Skip to content

Commit 33fd503

Browse files
leoromanovskyleoromanovsky
authored
Merge pull request #4071 from DataDog/leo.romanovsky/fix-openfeature-warning-exception
fix(feature-flags): make evaluation logging non-throwing
2 parents a732d12 + bf360ef commit 33fd503

10 files changed

Lines changed: 292 additions & 83 deletions

File tree

‎src/DDTrace/OpenFeature/DataDogProvider.php‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@
1212
use DDTrace\FeatureFlags\Internal\Metric\EvaluationMetric;
1313
use DDTrace\FeatureFlags\Internal\Metric\EvaluationMetricRecorder;
1414
use DDTrace\FeatureFlags\Internal\NativeEvaluator;
15+
use DDTrace\Log\Logger as GlobalLogger;
1516
use DDTrace\Log\LoggerInterface;
16-
use DDTrace\Log\TriggerErrorLogger;
17+
use DDTrace\Log\NonThrowingLogger;
1718
use OpenFeature\implementation\provider\AbstractProvider;
1819
use OpenFeature\implementation\provider\ResolutionDetailsBuilder;
1920
use OpenFeature\implementation\provider\ResolutionError;
@@ -30,16 +31,16 @@ final class DataDogProvider extends AbstractProvider
3031
protected static string $NAME = 'Datadog';
3132

3233
private Evaluator $evaluator;
33-
private LoggerInterface $warningLogger;
34-
private bool $warnedAboutNonProductionRuntime = false;
34+
private LoggerInterface $datadogLogger;
35+
private bool $warnedAboutRuntimeNotReady = false;
3536
private EvaluationMetricRecorder $metricRecorder;
3637

3738
public function __construct(?LoggerInterface $logger = null)
3839
{
3940
// Native evaluation metrics are disabled here because OpenFeature owns
4041
// the final provider outcome, including OF-level type mismatch mapping.
4142
$this->evaluator = NativeEvaluator::create(false);
42-
$this->warningLogger = $logger ?: new TriggerErrorLogger();
43+
$this->datadogLogger = new NonThrowingLogger($logger ?: GlobalLogger::get());
4344
$this->metricRecorder = EvaluationMetricRecorder::createDefault();
4445
}
4546

@@ -113,7 +114,7 @@ private function resolve(
113114
): ResolutionDetailsInterface {
114115
$normalizedContext = $this->normalizeContext($context);
115116
$details = $this->evaluate($flagKey, $expectedType, $defaultValue, $normalizedContext);
116-
$this->warnIfNonProductionRuntime($details);
117+
$this->warnIfRuntimeNotReady($details);
117118
// The PHP OpenFeature SDK does not pass ResolutionDetails to finally
118119
// hooks, so PHP records metrics here after native evaluation has the
119120
// final provider result.
@@ -214,24 +215,23 @@ private function normalizeContext(?EvaluationContext $context): array
214215
];
215216
}
216217

217-
private function warnIfNonProductionRuntime(EvaluationDetails $details): void
218+
private function warnIfRuntimeNotReady(EvaluationDetails $details): void
218219
{
219-
if ($this->warnedAboutNonProductionRuntime) {
220+
if ($this->warnedAboutRuntimeNotReady) {
220221
return;
221222
}
222223

223-
$providerState = $details->getProviderState();
224-
if (!array_key_exists('productionRuntime', $providerState) || $providerState['productionRuntime'] !== false) {
224+
if ($details->getErrorCode() !== EvaluationErrorCode::PROVIDER_NOT_READY) {
225225
return;
226226
}
227227

228228
$message = $details->getErrorMessage();
229229
if (!is_string($message) || $message === '') {
230-
$message = 'Datadog-backed PHP OpenFeature evaluation is not fully enabled yet.';
230+
$message = 'Datadog-backed PHP OpenFeature evaluation is not ready. Returning the default value.';
231231
}
232232

233-
$this->warningLogger->warning($message);
234-
$this->warnedAboutNonProductionRuntime = true;
233+
$this->warnedAboutRuntimeNotReady = true;
234+
$this->datadogLogger->warning($message);
235235
}
236236

237237
private function mapReason(string $reason): string

‎src/api/FeatureFlags/Client.php‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,16 @@
33
namespace DDTrace\FeatureFlags;
44

55
use DDTrace\FeatureFlags\Internal\NativeEvaluator;
6+
use DDTrace\Log\Logger as GlobalLogger;
67
use DDTrace\Log\LoggerInterface;
7-
use DDTrace\Log\TriggerErrorLogger;
8+
use DDTrace\Log\NonThrowingLogger;
89

910
final class Client
1011
{
1112
private $evaluator;
13+
/** @var LoggerInterface */
1214
private $logger;
13-
private $warnedAboutNonProductionRuntime = false;
15+
private $warnedAboutRuntimeNotReady = false;
1416

1517
public function __construct($logger = null)
1618
{
@@ -19,7 +21,7 @@ public function __construct($logger = null)
1921
}
2022

2123
$this->evaluator = NativeEvaluator::create();
22-
$this->logger = $logger ?: new TriggerErrorLogger();
24+
$this->logger = new NonThrowingLogger($logger ?: GlobalLogger::get());
2325
}
2426

2527
/**
@@ -115,7 +117,7 @@ private function evaluate($flagKey, $expectedType, $defaultValue, array $context
115117
$attributes
116118
);
117119

118-
$this->warnIfNonProductionRuntime($details);
120+
$this->warnIfRuntimeNotReady($details);
119121

120122
return $details;
121123
}
@@ -139,24 +141,23 @@ private function normalizeContext(array $context)
139141
return array($targetingKey, $attributes);
140142
}
141143

142-
private function warnIfNonProductionRuntime(EvaluationDetails $details)
144+
private function warnIfRuntimeNotReady(EvaluationDetails $details)
143145
{
144-
if ($this->warnedAboutNonProductionRuntime) {
146+
if ($this->warnedAboutRuntimeNotReady) {
145147
return;
146148
}
147149

148-
$providerState = $details->getProviderState();
149-
if (!array_key_exists('productionRuntime', $providerState) || $providerState['productionRuntime'] !== false) {
150+
if ($details->getErrorCode() !== EvaluationErrorCode::PROVIDER_NOT_READY) {
150151
return;
151152
}
152153

153154
$message = $details->getErrorMessage();
154155
if (!is_string($message) || $message === '') {
155-
$message = 'Datadog-backed PHP feature flag evaluation is running without exposure and metric reporting in this milestone.';
156+
$message = 'Datadog-backed PHP feature flag evaluation is not ready. Returning the default value.';
156157
}
157158

159+
$this->warnedAboutRuntimeNotReady = true;
158160
$this->logger->warning($message);
159-
$this->warnedAboutNonProductionRuntime = true;
160161
}
161162

162163
private function expectFlagKey($flagKey)

‎src/api/FeatureFlags/Internal/NativeEvaluator.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,9 @@ private function withProviderState($rawResult)
103103
'ready' => $hasConfig,
104104
'hasConfig' => $hasConfig,
105105
'configVersion' => $configVersion,
106-
'productionRuntime' => false,
106+
'productionRuntime' => true,
107107
'mode' => 'native_remote_config',
108-
'reason' => $hasConfig ? 'metrics_delivery_pending' : 'configuration_missing',
108+
'reason' => $hasConfig ? 'ready' : 'configuration_missing',
109109
);
110110

111111
if (is_array($rawResult)) {

‎src/api/Log/NonThrowingLogger.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
namespace DDTrace\Log;
4+
5+
/**
6+
* Prevents logger failures from escaping into application code.
7+
*
8+
* @internal
9+
*/
10+
final class NonThrowingLogger implements LoggerInterface
11+
{
12+
/** @var LoggerInterface */
13+
private $logger;
14+
15+
public function __construct(LoggerInterface $logger)
16+
{
17+
$this->logger = $logger;
18+
}
19+
20+
public function debug($message, array $context = array())
21+
{
22+
try {
23+
$this->logger->debug($message, $context);
24+
} catch (\Throwable $ignored) {
25+
}
26+
}
27+
28+
public function warning($message, array $context = array())
29+
{
30+
try {
31+
$this->logger->warning($message, $context);
32+
} catch (\Throwable $ignored) {
33+
}
34+
}
35+
36+
public function error($message, array $context = array())
37+
{
38+
try {
39+
$this->logger->error($message, $context);
40+
} catch (\Throwable $ignored) {
41+
}
42+
}
43+
44+
public function isLevelActive($level)
45+
{
46+
try {
47+
return (bool) $this->logger->isLevelActive($level);
48+
} catch (\Throwable $ignored) {
49+
return false;
50+
}
51+
}
52+
}

‎src/api/Log/TriggerErrorLogger.php‎

Lines changed: 0 additions & 37 deletions
This file was deleted.

‎src/bridge/_files_tracer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
__DIR__ . '/../api/Log/DatadogLogger.php',
2929
__DIR__ . '/../api/Log/ErrorLogLogger.php',
3030
__DIR__ . '/../api/Log/NullLogger.php',
31-
__DIR__ . '/../api/Log/TriggerErrorLogger.php',
31+
__DIR__ . '/../api/Log/NonThrowingLogger.php',
3232
__DIR__ . '/../api/Log/LoggingTrait.php',
3333
__DIR__ . '/../api/StartSpanOptions.php',
3434
__DIR__ . '/../DDTrace/SpanContext.php',

‎tests/OpenFeature/DataDogProviderTest.php‎

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use DDTrace\FeatureFlags\EvaluationReason;
1010
use DDTrace\FeatureFlags\EvaluationType;
1111
use DDTrace\FeatureFlags\Internal\Evaluator;
12-
use DDTrace\FeatureFlags\Internal\NativeEvaluator;
1312
use DDTrace\FeatureFlags\Internal\UnavailableEvaluator;
1413
use DDTrace\Log\LoggerInterface;
1514
use DDTrace\Log\LogLevel;
@@ -101,7 +100,7 @@ public function testEvaluationContextIsNormalizedForDatadogClient(): void
101100
public function testUnavailableRuntimeReturnsDefaultDetailsAndOneWarning(): void
102101
{
103102
$logger = new OpenFeatureRecordingLogger();
104-
$client = $this->openFeatureClientFor(new DataDogProvider($logger));
103+
$client = $this->openFeatureClientFor($this->providerForEvaluator(new UnavailableEvaluator(), $logger));
105104

106105
$value = $client->getBooleanValue('checkout.enabled', true);
107106
$details = $client->getStringDetails('checkout.copy', 'fallback');
@@ -110,10 +109,7 @@ public function testUnavailableRuntimeReturnsDefaultDetailsAndOneWarning(): void
110109
self::assertSame('fallback', $details->getValue());
111110
self::assertSame(Reason::ERROR, $details->getReason());
112111
self::assertSame(ErrorCode::PROVIDER_NOT_READY()->getValue(), $details->getError()->getResolutionErrorCode()->getValue());
113-
self::assertContains($details->getError()->getResolutionErrorMessage(), [
114-
NativeEvaluator::WARNING_MESSAGE,
115-
UnavailableEvaluator::WARNING_MESSAGE,
116-
]);
112+
self::assertSame(UnavailableEvaluator::WARNING_MESSAGE, $details->getError()->getResolutionErrorMessage());
117113
self::assertSame([$details->getError()->getResolutionErrorMessage()], $logger->warnings());
118114
}
119115

@@ -133,6 +129,26 @@ public function testProviderWarningIsEmittedOncePerProvider(): void
133129
self::assertSame(['temporary unavailable'], $logger->warnings());
134130
}
135131

132+
public function testThrowingLoggerDoesNotChangeProviderNotReadyEvaluation(): void
133+
{
134+
$evaluator = new OpenFeatureTestEvaluator();
135+
$evaluator->setUnavailable('preview.flag', false, 'runtime unavailable');
136+
$logger = new OpenFeatureThrowingLogger();
137+
$client = $this->openFeatureClientFor($this->providerForEvaluator($evaluator, $logger));
138+
139+
$firstDetails = $client->getBooleanDetails('preview.flag', false);
140+
$secondDetails = $client->getBooleanDetails('preview.flag', false);
141+
142+
self::assertFalse($firstDetails->getValue());
143+
self::assertSame(Reason::ERROR, $firstDetails->getReason());
144+
self::assertSame(
145+
ErrorCode::PROVIDER_NOT_READY()->getValue(),
146+
$firstDetails->getError()->getResolutionErrorCode()->getValue()
147+
);
148+
self::assertFalse($secondDetails->getValue());
149+
self::assertSame(1, $logger->warningCount());
150+
}
151+
136152
public function testProviderErrorsMapToOpenFeatureDetails(): void
137153
{
138154
$evaluator = new OpenFeatureTestEvaluator();
@@ -186,13 +202,19 @@ public function setSuccess(
186202
string $flagKey,
187203
mixed $value,
188204
string $reason = EvaluationReason::STATIC_REASON,
189-
?string $variant = null
205+
?string $variant = null,
206+
array $providerState = []
190207
): self {
191208
$this->details[$flagKey] = new EvaluationDetails(
192209
$value,
193210
$this->typeForValue($value),
194211
$reason,
195-
$variant
212+
$variant,
213+
null,
214+
null,
215+
[],
216+
[],
217+
$providerState
196218
);
197219

198220
return $this;
@@ -336,4 +358,33 @@ public function warnings(): array
336358
return $this->warnings;
337359
}
338360
}
361+
362+
final class OpenFeatureThrowingLogger implements LoggerInterface
363+
{
364+
private int $warningCount = 0;
365+
366+
public function debug($message, array $context = [])
367+
{
368+
}
369+
370+
public function warning($message, array $context = [])
371+
{
372+
++$this->warningCount;
373+
throw new \ErrorException($message);
374+
}
375+
376+
public function error($message, array $context = [])
377+
{
378+
}
379+
380+
public function isLevelActive($level)
381+
{
382+
return true;
383+
}
384+
385+
public function warningCount(): int
386+
{
387+
return $this->warningCount;
388+
}
389+
}
339390
}

0 commit comments

Comments
 (0)