Skip to content

Commit bf360ef

Browse files
leoromanovskyleoromanovsky
committed
fix(ffe): mark the native evaluator production ready
1 parent 51ed2b1 commit bf360ef

6 files changed

Lines changed: 46 additions & 63 deletions

File tree

‎src/DDTrace/OpenFeature/DataDogProvider.php‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ final class DataDogProvider extends AbstractProvider
3232

3333
private Evaluator $evaluator;
3434
private LoggerInterface $datadogLogger;
35-
private bool $warnedAboutNonProductionRuntime = false;
35+
private bool $warnedAboutRuntimeNotReady = false;
3636
private EvaluationMetricRecorder $metricRecorder;
3737

3838
public function __construct(?LoggerInterface $logger = null)
@@ -114,7 +114,7 @@ private function resolve(
114114
): ResolutionDetailsInterface {
115115
$normalizedContext = $this->normalizeContext($context);
116116
$details = $this->evaluate($flagKey, $expectedType, $defaultValue, $normalizedContext);
117-
$this->warnIfNonProductionRuntime($details);
117+
$this->warnIfRuntimeNotReady($details);
118118
// The PHP OpenFeature SDK does not pass ResolutionDetails to finally
119119
// hooks, so PHP records metrics here after native evaluation has the
120120
// final provider result.
@@ -215,23 +215,22 @@ private function normalizeContext(?EvaluationContext $context): array
215215
];
216216
}
217217

218-
private function warnIfNonProductionRuntime(EvaluationDetails $details): void
218+
private function warnIfRuntimeNotReady(EvaluationDetails $details): void
219219
{
220-
if ($this->warnedAboutNonProductionRuntime) {
220+
if ($this->warnedAboutRuntimeNotReady) {
221221
return;
222222
}
223223

224-
$providerState = $details->getProviderState();
225-
if (!array_key_exists('productionRuntime', $providerState) || $providerState['productionRuntime'] !== false) {
224+
if ($details->getErrorCode() !== EvaluationErrorCode::PROVIDER_NOT_READY) {
226225
return;
227226
}
228227

229228
$message = $details->getErrorMessage();
230229
if (!is_string($message) || $message === '') {
231-
$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.';
232231
}
233232

234-
$this->warnedAboutNonProductionRuntime = true;
233+
$this->warnedAboutRuntimeNotReady = true;
235234
$this->datadogLogger->warning($message);
236235
}
237236

‎src/api/FeatureFlags/Client.php‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ final class Client
1212
private $evaluator;
1313
/** @var LoggerInterface */
1414
private $logger;
15-
private $warnedAboutNonProductionRuntime = false;
15+
private $warnedAboutRuntimeNotReady = false;
1616

1717
public function __construct($logger = null)
1818
{
@@ -117,7 +117,7 @@ private function evaluate($flagKey, $expectedType, $defaultValue, array $context
117117
$attributes
118118
);
119119

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

122122
return $details;
123123
}
@@ -141,23 +141,22 @@ private function normalizeContext(array $context)
141141
return array($targetingKey, $attributes);
142142
}
143143

144-
private function warnIfNonProductionRuntime(EvaluationDetails $details)
144+
private function warnIfRuntimeNotReady(EvaluationDetails $details)
145145
{
146-
if ($this->warnedAboutNonProductionRuntime) {
146+
if ($this->warnedAboutRuntimeNotReady) {
147147
return;
148148
}
149149

150-
$providerState = $details->getProviderState();
151-
if (!array_key_exists('productionRuntime', $providerState) || $providerState['productionRuntime'] !== false) {
150+
if ($details->getErrorCode() !== EvaluationErrorCode::PROVIDER_NOT_READY) {
152151
return;
153152
}
154153

155154
$message = $details->getErrorMessage();
156155
if (!is_string($message) || $message === '') {
157-
$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.';
158157
}
159158

160-
$this->warnedAboutNonProductionRuntime = true;
159+
$this->warnedAboutRuntimeNotReady = true;
161160
$this->logger->warning($message);
162161
}
163162

‎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)) {

‎tests/OpenFeature/DataDogProviderTest.php‎

Lines changed: 11 additions & 19 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,27 +129,23 @@ public function testProviderWarningIsEmittedOncePerProvider(): void
133129
self::assertSame(['temporary unavailable'], $logger->warnings());
134130
}
135131

136-
public function testThrowingLoggerDoesNotChangeSuccessfulEvaluation(): void
132+
public function testThrowingLoggerDoesNotChangeProviderNotReadyEvaluation(): void
137133
{
138134
$evaluator = new OpenFeatureTestEvaluator();
139-
$evaluator->setSuccess(
140-
'preview.flag',
141-
true,
142-
EvaluationReason::STATIC_REASON,
143-
'on',
144-
['productionRuntime' => false]
145-
);
135+
$evaluator->setUnavailable('preview.flag', false, 'runtime unavailable');
146136
$logger = new OpenFeatureThrowingLogger();
147137
$client = $this->openFeatureClientFor($this->providerForEvaluator($evaluator, $logger));
148138

149139
$firstDetails = $client->getBooleanDetails('preview.flag', false);
150140
$secondDetails = $client->getBooleanDetails('preview.flag', false);
151141

152-
self::assertTrue($firstDetails->getValue());
153-
self::assertSame(EvaluationReason::STATIC_REASON, $firstDetails->getReason());
154-
self::assertSame('on', $firstDetails->getVariant());
155-
self::assertNull($firstDetails->getError());
156-
self::assertTrue($secondDetails->getValue());
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());
157149
self::assertSame(1, $logger->warningCount());
158150
}
159151

‎tests/api/Unit/FeatureFlags/ClientTest.php‎

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
use DDTrace\FeatureFlags\EvaluationReason;
99
use DDTrace\FeatureFlags\EvaluationType;
1010
use DDTrace\FeatureFlags\Internal\Evaluator;
11-
use DDTrace\FeatureFlags\Internal\NativeEvaluator;
1211
use DDTrace\FeatureFlags\Internal\UnavailableEvaluator;
1312
use DDTrace\Log\LoggerInterface;
1413
use PHPUnit\Framework\TestCase;
@@ -93,7 +92,7 @@ public function testContextNormalizesTargetingKeyAndPrimitiveAttributes()
9392
public function testUnavailableRuntimeReturnsDefaultWithProviderNotReadyDetailsAndWarning()
9493
{
9594
$logger = new RecordingLogger();
96-
$client = new Client($logger);
95+
$client = $this->clientForEvaluator(new UnavailableEvaluator(), $logger);
9796

9897
$value = $client->getBooleanValue('checkout-redesign', true);
9998
$details = $client->getStringDetails('checkout-copy', 'fallback');
@@ -102,25 +101,19 @@ public function testUnavailableRuntimeReturnsDefaultWithProviderNotReadyDetailsA
102101
$this->assertSame('fallback', $details->getValue());
103102
$this->assertSame(EvaluationReason::ERROR, $details->getReason());
104103
$this->assertSame(EvaluationErrorCode::PROVIDER_NOT_READY, $details->getErrorCode());
105-
$this->assertContains($details->getErrorMessage(), array(
106-
NativeEvaluator::WARNING_MESSAGE,
107-
UnavailableEvaluator::WARNING_MESSAGE,
108-
));
104+
$this->assertSame(UnavailableEvaluator::WARNING_MESSAGE, $details->getErrorMessage());
109105

110106
$providerState = $details->getProviderState();
111107
$this->assertSame(false, $providerState['ready']);
112108
$this->assertSame(false, $providerState['productionRuntime']);
113-
$this->assertTrue(in_array($providerState['reason'], array(
114-
'configuration_missing',
115-
'runtime_unavailable',
116-
), true));
109+
$this->assertSame('runtime_unavailable', $providerState['reason']);
117110
$this->assertSame(array($details->getErrorMessage()), $logger->warnings());
118111
}
119112

120113
public function testWarningIsEmittedOncePerClientNotOncePerEvaluation()
121114
{
122115
$logger = new RecordingLogger();
123-
$client = new Client($logger);
116+
$client = $this->clientForEvaluator(new ClientTestEvaluator(), $logger);
124117

125118
$client->getBooleanValue('flag-1', false);
126119
$client->getBooleanValue('flag-2', false);
@@ -129,29 +122,19 @@ public function testWarningIsEmittedOncePerClientNotOncePerEvaluation()
129122
$this->assertCount(1, $logger->warnings());
130123
}
131124

132-
public function testThrowingLoggerDoesNotChangeSuccessfulEvaluation()
125+
public function testThrowingLoggerDoesNotChangeProviderNotReadyEvaluation()
133126
{
134127
$evaluator = new ClientTestEvaluator();
135-
$evaluator->setSuccess(
136-
'preview.flag',
137-
true,
138-
EvaluationReason::STATIC_REASON,
139-
'on',
140-
array(),
141-
array(),
142-
array('productionRuntime' => false)
143-
);
144128
$logger = new ThrowingLogger();
145129
$client = $this->clientForEvaluator($evaluator, $logger);
146130

147131
$firstDetails = $client->getBooleanDetails('preview.flag', false);
148132
$secondDetails = $client->getBooleanDetails('preview.flag', false);
149133

150-
$this->assertTrue($firstDetails->getValue());
151-
$this->assertSame(EvaluationReason::STATIC_REASON, $firstDetails->getReason());
152-
$this->assertSame('on', $firstDetails->getVariant());
153-
$this->assertNull($firstDetails->getErrorCode());
154-
$this->assertTrue($secondDetails->getValue());
134+
$this->assertFalse($firstDetails->getValue());
135+
$this->assertSame(EvaluationReason::ERROR, $firstDetails->getReason());
136+
$this->assertSame(EvaluationErrorCode::PROVIDER_NOT_READY, $firstDetails->getErrorCode());
137+
$this->assertFalse($secondDetails->getValue());
155138
$this->assertSame(1, $logger->warningCount());
156139
}
157140

‎tests/ext/ffe/system_test_data_evaluate.phpt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,16 @@ function run_fixture_case($client, $fileName, $index, array $case, array &$failu
155155
$context
156156
);
157157

158+
$providerState = $details->getProviderState();
159+
if (($providerState['productionRuntime'] ?? null) !== true) {
160+
$failures[] = $fileName . '#' . $index . ': productionRuntime must be true';
161+
}
162+
if (($providerState['reason'] ?? null) !== 'ready') {
163+
$failures[] = $fileName . '#' . $index
164+
. ': runtime reason got=' . encode_value($providerState['reason'] ?? null)
165+
. ' want="ready"';
166+
}
167+
158168
if (!array_key_exists('value', $case['result'])) {
159169
$failures[] = $fileName . '#' . $index . ': result must include value';
160170
return;

0 commit comments

Comments
 (0)