From 24d062aae46590ba816136eb0ec01ac82a14105a Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Thu, 21 May 2026 17:52:01 -0400 Subject: [PATCH] test(ffe): specify exposure and metric semantics --- .../FeatureFlags/BufferedExposureWriter.php | 85 ++++++++++ .../FeatureFlags/CallableMetricsRecorder.php | 48 ++++++ src/api/FeatureFlags/Client.php | 73 ++++++++- src/api/FeatureFlags/ExposureWriter.php | 17 ++ src/api/FeatureFlags/MetricsRecorder.php | 12 ++ src/api/FeatureFlags/NoopExposureWriter.php | 14 ++ src/api/FeatureFlags/NoopMetricsRecorder.php | 10 ++ .../Unit/FeatureFlags/ExposureWriterTest.php | 153 ++++++++++++++++++ .../Unit/FeatureFlags/MetricsRecorderTest.php | 130 +++++++++++++++ 9 files changed, 537 insertions(+), 5 deletions(-) create mode 100644 src/api/FeatureFlags/BufferedExposureWriter.php create mode 100644 src/api/FeatureFlags/CallableMetricsRecorder.php create mode 100644 src/api/FeatureFlags/ExposureWriter.php create mode 100644 src/api/FeatureFlags/MetricsRecorder.php create mode 100644 src/api/FeatureFlags/NoopExposureWriter.php create mode 100644 src/api/FeatureFlags/NoopMetricsRecorder.php create mode 100644 tests/api/Unit/FeatureFlags/ExposureWriterTest.php create mode 100644 tests/api/Unit/FeatureFlags/MetricsRecorderTest.php diff --git a/src/api/FeatureFlags/BufferedExposureWriter.php b/src/api/FeatureFlags/BufferedExposureWriter.php new file mode 100644 index 0000000000..ad09799156 --- /dev/null +++ b/src/api/FeatureFlags/BufferedExposureWriter.php @@ -0,0 +1,85 @@ +sink = $sink; + $this->maxBatchSize = $maxBatchSize; + } + + public function write(array $event) + { + if (array_key_exists('doLog', $event) && $event['doLog'] === false) { + return; + } + + $dedupKey = $this->dedupKey($event); + $stateKey = $this->stateKey($event); + if (isset($this->dedup[$dedupKey]) && $this->dedup[$dedupKey] === $stateKey) { + return; + } + + $this->dedup[$dedupKey] = $stateKey; + $this->buffer[] = $event; + + if (count($this->buffer) >= $this->maxBatchSize) { + $this->flush(); + } + } + + public function flush() + { + if (!$this->buffer) { + return; + } + + $batch = $this->buffer; + $this->buffer = array(); + + call_user_func($this->sink, $batch); + } + + public function getBufferedCount() + { + return count($this->buffer); + } + + private function dedupKey(array $event) + { + return $this->eventValue($event, 'flagKey') . "\0" . $this->eventValue($event, 'targetingKey'); + } + + private function stateKey(array $event) + { + return $this->eventValue($event, 'allocationKey') . "\0" . $this->eventValue($event, 'variant'); + } + + private function eventValue(array $event, $key) + { + if (!array_key_exists($key, $event) || $event[$key] === null) { + return ''; + } + + if (is_scalar($event[$key])) { + return (string) $event[$key]; + } + + return json_encode($event[$key]); + } +} diff --git a/src/api/FeatureFlags/CallableMetricsRecorder.php b/src/api/FeatureFlags/CallableMetricsRecorder.php new file mode 100644 index 0000000000..b3d5f82fb4 --- /dev/null +++ b/src/api/FeatureFlags/CallableMetricsRecorder.php @@ -0,0 +1,48 @@ +sink = $sink; + $this->enabled = (bool) $enabled; + } + + public static function createFromEnvironment($sink) + { + return new self($sink, self::isTruthy(getenv('DD_METRICS_OTEL_ENABLED'))); + } + + public function recordEvaluation($flagKey, $valueType, $reason, $errorCode = null) + { + if (!$this->enabled) { + return; + } + + call_user_func($this->sink, array( + 'name' => self::METRIC_NAME, + 'attributes' => array( + 'feature_flag.key' => $flagKey, + 'feature_flag.result.reason' => $reason, + 'feature_flag.result.value_type' => $valueType, + 'feature_flag.error.code' => $errorCode === null ? 'none' : $errorCode, + ), + )); + } + + private static function isTruthy($value) + { + return in_array(strtolower((string) $value), array('1', 'true', 'yes', 'on'), true); + } +} diff --git a/src/api/FeatureFlags/Client.php b/src/api/FeatureFlags/Client.php index b4978c2935..84508b1f78 100644 --- a/src/api/FeatureFlags/Client.php +++ b/src/api/FeatureFlags/Client.php @@ -6,16 +6,36 @@ final class Client { private $evaluator; private $warningEmitter; + private $exposureWriter; + private $metricsRecorder; private $warnedAboutNonProductionRuntime = false; - public function __construct(Evaluator $evaluator, WarningEmitter $warningEmitter) - { + public function __construct( + Evaluator $evaluator, + WarningEmitter $warningEmitter, + $exposureWriter = null, + $metricsRecorder = null + ) { + if ($exposureWriter !== null && !$exposureWriter instanceof ExposureWriter) { + throw new \InvalidArgumentException('Expected an ExposureWriter instance'); + } + + if ($metricsRecorder !== null && !$metricsRecorder instanceof MetricsRecorder) { + throw new \InvalidArgumentException('Expected a MetricsRecorder instance'); + } + $this->evaluator = $evaluator; $this->warningEmitter = $warningEmitter; + $this->exposureWriter = $exposureWriter ?: new NoopExposureWriter(); + $this->metricsRecorder = $metricsRecorder ?: new NoopMetricsRecorder(); } - public static function create($evaluator = null, $warningEmitter = null) - { + public static function create( + $evaluator = null, + $warningEmitter = null, + $exposureWriter = null, + $metricsRecorder = null + ) { if ($evaluator !== null && !$evaluator instanceof Evaluator) { throw new \InvalidArgumentException('Expected an Evaluator instance'); } @@ -26,7 +46,9 @@ public static function create($evaluator = null, $warningEmitter = null) return new self( $evaluator ?: new UnavailableEvaluator(), - $warningEmitter ?: new TriggerErrorWarningEmitter() + $warningEmitter ?: new TriggerErrorWarningEmitter(), + $exposureWriter, + $metricsRecorder ); } @@ -94,10 +116,51 @@ private function evaluate($flagKey, $expectedType, $defaultValue, array $context ); $this->warnIfNonProductionRuntime($details); + $this->metricsRecorder->recordEvaluation( + $flagKey, + $details->getValueType(), + $details->getReason(), + $details->getErrorCode() + ); + $this->writeExposure($flagKey, $targetingKey, $attributes, $details); return $details; } + private function writeExposure($flagKey, $targetingKey, array $attributes, EvaluationDetails $details) + { + if ($details->isError()) { + return; + } + + $exposureData = $details->getExposureData(); + if (!$exposureData || (array_key_exists('doLog', $exposureData) && $exposureData['doLog'] === false)) { + return; + } + + $event = array( + 'flagKey' => $flagKey, + 'targetingKey' => $targetingKey, + 'attributes' => $attributes, + 'value' => $details->getValue(), + 'valueType' => $details->getValueType(), + 'reason' => $details->getReason(), + 'variant' => $details->getVariant(), + 'flagMetadata' => $details->getFlagMetadata(), + 'exposureData' => $exposureData, + ); + + if (array_key_exists('allocationKey', $exposureData)) { + $event['allocationKey'] = $exposureData['allocationKey']; + } + + if (array_key_exists('doLog', $exposureData)) { + $event['doLog'] = $exposureData['doLog']; + } + + $this->exposureWriter->write($event); + } + private function normalizeContext(array $context) { $targetingKey = null; diff --git a/src/api/FeatureFlags/ExposureWriter.php b/src/api/FeatureFlags/ExposureWriter.php new file mode 100644 index 0000000000..5a35d3ebe2 --- /dev/null +++ b/src/api/FeatureFlags/ExposureWriter.php @@ -0,0 +1,17 @@ + $event + * @return void + */ + public function write(array $event); + + /** + * @return void + */ + public function flush(); +} diff --git a/src/api/FeatureFlags/MetricsRecorder.php b/src/api/FeatureFlags/MetricsRecorder.php new file mode 100644 index 0000000000..609135d0ad --- /dev/null +++ b/src/api/FeatureFlags/MetricsRecorder.php @@ -0,0 +1,12 @@ +setSuccess( + 'checkout-redesign', + true, + EvaluationReason::SPLIT, + 'treatment', + array('owner' => 'ffe'), + array('allocationKey' => 'alloc-1', 'doLog' => true) + ); + + $client = Client::create( + $evaluator, + new ExposureTestWarningEmitter(), + $writer, + new NoopMetricsRecorder() + ); + + $client->getBooleanValue('checkout-redesign', false, array( + 'targetingKey' => 'user-123', + 'attributes' => array('plan' => 'pro'), + )); + $writer->flush(); + + $this->assertCount(1, $batches); + $this->assertCount(1, $batches[0]); + $this->assertSame(array( + 'flagKey' => 'checkout-redesign', + 'targetingKey' => 'user-123', + 'attributes' => array('plan' => 'pro'), + 'value' => true, + 'valueType' => 'boolean', + 'reason' => EvaluationReason::SPLIT, + 'variant' => 'treatment', + 'flagMetadata' => array('owner' => 'ffe'), + 'exposureData' => array('allocationKey' => 'alloc-1', 'doLog' => true), + 'allocationKey' => 'alloc-1', + 'doLog' => true, + ), $batches[0][0]); + } + + public function testClientSkipsExposureWhenDoLogFalseOrEvaluationErrors() + { + $batches = array(); + $writer = new BufferedExposureWriter(function (array $batch) use (&$batches) { + $batches[] = $batch; + }); + + $evaluator = new FakeEvaluator(); + $evaluator + ->setSuccess( + 'do-not-log', + true, + EvaluationReason::SPLIT, + 'control', + array(), + array('allocationKey' => 'alloc-1', 'doLog' => false) + ) + ->setProviderNotReady('provider-not-ready'); + + $client = Client::create( + $evaluator, + new ExposureTestWarningEmitter(), + $writer, + new NoopMetricsRecorder() + ); + + $client->getBooleanValue('do-not-log', false); + $client->getBooleanValue('provider-not-ready', false); + $writer->flush(); + + $this->assertSame(array(), $batches); + } + + public function testBufferedWriterSuppressesDuplicatesAndReemitsChangedAssignments() + { + $batches = array(); + $writer = new BufferedExposureWriter(function (array $batch) use (&$batches) { + $batches[] = $batch; + }); + + $writer->write($this->event('checkout', 'user-1', 'alloc-1', 'control')); + $writer->write($this->event('checkout', 'user-1', 'alloc-1', 'control')); + $writer->write($this->event('checkout', 'user-1', 'alloc-1', 'treatment')); + $writer->write($this->event('checkout', 'user-1', 'alloc-2', 'treatment')); + $writer->flush(); + + $this->assertCount(1, $batches); + $this->assertSame(array('control', 'treatment', 'treatment'), array( + $batches[0][0]['variant'], + $batches[0][1]['variant'], + $batches[0][2]['variant'], + )); + $this->assertSame('alloc-2', $batches[0][2]['allocationKey']); + } + + public function testBufferedWriterFlushesAtBatchCapAndEmptyFlushIsNoop() + { + $batches = array(); + $writer = new BufferedExposureWriter(function (array $batch) use (&$batches) { + $batches[] = $batch; + }, 2); + + $writer->write($this->event('flag-a', 'user-1', 'alloc-1', 'control')); + $this->assertSame(1, $writer->getBufferedCount()); + $writer->write($this->event('flag-b', 'user-2', 'alloc-1', 'control')); + + $this->assertSame(0, $writer->getBufferedCount()); + $this->assertCount(1, $batches); + $this->assertCount(2, $batches[0]); + + $writer->flush(); + $this->assertCount(1, $batches); + } + + private function event($flagKey, $targetingKey, $allocationKey, $variant) + { + return array( + 'flagKey' => $flagKey, + 'targetingKey' => $targetingKey, + 'allocationKey' => $allocationKey, + 'variant' => $variant, + 'doLog' => true, + ); + } +} + +final class ExposureTestWarningEmitter implements WarningEmitter +{ + public function warning($message) + { + } +} diff --git a/tests/api/Unit/FeatureFlags/MetricsRecorderTest.php b/tests/api/Unit/FeatureFlags/MetricsRecorderTest.php new file mode 100644 index 0000000000..2d4ea6675f --- /dev/null +++ b/tests/api/Unit/FeatureFlags/MetricsRecorderTest.php @@ -0,0 +1,130 @@ +recordEvaluation('checkout-redesign', 'BOOLEAN', EvaluationReason::SPLIT); + + $this->assertSame(array(array( + 'name' => CallableMetricsRecorder::METRIC_NAME, + 'attributes' => array( + 'feature_flag.key' => 'checkout-redesign', + 'feature_flag.result.reason' => EvaluationReason::SPLIT, + 'feature_flag.result.value_type' => 'BOOLEAN', + 'feature_flag.error.code' => 'none', + ), + )), $metrics); + } + + public function testMetricsRecorderCanBeGatedByEnvironment() + { + $previous = getenv('DD_METRICS_OTEL_ENABLED'); + $metrics = array(); + + try { + putenv('DD_METRICS_OTEL_ENABLED=false'); + $disabled = CallableMetricsRecorder::createFromEnvironment(function (array $metric) use (&$metrics) { + $metrics[] = $metric; + }); + $disabled->recordEvaluation('checkout-redesign', 'BOOLEAN', EvaluationReason::SPLIT); + $this->assertSame(array(), $metrics); + + putenv('DD_METRICS_OTEL_ENABLED=true'); + $enabled = CallableMetricsRecorder::createFromEnvironment(function (array $metric) use (&$metrics) { + $metrics[] = $metric; + }); + $enabled->recordEvaluation('checkout-redesign', 'BOOLEAN', EvaluationReason::SPLIT); + $this->assertCount(1, $metrics); + } finally { + if ($previous === false) { + putenv('DD_METRICS_OTEL_ENABLED'); + } else { + putenv('DD_METRICS_OTEL_ENABLED=' . $previous); + } + } + } + + public function testClientRecordsMetricsOncePerEvaluationForSuccessAndErrors() + { + $metrics = array(); + $recorder = new CallableMetricsRecorder(function (array $metric) use (&$metrics) { + $metrics[] = $metric; + }, true); + + $evaluator = new FakeEvaluator(); + $evaluator + ->setSuccess('success.flag', true) + ->setTypeMismatch('type-mismatch.flag'); + + $client = Client::create( + $evaluator, + new MetricsTestWarningEmitter(), + new NoopExposureWriter(), + $recorder + ); + + $client->getBooleanValue('success.flag', false); + $client->getBooleanValue('type-mismatch.flag', false); + + $this->assertCount(2, $metrics); + $this->assertSame('none', $metrics[0]['attributes']['feature_flag.error.code']); + $this->assertSame( + EvaluationErrorCode::TYPE_MISMATCH, + $metrics[1]['attributes']['feature_flag.error.code'] + ); + } + + public function testProviderNotReadyMetricsUseErrorCode() + { + $metrics = array(); + $recorder = new CallableMetricsRecorder(function (array $metric) use (&$metrics) { + $metrics[] = $metric; + }, true); + + $client = Client::create( + new UnavailableEvaluator(), + new MetricsTestWarningEmitter(), + new NoopExposureWriter(), + $recorder + ); + + $client->getBooleanValue('not-ready.flag', false); + + $this->assertSame( + EvaluationErrorCode::PROVIDER_NOT_READY, + $metrics[0]['attributes']['feature_flag.error.code'] + ); + } + + public function testRootComposerDoesNotRequireOpenTelemetrySdk() + { + $composer = json_decode((string) file_get_contents(__DIR__ . '/../../../../composer.json'), true); + + $this->assertArrayNotHasKey('open-telemetry/sdk', isset($composer['require']) ? $composer['require'] : array()); + } +} + +final class MetricsTestWarningEmitter implements WarningEmitter +{ + public function warning($message) + { + } +}