From 053f9d83c1e7aa0627565ad250a4290994825980 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Mon, 15 Jun 2026 23:30:45 -0400 Subject: [PATCH 01/12] feat(02-05): thread split serial_id Rust -> C -> PHP mapper + add span-enrichment gate - Add serial_id (i64) + has_serial_id (bool) to the Rust FfeResult struct and populate from assignment.serial_id (unwrap_or(0) + is_some()) in all ctors; regenerate the cbindgen common.h ABI to match. - Surface serialId as a nullable int on the DDTrace\FfeResult object in the C reader (tracer/functions.c), guarded by has_serial_id so absence stays null (Pattern B: missing != 0); update the stub + arginfo. - Thread serialId into ResultMapper::exposureData (only when present). - Add the gate CONFIG(BOOL, DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED, "false") to ext/configuration.h (distinct from the provider-enabled gate). - Update existing FFE phpt EXPECT blocks for the new serialId field. --- components-rs/common.h | 2 ++ components-rs/ffe.rs | 14 ++++++++++++++ ext/configuration.h | 3 ++- src/api/FeatureFlags/Internal/ResultMapper.php | 9 +++++++++ tests/ext/ffe/evaluation_metrics_native.phpt | 4 ++-- tests/ext/ffe/native_bridge_evaluate.phpt | 14 +++++++------- tests/ext/ffe/remote_config_lifecycle.phpt | 2 +- tracer/ddtrace.stub.php | 1 + tracer/ddtrace_arginfo.h | 6 ++++++ tracer/functions.c | 7 +++++++ 10 files changed, 51 insertions(+), 11 deletions(-) diff --git a/components-rs/common.h b/components-rs/common.h index 5c0b9606b79..07175ca8c83 100644 --- a/components-rs/common.h +++ b/components-rs/common.h @@ -484,6 +484,8 @@ typedef struct ddog_FfeResult { _zend_string * allocation_key; int32_t reason; int32_t error_code; + int64_t serial_id; + bool has_serial_id; bool do_log; bool valid; } ddog_FfeResult; diff --git a/components-rs/ffe.rs b/components-rs/ffe.rs index f7684d069fa..d2f487e7767 100644 --- a/components-rs/ffe.rs +++ b/components-rs/ffe.rs @@ -97,6 +97,14 @@ pub struct FfeResult { pub allocation_key: MaybeOwnedZendString, pub reason: i32, pub error_code: i32, + // serial_id is the selected split's serial id, carried for APM span + // enrichment (ffe_flags_enc). The source field is Option; since the + // C ABI cannot represent Option as a plain field, we surface the + // presence separately via has_serial_id. Consumers MUST gate on + // has_serial_id (the Pattern B "missing variant => default" semantic) and + // never treat serial_id == 0 as "absent". + pub serial_id: i64, + pub has_serial_id: bool, pub do_log: bool, pub valid: bool, } @@ -220,6 +228,8 @@ fn result_from_assignment(assignment: Result) AssignmentReason::Default => REASON_DEFAULT, }, error_code: ERROR_NONE, + serial_id: assignment.serial_id.unwrap_or(0) as i64, + has_serial_id: assignment.serial_id.is_some(), do_log: assignment.do_log, valid: true, } @@ -244,6 +254,8 @@ fn result_from_assignment(assignment: Result) allocation_key: None, reason, error_code, + serial_id: 0, + has_serial_id: false, do_log: false, valid: true, } @@ -258,6 +270,8 @@ fn invalid_result() -> FfeResult { allocation_key: None, reason: REASON_ERROR, error_code: ERROR_GENERAL, + serial_id: 0, + has_serial_id: false, do_log: false, valid: false, } diff --git a/ext/configuration.h b/ext/configuration.h index f3d1b811db9..72732602af6 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -108,7 +108,8 @@ enum datadog_sidecar_connection_mode { CONFIG(DOUBLE, DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS, "5.0", .ini_change = zai_config_system_ini_change) \ CONFIG(BOOL, DD_REMOTE_CONFIG_ENABLED, "true", .ini_change = zai_config_system_ini_change) \ CONFIG(INT, DD_TRACE_RETRY_INTERVAL, "100", .ini_change = zai_config_system_ini_change) \ - CONFIG(BOOL, DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED, "true") + CONFIG(BOOL, DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED, "true") \ + CONFIG(BOOL, DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED, "false") #define DD_CONFIGURATIONS_ONLY #ifdef DDTRACE diff --git a/src/api/FeatureFlags/Internal/ResultMapper.php b/src/api/FeatureFlags/Internal/ResultMapper.php index 4dd30853dc5..d717d24b9db 100644 --- a/src/api/FeatureFlags/Internal/ResultMapper.php +++ b/src/api/FeatureFlags/Internal/ResultMapper.php @@ -274,6 +274,15 @@ private function exposureData($rawResult) $exposureData['doLog'] = (bool) $this->read($rawResult, array('do_log', 'doLog'), false); } + // serialId is the selected split's serial id, surfaced from the native + // bridge for APM span enrichment. It is only present when the native + // result actually carried one; a null/absent value must be left out so + // downstream consumers can treat "no serialId" as a runtime default. + $serialId = $this->read($rawResult, array('serial_id', 'serialId'), null); + if ($serialId !== null) { + $exposureData['serialId'] = (int) $serialId; + } + return $exposureData; } diff --git a/tests/ext/ffe/evaluation_metrics_native.phpt b/tests/ext/ffe/evaluation_metrics_native.phpt index 572ea09fbbe..c0d88766433 100644 --- a/tests/ext/ffe/evaluation_metrics_native.phpt +++ b/tests/ext/ffe/evaluation_metrics_native.phpt @@ -53,5 +53,5 @@ old_metrics_forwarder_exists=false old_exposure_forwarder_exists=false recorded=true load=true -evaluation_without_native_metric={"valueJson":"\"blue\"","variant":"blue","allocationKey":"alloc-string","reason":0,"errorCode":0,"doLog":true,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} -missing_flag_without_native_metric={"valueJson":"null","variant":null,"allocationKey":null,"reason":5,"errorCode":3,"doLog":false,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} +evaluation_without_native_metric={"valueJson":"\"blue\"","variant":"blue","allocationKey":"alloc-string","reason":0,"errorCode":0,"doLog":true,"serialId":7,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} +missing_flag_without_native_metric={"valueJson":"null","variant":null,"allocationKey":null,"reason":5,"errorCode":3,"doLog":false,"serialId":null,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} diff --git a/tests/ext/ffe/native_bridge_evaluate.phpt b/tests/ext/ffe/native_bridge_evaluate.phpt index 3b113f1300f..f9fdb8761be 100644 --- a/tests/ext/ffe/native_bridge_evaluate.phpt +++ b/tests/ext/ffe/native_bridge_evaluate.phpt @@ -132,14 +132,14 @@ has_config_before=false native_exposure_flush_exists=true internal_exposure_flush_exists=false old_exposure_forwarder_exists=false -provider_not_ready={"valueJson":"null","variant":null,"allocationKey":null,"reason":5,"errorCode":6,"doLog":false,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} +provider_not_ready={"valueJson":"null","variant":null,"allocationKey":null,"reason":5,"errorCode":6,"doLog":false,"serialId":null,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} load=true has_config_after=true -success={"valueJson":"\"blue\"","variant":"blue","allocationKey":"alloc-string","reason":0,"errorCode":0,"doLog":true,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} +success={"valueJson":"\"blue\"","variant":"blue","allocationKey":"alloc-string","reason":0,"errorCode":0,"doLog":true,"serialId":7,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} object_success_value={"enabled":true,"threshold":2} object_success_metadata={"variant":"json-a","allocation_key":"alloc-json","reason":0,"error_code":0,"do_log":true} -numeric_attribute_key={"valueJson":"\"numeric-attribute-name\"","variant":"numeric-key","allocationKey":"alloc-numeric-attribute","reason":2,"errorCode":0,"doLog":true,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} -empty_targeting_key={"valueJson":"\"empty-targeting-key\"","variant":"empty-target","allocationKey":"alloc-empty-targeting-key","reason":3,"errorCode":0,"doLog":true,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} -missing={"valueJson":"null","variant":null,"allocationKey":null,"reason":5,"errorCode":3,"doLog":false,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} -type_mismatch={"valueJson":"null","variant":null,"allocationKey":null,"reason":5,"errorCode":1,"doLog":false,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} -parse_error={"valueJson":"null","variant":null,"allocationKey":null,"reason":5,"errorCode":2,"doLog":false,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} +numeric_attribute_key={"valueJson":"\"numeric-attribute-name\"","variant":"numeric-key","allocationKey":"alloc-numeric-attribute","reason":2,"errorCode":0,"doLog":true,"serialId":null,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} +empty_targeting_key={"valueJson":"\"empty-targeting-key\"","variant":"empty-target","allocationKey":"alloc-empty-targeting-key","reason":3,"errorCode":0,"doLog":true,"serialId":null,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} +missing={"valueJson":"null","variant":null,"allocationKey":null,"reason":5,"errorCode":3,"doLog":false,"serialId":null,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} +type_mismatch={"valueJson":"null","variant":null,"allocationKey":null,"reason":5,"errorCode":1,"doLog":false,"serialId":null,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} +parse_error={"valueJson":"null","variant":null,"allocationKey":null,"reason":5,"errorCode":2,"doLog":false,"serialId":null,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} diff --git a/tests/ext/ffe/remote_config_lifecycle.phpt b/tests/ext/ffe/remote_config_lifecycle.phpt index 1af6f2d41f3..ea7f571a1ed 100644 --- a/tests/ext/ffe/remote_config_lifecycle.phpt +++ b/tests/ext/ffe/remote_config_lifecycle.phpt @@ -84,7 +84,7 @@ reset_request_replayer(); before=false loaded=true has_config_after_add=true -success={"valueJson":"\"blue\"","variant":"blue","allocationKey":"alloc-string","reason":0,"errorCode":0,"doLog":true,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} +success={"valueJson":"\"blue\"","variant":"blue","allocationKey":"alloc-string","reason":0,"errorCode":0,"doLog":true,"serialId":7,"providerState":[],"errorMessage":null,"hasConfig":null,"configVersion":null} removed=true has_config_after_remove=false version_increased=true diff --git a/tracer/ddtrace.stub.php b/tracer/ddtrace.stub.php index 1499091ad71..adac5b1f10c 100644 --- a/tracer/ddtrace.stub.php +++ b/tracer/ddtrace.stub.php @@ -66,6 +66,7 @@ final class FfeResult { public int $reason = 0; public int $errorCode = 0; public bool $doLog = false; + public ?int $serialId = null; public array $providerState = []; public ?string $errorMessage = null; public ?bool $hasConfig = null; diff --git a/tracer/ddtrace_arginfo.h b/tracer/ddtrace_arginfo.h index 9a9b8a16d69..afa0f62d9f7 100644 --- a/tracer/ddtrace_arginfo.h +++ b/tracer/ddtrace_arginfo.h @@ -673,6 +673,12 @@ static zend_class_entry *register_class_DDTrace_FfeResult(void) zend_declare_typed_property(class_entry, property_doLog_name, &property_doLog_default_value, ZEND_ACC_PUBLIC, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_BOOL)); zend_string_release_ex(property_doLog_name, true); + zval property_serialId_default_value; + ZVAL_NULL(&property_serialId_default_value); + zend_string *property_serialId_name = zend_string_init("serialId", sizeof("serialId") - 1, true); + zend_declare_typed_property(class_entry, property_serialId_name, &property_serialId_default_value, ZEND_ACC_PUBLIC, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_LONG|MAY_BE_NULL)); + zend_string_release_ex(property_serialId_name, true); + zval property_providerState_default_value; ZVAL_EMPTY_ARRAY(&property_providerState_default_value); zend_string *property_providerState_name = zend_string_init("providerState", sizeof("providerState") - 1, true); diff --git a/tracer/functions.c b/tracer/functions.c index aca31566a7a..11aac813c09 100644 --- a/tracer/functions.c +++ b/tracer/functions.c @@ -1878,6 +1878,13 @@ PHP_FUNCTION(DDTrace_ffe_evaluate) { ddtrace_ffe_update_long_property(return_value, ZEND_STRL("reason"), ddtrace_ffe_effective_reason(result.reason, result.error_code)); ddtrace_ffe_update_long_property(return_value, ZEND_STRL("errorCode"), result.error_code); ddtrace_ffe_update_bool_property(return_value, ZEND_STRL("doLog"), result.do_log); + // serialId is only populated when the native result actually carried one + // (has_serial_id). It stays null otherwise so the PHP accumulator can use + // the Pattern B "missing variant => runtime default" branch, rather than + // mistaking a 0 sentinel for a real serial id. + if (result.has_serial_id) { + ddtrace_ffe_update_long_property(return_value, ZEND_STRL("serialId"), (zend_long)result.serial_id); + } ddtrace_ffe_update_empty_array_property(return_value, ZEND_STRL("providerState")); } From 8027165b7304d914e27d94588e196c4d4bb9f5fa Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Mon, 15 Jun 2026 23:31:13 -0400 Subject: [PATCH 02/12] feat(02-05): inline span-enrichment accumulation (DG-004) + codec + root-close write - Add DDTrace\FeatureFlags\SpanEnrichmentAccumulator: per-root-span accumulator + ULEB128 delta-varint/base64/SHA256 codec ported verbatim from the frozen Node reference (dd-trace-js#8343). Limits 200/10/20/5/64, dedupe+sort, object defaults via json_encode, UTF-8-safe 64-char truncation; tag shapes ffe_flags_enc (bare base64), ffe_subjects_enc / ffe_runtime_defaults (JSON objects). - DataDogProvider: accumulate INLINE in resolve() right after recordEvaluationMetric (DG-004, no finally hook); gate-gated lazy accumulator (DG-005 zero-idle); error isolation via try/catch(\Throwable); runtime-default detection via missing variant. - Native request-scoped staging store in tracer/ffe.c (+ ddtrace_globals.h) flushed into the root span meta on the ddtrace_close_span root branch and cleared on root close / RSHUTDOWN (no cross-request leak); gate-off path does no work. - Add DDTrace\Internal\set_ffe_span_enrichment_tags() PHP-callable staging fn. - Tests: SpanEnrichmentAccumulatorTest (7 required L0 cases incl. gate-off control + codec golden round-trip), serial_id_passthrough.phpt (C bridge), ResultMapper serialId threading cases. --- src/DDTrace/OpenFeature/DataDogProvider.php | 98 +++- .../SpanEnrichmentAccumulator.php | 302 ++++++++++++ .../SpanEnrichmentAccumulatorTest.php | 456 ++++++++++++++++++ .../Unit/FeatureFlags/ResultMapperTest.php | 52 ++ tests/ext/ffe/serial_id_passthrough.phpt | 81 ++++ tracer/ddtrace.c | 4 + tracer/ddtrace.stub.php | 12 + tracer/ddtrace_arginfo.h | 8 + tracer/ddtrace_globals.h | 10 + tracer/ffe.c | 76 +++ tracer/ffe.h | 7 + tracer/functions.c | 14 + tracer/span.c | 8 + 13 files changed, 1127 insertions(+), 1 deletion(-) create mode 100644 src/api/FeatureFlags/SpanEnrichmentAccumulator.php create mode 100644 tests/OpenFeature/SpanEnrichmentAccumulatorTest.php create mode 100644 tests/ext/ffe/serial_id_passthrough.phpt diff --git a/src/DDTrace/OpenFeature/DataDogProvider.php b/src/DDTrace/OpenFeature/DataDogProvider.php index 29d182221e7..42e4dea4596 100644 --- a/src/DDTrace/OpenFeature/DataDogProvider.php +++ b/src/DDTrace/OpenFeature/DataDogProvider.php @@ -12,6 +12,7 @@ use DDTrace\FeatureFlags\Internal\Metric\EvaluationMetric; use DDTrace\FeatureFlags\Internal\Metric\EvaluationMetricRecorder; use DDTrace\FeatureFlags\Internal\NativeEvaluator; +use DDTrace\FeatureFlags\SpanEnrichmentAccumulator; use DDTrace\Log\LoggerInterface; use DDTrace\Log\TriggerErrorLogger; use OpenFeature\implementation\provider\AbstractProvider; @@ -26,6 +27,9 @@ final class DataDogProvider extends AbstractProvider { private const ALLOCATION_KEY_METADATA_KEY = 'allocationKey'; + private const SERIAL_ID_METADATA_KEY = 'serialId'; + private const DO_LOG_METADATA_KEY = 'doLog'; + private const SPAN_ENRICHMENT_CONFIG_KEY = 'DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED'; protected static string $NAME = 'Datadog'; @@ -33,6 +37,8 @@ final class DataDogProvider extends AbstractProvider private LoggerInterface $warningLogger; private bool $warnedAboutNonProductionRuntime = false; private EvaluationMetricRecorder $metricRecorder; + private bool $spanEnrichmentEnabled = false; + private ?SpanEnrichmentAccumulator $spanEnrichmentAccumulator = null; public function __construct(?LoggerInterface $logger = null) { @@ -41,6 +47,24 @@ public function __construct(?LoggerInterface $logger = null) $this->evaluator = NativeEvaluator::create(false); $this->warningLogger = $logger ?: new TriggerErrorLogger(); $this->metricRecorder = EvaluationMetricRecorder::createDefault(); + + // DG-005: only read the gate / allocate the accumulator when the + // experimental span-enrichment feature is opted in. With the gate off we + // construct nothing and never stage anything, so the close-span write + // path stays a cheap no-op and there is no idle per-span overhead. + $this->spanEnrichmentEnabled = self::spanEnrichmentGateEnabled(); + if ($this->spanEnrichmentEnabled) { + $this->spanEnrichmentAccumulator = new SpanEnrichmentAccumulator(); + } + } + + private static function spanEnrichmentGateEnabled(): bool + { + // Mirrors EvaluationMetricRecorder's gate read. dd_trace_env_config + // resolves the extension-managed config; absent (extension not loaded) + // means the feature is off. + return \function_exists('dd_trace_env_config') + && \dd_trace_env_config(self::SPAN_ENRICHMENT_CONFIG_KEY) === true; } /** @@ -111,12 +135,18 @@ private function resolve( mixed $defaultValue, ?EvaluationContext $context ): ResolutionDetailsInterface { - $details = $this->evaluate($flagKey, $expectedType, $defaultValue, $this->normalizeContext($context)); + $normalizedContext = $this->normalizeContext($context); + $details = $this->evaluate($flagKey, $expectedType, $defaultValue, $normalizedContext); $this->warnIfNonProductionRuntime($details); // The PHP OpenFeature SDK does not pass ResolutionDetails to finally // hooks, so PHP records metrics here after native evaluation has the // final provider result. $this->recordEvaluationMetric($flagKey, $details); + // DG-004: PHP has no finally hook with ResolutionDetails, so APM span + // enrichment is accumulated INLINE here, on the same code path and from + // the same $details, immediately after the metrics hook. No-op when the + // gate is off. + $this->accumulateSpanEnrichment($flagKey, $details, $normalizedContext['targetingKey'] ?? null); $builder = (new ResolutionDetailsBuilder()) ->withValue($details->getValue()) @@ -161,6 +191,72 @@ private function allocationKey(EvaluationDetails $details): ?string return $exposure[$key] !== '' ? $exposure[$key] : null; } + /** + * Accumulate APM span enrichment data from a single evaluation (DG-004 + * inline path). Mirrors the frozen Node reference branch: a present serial + * id is recorded (and, when do_log authorizes and a targeting key exists, a + * subject); an evaluation with no variant is treated as a runtime default. + * Errors are swallowed (Pattern D) — enrichment must never break the eval. + */ + private function accumulateSpanEnrichment( + string $flagKey, + EvaluationDetails $details, + ?string $targetingKey + ): void { + if (!$this->spanEnrichmentEnabled || $this->spanEnrichmentAccumulator === null) { + return; + } + + try { + $exposure = $details->getExposureData(); + $serialId = is_array($exposure) && array_key_exists(self::SERIAL_ID_METADATA_KEY, $exposure) + ? $exposure[self::SERIAL_ID_METADATA_KEY] + : null; + $doLog = is_array($exposure) && !empty($exposure[self::DO_LOG_METADATA_KEY]); + + if ($serialId !== null) { + $this->spanEnrichmentAccumulator->addSerialId((int) $serialId); + if ($doLog && $targetingKey !== null && $targetingKey !== '') { + $this->spanEnrichmentAccumulator->addSubject($targetingKey, (int) $serialId); + } + } else { + // Runtime-default detection = MISSING variant (Pattern C), never + // a reason enum. + $variant = $details->getVariant(); + if ($variant === null || $variant === '') { + $this->spanEnrichmentAccumulator->addDefault($flagKey, $details->getValue()); + } + } + + $this->stageSpanEnrichmentTags(); + } catch (\Throwable $e) { + // Never let span enrichment break flag evaluation. + $this->warningLogger->debug('FFE span enrichment accumulation failed: ' . $e->getMessage()); + } + } + + /** + * Push the currently-accumulated, encoded tag set into native request-local + * memory so the root-span close path can write it onto the root span. Only + * stages when there is data; the native side stays empty (and the close-span + * write is a no-op) otherwise. + */ + private function stageSpanEnrichmentTags(): void + { + if ($this->spanEnrichmentAccumulator === null + || !$this->spanEnrichmentAccumulator->hasData() + || !\function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')) { + return; + } + + $tags = $this->spanEnrichmentAccumulator->toSpanTags(); + \DDTrace\Internal\set_ffe_span_enrichment_tags( + $tags[SpanEnrichmentAccumulator::TAG_FLAGS] ?? null, + $tags[SpanEnrichmentAccumulator::TAG_SUBJECTS] ?? null, + $tags[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS] ?? null + ); + } + /** * @param bool|string|int|float|array $defaultValue * @param array $context diff --git a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php new file mode 100644 index 00000000000..e82061ea352 --- /dev/null +++ b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php @@ -0,0 +1,302 @@ + Set of unique serial ids (dedupe-before-encode). */ + private $serialIds = array(); + + /** @var array> sha256hex => set of serial ids. */ + private $subjects = array(); + + /** @var array flagKey => stringified default value. */ + private $defaults = array(); + + /** + * Record a serial id seen during evaluation. Deduped via a set; dropped + * (with no error) once the frozen cap is reached. + */ + public function addSerialId($id) + { + $id = (int) $id; + if (isset($this->serialIds[$id])) { + return; + } + if (count($this->serialIds) >= self::MAX_SERIAL_IDS) { + return; + } + $this->serialIds[$id] = true; + } + + /** + * Associate a serial id with a (hashed) subject. The targeting key is + * SHA256-hashed before storage (privacy contract DG-003) and is only ever + * called by the provider when `do_log` authorizes it. + */ + public function addSubject($targetingKey, $id) + { + $id = (int) $id; + $hashed = $this->hashTargetingKey((string) $targetingKey); + + if (isset($this->subjects[$hashed])) { + if (isset($this->subjects[$hashed][$id])) { + return; + } + if (count($this->subjects[$hashed]) >= self::MAX_EXPERIMENTS_PER_SUBJECT) { + return; + } + $this->subjects[$hashed][$id] = true; + return; + } + + if (count($this->subjects) >= self::MAX_SUBJECTS) { + return; + } + $this->subjects[$hashed] = array($id => true); + } + + /** + * Record a runtime-default value for a flag (first-wins). Object/array + * values are JSON-encoded (never the implicit "Array"/"[object Object]" + * cast); the stringified value is truncated to the frozen length budget in + * a UTF-8-safe manner. + * + * @param mixed $value + */ + public function addDefault($flagKey, $value) + { + $flagKey = (string) $flagKey; + if (array_key_exists($flagKey, $this->defaults)) { + return; + } + if (count($this->defaults) >= self::MAX_DEFAULTS) { + return; + } + + $this->defaults[$flagKey] = $this->stringifyDefault($value); + } + + /** + * Whether the accumulator carries anything worth writing. Mirrors the Node + * reference: subjects are intentionally NOT checked, because addSubject is + * never reached without a preceding addSerialId. + */ + public function hasData() + { + return count($this->serialIds) > 0 || count($this->defaults) > 0; + } + + /** + * Encode the accumulated state into the frozen `ffe_*` span tag set. + * + * Output-shape contract (Pattern F): + * - ffe_flags_enc => BARE base64 string + * - ffe_subjects_enc => JSON-stringified object {sha256hex: base64} + * - ffe_runtime_defaults => JSON-stringified object {flagKey: valueStr} + * + * Empty components are omitted entirely. + * + * @return array + */ + public function toSpanTags() + { + $tags = array(); + + if (count($this->serialIds) > 0) { + $tags[self::TAG_FLAGS] = $this->encodeDeltaVarint(array_keys($this->serialIds)); + } + + if (count($this->subjects) > 0) { + $encodedSubjects = array(); + foreach ($this->subjects as $hashed => $ids) { + $encodedSubjects[$hashed] = $this->encodeDeltaVarint(array_keys($ids)); + } + $tags[self::TAG_SUBJECTS] = json_encode($encodedSubjects); + } + + if (count($this->defaults) > 0) { + $tags[self::TAG_RUNTIME_DEFAULTS] = json_encode($this->defaults); + } + + return $tags; + } + + /** + * Reset all accumulated state. Called after the tags are flushed onto the + * root span so a reused accumulator never leaks across spans/requests. + */ + public function clear() + { + $this->serialIds = array(); + $this->subjects = array(); + $this->defaults = array(); + } + + /** + * ULEB128 delta-varint + base64 encoder (frozen). + * + * Rules (must match Node + the L2 decoder exactly): dedupe via set, sort + * ascending, delta-from-previous (first delta = first value), UNSIGNED + * LEB128 (7 bits/byte, MSB = continuation). Empty input encodes to "" so + * the caller omits the tag. + * + * Golden vector: [100,108,128,130] => "ZAgUAg==". + * + * @param array $ids + * @return string + */ + private function encodeDeltaVarint(array $ids) + { + if (count($ids) === 0) { + return ''; + } + + $unique = array(); + foreach ($ids as $id) { + $unique[(int) $id] = true; + } + $sorted = array_keys($unique); + sort($sorted, SORT_NUMERIC); + + $buffer = ''; + $prev = 0; + foreach ($sorted as $id) { + $delta = $id - $prev; + $prev = $id; + + // Unsigned LEB128 of the non-negative delta. + while ($delta > 0x7F) { + $buffer .= chr(($delta & 0x7F) | 0x80); + $delta >>= 7; + } + $buffer .= chr($delta & 0x7F); + } + + return base64_encode($buffer); + } + + /** + * Decode a delta-varint base64 string back into the serial-id set. Provided + * for round-trip self-tests (the L2 `utils.py` decoder is the authority); + * not used on the write path. + * + * @return array + */ + public function decodeDeltaVarint($encoded) + { + $ids = array(); + if (!is_string($encoded) || $encoded === '') { + return $ids; + } + + $bytes = base64_decode($encoded, true); + if ($bytes === false) { + return $ids; + } + + $prev = 0; + $shift = 0; + $delta = 0; + $length = strlen($bytes); + for ($i = 0; $i < $length; $i++) { + $byte = ord($bytes[$i]); + $delta |= ($byte & 0x7F) << $shift; + if (($byte & 0x80) === 0) { + $prev += $delta; + $ids[] = $prev; + $delta = 0; + $shift = 0; + } else { + $shift += 7; + } + } + + return $ids; + } + + /** + * Lowercase hex SHA256 of the targeting key (frozen; stdlib ext-hash). + */ + private function hashTargetingKey($key) + { + return hash('sha256', $key); + } + + /** + * @param mixed $value + * @return string + */ + private function stringifyDefault($value) + { + if (is_array($value) || is_object($value)) { + $encoded = json_encode($value); + $valueStr = $encoded === false ? '' : $encoded; + } elseif (is_bool($value)) { + // Match the Node String(boolean) form: "true"/"false". + $valueStr = $value ? 'true' : 'false'; + } elseif ($value === null) { + $valueStr = 'null'; + } else { + $valueStr = (string) $value; + } + + return $this->truncateUtf8($valueStr, self::MAX_DEFAULT_VALUE_LENGTH); + } + + /** + * Truncate to at most $maxLength characters without splitting a multi-byte + * UTF-8 sequence. Falls back to a byte-safe trim if the multibyte helpers + * are unavailable. + */ + private function truncateUtf8($value, $maxLength) + { + if (function_exists('mb_substr') && function_exists('mb_strlen')) { + if (mb_strlen($value, 'UTF-8') <= $maxLength) { + return $value; + } + return mb_substr($value, 0, $maxLength, 'UTF-8'); + } + + if (strlen($value) <= $maxLength) { + return $value; + } + + $truncated = substr($value, 0, $maxLength); + // Drop a trailing partial multi-byte sequence (bytes with 0x80 bit set + // that do not form a complete code point). + $i = strlen($truncated); + while ($i > 0 && (ord($truncated[$i - 1]) & 0xC0) === 0x80) { + $i--; + } + if ($i > 0 && (ord($truncated[$i - 1]) & 0x80) !== 0) { + $i--; + } + + return substr($truncated, 0, $i); + } +} diff --git a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php new file mode 100644 index 00000000000..38326c6b1bf --- /dev/null +++ b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php @@ -0,0 +1,456 @@ +addSerialId(100); + $acc->addSerialId(108); + $acc->addSerialId(128); + $acc->addSerialId(130); + + $tags = $acc->toSpanTags(); + + // [100,108,128,130] -> deltas [100,8,20,2] -> ULEB128 [0x64,0x08,0x14,0x02] + // -> base64 "ZAgUAg==". This is the frozen oracle shared with the L2 + // decode side; any divergence breaks backend/Trino parity. + self::assertSame('ZAgUAg==', $tags[SpanEnrichmentAccumulator::TAG_FLAGS]); + } + + public function testCodecRoundTrips(): void + { + $acc = new SpanEnrichmentAccumulator(); + $input = [130, 100, 108, 128, 100]; // out of order + a duplicate + foreach ($input as $id) { + $acc->addSerialId($id); + } + + $encoded = $acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_FLAGS]; + $decoded = $acc->decodeDeltaVarint($encoded); + + self::assertSame([100, 108, 128, 130], $decoded); + } + + public function testCodecMultiByteVarint(): void + { + // 300 needs two ULEB128 bytes: delta 300 = 0b100101100 -> + // [0xAC, 0x02]. Round-tripping proves the continuation-bit handling. + $acc = new SpanEnrichmentAccumulator(); + $acc->addSerialId(300); + $acc->addSerialId(301); + + $decoded = $acc->decodeDeltaVarint( + $acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_FLAGS] + ); + + self::assertSame([300, 301], $decoded); + } + + public function testEmptySerialIdsOmitsFlagsTag(): void + { + $acc = new SpanEnrichmentAccumulator(); + + self::assertFalse($acc->hasData()); + self::assertArrayNotHasKey(SpanEnrichmentAccumulator::TAG_FLAGS, $acc->toSpanTags()); + } + + // ---- dedupe + sort ---------------------------------------------------- + + public function testSerialIdsAreDedupedAndSorted(): void + { + $acc = new SpanEnrichmentAccumulator(); + foreach ([5, 1, 5, 3, 1] as $id) { + $acc->addSerialId($id); + } + + $decoded = $acc->decodeDeltaVarint( + $acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_FLAGS] + ); + + self::assertSame([1, 3, 5], $decoded); + } + + // ---- Case 4: limits --------------------------------------------------- + + public function testMax200SerialIdsEnforced(): void + { + $acc = new SpanEnrichmentAccumulator(); + for ($i = 1; $i <= 250; $i++) { + $acc->addSerialId($i); + } + + $decoded = $acc->decodeDeltaVarint( + $acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_FLAGS] + ); + + self::assertCount(200, $decoded); + self::assertSame(1, $decoded[0]); + self::assertSame(200, $decoded[199]); + } + + public function testMax10SubjectsEnforced(): void + { + $acc = new SpanEnrichmentAccumulator(); + for ($i = 1; $i <= 15; $i++) { + $acc->addSubject('subject-' . $i, $i); + } + + $subjects = json_decode($acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_SUBJECTS], true); + + self::assertCount(10, $subjects); + } + + public function testMax20ExperimentsPerSubjectEnforced(): void + { + $acc = new SpanEnrichmentAccumulator(); + $acc->addSerialId(1); // ensure hasData() + for ($i = 1; $i <= 25; $i++) { + $acc->addSubject('user-123', $i); + } + + $subjects = json_decode($acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_SUBJECTS], true); + $hashed = hash('sha256', 'user-123'); + $decoded = $acc->decodeDeltaVarint($subjects[$hashed]); + + self::assertCount(20, $decoded); + } + + public function testMax5DefaultsEnforced(): void + { + $acc = new SpanEnrichmentAccumulator(); + for ($i = 1; $i <= 8; $i++) { + $acc->addDefault('flag-' . $i, 'value-' . $i); + } + + $defaults = json_decode($acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS], true); + + self::assertCount(5, $defaults); + } + + public function testDefaultIsFirstWins(): void + { + $acc = new SpanEnrichmentAccumulator(); + $acc->addDefault('flag', 'first'); + $acc->addDefault('flag', 'second'); + + $defaults = json_decode($acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS], true); + + self::assertSame('first', $defaults['flag']); + } + + public function testDefaultValueTruncatedTo64Chars(): void + { + $acc = new SpanEnrichmentAccumulator(); + $acc->addDefault('flag', str_repeat('a', 100)); + + $defaults = json_decode($acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS], true); + + self::assertSame(64, strlen($defaults['flag'])); + } + + public function testDefaultValueTruncationIsUtf8Safe(): void + { + // 80 two-byte characters (160 bytes) — exceeds the 64-CHARACTER budget. + // A naive 64-byte cut would split a multi-byte char and corrupt the + // string; the UTF-8-safe truncation keeps exactly 64 whole characters. + $acc = new SpanEnrichmentAccumulator(); + $acc->addDefault('flag', str_repeat("\u{00e9}", 80)); + + $defaults = json_decode($acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS], true); + + // Valid UTF-8 (json_decode would have returned null on a broken string). + self::assertNotNull($defaults); + self::assertSame(64, mb_strlen($defaults['flag'], 'UTF-8')); + } + + // ---- Case 5: JSON / object default ------------------------------------ + + public function testObjectDefaultIsJsonStringified(): void + { + $acc = new SpanEnrichmentAccumulator(); + $acc->addDefault('flag', ['enabled' => true, 'n' => 3]); + + $defaults = json_decode($acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS], true); + + // Must be JSON, never "Array"/"[object Object]". + self::assertSame('{"enabled":true,"n":3}', $defaults['flag']); + } + + public function testSubjectsTagIsJsonObjectOfBase64(): void + { + $acc = new SpanEnrichmentAccumulator(); + $acc->addSerialId(100); + $acc->addSubject('user-123', 100); + + $raw = $acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_SUBJECTS]; + $subjects = json_decode($raw, true); + + self::assertIsArray($subjects); + $hashed = hash('sha256', 'user-123'); + // SHA256("user-123") is the frozen fixture digest. + self::assertSame( + 'fcdec6df4d44dbc637c7c5b58efface52a7f8a88535423430255be0bb89bedd8', + $hashed + ); + self::assertArrayHasKey($hashed, $subjects); + self::assertSame([100], $acc->decodeDeltaVarint($subjects[$hashed])); + } + + public function testFlagsTagIsBareBase64NotJson(): void + { + $acc = new SpanEnrichmentAccumulator(); + $acc->addSerialId(100); + + $flags = $acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_FLAGS]; + + // Bare base64 string, NOT a JSON-wrapped value. + self::assertSame('"', substr(json_encode($flags), 0, 1)); + self::assertNull(json_decode($flags, true)); + self::assertSame($flags, base64_encode(base64_decode($flags, true))); + } + + public function testClearResetsState(): void + { + $acc = new SpanEnrichmentAccumulator(); + $acc->addSerialId(1); + $acc->addDefault('flag', 'value'); + $acc->addSubject('user', 1); + + $acc->clear(); + + self::assertFalse($acc->hasData()); + self::assertSame([], $acc->toSpanTags()); + } + + // ---- DG-004 inline accumulation via DataDogProvider ------------------ + + public function testInlineAccumulationRecordsSerialIdAndSubject(): void + { + $accumulator = new SpanEnrichmentAccumulator(); + $provider = $this->providerWithAccumulator($accumulator); + + $provider->resolveStringValue('flag', 'fallback', new EvaluationContext('user-123')); + + self::assertTrue($accumulator->hasData()); + $tags = $accumulator->toSpanTags(); + self::assertSame([4242], $accumulator->decodeDeltaVarint($tags[SpanEnrichmentAccumulator::TAG_FLAGS])); + // do_log was true and a targeting key was present -> a subject is added. + $subjects = json_decode($tags[SpanEnrichmentAccumulator::TAG_SUBJECTS], true); + self::assertArrayHasKey(hash('sha256', 'user-123'), $subjects); + } + + public function testInlineAccumulationSkipsSubjectWhenDoLogFalse(): void + { + $accumulator = new SpanEnrichmentAccumulator(); + $evaluator = new SpanEnrichmentTestEvaluator(); + $evaluator->setResult('flag', 'on', 'on', ['serialId' => 7, 'doLog' => false]); + $provider = $this->providerWithAccumulator($accumulator, $evaluator); + + $provider->resolveStringValue('flag', 'fallback', new EvaluationContext('user-123')); + + $tags = $accumulator->toSpanTags(); + self::assertArrayHasKey(SpanEnrichmentAccumulator::TAG_FLAGS, $tags); + self::assertArrayNotHasKey(SpanEnrichmentAccumulator::TAG_SUBJECTS, $tags); + } + + // ---- Case 3: error / default variant = runtime default --------------- + + public function testInlineAccumulationRuntimeDefaultOnMissingVariant(): void + { + $accumulator = new SpanEnrichmentAccumulator(); + $evaluator = new SpanEnrichmentTestEvaluator(); + // No serialId, no variant -> runtime default (Pattern C). + $evaluator->setResult('flag', 'computed-default', null, []); + $provider = $this->providerWithAccumulator($accumulator, $evaluator); + + $provider->resolveStringValue('flag', 'fallback', new EvaluationContext('user-123')); + + $defaults = json_decode( + $accumulator->toSpanTags()[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS], + true + ); + self::assertSame('computed-default', $defaults['flag']); + } + + public function testInlineAccumulationDoesNotRaiseWithoutTargetingKey(): void + { + // Case "no-span"/no-context analog: accumulation must not throw when + // there is no targeting key. The serial id is still recorded; no subject. + $accumulator = new SpanEnrichmentAccumulator(); + $provider = $this->providerWithAccumulator($accumulator); + + $provider->resolveStringValue('flag', 'fallback'); + + $tags = $accumulator->toSpanTags(); + self::assertArrayHasKey(SpanEnrichmentAccumulator::TAG_FLAGS, $tags); + self::assertArrayNotHasKey(SpanEnrichmentAccumulator::TAG_SUBJECTS, $tags); + } + + // ---- Case 6: gate-off negative control (DG-005) ---------------------- + + public function testGateOffAllocatesNoAccumulatorAndStagesNothing(): void + { + // With the gate off the provider must construct NO accumulator at all + // (DG-005 zero-idle). We assert that via reflection on a default-built + // provider; in this unit context dd_trace_env_config is unavailable, so + // the gate reads as off. + $provider = new DataDogProvider(new NullLogger(LogLevel::EMERGENCY)); + + $enabledProp = self::accessibleProperty($provider, 'spanEnrichmentEnabled'); + $accProp = self::accessibleProperty($provider, 'spanEnrichmentAccumulator'); + + self::assertFalse($enabledProp->getValue($provider)); + self::assertNull($accProp->getValue($provider)); + } + + public function testGateOffResolveProducesNoSpanTags(): void + { + $evaluator = new SpanEnrichmentTestEvaluator(); + $evaluator->setResult('flag', 'on', 'on', ['serialId' => 99, 'doLog' => true]); + // Gate off: createWithDependencies leaves spanEnrichmentEnabled false. + $provider = DataDogProvider::createWithDependencies( + $evaluator, + new NullLogger(LogLevel::EMERGENCY) + ); + + $provider->resolveStringValue('flag', 'fallback', new EvaluationContext('user-123')); + + $accProp = self::accessibleProperty($provider, 'spanEnrichmentAccumulator'); + + // No accumulator was constructed and nothing was accumulated. + self::assertNull($accProp->getValue($provider)); + } + + // ---- helpers ---------------------------------------------------------- + + /** + * Make a private property accessible across PHP versions without emitting + * the PHP 8.1+ "setAccessible has no effect" deprecation notice (which would + * mark the test risky). On <8.1 setAccessible(true) is still required. + */ + private static function accessibleProperty(object $object, string $name): \ReflectionProperty + { + $property = (new \ReflectionObject($object))->getProperty($name); + if (\PHP_VERSION_ID < 80100) { + $property->setAccessible(true); + } + + return $property; + } + + private function providerWithAccumulator( + SpanEnrichmentAccumulator $accumulator, + ?Evaluator $evaluator = null + ): DataDogProvider { + if ($evaluator === null) { + $evaluator = new SpanEnrichmentTestEvaluator(); + $evaluator->setResult('flag', 'on', 'on', ['serialId' => 4242, 'doLog' => true]); + } + + $provider = DataDogProvider::createWithDependencies( + $evaluator, + new NullLogger(LogLevel::EMERGENCY) + ); + + // Force-enable the feature and inject a test accumulator so we can + // inspect the accumulated state without the native extension. + self::accessibleProperty($provider, 'spanEnrichmentEnabled')->setValue($provider, true); + self::accessibleProperty($provider, 'spanEnrichmentAccumulator')->setValue($provider, $accumulator); + + return $provider; + } +} + +/** + * Minimal Evaluator that returns canned EvaluationDetails (with exposureData + * carrying serialId/doLog) so the provider's inline accumulation can be tested + * without the native bridge. + */ +final class SpanEnrichmentTestEvaluator implements Evaluator +{ + /** @var array */ + private array $results = []; + + /** + * @param mixed $value + * @param array $exposureData + */ + public function setResult(string $flagKey, $value, ?string $variant, array $exposureData): self + { + $this->results[$flagKey] = new EvaluationDetails( + $value, + $this->typeForValue($value), + EvaluationReason::TARGETING_MATCH, + $variant, + null, + null, + [], + $exposureData, + [] + ); + + return $this; + } + + public function evaluate($flagKey, $expectedType, $defaultValue, $targetingKey = null, array $attributes = []) + { + if (isset($this->results[$flagKey])) { + return $this->results[$flagKey]; + } + + return new EvaluationDetails( + $defaultValue, + EvaluationType::STRING, + EvaluationReason::DEFAULT_REASON + ); + } + + /** + * @param mixed $value + */ + private function typeForValue($value): string + { + if (is_bool($value)) { + return EvaluationType::BOOLEAN; + } + if (is_int($value)) { + return EvaluationType::INTEGER; + } + if (is_float($value)) { + return EvaluationType::FLOAT; + } + if (is_array($value)) { + return EvaluationType::OBJECT; + } + return EvaluationType::STRING; + } +} diff --git a/tests/api/Unit/FeatureFlags/ResultMapperTest.php b/tests/api/Unit/FeatureFlags/ResultMapperTest.php index 1f9d4f81e93..075a45d9da6 100644 --- a/tests/api/Unit/FeatureFlags/ResultMapperTest.php +++ b/tests/api/Unit/FeatureFlags/ResultMapperTest.php @@ -227,4 +227,56 @@ public function testMapsObjectBridgeResultToEvaluationDetails() $details->getProviderState() ); } + + public function testThreadsSerialIdIntoExposureDataWhenPresent() + { + $details = (new ResultMapper())->map(array( + 'value_json' => '"blue"', + 'variant' => 'variant-a', + 'allocation_key' => 'alloc-1', + 'reason' => ResultMapper::BRIDGE_REASON_SPLIT, + 'error_code' => ResultMapper::BRIDGE_ERROR_NONE, + 'do_log' => true, + 'serial_id' => 4242, + ), EvaluationType::STRING, 'red'); + + $this->assertSame( + array('allocationKey' => 'alloc-1', 'doLog' => true, 'serialId' => 4242), + $details->getExposureData() + ); + $this->assertIsInt($details->getExposureData()['serialId']); + } + + public function testThreadsCamelCaseSerialIdFromObjectResult() + { + $rawResult = new \stdClass(); + $rawResult->valueJson = '"green"'; + $rawResult->variant = 'variant-b'; + $rawResult->allocationKey = 'alloc-2'; + $rawResult->reason = ResultMapper::BRIDGE_REASON_SPLIT; + $rawResult->errorCode = ResultMapper::BRIDGE_ERROR_NONE; + $rawResult->doLog = true; + $rawResult->serialId = 7; + + $details = (new ResultMapper())->map($rawResult, EvaluationType::STRING, 'fallback'); + + $this->assertSame( + array('allocationKey' => 'alloc-2', 'doLog' => true, 'serialId' => 7), + $details->getExposureData() + ); + } + + public function testOmitsSerialIdFromExposureDataWhenAbsent() + { + $details = (new ResultMapper())->map(array( + 'value_json' => '"blue"', + 'variant' => 'variant-a', + 'allocation_key' => 'alloc-1', + 'reason' => ResultMapper::BRIDGE_REASON_SPLIT, + 'error_code' => ResultMapper::BRIDGE_ERROR_NONE, + 'do_log' => true, + ), EvaluationType::STRING, 'red'); + + $this->assertArrayNotHasKey('serialId', $details->getExposureData()); + } } diff --git a/tests/ext/ffe/serial_id_passthrough.phpt b/tests/ext/ffe/serial_id_passthrough.phpt new file mode 100644 index 00000000000..25d65e2f06c --- /dev/null +++ b/tests/ext/ffe/serial_id_passthrough.phpt @@ -0,0 +1,81 @@ +--TEST-- +FFE native bridge surfaces the split serial_id (Rust FfeResult -> C reader -> PHP) for APM span enrichment +--FILE-- +serialId); + +$config = <<<'JSON' +{ + "createdAt": "2024-01-01T00:00:00Z", + "environment": {"name": "test"}, + "flags": { + "string.flag": { + "key": "string.flag", + "enabled": true, + "variationType": "STRING", + "variations": { + "blue": {"key": "blue", "value": "blue"} + }, + "allocations": [{ + "key": "alloc-string", + "rules": [], + "splits": [{"variationKey": "blue", "serialId": 4242, "shards": []}], + "doLog": true + }] + }, + "no.serial.flag": { + "key": "no.serial.flag", + "enabled": true, + "variationType": "STRING", + "variations": { + "green": {"key": "green", "value": "green"} + }, + "allocations": [{ + "key": "alloc-no-serial", + "rules": [], + "splits": [{"variationKey": "green", "shards": []}], + "doLog": false + }] + } + } +} +JSON; + +show('load', \DDTrace\Testing\ffe_load_config($config)); + +// A split that declares serialId surfaces it as an int on the result object. +$withSerial = \DDTrace\ffe_evaluate('string.flag', 0, 'user-1', array()); +show('with_serial_variant', $withSerial->variant); +show('with_serial_id', $withSerial->serialId); +show('with_serial_id_is_int', is_int($withSerial->serialId)); + +// A split that omits serialId leaves it null (Pattern B: absence != 0). This is +// what lets the PHP accumulator treat the evaluation as a runtime default rather +// than mistaking a 0 sentinel for a real serial id. +$noSerial = \DDTrace\ffe_evaluate('no.serial.flag', 0, 'user-1', array()); +show('no_serial_variant', $noSerial->variant); +show('no_serial_id', $noSerial->serialId); + +// Errors / unknown flags also carry a null serial id. +$missing = \DDTrace\ffe_evaluate('missing.flag', 0, 'user-1', array()); +show('missing_serial', $missing->serialId); + +// The native staging entrypoint used by the provider is registered. +show('stage_fn_exists', function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')); +?> +--EXPECT-- +not_ready_serial=null +load=true +with_serial_variant="blue" +with_serial_id=4242 +with_serial_id_is_int=true +no_serial_variant="green" +no_serial_id=null +missing_serial=null +stage_fn_exists=true diff --git a/tracer/ddtrace.c b/tracer/ddtrace.c index 7f5e9681d92..67cdffe5e0a 100644 --- a/tracer/ddtrace.c +++ b/tracer/ddtrace.c @@ -612,6 +612,10 @@ void ddtrace_rshutdown(bool fast_shutdown) { ddtrace_ffe_flush_exposures(); ddtrace_ffe_flush_evaluation_metrics(); + // Drop any APM feature-flag span enrichment tags that were staged but never + // flushed (e.g. the root span was dropped, or the request ended without a + // root close). Request-scoped: prevents cross-request leakage (DG-005). + ddtrace_ffe_clear_span_enrichment_tags(); ddtrace_clean_git_object(); ddtrace_weak_resources_rshutdown(); diff --git a/tracer/ddtrace.stub.php b/tracer/ddtrace.stub.php index adac5b1f10c..a8ec9f7bb94 100644 --- a/tracer/ddtrace.stub.php +++ b/tracer/ddtrace.stub.php @@ -1093,6 +1093,18 @@ function record_ffe_evaluation_metric(string $flagKey, ?string $variant, ?string */ function flush_ffe_evaluation_metrics(): bool {} + /** + * Stage the encoded APM feature-flag span enrichment tags in native + * request-local memory. The values are written onto the root span's meta + * when the root span closes and cleared afterwards (request-scoped). Passing + * null/empty for a tag clears that slot. This is gated by + * DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED; the provider + * only calls it when the gate is on (DG-004 inline accumulation / DG-005). + * + * @internal + */ + function set_ffe_span_enrichment_tags(?string $flagsEnc, ?string $subjectsEnc, ?string $runtimeDefaults): void {} + } namespace datadog\appsec\v2 { diff --git a/tracer/ddtrace_arginfo.h b/tracer/ddtrace_arginfo.h index afa0f62d9f7..77d8bbb2da4 100644 --- a/tracer/ddtrace_arginfo.h +++ b/tracer/ddtrace_arginfo.h @@ -258,6 +258,12 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_DDTrace_Internal_flush_ffe_evaluation_metrics, 0, 0, _IS_BOOL, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_DDTrace_Internal_set_ffe_span_enrichment_tags, 0, 3, IS_VOID, 0) + ZEND_ARG_TYPE_INFO(0, flagsEnc, IS_STRING, 1) + ZEND_ARG_TYPE_INFO(0, subjectsEnc, IS_STRING, 1) + ZEND_ARG_TYPE_INFO(0, runtimeDefaults, IS_STRING, 1) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_datadog_appsec_v2_track_user_login_success, 0, 1, IS_VOID, 0) ZEND_ARG_TYPE_INFO(0, login, IS_STRING, 0) ZEND_ARG_TYPE_MASK(0, user, MAY_BE_STRING|MAY_BE_ARRAY|MAY_BE_NULL, "null") @@ -445,6 +451,7 @@ ZEND_FUNCTION(DDTrace_Internal_add_span_flag); ZEND_FUNCTION(DDTrace_Internal_handle_fork); ZEND_FUNCTION(DDTrace_Internal_record_ffe_evaluation_metric); ZEND_FUNCTION(DDTrace_Internal_flush_ffe_evaluation_metrics); +ZEND_FUNCTION(DDTrace_Internal_set_ffe_span_enrichment_tags); ZEND_FUNCTION(datadog_appsec_v2_track_user_login_success); ZEND_FUNCTION(datadog_appsec_v2_track_user_login_failure); ZEND_FUNCTION(dd_trace_env_config); @@ -547,6 +554,7 @@ static const zend_function_entry ext_functions[] = { ZEND_RAW_FENTRY(ZEND_NS_NAME("DDTrace\\Internal", "handle_fork"), zif_DDTrace_Internal_handle_fork, arginfo_DDTrace_Internal_handle_fork, 0, NULL, NULL) ZEND_RAW_FENTRY(ZEND_NS_NAME("DDTrace\\Internal", "record_ffe_evaluation_metric"), zif_DDTrace_Internal_record_ffe_evaluation_metric, arginfo_DDTrace_Internal_record_ffe_evaluation_metric, 0, NULL, NULL) ZEND_RAW_FENTRY(ZEND_NS_NAME("DDTrace\\Internal", "flush_ffe_evaluation_metrics"), zif_DDTrace_Internal_flush_ffe_evaluation_metrics, arginfo_DDTrace_Internal_flush_ffe_evaluation_metrics, 0, NULL, NULL) + ZEND_RAW_FENTRY(ZEND_NS_NAME("DDTrace\\Internal", "set_ffe_span_enrichment_tags"), zif_DDTrace_Internal_set_ffe_span_enrichment_tags, arginfo_DDTrace_Internal_set_ffe_span_enrichment_tags, 0, NULL, NULL) ZEND_RAW_FENTRY(ZEND_NS_NAME("datadog\\appsec\\v2", "track_user_login_success"), zif_datadog_appsec_v2_track_user_login_success, arginfo_datadog_appsec_v2_track_user_login_success, 0, NULL, NULL) ZEND_RAW_FENTRY(ZEND_NS_NAME("datadog\\appsec\\v2", "track_user_login_failure"), zif_datadog_appsec_v2_track_user_login_failure, arginfo_datadog_appsec_v2_track_user_login_failure, 0, NULL, NULL) ZEND_FE(dd_trace_env_config, arginfo_dd_trace_env_config) diff --git a/tracer/ddtrace_globals.h b/tracer/ddtrace_globals.h index 3865b2e48f9..19f44dcbea2 100644 --- a/tracer/ddtrace_globals.h +++ b/tracer/ddtrace_globals.h @@ -100,6 +100,16 @@ typedef struct { size_t ffe_metric_buffer_len; size_t ffe_metric_buffer_cap; + // Request-scoped, gate-gated staging area for the APM feature-flag span + // enrichment tags (ffe_flags_enc / ffe_subjects_enc / ffe_runtime_defaults). + // The OpenFeature provider stages the latest encoded values here while + // evaluating; the root-span close path flushes them into the root span meta + // and clears them. All three are NULL when the gate is off (DG-005: no idle + // allocation) or when nothing has been accumulated yet. + zend_string *ffe_span_flags_enc; + zend_string *ffe_span_subjects_enc; + zend_string *ffe_span_runtime_defaults; + } ddtrace_globals; #define DDTRACE_G(v) (DATADOG_G(ddtrace).v) diff --git a/tracer/ffe.c b/tracer/ffe.c index 13653a2fab0..d4fe28e7670 100644 --- a/tracer/ffe.c +++ b/tracer/ffe.c @@ -250,3 +250,79 @@ bool ddtrace_ffe_flush_exposures(void) { dd_ffe_clear_exposures(); return flushed; } + +// --- APM feature-flag span enrichment tag staging (PHP-01) --------------- +// +// The OpenFeature provider accumulates serial ids / subjects / runtime +// defaults inline during evaluation (DG-004) and stages the encoded tag set +// here. The values are flushed into the root span meta when the root span +// closes (ddtrace_close_span) and cleared on root close / request shutdown so +// nothing leaks across requests. The whole feature is gated: when the gate is +// off the provider never stages anything, so this store stays empty and the +// close-span path is a cheap early-return (DG-005). + +static void dd_ffe_set_span_tag(zend_string **slot, zend_string *value) { + if (*slot) { + zend_string_release(*slot); + *slot = NULL; + } + if (value && ZSTR_LEN(value) > 0) { + *slot = zend_string_copy(value); + } +} + +void ddtrace_ffe_set_span_enrichment_tags(zend_string *flags_enc, zend_string *subjects_enc, zend_string *runtime_defaults) { + dd_ffe_set_span_tag(&DDTRACE_G(ffe_span_flags_enc), flags_enc); + dd_ffe_set_span_tag(&DDTRACE_G(ffe_span_subjects_enc), subjects_enc); + dd_ffe_set_span_tag(&DDTRACE_G(ffe_span_runtime_defaults), runtime_defaults); +} + +bool ddtrace_ffe_has_span_enrichment_tags(void) { + return DDTRACE_G(ffe_span_flags_enc) != NULL + || DDTRACE_G(ffe_span_subjects_enc) != NULL + || DDTRACE_G(ffe_span_runtime_defaults) != NULL; +} + +void ddtrace_ffe_clear_span_enrichment_tags(void) { + if (DDTRACE_G(ffe_span_flags_enc)) { + zend_string_release(DDTRACE_G(ffe_span_flags_enc)); + DDTRACE_G(ffe_span_flags_enc) = NULL; + } + if (DDTRACE_G(ffe_span_subjects_enc)) { + zend_string_release(DDTRACE_G(ffe_span_subjects_enc)); + DDTRACE_G(ffe_span_subjects_enc) = NULL; + } + if (DDTRACE_G(ffe_span_runtime_defaults)) { + zend_string_release(DDTRACE_G(ffe_span_runtime_defaults)); + DDTRACE_G(ffe_span_runtime_defaults) = NULL; + } +} + +static void dd_ffe_add_span_tag_to_meta(zend_array *meta, const char *key, size_t key_len, zend_string *value) { + if (!value || ZSTR_LEN(value) == 0) { + return; + } + zval tag; + ZVAL_STR_COPY(&tag, value); + zend_hash_str_update(meta, key, key_len, &tag); +} + +void ddtrace_ffe_flush_span_enrichment_tags(zend_array *meta) { + // Cheap gate check first: if the feature is off there is nothing staged and + // we must do no work (DG-005). zai_config may not be initialized yet during + // early shutdown, so fall back to the global value in that case. + bool enabled = zai_config_is_initialized() + ? get_DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED() + : get_global_DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED(); + + if (!enabled || !meta || !ddtrace_ffe_has_span_enrichment_tags()) { + ddtrace_ffe_clear_span_enrichment_tags(); + return; + } + + dd_ffe_add_span_tag_to_meta(meta, ZEND_STRL("ffe_flags_enc"), DDTRACE_G(ffe_span_flags_enc)); + dd_ffe_add_span_tag_to_meta(meta, ZEND_STRL("ffe_subjects_enc"), DDTRACE_G(ffe_span_subjects_enc)); + dd_ffe_add_span_tag_to_meta(meta, ZEND_STRL("ffe_runtime_defaults"), DDTRACE_G(ffe_span_runtime_defaults)); + + ddtrace_ffe_clear_span_enrichment_tags(); +} diff --git a/tracer/ffe.h b/tracer/ffe.h index 0b61ab9a210..717763f26fc 100644 --- a/tracer/ffe.h +++ b/tracer/ffe.h @@ -11,4 +11,11 @@ bool ddtrace_ffe_flush_evaluation_metrics(void); void ddtrace_ffe_record_exposure(zend_string *flag_key, zend_string *targeting_key, zend_string *subject_attributes_json, zend_string *allocation_key, zend_string *variant); bool ddtrace_ffe_flush_exposures(void); +// APM feature-flag span enrichment (PHP-01): request-scoped, gate-gated tag +// staging written onto the root span when it closes. +void ddtrace_ffe_set_span_enrichment_tags(zend_string *flags_enc, zend_string *subjects_enc, zend_string *runtime_defaults); +bool ddtrace_ffe_has_span_enrichment_tags(void); +void ddtrace_ffe_clear_span_enrichment_tags(void); +void ddtrace_ffe_flush_span_enrichment_tags(zend_array *meta); + #endif // DDTRACE_FFE_H diff --git a/tracer/functions.c b/tracer/functions.c index 11aac813c09..a1df8f4b4ef 100644 --- a/tracer/functions.c +++ b/tracer/functions.c @@ -2819,6 +2819,20 @@ PHP_FUNCTION(DDTrace_Internal_flush_ffe_evaluation_metrics) { RETURN_BOOL(ddtrace_ffe_flush_evaluation_metrics()); } +PHP_FUNCTION(DDTrace_Internal_set_ffe_span_enrichment_tags) { + zend_string *flags_enc = NULL; + zend_string *subjects_enc = NULL; + zend_string *runtime_defaults = NULL; + + ZEND_PARSE_PARAMETERS_START(3, 3) + Z_PARAM_STR_OR_NULL(flags_enc) + Z_PARAM_STR_OR_NULL(subjects_enc) + Z_PARAM_STR_OR_NULL(runtime_defaults) + ZEND_PARSE_PARAMETERS_END(); + + ddtrace_ffe_set_span_enrichment_tags(flags_enc, subjects_enc, runtime_defaults); +} + /* {{{ proto array generate_distributed_tracing_headers() */ PHP_FUNCTION(DDTrace_generate_distributed_tracing_headers) { zend_array *inject = NULL; diff --git a/tracer/span.c b/tracer/span.c index a744e020cc2..27bc67efea3 100644 --- a/tracer/span.c +++ b/tracer/span.c @@ -25,6 +25,7 @@ #include "standalone_limiter.h" #include "code_origins.h" #include "endpoint_guessing.h" +#include "ffe.h" #define USE_REALTIME_CLOCK 0 #define USE_MONOTONIC_CLOCK 1 @@ -976,6 +977,13 @@ void ddtrace_close_top_span_without_stack_swap(ddtrace_span_data *span) { if (span->std.ce == ddtrace_ce_root_span_data) { ddtrace_root_span_data *root = ROOTSPANDATA(&span->std); LOG(SPAN_TRACE, "Closing root span: trace_id=%s, span_id=%" PRIu64, Z_STRVAL(root->property_trace_id), span->span_id); + + // APM feature-flag span enrichment (PHP-01): when the root span closes, + // flush any staged ffe_* tags into its meta. This is gated and a no-op + // (cheap early-return) when the feature is off or nothing was staged + // (DG-005). The accumulator is request-scoped and cleared here so it + // never leaks across requests. + ddtrace_ffe_flush_span_enrichment_tags(ddtrace_property_array(&root->property_meta)); } else { LOG(SPAN_TRACE, "Closing span: trace_id=%s, span_id=%" PRIu64, Z_STRVAL(span->root->property_trace_id), span->span_id); } From cf47e8e68b87eff0133efca58db336d2c1a7bd07 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 16 Jun 2026 00:35:18 -0400 Subject: [PATCH 03/12] fix(02-05): reset FFE span-enrichment accumulator on root-span boundary (CR-01) The per-provider SpanEnrichmentAccumulator was only ever added to: clear() had zero production callers and accumulateSpanEnrichment() re-staged the FULL accumulated set on every resolve(). After a root span closed, the next root span re-staged the prior root's serial ids / hashed subjects / runtime defaults (within-request multi-root contamination), and because OpenFeature providers are process-level singletons the accumulator leaked across requests in persistent SAPIs -- a privacy leak of SHA256 subject keys. Fix: reset the PHP accumulator on the root-span boundary, in lockstep with the native close-span flush (which already clears the native staging slots on the same ddtrace_close_span root branch + RSHUTDOWN): - Track the active root span id (spl_object_id of DDTrace\root_span()). On any boundary transition, clear the accumulator + native staging store so a dropped/abandoned root (which never runs its onClose) and a new request both start clean. - Bind a one-shot accumulator clear to the root span's $onClose so the PHP object is reset when the root closes (mirrors the frozen Node reference #onSpanFinish cleanup). - Lifecycle is injectable (rootIdResolver / rootCloseScheduler) so the pure-PHP L0 suite can drive root transitions without the extension. Regression tests (fail-before / pass-after): two sequential root spans in one request -> root 2 stages only its own serial ids/subjects/ defaults; dropped-root and cross-request reset -> no carryover incl. no leaked hashed subject keys; root close clears the accumulator with no subsequent eval. Plus a Node String(value) runtime-default parity test (null/true/false/scalars/objects). Native ABI passthrough, codec (ZAgUAg==), limits, gate-off DG-005, and DG-004 inline accumulation are unchanged. --- src/DDTrace/OpenFeature/DataDogProvider.php | 149 +++++++++ .../SpanEnrichmentAccumulatorTest.php | 311 ++++++++++++++++++ 2 files changed, 460 insertions(+) diff --git a/src/DDTrace/OpenFeature/DataDogProvider.php b/src/DDTrace/OpenFeature/DataDogProvider.php index 42e4dea4596..106aac64bdb 100644 --- a/src/DDTrace/OpenFeature/DataDogProvider.php +++ b/src/DDTrace/OpenFeature/DataDogProvider.php @@ -39,6 +39,29 @@ final class DataDogProvider extends AbstractProvider private EvaluationMetricRecorder $metricRecorder; private bool $spanEnrichmentEnabled = false; private ?SpanEnrichmentAccumulator $spanEnrichmentAccumulator = null; + /** + * Identity (spl_object_id) of the root span the accumulator is currently + * bound to. Lets the provider detect a root-span boundary and reset the + * accumulator per root span (CR-01: otherwise it grows for the provider's + * whole lifetime, contaminating later spans and leaking across requests in + * persistent SAPIs). + */ + private ?int $spanEnrichmentRootId = null; + /** + * Test seam: resolve the current root-span id. Null in production, where + * currentRootSpanId() reads \DDTrace\root_span() directly. + * + * @var (callable(): ?int)|null + */ + private $rootIdResolver = null; + /** + * Test seam: register a one-shot accumulator-reset bound to a root-span + * close. Null in production, where the reset is appended to the root span's + * $onClose. Signature: function(int $rootId, callable $reset): void. + * + * @var (callable(int, callable): void)|null + */ + private $rootCloseScheduler = null; public function __construct(?LoggerInterface $logger = null) { @@ -208,6 +231,18 @@ private function accumulateSpanEnrichment( } try { + // CR-01: reset the accumulator on a root-span boundary so it carries + // ONLY the current root span's evaluations. Without this the + // per-provider accumulator grows for the provider's whole lifetime, + // re-staging stale serial ids / hashed subjects / runtime defaults + // onto later root spans (within-request multi-root contamination) + // and, since OpenFeature providers are process-level singletons, + // across requests in persistent SAPIs (a privacy leak of SHA256 + // subject keys). The native staging store is flushed + cleared on the + // same ddtrace_close_span root branch; here we keep the PHP-side + // accumulator in lockstep. + $this->resetSpanEnrichmentForRootBoundary(); + $exposure = $details->getExposureData(); $serialId = is_array($exposure) && array_key_exists(self::SERIAL_ID_METADATA_KEY, $exposure) ? $exposure[self::SERIAL_ID_METADATA_KEY] @@ -235,6 +270,38 @@ private function accumulateSpanEnrichment( } } + /** + * Detect a root-span boundary and, on a change, reset the accumulator so it + * only ever carries the active root span's evaluations (CR-01). The reset + * fires on ANY transition (a new root, or losing the active root) so a + * dropped/abandoned root — which never runs its $onClose handler — cannot + * leak into the next root span or the next request. On entering a new root + * span a one-shot reset is also bound to that root's close, keeping the + * PHP accumulator in lockstep with the native close-span flush. + */ + private function resetSpanEnrichmentForRootBoundary(): void + { + if ($this->spanEnrichmentAccumulator === null) { + return; + } + + $rootId = $this->currentRootSpanId(); + if ($rootId === $this->spanEnrichmentRootId) { + return; + } + + // Boundary crossed: start clean for the new root (or no-root) context. + $this->spanEnrichmentAccumulator->clear(); + $this->resetSpanEnrichmentStaging(); + $this->spanEnrichmentRootId = $rootId; + + // Bind a one-shot clear to the new root's close (mirrors the Node + // #onSpanFinish cleanup). No root => nothing to bind to. + if ($rootId !== null) { + $this->scheduleAccumulatorResetOnRootClose($rootId); + } + } + /** * Push the currently-accumulated, encoded tag set into native request-local * memory so the root-span close path can write it onto the root span. Only @@ -257,6 +324,88 @@ private function stageSpanEnrichmentTags(): void ); } + /** + * Identity of the currently active root span (or null if there is none / + * tracing is off). Used to detect a root-span boundary so the accumulator is + * reset per root span rather than growing for the provider's lifetime. + * + * The resolver is injectable (`$rootIdResolver`) so the lifecycle can be + * driven in tests without the native extension; in production it reads + * `\DDTrace\root_span()`. + */ + private function currentRootSpanId(): ?int + { + if ($this->rootIdResolver !== null) { + $id = ($this->rootIdResolver)(); + return $id === null ? null : (int) $id; + } + + if (!\function_exists('DDTrace\\root_span')) { + return null; + } + + $root = \DDTrace\root_span(); + + return $root !== null ? \spl_object_id($root) : null; + } + + /** + * Bind a one-shot reset to the active root span's close so the PHP + * accumulator is cleared in lockstep with the native flush (which runs on the + * same `ddtrace_close_span` root branch, AFTER the engine invokes the + * `$onClose` handlers). Mirrors the frozen Node reference's `#onSpanFinish` + * cleanup. The engine empties `$onClose` after firing it, so the closure is + * inherently one-shot per root span. + * + * The scheduler is injectable (`$rootCloseScheduler`) for tests; in + * production it appends to the root span's `$onClose`. + */ + private function scheduleAccumulatorResetOnRootClose(int $rootId): void + { + $reset = function () use ($rootId): void { + if ($this->spanEnrichmentRootId === $rootId && $this->spanEnrichmentAccumulator !== null) { + $this->spanEnrichmentAccumulator->clear(); + } + // The native staging store is cleared by the close-span flush + // itself; nothing to do here for it. + if ($this->spanEnrichmentRootId === $rootId) { + $this->spanEnrichmentRootId = null; + } + }; + + if ($this->rootCloseScheduler !== null) { + ($this->rootCloseScheduler)($rootId, $reset); + return; + } + + if (!\function_exists('DDTrace\\root_span')) { + return; + } + + $root = \DDTrace\root_span(); + if ($root === null) { + return; + } + + $root->onClose[] = static function () use ($reset): void { + $reset(); + }; + } + + /** + * Clear the native request-local staging slots. Used on a root-span boundary + * so a previous (possibly dropped, never-flushed) root's staged tags never + * bleed onto a later root span. No-op without the extension or in tests. + */ + private function resetSpanEnrichmentStaging(): void + { + if (!\function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')) { + return; + } + + \DDTrace\Internal\set_ffe_span_enrichment_tags(null, null, null); + } + /** * @param bool|string|int|float|array $defaultValue * @param array $context diff --git a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php index 38326c6b1bf..29f67d3d725 100644 --- a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php +++ b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php @@ -205,6 +205,41 @@ public function testObjectDefaultIsJsonStringified(): void self::assertSame('{"enabled":true,"n":3}', $defaults['flag']); } + /** + * Runtime-default rendering must match the frozen Node `String(value)` + * (RESEARCH.md:102): null => "null", booleans => "true"/"false", scalars + * via string cast, objects via JSON. This locks the scalar/null/bool parity + * the L2 decode side expects. + * + * @dataProvider nodeStringValueRenderings + * @param mixed $value + */ + public function testDefaultRenderingMatchesNodeStringValue($value, string $expected): void + { + $acc = new SpanEnrichmentAccumulator(); + $acc->addDefault('flag', $value); + + $defaults = json_decode($acc->toSpanTags()[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS], true); + + self::assertSame($expected, $defaults['flag']); + } + + /** + * @return array + */ + public static function nodeStringValueRenderings(): array + { + return [ + 'null => "null"' => [null, 'null'], + 'true => "true"' => [true, 'true'], + 'false => "false"' => [false, 'false'], + 'int => decimal string' => [42, '42'], + 'float => string' => [4.5, '4.5'], + 'string passthrough' => ['plain', 'plain'], + 'stdClass => JSON' => [(object) ['a' => 1], '{"a":1}'], + ]; + } + public function testSubjectsTagIsJsonObjectOfBase64(): void { $acc = new SpanEnrichmentAccumulator(); @@ -251,6 +286,186 @@ public function testClearResetsState(): void self::assertSame([], $acc->toSpanTags()); } + // ---- CR-01 regression: per-root reset (multi-root + cross-request) ----- + // + // The pre-fix provider only ever ADDED to the per-provider accumulator and + // re-staged the FULL accumulated set on every resolve(); clear() had no + // production caller. So after root span 1 closed, span 2 re-staged span 1's + // data (within-request contamination), and in persistent SAPIs the + // accumulator (incl. SHA256 subject keys) leaked across requests. These + // tests drive the provider across simulated root-span boundaries and assert + // the staged tags reflect ONLY the current root's evaluations. + + public function testSecondRootSpanDoesNotInheritFirstRootSerialIds(): void + { + // Within ONE request, two sequential root spans. Span 2 must stage only + // its own serial id — not span 1's (CR-01 consequence #1). + $evaluator = new SpanEnrichmentTestEvaluator(); + $evaluator->setResult('flag-a', 'a', 'a', ['serialId' => 11, 'doLog' => false]); + $evaluator->setResult('flag-b', 'b', 'b', ['serialId' => 22, 'doLog' => false]); + $provider = $this->multiRootProvider($evaluator); + + // Root span #1. + $provider->enterRootSpan(1); + $provider->resolveStringValue('flag-a', 'fallback', new EvaluationContext('user-1')); + $firstStaged = $provider->lastStagedTags(); + self::assertSame([11], $this->decodeFlags($firstStaged)); + + // Root span #1 closes (fires the registered one-shot clear). + $provider->closeRootSpan(1); + + // Root span #2 — a fresh root in the same request/provider. + $provider->enterRootSpan(2); + $provider->resolveStringValue('flag-b', 'fallback', new EvaluationContext('user-2')); + $secondStaged = $provider->lastStagedTags(); + + // BUG (pre-fix): would decode to [11, 22] (span 1's id leaked into span 2). + self::assertSame([22], $this->decodeFlags($secondStaged)); + } + + public function testSecondRootSpanDoesNotInheritFirstRootSubjects(): void + { + // Hashed subject keys must not carry from root #1 to root #2 (privacy + // dimension of CR-01). + $evaluator = new SpanEnrichmentTestEvaluator(); + $evaluator->setResult('flag-a', 'a', 'a', ['serialId' => 11, 'doLog' => true]); + $evaluator->setResult('flag-b', 'b', 'b', ['serialId' => 22, 'doLog' => true]); + $provider = $this->multiRootProvider($evaluator); + + $provider->enterRootSpan(1); + $provider->resolveStringValue('flag-a', 'fallback', new EvaluationContext('alice')); + $provider->closeRootSpan(1); + + $provider->enterRootSpan(2); + $provider->resolveStringValue('flag-b', 'fallback', new EvaluationContext('bob')); + $staged = $provider->lastStagedTags(); + + $subjects = json_decode($staged[SpanEnrichmentAccumulator::TAG_SUBJECTS] ?? '{}', true); + // Only bob's hashed key may be present; alice's must be gone. + self::assertArrayHasKey(hash('sha256', 'bob'), $subjects); + self::assertArrayNotHasKey(hash('sha256', 'alice'), $subjects); + } + + public function testSecondRootSpanDoesNotInheritFirstRootDefaults(): void + { + // Runtime defaults must also reset per root span. + $evaluator = new SpanEnrichmentTestEvaluator(); + $evaluator->setResult('flag-a', 'default-a', null, []); // missing variant => default + $evaluator->setResult('flag-b', 'default-b', null, []); + $provider = $this->multiRootProvider($evaluator); + + $provider->enterRootSpan(1); + $provider->resolveStringValue('flag-a', 'fallback', new EvaluationContext('user-1')); + $provider->closeRootSpan(1); + + $provider->enterRootSpan(2); + $provider->resolveStringValue('flag-b', 'fallback', new EvaluationContext('user-2')); + $staged = $provider->lastStagedTags(); + + $defaults = json_decode($staged[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS] ?? '{}', true); + self::assertSame(['flag-b' => 'default-b'], $defaults); + } + + public function testNewRootResetsEvenWhenPreviousRootWasNeverClosed(): void + { + // Defensive path: a dropped/abandoned root never fires onClose, so the + // provider must still reset when it observes a NEW root id on the next + // evaluation (otherwise a dropped root's data leaks into the next root / + // next request). + $evaluator = new SpanEnrichmentTestEvaluator(); + $evaluator->setResult('flag-a', 'a', 'a', ['serialId' => 11, 'doLog' => false]); + $evaluator->setResult('flag-b', 'b', 'b', ['serialId' => 22, 'doLog' => false]); + $provider = $this->multiRootProvider($evaluator); + + $provider->enterRootSpan(1); + $provider->resolveStringValue('flag-a', 'fallback', new EvaluationContext('user-1')); + // NOTE: no closeRootSpan(1) — the root was dropped/abandoned. + + $provider->enterRootSpan(2); + $provider->resolveStringValue('flag-b', 'fallback', new EvaluationContext('user-2')); + + self::assertSame([22], $this->decodeFlags($provider->lastStagedTags())); + } + + public function testCrossRequestDoesNotLeakAccumulatedStateOnSharedProvider(): void + { + // Persistent-SAPI model (php-fpm / RoadRunner / Swoole): ONE provider + // instance serves multiple requests. Each request has its own root span. + // Request 2 must NOT carry request 1's serial ids or hashed subjects + // (CR-01 consequence #2 — the cross-request privacy leak). + $evaluator = new SpanEnrichmentTestEvaluator(); + $evaluator->setResult('flag-a', 'a', 'a', ['serialId' => 11, 'doLog' => true]); + $evaluator->setResult('flag-b', 'b', 'b', ['serialId' => 22, 'doLog' => true]); + $provider = $this->multiRootProvider($evaluator); + + // ---- Request 1 ---- + $provider->enterRootSpan(101); + $provider->resolveStringValue('flag-a', 'fallback', new EvaluationContext('req1-user')); + $provider->closeRootSpan(101); // request ends: root span closes + + // ---- Request 2 (same provider instance) ---- + $provider->enterRootSpan(202); + $provider->resolveStringValue('flag-b', 'fallback', new EvaluationContext('req2-user')); + $staged = $provider->lastStagedTags(); + + self::assertSame([22], $this->decodeFlags($staged), 'serial ids leaked across requests'); + $subjects = json_decode($staged[SpanEnrichmentAccumulator::TAG_SUBJECTS] ?? '{}', true); + self::assertArrayHasKey(hash('sha256', 'req2-user'), $subjects); + self::assertArrayNotHasKey( + hash('sha256', 'req1-user'), + $subjects, + 'request 1 hashed subject key leaked into request 2' + ); + } + + public function testRootCloseClearsAccumulatorEvenWithNoSubsequentEval(): void + { + // After a root span closes, the accumulator must be empty even if no + // further evaluation happens (lockstep with the native flush; mirrors + // the Node #onSpanFinish cleanup). Otherwise stale data lingers on the + // provider until the next eval. + $accumulator = new SpanEnrichmentAccumulator(); + $evaluator = new SpanEnrichmentTestEvaluator(); + $evaluator->setResult('flag-a', 'a', 'a', ['serialId' => 11, 'doLog' => false]); + $provider = $this->multiRootProvider($evaluator, $accumulator); + + $provider->enterRootSpan(1); + $provider->resolveStringValue('flag-a', 'fallback', new EvaluationContext('user-1')); + self::assertTrue($accumulator->hasData()); + + $provider->closeRootSpan(1); + + self::assertFalse($accumulator->hasData(), 'accumulator not cleared on root close'); + } + + /** + * @param array $staged + * @return array + */ + private function decodeFlags(array $staged): array + { + $flags = $staged[SpanEnrichmentAccumulator::TAG_FLAGS] ?? null; + if ($flags === null) { + return []; + } + return (new SpanEnrichmentAccumulator())->decodeDeltaVarint($flags); + } + + private function multiRootProvider( + Evaluator $evaluator, + ?SpanEnrichmentAccumulator $accumulator = null + ): MultiRootSpanEnrichmentProvider { + $accumulator = $accumulator ?? new SpanEnrichmentAccumulator(); + $provider = DataDogProvider::createWithDependencies( + $evaluator, + new NullLogger(LogLevel::EMERGENCY) + ); + self::accessibleProperty($provider, 'spanEnrichmentEnabled')->setValue($provider, true); + self::accessibleProperty($provider, 'spanEnrichmentAccumulator')->setValue($provider, $accumulator); + + return new MultiRootSpanEnrichmentProvider($provider, $accumulator); + } + // ---- DG-004 inline accumulation via DataDogProvider ------------------ public function testInlineAccumulationRecordsSerialIdAndSubject(): void @@ -454,3 +669,99 @@ private function typeForValue($value): string return EvaluationType::STRING; } } + +/** + * Pure-PHP simulator of the native root-span lifecycle so the CR-01 regression + * tests run without the extension. DataDogProvider is `final`, so rather than + * subclassing it this wraps a real provider and injects the provider's two + * root-span seams via reflection: + * - $rootIdResolver: returns the current simulated root id, standing in for + * spl_object_id(\DDTrace\root_span()). + * - $rootCloseScheduler: records the one-shot accumulator reset that the real + * provider binds to $root->onClose; closeRootSpan() fires it, mirroring + * span.c invoking the onClose handlers before the native flush. + * + * The provider's real accumulate path runs unchanged; the accumulator IS the + * payload that stageSpanEnrichmentTags() pushes to the native store, so the + * regression tests assert on the accumulator's toSpanTags() (what would be + * staged for the current root). + */ +final class MultiRootSpanEnrichmentProvider +{ + /** @var DataDogProvider */ + private $provider; + + /** @var SpanEnrichmentAccumulator */ + private $accumulator; + + /** @var int|null current simulated root span id (null = no active root). */ + private $currentRootId = null; + + /** @var array> root id => registered one-shot resets. */ + private $onRootClose = []; + + public function __construct(DataDogProvider $provider, SpanEnrichmentAccumulator $accumulator) + { + $this->provider = $provider; + $this->accumulator = $accumulator; + + self::setProp($provider, 'rootIdResolver', function (): ?int { + return $this->currentRootId; + }); + self::setProp($provider, 'rootCloseScheduler', function (int $rootId, callable $reset): void { + $this->onRootClose[$rootId][] = $reset; + }); + } + + public function enterRootSpan(int $rootId): void + { + $this->currentRootId = $rootId; + } + + public function closeRootSpan(int $rootId): void + { + // Fire the resets bound to this root, then drop the active root (the + // engine empties $root->onClose after invoking it — one-shot). + $callbacks = $this->onRootClose[$rootId] ?? []; + unset($this->onRootClose[$rootId]); + foreach ($callbacks as $callback) { + $callback(); + } + if ($this->currentRootId === $rootId) { + $this->currentRootId = null; + } + } + + /** + * @param mixed ...$args + */ + public function resolveStringValue(...$args): void + { + $this->provider->resolveStringValue(...$args); + } + + /** + * The encoded tag set that would be staged for the CURRENT root span — i.e. + * exactly what stageSpanEnrichmentTags() pushes into the native store. + * + * @return array + */ + public function lastStagedTags(): array + { + return $this->accumulator->toSpanTags(); + } + + public function accumulator(): SpanEnrichmentAccumulator + { + return $this->accumulator; + } + + private static function setProp(object $object, string $name, $value): void + { + $property = (new \ReflectionObject($object))->getProperty($name); + if (\PHP_VERSION_ID < 80100) { + $property->setAccessible(true); + } + $property->setValue($object, $value); + } +} From e5fa42f65e636d3f92d374f79e0b3d16d4579146 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 16 Jun 2026 19:56:03 -0400 Subject: [PATCH 04/12] test(ffe): add await_ffe_config internal fn to pump RC in CLI servers Long-running CLI servers (parametric test apps) starve the SIGVTALRM-driven remote-config refresh because the process is mostly blocked in IO rather than burning CPU time, so an FFE evaluation issued right after the agent ACKs a pushed UFC config still sees no config and falls back to defaults. Add a dd_trace_internal_fn('await_ffe_config') testing hook that actively pumps remote configs (mirrors await_agent_info) until ddog_ffe_has_config() is true. Enables the FROZEN system-tests span-enrichment parametric suite to load UFC via Remote Config in the long-running PHP parametric server. --- tracer/functions.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tracer/functions.c b/tracer/functions.c index a1df8f4b4ef..f1293e9ddea 100644 --- a/tracer/functions.c +++ b/tracer/functions.c @@ -2123,6 +2123,26 @@ PHP_FUNCTION(dd_trace_internal_fn) { waited += 10; } RETVAL_BOOL(ddog_is_agent_info_ready()); + } else if (FUNCTION_NAME_MATCHES("await_ffe_config")) { + // Block until the sidecar has delivered an FFE (FFE_FLAGS) Remote Config update and the + // worker has applied it. In long-running CLI servers (e.g. the parametric apps) the + // SIGVTALRM-driven remote-config refresh is starved because the process spends most of + // its time blocked in IO rather than burning CPU time, so an evaluation issued right + // after the agent ACKnowledges the config would otherwise still see no config and fall + // back to defaults. Actively pump remote configs here (same shape as await_agent_info) + // so feature-flag evaluation observes the pushed UFC. Times out after 5 seconds. + uint32_t timeout_ms = 5000; + if (params_count == 1) { + timeout_ms = (uint32_t)Z_LVAL_P(ZVAL_VARARG_PARAM(params, 0)); + } + uint32_t waited = 0; + while (!ddog_ffe_has_config() && waited < timeout_ms) { + // Actively read the SHM so we pick up the update the sidecar wrote. + datadog_check_for_new_config_now(); + usleep(10000); // 10ms + waited += 10; + } + RETVAL_BOOL(ddog_ffe_has_config()); } else if (FUNCTION_NAME_MATCHES("get_loaded_remote_configs")) { // Returns a PHP array mapping loaded RC config IDs to their content summary. // e.g. ["datadog/2/LIVE_DEBUGGING/logProbe_log.../config" => ["type"=>"probe","id"=>"log..."]] From 102d3be45f3a3760ffe66511fef53d9ee09e0d3f Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 16 Jun 2026 20:01:26 -0400 Subject: [PATCH 05/12] feat(FeatureFlags): bind APM span enrichment to native Client path Span enrichment was accumulated only inside the OpenFeature DataDogProvider (DG-004 inline path). The native DDTrace\FeatureFlags\Client evaluates flags without going through the provider, so consumers on the native path (the parametric system-tests app, and any non-OpenFeature caller) produced ffe_* tags on the root span for OpenFeature but NOT for the native Client. Extract the per-root-span accumulate/encode/root-boundary lifecycle into a reusable PHP7-compatible SpanEnrichmentBinder and bind it on Client::evaluate(), so both the provider and the native Client stage identical ffe_* tags from the same EvaluationDetails and stay in lockstep with the native close-span write. Honours the FROZEN contract (limits 200/10/20/5/64, delta-varint, SHA256 subjects, runtime-default detection). DG-005: no-op with the gate off. --- src/api/FeatureFlags/Client.php | 11 + src/api/FeatureFlags/SpanEnrichmentBinder.php | 197 ++++++++++++++++++ 2 files changed, 208 insertions(+) create mode 100644 src/api/FeatureFlags/SpanEnrichmentBinder.php diff --git a/src/api/FeatureFlags/Client.php b/src/api/FeatureFlags/Client.php index b42bbfcbcdd..f365ed76386 100644 --- a/src/api/FeatureFlags/Client.php +++ b/src/api/FeatureFlags/Client.php @@ -11,6 +11,8 @@ final class Client private $evaluator; private $logger; private $warnedAboutNonProductionRuntime = false; + /** @var SpanEnrichmentBinder */ + private $spanEnrichmentBinder; public function __construct($logger = null) { @@ -20,6 +22,11 @@ public function __construct($logger = null) $this->evaluator = NativeEvaluator::create(); $this->logger = $logger ?: new TriggerErrorLogger(); + // DG-004/DG-005: the native Client does NOT go through the OpenFeature + // provider, so APM span enrichment is bound here on the same evaluation + // path. The binder is a cheap no-op (allocates nothing, stages nothing) + // unless the experimental span-enrichment gate is on. + $this->spanEnrichmentBinder = new SpanEnrichmentBinder(); } /** @@ -116,6 +123,10 @@ private function evaluate($flagKey, $expectedType, $defaultValue, array $context ); $this->warnIfNonProductionRuntime($details); + // APM span enrichment (no-op when the gate is off). Accumulates from the + // same EvaluationDetails the caller receives; the native close-span path + // writes the staged ffe_* tags onto the root span. + $this->spanEnrichmentBinder->accumulate($flagKey, $details, $targetingKey); return $details; } diff --git a/src/api/FeatureFlags/SpanEnrichmentBinder.php b/src/api/FeatureFlags/SpanEnrichmentBinder.php new file mode 100644 index 00000000000..c8e166556dd --- /dev/null +++ b/src/api/FeatureFlags/SpanEnrichmentBinder.php @@ -0,0 +1,197 @@ +rootIdResolver = $rootIdResolver; + $this->rootCloseScheduler = $rootCloseScheduler; + + $this->enabled = self::gateEnabled(); + if ($this->enabled) { + $this->accumulator = new SpanEnrichmentAccumulator(); + } + } + + private static function gateEnabled() + { + return \function_exists('dd_trace_env_config') + && \dd_trace_env_config(self::SPAN_ENRICHMENT_CONFIG_KEY) === true; + } + + /** + * Accumulate one evaluation's enrichment data and stage the encoded tag set + * for the native close-span write. No-op when the gate is off. Mirrors the + * frozen Node reference branch: a present serial id is recorded (and, when + * do_log authorizes and a targeting key exists, a hashed subject); an + * evaluation with no variant is treated as a runtime default. Errors are + * swallowed -- enrichment must never break flag evaluation. + * + * @param string $flagKey + * @param EvaluationDetails $details + * @param string|null $targetingKey + */ + public function accumulate($flagKey, $details, $targetingKey) + { + if (!$this->enabled || $this->accumulator === null) { + return; + } + + try { + $this->resetForRootBoundary(); + + $exposure = $details->getExposureData(); + $serialId = is_array($exposure) && array_key_exists(self::SERIAL_ID_METADATA_KEY, $exposure) + ? $exposure[self::SERIAL_ID_METADATA_KEY] + : null; + $doLog = is_array($exposure) && !empty($exposure[self::DO_LOG_METADATA_KEY]); + + if ($serialId !== null) { + $this->accumulator->addSerialId((int) $serialId); + if ($doLog && $targetingKey !== null && $targetingKey !== '') { + $this->accumulator->addSubject($targetingKey, (int) $serialId); + } + } else { + $variant = $details->getVariant(); + if ($variant === null || $variant === '') { + $this->accumulator->addDefault((string) $flagKey, $details->getValue()); + } + } + + $this->stage(); + } catch (\Throwable $e) { + // Never let span enrichment break flag evaluation. + } + } + + /** + * Reset the accumulator on a root-span boundary (CR-01) so it carries only + * the active root span's evaluations and never leaks across spans/requests. + */ + private function resetForRootBoundary() + { + if ($this->accumulator === null) { + return; + } + + $rootId = $this->currentRootSpanId(); + if ($rootId === $this->rootId) { + return; + } + + $this->accumulator->clear(); + $this->resetStaging(); + $this->rootId = $rootId; + + if ($rootId !== null) { + $this->scheduleResetOnRootClose($rootId); + } + } + + private function stage() + { + if ($this->accumulator === null + || !$this->accumulator->hasData() + || !\function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')) { + return; + } + + $tags = $this->accumulator->toSpanTags(); + \DDTrace\Internal\set_ffe_span_enrichment_tags( + isset($tags[SpanEnrichmentAccumulator::TAG_FLAGS]) ? $tags[SpanEnrichmentAccumulator::TAG_FLAGS] : null, + isset($tags[SpanEnrichmentAccumulator::TAG_SUBJECTS]) ? $tags[SpanEnrichmentAccumulator::TAG_SUBJECTS] : null, + isset($tags[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS]) ? $tags[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS] : null + ); + } + + private function resetStaging() + { + if (!\function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')) { + return; + } + \DDTrace\Internal\set_ffe_span_enrichment_tags(null, null, null); + } + + private function currentRootSpanId() + { + if ($this->rootIdResolver !== null) { + $id = \call_user_func($this->rootIdResolver); + return $id === null ? null : (int) $id; + } + + if (!\function_exists('DDTrace\\root_span')) { + return null; + } + + $root = \DDTrace\root_span(); + return $root !== null ? \spl_object_id($root) : null; + } + + private function scheduleResetOnRootClose($rootId) + { + $self = $this; + $reset = function () use ($self, $rootId) { + if ($self->rootId === $rootId && $self->accumulator !== null) { + $self->accumulator->clear(); + } + if ($self->rootId === $rootId) { + $self->rootId = null; + } + }; + + if ($this->rootCloseScheduler !== null) { + \call_user_func($this->rootCloseScheduler, $rootId, $reset); + return; + } + + if (!\function_exists('DDTrace\\root_span')) { + return; + } + + $root = \DDTrace\root_span(); + if ($root === null) { + return; + } + + $root->onClose[] = static function () use ($reset) { + $reset(); + }; + } +} From 2cf40629e126ce08e2a905ab70674649042375af Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Wed, 17 Jun 2026 00:41:52 -0400 Subject: [PATCH 06/12] fix(config): regenerate supported-configurations.json for span-enrichment gate Register DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED in metadata/supported-configurations.json by running tooling/generate-supported-configurations.sh. The config was added to ext/configuration.h but the generated metadata was not regenerated, causing the Configuration Consistency CI check to fail. --- metadata/supported-configurations.json | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index 1cf6ad217ff..61ed8169083 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -396,6 +396,13 @@ "default": "false" } ], + "DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED": [ + { + "implementation": "A", + "type": "boolean", + "default": "false" + } + ], "DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED": [ { "implementation": "B", From 50c80d5066b0971f42198e89999d9168638ca6b8 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Wed, 17 Jun 2026 00:42:01 -0400 Subject: [PATCH 07/12] test(ffe): use PHP 7.0-compatible int assertion in ResultMapperTest assertIsInt() is only available in PHPUnit 7.5+, so the new serialId exposure-data test errored on the PHP 7.0 API unit-test job (older PHPUnit). assertInternalType() is unavailable too (removed in PHPUnit 9, and the matrix runs up to PHPUnit <10). Replace with assertTrue(is_int(...)), which works across the whole 7.0-8.5 matrix. The preceding strict assertSame already enforces the integer type. --- tests/api/Unit/FeatureFlags/ResultMapperTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/api/Unit/FeatureFlags/ResultMapperTest.php b/tests/api/Unit/FeatureFlags/ResultMapperTest.php index 075a45d9da6..bc2881f81a4 100644 --- a/tests/api/Unit/FeatureFlags/ResultMapperTest.php +++ b/tests/api/Unit/FeatureFlags/ResultMapperTest.php @@ -244,7 +244,10 @@ public function testThreadsSerialIdIntoExposureDataWhenPresent() array('allocationKey' => 'alloc-1', 'doLog' => true, 'serialId' => 4242), $details->getExposureData() ); - $this->assertIsInt($details->getExposureData()['serialId']); + // assertSame above already enforces the strict int type (4242 !== "4242"); + // avoid assertIsInt()/assertInternalType() which are unavailable on the + // PHP 7.0 PHPUnit and removed in PHPUnit 9 respectively. + $this->assertTrue(is_int($details->getExposureData()['serialId'])); } public function testThreadsCamelCaseSerialIdFromObjectResult() From 547747770f1bf2e3bed932cf482b0a92213c293d Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Wed, 17 Jun 2026 01:52:17 -0400 Subject: [PATCH 08/12] fix(ffe): add non-creating root accessor + gate await_ffe_config test-only PR review (DataDog/dd-trace-php#3996), two native findings: - should-fix: DDTrace\root_span() calls dd_ensure_root_span(), which CREATES an autoroot span when none exists. Resolving the root id while merely evaluating a feature flag must not have that side effect. Add a non-creating DDTrace\Internal\peek_root_span_id() that reads DDTRACE_G(active_stack)-> root_span directly (no dd_ensure_root_span) and returns its object handle, identical to spl_object_id(\DDTrace\root_span()) but without trace-state creation. Wired into the stub + committed arginfo (phpize build uses the committed header as-is; no CI stub-hash gate). - should-fix: await_ffe_config sits in the production dd_trace_internal_fn dispatcher and actively pumps Remote Config, blocking up to 5s. Guard it behind a new DD_TEST_HELPERS compile flag (config.m4, defined for the standard CI/test/package builds the system-tests + ffe-dogfooding harnesses run against) so a hardened production build can compile the heavyweight test helper out of the dispatcher entirely. ZTS-safe (DDTRACE_G accessor); no allocation, no refcount changes. --- config.m4 | 9 +++++++++ tracer/ddtrace.stub.php | 11 +++++++++++ tracer/ddtrace_arginfo.h | 5 +++++ tracer/functions.c | 27 +++++++++++++++++++++++++++ 4 files changed, 52 insertions(+) diff --git a/config.m4 b/config.m4 index aa69d309a43..d34c8627821 100644 --- a/config.m4 +++ b/config.m4 @@ -262,6 +262,15 @@ if test "$PHP_DDTRACE" != "no"; then DATADOG_EXTENSION_FLAGS="-DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -Wall -std=gnu11" + dnl Test-only internal helpers exposed via dd_trace_internal_fn (e.g. + dnl await_ffe_config, which actively pumps Remote Config and can block for + dnl seconds). Defined for the standard CI/test/package builds the + dnl system-tests and ffe-dogfooding harnesses run against; a hardened + dnl production build can drop -DDD_TEST_HELPERS to compile these heavyweight + dnl test surfaces out of the dispatcher entirely so they have no production + dnl effect. + DATADOG_EXTENSION_FLAGS="$DATADOG_EXTENSION_FLAGS -DDD_TEST_HELPERS=1" + ALL_DATADOG_SOURCES=" \ $DATADOG_PHP_SOURCES \ $ZAI_SOURCES \ diff --git a/tracer/ddtrace.stub.php b/tracer/ddtrace.stub.php index a8ec9f7bb94..9d0a4401f7d 100644 --- a/tracer/ddtrace.stub.php +++ b/tracer/ddtrace.stub.php @@ -1105,6 +1105,17 @@ function flush_ffe_evaluation_metrics(): bool {} */ function set_ffe_span_enrichment_tags(?string $flagsEnc, ?string $subjectsEnc, ?string $runtimeDefaults): void {} + /** + * Identity (spl_object_id) of the active root span, or null when there is + * none. Unlike \DDTrace\root_span(), this NEVER creates a root span as a + * side effect (it does not call dd_ensure_root_span()). Used by APM + * feature-flag span enrichment to detect a root-span boundary while merely + * evaluating a flag, without forcing autoroot creation. + * + * @internal + */ + function peek_root_span_id(): ?int {} + } namespace datadog\appsec\v2 { diff --git a/tracer/ddtrace_arginfo.h b/tracer/ddtrace_arginfo.h index 77d8bbb2da4..adb4fed6c80 100644 --- a/tracer/ddtrace_arginfo.h +++ b/tracer/ddtrace_arginfo.h @@ -264,6 +264,9 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_DDTrace_Internal_set_ffe_span_en ZEND_ARG_TYPE_INFO(0, runtimeDefaults, IS_STRING, 1) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_DDTrace_Internal_peek_root_span_id, 0, 0, IS_LONG, 1) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_datadog_appsec_v2_track_user_login_success, 0, 1, IS_VOID, 0) ZEND_ARG_TYPE_INFO(0, login, IS_STRING, 0) ZEND_ARG_TYPE_MASK(0, user, MAY_BE_STRING|MAY_BE_ARRAY|MAY_BE_NULL, "null") @@ -452,6 +455,7 @@ ZEND_FUNCTION(DDTrace_Internal_handle_fork); ZEND_FUNCTION(DDTrace_Internal_record_ffe_evaluation_metric); ZEND_FUNCTION(DDTrace_Internal_flush_ffe_evaluation_metrics); ZEND_FUNCTION(DDTrace_Internal_set_ffe_span_enrichment_tags); +ZEND_FUNCTION(DDTrace_Internal_peek_root_span_id); ZEND_FUNCTION(datadog_appsec_v2_track_user_login_success); ZEND_FUNCTION(datadog_appsec_v2_track_user_login_failure); ZEND_FUNCTION(dd_trace_env_config); @@ -555,6 +559,7 @@ static const zend_function_entry ext_functions[] = { ZEND_RAW_FENTRY(ZEND_NS_NAME("DDTrace\\Internal", "record_ffe_evaluation_metric"), zif_DDTrace_Internal_record_ffe_evaluation_metric, arginfo_DDTrace_Internal_record_ffe_evaluation_metric, 0, NULL, NULL) ZEND_RAW_FENTRY(ZEND_NS_NAME("DDTrace\\Internal", "flush_ffe_evaluation_metrics"), zif_DDTrace_Internal_flush_ffe_evaluation_metrics, arginfo_DDTrace_Internal_flush_ffe_evaluation_metrics, 0, NULL, NULL) ZEND_RAW_FENTRY(ZEND_NS_NAME("DDTrace\\Internal", "set_ffe_span_enrichment_tags"), zif_DDTrace_Internal_set_ffe_span_enrichment_tags, arginfo_DDTrace_Internal_set_ffe_span_enrichment_tags, 0, NULL, NULL) + ZEND_RAW_FENTRY(ZEND_NS_NAME("DDTrace\\Internal", "peek_root_span_id"), zif_DDTrace_Internal_peek_root_span_id, arginfo_DDTrace_Internal_peek_root_span_id, 0, NULL, NULL) ZEND_RAW_FENTRY(ZEND_NS_NAME("datadog\\appsec\\v2", "track_user_login_success"), zif_datadog_appsec_v2_track_user_login_success, arginfo_datadog_appsec_v2_track_user_login_success, 0, NULL, NULL) ZEND_RAW_FENTRY(ZEND_NS_NAME("datadog\\appsec\\v2", "track_user_login_failure"), zif_datadog_appsec_v2_track_user_login_failure, arginfo_datadog_appsec_v2_track_user_login_failure, 0, NULL, NULL) ZEND_FE(dd_trace_env_config, arginfo_dd_trace_env_config) diff --git a/tracer/functions.c b/tracer/functions.c index f1293e9ddea..e1750b7d194 100644 --- a/tracer/functions.c +++ b/tracer/functions.c @@ -2123,6 +2123,14 @@ PHP_FUNCTION(dd_trace_internal_fn) { waited += 10; } RETVAL_BOOL(ddog_is_agent_info_ready()); +#ifdef DD_TEST_HELPERS + // await_ffe_config is a TEST-ONLY helper: it actively pumps Remote + // Config and can block for up to 5s. It exists solely so long-running + // CLI test servers (the system-tests parametric app / ffe-dogfooding + // harness) can deterministically wait for the pushed UFC before + // evaluating. It is compiled in only when DD_TEST_HELPERS is defined + // (the standard CI/test/package builds; see config.m4) so it has no + // production effect in a hardened build that drops the flag. } else if (FUNCTION_NAME_MATCHES("await_ffe_config")) { // Block until the sidecar has delivered an FFE (FFE_FLAGS) Remote Config update and the // worker has applied it. In long-running CLI servers (e.g. the parametric apps) the @@ -2143,6 +2151,7 @@ PHP_FUNCTION(dd_trace_internal_fn) { waited += 10; } RETVAL_BOOL(ddog_ffe_has_config()); +#endif } else if (FUNCTION_NAME_MATCHES("get_loaded_remote_configs")) { // Returns a PHP array mapping loaded RC config IDs to their content summary. // e.g. ["datadog/2/LIVE_DEBUGGING/logProbe_log.../config" => ["type"=>"probe","id"=>"log..."]] @@ -2853,6 +2862,24 @@ PHP_FUNCTION(DDTrace_Internal_set_ffe_span_enrichment_tags) { ddtrace_ffe_set_span_enrichment_tags(flags_enc, subjects_enc, runtime_defaults); } +PHP_FUNCTION(DDTrace_Internal_peek_root_span_id) { + if (zend_parse_parameters_none() == FAILURE) { + RETURN_THROWS(); + } + + // Non-creating root accessor: read the active root span directly WITHOUT + // dd_ensure_root_span(), so resolving the root id while merely evaluating a + // feature flag never creates an autoroot span as a side effect. Returns the + // span object's identity (spl_object_id == its zend object handle), matching + // what PHP's spl_object_id(\DDTrace\root_span()) would yield, so the + // PHP-side accumulator can detect a root-span boundary consistently. + if (!get_DD_TRACE_ENABLED() || !DDTRACE_G(active_stack) || !DDTRACE_G(active_stack)->root_span) { + RETURN_NULL(); + } + + RETURN_LONG((zend_long) DDTRACE_G(active_stack)->root_span->std.handle); +} + /* {{{ proto array generate_distributed_tracing_headers() */ PHP_FUNCTION(DDTrace_generate_distributed_tracing_headers) { zend_array *inject = NULL; From f0a0351242a463b28213468f3cce704132dbed4d Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Wed, 17 Jun 2026 01:52:31 -0400 Subject: [PATCH 09/12] fix(ffe): share one request-scoped span-enrichment accumulator across all paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR review (DataDog/dd-trace-php#3996) blocker + should-fix. blocker: tracer/ffe.c set_ffe_span_enrichment_tags() REPLACES the three request-global tag slots on every call. Both DataDogProvider and each FeatureFlags\Client/SpanEnrichmentBinder owned a SEPARATE accumulator and staged independently, so two clients, two providers, or a mixed OpenFeature + native-client evaluation under ONE root span would OVERWRITE earlier serial ids / hashed subjects / runtime defaults instead of aggregating them. Fix: introduce SpanEnrichmentRegistry, a single request-scoped accumulator that ALL PHP evaluation paths feed. The staged tag set is now the union of every evaluation on the active root span, matching the frozen Node contract. No tag/encoding/limit semantics changed. should-fix (per-binder onClose retention): the lifecycle is centralized in the registry, which binds AT MOST ONE root-close reset per root span (tracked by rootCloseBoundRootId). Many short-lived clients under one long-lived root no longer each retain a closure + accumulator. SpanEnrichmentBinder is now a thin gate-checked adapter; DataDogProvider drops its inline accumulator + lifecycle. should-fix (gate-off not inert): Client and DataDogProvider now construct NO binder unless DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED is on, and evaluate()/resolve() skip the enrichment call entirely when the binder is absent — no per-evaluation config read with the gate off (DG-005). should-fix (root side effect): the registry resolves the root id via the new non-creating DDTrace\Internal\peek_root_span_id(), falling back to the (creating) DDTrace\root_span() only on older extensions. --- src/DDTrace/OpenFeature/DataDogProvider.php | 264 ++------------- src/api/FeatureFlags/Client.php | 26 +- src/api/FeatureFlags/SpanEnrichmentBinder.php | 201 ++---------- .../FeatureFlags/SpanEnrichmentRegistry.php | 301 ++++++++++++++++++ 4 files changed, 376 insertions(+), 416 deletions(-) create mode 100644 src/api/FeatureFlags/SpanEnrichmentRegistry.php diff --git a/src/DDTrace/OpenFeature/DataDogProvider.php b/src/DDTrace/OpenFeature/DataDogProvider.php index 106aac64bdb..cd4ca2493fa 100644 --- a/src/DDTrace/OpenFeature/DataDogProvider.php +++ b/src/DDTrace/OpenFeature/DataDogProvider.php @@ -12,7 +12,7 @@ use DDTrace\FeatureFlags\Internal\Metric\EvaluationMetric; use DDTrace\FeatureFlags\Internal\Metric\EvaluationMetricRecorder; use DDTrace\FeatureFlags\Internal\NativeEvaluator; -use DDTrace\FeatureFlags\SpanEnrichmentAccumulator; +use DDTrace\FeatureFlags\SpanEnrichmentBinder; use DDTrace\Log\LoggerInterface; use DDTrace\Log\TriggerErrorLogger; use OpenFeature\implementation\provider\AbstractProvider; @@ -27,9 +27,6 @@ final class DataDogProvider extends AbstractProvider { private const ALLOCATION_KEY_METADATA_KEY = 'allocationKey'; - private const SERIAL_ID_METADATA_KEY = 'serialId'; - private const DO_LOG_METADATA_KEY = 'doLog'; - private const SPAN_ENRICHMENT_CONFIG_KEY = 'DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED'; protected static string $NAME = 'Datadog'; @@ -37,31 +34,16 @@ final class DataDogProvider extends AbstractProvider private LoggerInterface $warningLogger; private bool $warnedAboutNonProductionRuntime = false; private EvaluationMetricRecorder $metricRecorder; - private bool $spanEnrichmentEnabled = false; - private ?SpanEnrichmentAccumulator $spanEnrichmentAccumulator = null; /** - * Identity (spl_object_id) of the root span the accumulator is currently - * bound to. Lets the provider detect a root-span boundary and reset the - * accumulator per root span (CR-01: otherwise it grows for the provider's - * whole lifetime, contaminating later spans and leaking across requests in - * persistent SAPIs). + * Gate-gated adapter to the SHARED request-scoped span-enrichment + * accumulator (SpanEnrichmentRegistry). Null unless the experimental + * span-enrichment gate is on (DG-005: gate off => no binder, no per-span + * overhead). Routing through the shared registry rather than a per-provider + * accumulator is what lets multiple providers / native clients / mixed + * evaluations under one root span AGGREGATE rather than OVERWRITE one + * another's tags (PR review blocker). */ - private ?int $spanEnrichmentRootId = null; - /** - * Test seam: resolve the current root-span id. Null in production, where - * currentRootSpanId() reads \DDTrace\root_span() directly. - * - * @var (callable(): ?int)|null - */ - private $rootIdResolver = null; - /** - * Test seam: register a one-shot accumulator-reset bound to a root-span - * close. Null in production, where the reset is appended to the root span's - * $onClose. Signature: function(int $rootId, callable $reset): void. - * - * @var (callable(int, callable): void)|null - */ - private $rootCloseScheduler = null; + private ?SpanEnrichmentBinder $spanEnrichmentBinder = null; public function __construct(?LoggerInterface $logger = null) { @@ -71,25 +53,15 @@ public function __construct(?LoggerInterface $logger = null) $this->warningLogger = $logger ?: new TriggerErrorLogger(); $this->metricRecorder = EvaluationMetricRecorder::createDefault(); - // DG-005: only read the gate / allocate the accumulator when the - // experimental span-enrichment feature is opted in. With the gate off we - // construct nothing and never stage anything, so the close-span write - // path stays a cheap no-op and there is no idle per-span overhead. - $this->spanEnrichmentEnabled = self::spanEnrichmentGateEnabled(); - if ($this->spanEnrichmentEnabled) { - $this->spanEnrichmentAccumulator = new SpanEnrichmentAccumulator(); + // DG-005: only construct the binder when the experimental span-enrichment + // feature is opted in. With the gate off we construct nothing and never + // accumulate/stage anything, so the close-span write path stays a cheap + // no-op and there is no idle per-span overhead. + if (SpanEnrichmentBinder::gateEnabled()) { + $this->spanEnrichmentBinder = new SpanEnrichmentBinder(); } } - private static function spanEnrichmentGateEnabled(): bool - { - // Mirrors EvaluationMetricRecorder's gate read. dd_trace_env_config - // resolves the extension-managed config; absent (extension not loaded) - // means the feature is off. - return \function_exists('dd_trace_env_config') - && \dd_trace_env_config(self::SPAN_ENRICHMENT_CONFIG_KEY) === true; - } - /** * @internal Tests and Datadog-owned bridge adapters only. */ @@ -167,9 +139,17 @@ private function resolve( $this->recordEvaluationMetric($flagKey, $details); // DG-004: PHP has no finally hook with ResolutionDetails, so APM span // enrichment is accumulated INLINE here, on the same code path and from - // the same $details, immediately after the metrics hook. No-op when the - // gate is off. - $this->accumulateSpanEnrichment($flagKey, $details, $normalizedContext['targetingKey'] ?? null); + // the same $details, immediately after the metrics hook. Skipped entirely + // with the gate off (no binder was constructed); when on, it feeds the + // SHARED registry so this provider's tags aggregate with any other + // evaluation path active on the same root span. + if ($this->spanEnrichmentBinder !== null) { + $this->spanEnrichmentBinder->accumulate( + $flagKey, + $details, + $normalizedContext['targetingKey'] ?? null + ); + } $builder = (new ResolutionDetailsBuilder()) ->withValue($details->getValue()) @@ -214,198 +194,6 @@ private function allocationKey(EvaluationDetails $details): ?string return $exposure[$key] !== '' ? $exposure[$key] : null; } - /** - * Accumulate APM span enrichment data from a single evaluation (DG-004 - * inline path). Mirrors the frozen Node reference branch: a present serial - * id is recorded (and, when do_log authorizes and a targeting key exists, a - * subject); an evaluation with no variant is treated as a runtime default. - * Errors are swallowed (Pattern D) — enrichment must never break the eval. - */ - private function accumulateSpanEnrichment( - string $flagKey, - EvaluationDetails $details, - ?string $targetingKey - ): void { - if (!$this->spanEnrichmentEnabled || $this->spanEnrichmentAccumulator === null) { - return; - } - - try { - // CR-01: reset the accumulator on a root-span boundary so it carries - // ONLY the current root span's evaluations. Without this the - // per-provider accumulator grows for the provider's whole lifetime, - // re-staging stale serial ids / hashed subjects / runtime defaults - // onto later root spans (within-request multi-root contamination) - // and, since OpenFeature providers are process-level singletons, - // across requests in persistent SAPIs (a privacy leak of SHA256 - // subject keys). The native staging store is flushed + cleared on the - // same ddtrace_close_span root branch; here we keep the PHP-side - // accumulator in lockstep. - $this->resetSpanEnrichmentForRootBoundary(); - - $exposure = $details->getExposureData(); - $serialId = is_array($exposure) && array_key_exists(self::SERIAL_ID_METADATA_KEY, $exposure) - ? $exposure[self::SERIAL_ID_METADATA_KEY] - : null; - $doLog = is_array($exposure) && !empty($exposure[self::DO_LOG_METADATA_KEY]); - - if ($serialId !== null) { - $this->spanEnrichmentAccumulator->addSerialId((int) $serialId); - if ($doLog && $targetingKey !== null && $targetingKey !== '') { - $this->spanEnrichmentAccumulator->addSubject($targetingKey, (int) $serialId); - } - } else { - // Runtime-default detection = MISSING variant (Pattern C), never - // a reason enum. - $variant = $details->getVariant(); - if ($variant === null || $variant === '') { - $this->spanEnrichmentAccumulator->addDefault($flagKey, $details->getValue()); - } - } - - $this->stageSpanEnrichmentTags(); - } catch (\Throwable $e) { - // Never let span enrichment break flag evaluation. - $this->warningLogger->debug('FFE span enrichment accumulation failed: ' . $e->getMessage()); - } - } - - /** - * Detect a root-span boundary and, on a change, reset the accumulator so it - * only ever carries the active root span's evaluations (CR-01). The reset - * fires on ANY transition (a new root, or losing the active root) so a - * dropped/abandoned root — which never runs its $onClose handler — cannot - * leak into the next root span or the next request. On entering a new root - * span a one-shot reset is also bound to that root's close, keeping the - * PHP accumulator in lockstep with the native close-span flush. - */ - private function resetSpanEnrichmentForRootBoundary(): void - { - if ($this->spanEnrichmentAccumulator === null) { - return; - } - - $rootId = $this->currentRootSpanId(); - if ($rootId === $this->spanEnrichmentRootId) { - return; - } - - // Boundary crossed: start clean for the new root (or no-root) context. - $this->spanEnrichmentAccumulator->clear(); - $this->resetSpanEnrichmentStaging(); - $this->spanEnrichmentRootId = $rootId; - - // Bind a one-shot clear to the new root's close (mirrors the Node - // #onSpanFinish cleanup). No root => nothing to bind to. - if ($rootId !== null) { - $this->scheduleAccumulatorResetOnRootClose($rootId); - } - } - - /** - * Push the currently-accumulated, encoded tag set into native request-local - * memory so the root-span close path can write it onto the root span. Only - * stages when there is data; the native side stays empty (and the close-span - * write is a no-op) otherwise. - */ - private function stageSpanEnrichmentTags(): void - { - if ($this->spanEnrichmentAccumulator === null - || !$this->spanEnrichmentAccumulator->hasData() - || !\function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')) { - return; - } - - $tags = $this->spanEnrichmentAccumulator->toSpanTags(); - \DDTrace\Internal\set_ffe_span_enrichment_tags( - $tags[SpanEnrichmentAccumulator::TAG_FLAGS] ?? null, - $tags[SpanEnrichmentAccumulator::TAG_SUBJECTS] ?? null, - $tags[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS] ?? null - ); - } - - /** - * Identity of the currently active root span (or null if there is none / - * tracing is off). Used to detect a root-span boundary so the accumulator is - * reset per root span rather than growing for the provider's lifetime. - * - * The resolver is injectable (`$rootIdResolver`) so the lifecycle can be - * driven in tests without the native extension; in production it reads - * `\DDTrace\root_span()`. - */ - private function currentRootSpanId(): ?int - { - if ($this->rootIdResolver !== null) { - $id = ($this->rootIdResolver)(); - return $id === null ? null : (int) $id; - } - - if (!\function_exists('DDTrace\\root_span')) { - return null; - } - - $root = \DDTrace\root_span(); - - return $root !== null ? \spl_object_id($root) : null; - } - - /** - * Bind a one-shot reset to the active root span's close so the PHP - * accumulator is cleared in lockstep with the native flush (which runs on the - * same `ddtrace_close_span` root branch, AFTER the engine invokes the - * `$onClose` handlers). Mirrors the frozen Node reference's `#onSpanFinish` - * cleanup. The engine empties `$onClose` after firing it, so the closure is - * inherently one-shot per root span. - * - * The scheduler is injectable (`$rootCloseScheduler`) for tests; in - * production it appends to the root span's `$onClose`. - */ - private function scheduleAccumulatorResetOnRootClose(int $rootId): void - { - $reset = function () use ($rootId): void { - if ($this->spanEnrichmentRootId === $rootId && $this->spanEnrichmentAccumulator !== null) { - $this->spanEnrichmentAccumulator->clear(); - } - // The native staging store is cleared by the close-span flush - // itself; nothing to do here for it. - if ($this->spanEnrichmentRootId === $rootId) { - $this->spanEnrichmentRootId = null; - } - }; - - if ($this->rootCloseScheduler !== null) { - ($this->rootCloseScheduler)($rootId, $reset); - return; - } - - if (!\function_exists('DDTrace\\root_span')) { - return; - } - - $root = \DDTrace\root_span(); - if ($root === null) { - return; - } - - $root->onClose[] = static function () use ($reset): void { - $reset(); - }; - } - - /** - * Clear the native request-local staging slots. Used on a root-span boundary - * so a previous (possibly dropped, never-flushed) root's staged tags never - * bleed onto a later root span. No-op without the extension or in tests. - */ - private function resetSpanEnrichmentStaging(): void - { - if (!\function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')) { - return; - } - - \DDTrace\Internal\set_ffe_span_enrichment_tags(null, null, null); - } - /** * @param bool|string|int|float|array $defaultValue * @param array $context diff --git a/src/api/FeatureFlags/Client.php b/src/api/FeatureFlags/Client.php index f365ed76386..d83d76a4ba1 100644 --- a/src/api/FeatureFlags/Client.php +++ b/src/api/FeatureFlags/Client.php @@ -11,8 +11,8 @@ final class Client private $evaluator; private $logger; private $warnedAboutNonProductionRuntime = false; - /** @var SpanEnrichmentBinder */ - private $spanEnrichmentBinder; + /** @var SpanEnrichmentBinder|null Null unless the span-enrichment gate is on. */ + private $spanEnrichmentBinder = null; public function __construct($logger = null) { @@ -24,9 +24,14 @@ public function __construct($logger = null) $this->logger = $logger ?: new TriggerErrorLogger(); // DG-004/DG-005: the native Client does NOT go through the OpenFeature // provider, so APM span enrichment is bound here on the same evaluation - // path. The binder is a cheap no-op (allocates nothing, stages nothing) - // unless the experimental span-enrichment gate is on. - $this->spanEnrichmentBinder = new SpanEnrichmentBinder(); + // path. To stay fully inert with the gate off (PR review should-fix: + // gate-off must allocate no binder and read no per-evaluation config), + // construct the binder ONLY when the experimental span-enrichment gate + // is on; when it is off $spanEnrichmentBinder stays null and evaluate() + // skips the enrichment call entirely. + if (SpanEnrichmentBinder::gateEnabled()) { + $this->spanEnrichmentBinder = new SpanEnrichmentBinder(); + } } /** @@ -123,10 +128,13 @@ private function evaluate($flagKey, $expectedType, $defaultValue, array $context ); $this->warnIfNonProductionRuntime($details); - // APM span enrichment (no-op when the gate is off). Accumulates from the - // same EvaluationDetails the caller receives; the native close-span path - // writes the staged ffe_* tags onto the root span. - $this->spanEnrichmentBinder->accumulate($flagKey, $details, $targetingKey); + // APM span enrichment. Skipped entirely with the gate off (no binder was + // constructed). When on, accumulates from the same EvaluationDetails the + // caller receives into the shared request-scoped registry; the native + // close-span path writes the staged ffe_* tags onto the root span. + if ($this->spanEnrichmentBinder !== null) { + $this->spanEnrichmentBinder->accumulate($flagKey, $details, $targetingKey); + } return $details; } diff --git a/src/api/FeatureFlags/SpanEnrichmentBinder.php b/src/api/FeatureFlags/SpanEnrichmentBinder.php index c8e166556dd..fe667def3ec 100644 --- a/src/api/FeatureFlags/SpanEnrichmentBinder.php +++ b/src/api/FeatureFlags/SpanEnrichmentBinder.php @@ -3,66 +3,51 @@ namespace DDTrace\FeatureFlags; /** - * Binds APM feature-flag span enrichment to an evaluation code path. + * Thin, gate-gated adapter that binds an evaluation code path to the shared + * request-scoped span-enrichment accumulator (SpanEnrichmentRegistry). * - * The per-root-span accumulation + encoding + root-boundary lifecycle was first - * implemented inline in DDTrace\OpenFeature\DataDogProvider (DG-004: PHP's - * OpenFeature SDK has no finally hook carrying ResolutionDetails, so enrichment - * is accumulated INLINE on the evaluation path). That same machinery is needed - * by the native DDTrace\FeatureFlags\Client, which evaluates flags WITHOUT going - * through the OpenFeature provider (e.g. the parametric system-tests app and any - * non-OpenFeature consumer). Extracting it here lets both paths stage identical - * ffe_* tags from the same EvaluationDetails, keeping them in lockstep with the - * native root-span close-write while honouring the FROZEN contract (limits, - * delta-varint encoding, SHA256 subjects, runtime-default detection). + * History: the per-root-span accumulation + encoding + root-boundary lifecycle + * was first implemented inline in DDTrace\OpenFeature\DataDogProvider (DG-004: + * PHP's OpenFeature SDK has no finally hook carrying ResolutionDetails, so + * enrichment is accumulated INLINE on the evaluation path). The same machinery + * is needed by the native DDTrace\FeatureFlags\Client, which evaluates WITHOUT + * the OpenFeature provider. * - * DG-005: the binder allocates the accumulator lazily and only when the - * experimental span-enrichment gate is on, so with the gate off there is no idle - * per-evaluation or per-span overhead. + * Originally each binder owned its OWN accumulator and staged independently. + * That overwrote rather than aggregated when two clients/providers (or a mixed + * OpenFeature + native evaluation) ran under one root span, because the native + * staging API replaces the request-global tag slots wholesale (PR review + * blocker). It also retained a per-binder onClose closure per root span (PR + * review should-fix). Both are fixed by delegating to the single shared + * SpanEnrichmentRegistry: one accumulator aggregates all paths, and the + * registry binds at most one root-close reset per root span. * - * PHP 7 compatible (lives in src/api alongside Client/SpanEnrichmentAccumulator). + * DG-005: this binder is constructed ONLY when the experimental span-enrichment + * gate is on (see DDTrace\FeatureFlags\Client). With the gate off no binder is + * allocated and accumulate() is never reached, so there is no idle per-span + * overhead. + * + * PHP 7 compatible (lives in src/api alongside Client / SpanEnrichmentRegistry). */ final class SpanEnrichmentBinder { - const SERIAL_ID_METADATA_KEY = 'serialId'; - const DO_LOG_METADATA_KEY = 'doLog'; const SPAN_ENRICHMENT_CONFIG_KEY = 'DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED'; - /** @var bool */ - private $enabled = false; - /** @var SpanEnrichmentAccumulator|null */ - private $accumulator = null; - /** @var int|null Identity (spl_object_id) of the root span currently bound. */ - private $rootId = null; - /** @var callable|null Test seam: resolve the current root-span id. */ - private $rootIdResolver = null; - /** @var callable|null Test seam: schedule a one-shot reset on root close. */ - private $rootCloseScheduler = null; - - public function __construct($rootIdResolver = null, $rootCloseScheduler = null) - { - $this->rootIdResolver = $rootIdResolver; - $this->rootCloseScheduler = $rootCloseScheduler; - - $this->enabled = self::gateEnabled(); - if ($this->enabled) { - $this->accumulator = new SpanEnrichmentAccumulator(); - } - } - - private static function gateEnabled() + /** + * Whether the experimental span-enrichment gate is on. Read once: the gate + * is process-level (an env-backed config) and cannot toggle mid-request. + * + * @return bool + */ + public static function gateEnabled() { return \function_exists('dd_trace_env_config') && \dd_trace_env_config(self::SPAN_ENRICHMENT_CONFIG_KEY) === true; } /** - * Accumulate one evaluation's enrichment data and stage the encoded tag set - * for the native close-span write. No-op when the gate is off. Mirrors the - * frozen Node reference branch: a present serial id is recorded (and, when - * do_log authorizes and a targeting key exists, a hashed subject); an - * evaluation with no variant is treated as a runtime default. Errors are - * swallowed -- enrichment must never break flag evaluation. + * Accumulate one evaluation's enrichment via the shared registry. Only + * called when the gate is on (the caller constructs no binder otherwise). * * @param string $flagKey * @param EvaluationDetails $details @@ -70,128 +55,6 @@ private static function gateEnabled() */ public function accumulate($flagKey, $details, $targetingKey) { - if (!$this->enabled || $this->accumulator === null) { - return; - } - - try { - $this->resetForRootBoundary(); - - $exposure = $details->getExposureData(); - $serialId = is_array($exposure) && array_key_exists(self::SERIAL_ID_METADATA_KEY, $exposure) - ? $exposure[self::SERIAL_ID_METADATA_KEY] - : null; - $doLog = is_array($exposure) && !empty($exposure[self::DO_LOG_METADATA_KEY]); - - if ($serialId !== null) { - $this->accumulator->addSerialId((int) $serialId); - if ($doLog && $targetingKey !== null && $targetingKey !== '') { - $this->accumulator->addSubject($targetingKey, (int) $serialId); - } - } else { - $variant = $details->getVariant(); - if ($variant === null || $variant === '') { - $this->accumulator->addDefault((string) $flagKey, $details->getValue()); - } - } - - $this->stage(); - } catch (\Throwable $e) { - // Never let span enrichment break flag evaluation. - } - } - - /** - * Reset the accumulator on a root-span boundary (CR-01) so it carries only - * the active root span's evaluations and never leaks across spans/requests. - */ - private function resetForRootBoundary() - { - if ($this->accumulator === null) { - return; - } - - $rootId = $this->currentRootSpanId(); - if ($rootId === $this->rootId) { - return; - } - - $this->accumulator->clear(); - $this->resetStaging(); - $this->rootId = $rootId; - - if ($rootId !== null) { - $this->scheduleResetOnRootClose($rootId); - } - } - - private function stage() - { - if ($this->accumulator === null - || !$this->accumulator->hasData() - || !\function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')) { - return; - } - - $tags = $this->accumulator->toSpanTags(); - \DDTrace\Internal\set_ffe_span_enrichment_tags( - isset($tags[SpanEnrichmentAccumulator::TAG_FLAGS]) ? $tags[SpanEnrichmentAccumulator::TAG_FLAGS] : null, - isset($tags[SpanEnrichmentAccumulator::TAG_SUBJECTS]) ? $tags[SpanEnrichmentAccumulator::TAG_SUBJECTS] : null, - isset($tags[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS]) ? $tags[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS] : null - ); - } - - private function resetStaging() - { - if (!\function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')) { - return; - } - \DDTrace\Internal\set_ffe_span_enrichment_tags(null, null, null); - } - - private function currentRootSpanId() - { - if ($this->rootIdResolver !== null) { - $id = \call_user_func($this->rootIdResolver); - return $id === null ? null : (int) $id; - } - - if (!\function_exists('DDTrace\\root_span')) { - return null; - } - - $root = \DDTrace\root_span(); - return $root !== null ? \spl_object_id($root) : null; - } - - private function scheduleResetOnRootClose($rootId) - { - $self = $this; - $reset = function () use ($self, $rootId) { - if ($self->rootId === $rootId && $self->accumulator !== null) { - $self->accumulator->clear(); - } - if ($self->rootId === $rootId) { - $self->rootId = null; - } - }; - - if ($this->rootCloseScheduler !== null) { - \call_user_func($this->rootCloseScheduler, $rootId, $reset); - return; - } - - if (!\function_exists('DDTrace\\root_span')) { - return; - } - - $root = \DDTrace\root_span(); - if ($root === null) { - return; - } - - $root->onClose[] = static function () use ($reset) { - $reset(); - }; + SpanEnrichmentRegistry::instance()->accumulate($flagKey, $details, $targetingKey); } } diff --git a/src/api/FeatureFlags/SpanEnrichmentRegistry.php b/src/api/FeatureFlags/SpanEnrichmentRegistry.php new file mode 100644 index 00000000000..93addfe3306 --- /dev/null +++ b/src/api/FeatureFlags/SpanEnrichmentRegistry.php @@ -0,0 +1,301 @@ +rootIdResolver = $rootIdResolver; + $this->rootCloseScheduler = $rootCloseScheduler; + } + + /** + * Test seam: inject the shared accumulator so a test can inspect the exact + * state every evaluation path feeds into. In production the accumulator is + * allocated lazily on first use. + * + * @param SpanEnrichmentAccumulator $accumulator + */ + public function setAccumulator($accumulator) + { + $this->accumulator = $accumulator; + } + + /** + * Accumulate one evaluation's enrichment into the SHARED accumulator and + * stage the encoded union for the native close-span write. Mirrors the + * frozen Node reference branch: a present serial id is recorded (and, when + * do_log authorizes and a targeting key exists, a hashed subject); an + * evaluation with no variant is treated as a runtime default. Errors are + * swallowed -- enrichment must never break flag evaluation. + * + * @param string $flagKey + * @param EvaluationDetails $details + * @param string|null $targetingKey + */ + public function accumulate($flagKey, $details, $targetingKey) + { + try { + $this->resetForRootBoundary(); + + if ($this->accumulator === null) { + $this->accumulator = new SpanEnrichmentAccumulator(); + } + + $exposure = $details->getExposureData(); + $serialId = is_array($exposure) && array_key_exists(self::SERIAL_ID_METADATA_KEY, $exposure) + ? $exposure[self::SERIAL_ID_METADATA_KEY] + : null; + $doLog = is_array($exposure) && !empty($exposure[self::DO_LOG_METADATA_KEY]); + + if ($serialId !== null) { + $this->accumulator->addSerialId((int) $serialId); + if ($doLog && $targetingKey !== null && $targetingKey !== '') { + $this->accumulator->addSubject($targetingKey, (int) $serialId); + } + } else { + $variant = $details->getVariant(); + if ($variant === null || $variant === '') { + $this->accumulator->addDefault((string) $flagKey, $details->getValue()); + } + } + + $this->stage(); + } catch (\Throwable $e) { + // Never let span enrichment break flag evaluation. + } + } + + /** + * Reset the shared accumulator on a root-span boundary (CR-01) so it carries + * only the active root span's evaluations and never leaks across spans / + * requests. Fires on ANY transition (new root, or losing the active root) so + * a dropped/abandoned root -- which never runs its $onClose handler -- cannot + * leak into the next root or request. + */ + private function resetForRootBoundary() + { + $rootId = $this->currentRootSpanId(); + if ($rootId === $this->rootId) { + return; + } + + if ($this->accumulator !== null) { + $this->accumulator->clear(); + } + $this->resetStaging(); + $this->rootId = $rootId; + + if ($rootId !== null) { + $this->scheduleResetOnRootClose($rootId); + } + } + + private function stage() + { + if ($this->accumulator === null + || !$this->accumulator->hasData() + || !\function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')) { + return; + } + + $tags = $this->accumulator->toSpanTags(); + \DDTrace\Internal\set_ffe_span_enrichment_tags( + isset($tags[SpanEnrichmentAccumulator::TAG_FLAGS]) ? $tags[SpanEnrichmentAccumulator::TAG_FLAGS] : null, + isset($tags[SpanEnrichmentAccumulator::TAG_SUBJECTS]) ? $tags[SpanEnrichmentAccumulator::TAG_SUBJECTS] : null, + isset($tags[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS]) ? $tags[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS] : null + ); + } + + private function resetStaging() + { + if (!\function_exists('DDTrace\\Internal\\set_ffe_span_enrichment_tags')) { + return; + } + \DDTrace\Internal\set_ffe_span_enrichment_tags(null, null, null); + } + + /** + * Identity of the active root span (or null). Uses a NON-creating accessor + * (PR review should-fix, DDTrace\root_span side effect): resolving a root id + * while merely evaluating a flag must NOT create an autoroot span. The + * extension exposes DDTrace\Internal\peek_root_span_id() which reads the + * active root span WITHOUT calling dd_ensure_root_span(); we fall back to + * the (creating) DDTrace\root_span() only on older extensions that predate + * the peek helper, preserving behaviour there. + * + * @return int|null + */ + private function currentRootSpanId() + { + if ($this->rootIdResolver !== null) { + $id = \call_user_func($this->rootIdResolver); + return $id === null ? null : (int) $id; + } + + if (\function_exists('DDTrace\\Internal\\peek_root_span_id')) { + $id = \DDTrace\Internal\peek_root_span_id(); + return $id === null ? null : (int) $id; + } + + if (!\function_exists('DDTrace\\root_span')) { + return null; + } + + $root = \DDTrace\root_span(); + return $root !== null ? \spl_object_id($root) : null; + } + + /** + * Bind AT MOST ONE one-shot reset to the active root span's close. Tracking + * $rootCloseBoundRootId guarantees that many short-lived clients/providers + * under one root do not each append a closure (the per-instance onClose + * retention the review flagged). + * + * @param int $rootId + */ + private function scheduleResetOnRootClose($rootId) + { + if ($this->rootCloseBoundRootId === $rootId) { + return; + } + $this->rootCloseBoundRootId = $rootId; + + $reset = function () use ($rootId) { + if ($this->rootId === $rootId && $this->accumulator !== null) { + $this->accumulator->clear(); + } + if ($this->rootId === $rootId) { + $this->rootId = null; + } + if ($this->rootCloseBoundRootId === $rootId) { + $this->rootCloseBoundRootId = null; + } + }; + + if ($this->rootCloseScheduler !== null) { + \call_user_func($this->rootCloseScheduler, $rootId, $reset); + return; + } + + if (!\function_exists('DDTrace\\root_span')) { + return; + } + + $root = \DDTrace\root_span(); + if ($root === null) { + return; + } + + $root->onClose[] = static function () use ($reset) { + $reset(); + }; + } + + /** + * Test accessor for the shared accumulator's currently-staged tag set + * (the union of all evaluations seen on the active root). Returns an empty + * array when nothing has been accumulated. + * + * @return array + */ + public function stagedTags() + { + if ($this->accumulator === null) { + return array(); + } + + return $this->accumulator->toSpanTags(); + } +} From 5a037d10cea37c0e12dbc21a92664f1b501504ee Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Wed, 17 Jun 2026 01:52:47 -0400 Subject: [PATCH 10/12] test(ffe): cover shared-accumulator aggregation, gate-off inertness, non-creating root PR review (DataDog/dd-trace-php#3996) regression coverage. - SpanEnrichmentRegistryTest (PHPUnit, runs without the native ext): two binders (standing in for two clients / a client + a provider) under one simulated root AGGREGATE their serial ids, hashed subjects, and runtime defaults into one staged payload rather than overwriting; CR-01 per-root reset still holds; at most ONE root-close reset is bound across many short-lived binders; the root-close reset clears the shared accumulator. - ClientTest: gate-off Client allocates no SpanEnrichmentBinder and evaluate() short-circuits enrichment without error. - SpanEnrichmentAccumulatorTest: rewired the DG-004 inline + CR-01 multi-root harness to drive the shared registry's seams (the lifecycle moved out of the provider); gate-off assertions now check spanEnrichmentBinder is null. - peek_root_span_id_non_creating.phpt (orchestrator L2, needs built ext): proves peek_root_span_id() returns null without creating a root span (active_span() stays null) and otherwise equals spl_object_id(root_span()). --- .../SpanEnrichmentAccumulatorTest.php | 101 +++++---- tests/api/Unit/FeatureFlags/ClientTest.php | 28 +++ .../SpanEnrichmentRegistryTest.php | 212 ++++++++++++++++++ .../ffe/peek_root_span_id_non_creating.phpt | 42 ++++ 4 files changed, 336 insertions(+), 47 deletions(-) create mode 100644 tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php create mode 100644 tests/ext/ffe/peek_root_span_id_non_creating.phpt diff --git a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php index 29f67d3d725..4a3f4804d30 100644 --- a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php +++ b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php @@ -9,6 +9,8 @@ use DDTrace\FeatureFlags\EvaluationType; use DDTrace\FeatureFlags\Internal\Evaluator; use DDTrace\FeatureFlags\SpanEnrichmentAccumulator; +use DDTrace\FeatureFlags\SpanEnrichmentBinder; +use DDTrace\FeatureFlags\SpanEnrichmentRegistry; use DDTrace\Log\LogLevel; use DDTrace\Log\NullLogger; use DDTrace\OpenFeature\DataDogProvider; @@ -460,8 +462,14 @@ private function multiRootProvider( $evaluator, new NullLogger(LogLevel::EMERGENCY) ); - self::accessibleProperty($provider, 'spanEnrichmentEnabled')->setValue($provider, true); - self::accessibleProperty($provider, 'spanEnrichmentAccumulator')->setValue($provider, $accumulator); + // Force-enable enrichment (give the provider a binder) and route the + // shared registry at the injected accumulator. The lifecycle now lives + // in the registry, so MultiRootSpanEnrichmentProvider drives the + // registry's root-span seams rather than the provider's. + SpanEnrichmentRegistry::reset(); + self::accessibleProperty($provider, 'spanEnrichmentBinder') + ->setValue($provider, new SpanEnrichmentBinder()); + SpanEnrichmentRegistry::instance()->setAccumulator($accumulator); return new MultiRootSpanEnrichmentProvider($provider, $accumulator); } @@ -532,26 +540,25 @@ public function testInlineAccumulationDoesNotRaiseWithoutTargetingKey(): void // ---- Case 6: gate-off negative control (DG-005) ---------------------- - public function testGateOffAllocatesNoAccumulatorAndStagesNothing(): void + public function testGateOffAllocatesNoBinder(): void { - // With the gate off the provider must construct NO accumulator at all - // (DG-005 zero-idle). We assert that via reflection on a default-built - // provider; in this unit context dd_trace_env_config is unavailable, so - // the gate reads as off. + // With the gate off the provider must construct NO SpanEnrichmentBinder + // at all (DG-005 zero-idle; PR-review should-fix). We assert that via + // reflection on a default-built provider; in this unit context + // dd_trace_env_config is unavailable, so the gate reads as off. $provider = new DataDogProvider(new NullLogger(LogLevel::EMERGENCY)); - $enabledProp = self::accessibleProperty($provider, 'spanEnrichmentEnabled'); - $accProp = self::accessibleProperty($provider, 'spanEnrichmentAccumulator'); + $binderProp = self::accessibleProperty($provider, 'spanEnrichmentBinder'); - self::assertFalse($enabledProp->getValue($provider)); - self::assertNull($accProp->getValue($provider)); + self::assertNull($binderProp->getValue($provider)); } public function testGateOffResolveProducesNoSpanTags(): void { + SpanEnrichmentRegistry::reset(); $evaluator = new SpanEnrichmentTestEvaluator(); $evaluator->setResult('flag', 'on', 'on', ['serialId' => 99, 'doLog' => true]); - // Gate off: createWithDependencies leaves spanEnrichmentEnabled false. + // Gate off: createWithDependencies leaves spanEnrichmentBinder null. $provider = DataDogProvider::createWithDependencies( $evaluator, new NullLogger(LogLevel::EMERGENCY) @@ -559,10 +566,11 @@ public function testGateOffResolveProducesNoSpanTags(): void $provider->resolveStringValue('flag', 'fallback', new EvaluationContext('user-123')); - $accProp = self::accessibleProperty($provider, 'spanEnrichmentAccumulator'); - - // No accumulator was constructed and nothing was accumulated. - self::assertNull($accProp->getValue($provider)); + // No binder was constructed and nothing was accumulated into the shared + // registry. + $binderProp = self::accessibleProperty($provider, 'spanEnrichmentBinder'); + self::assertNull($binderProp->getValue($provider)); + self::assertSame([], SpanEnrichmentRegistry::instance()->stagedTags()); } // ---- helpers ---------------------------------------------------------- @@ -596,10 +604,16 @@ private function providerWithAccumulator( new NullLogger(LogLevel::EMERGENCY) ); - // Force-enable the feature and inject a test accumulator so we can - // inspect the accumulated state without the native extension. - self::accessibleProperty($provider, 'spanEnrichmentEnabled')->setValue($provider, true); - self::accessibleProperty($provider, 'spanEnrichmentAccumulator')->setValue($provider, $accumulator); + // Force-enable the feature by giving the provider a binder (in + // production the gate read does this), and inject the test accumulator + // into the SHARED registry that all evaluation paths now feed, so we can + // inspect the accumulated state without the native extension. No root + // is simulated here, so the registry's no-root path keeps the injected + // accumulator (rootId stays null across evaluations). + SpanEnrichmentRegistry::reset(); + self::accessibleProperty($provider, 'spanEnrichmentBinder') + ->setValue($provider, new SpanEnrichmentBinder()); + SpanEnrichmentRegistry::instance()->setAccumulator($accumulator); return $provider; } @@ -672,19 +686,19 @@ private function typeForValue($value): string /** * Pure-PHP simulator of the native root-span lifecycle so the CR-01 regression - * tests run without the extension. DataDogProvider is `final`, so rather than - * subclassing it this wraps a real provider and injects the provider's two - * root-span seams via reflection: - * - $rootIdResolver: returns the current simulated root id, standing in for - * spl_object_id(\DDTrace\root_span()). - * - $rootCloseScheduler: records the one-shot accumulator reset that the real - * provider binds to $root->onClose; closeRootSpan() fires it, mirroring + * tests run without the extension. The per-root accumulation lifecycle now + * lives in the SHARED SpanEnrichmentRegistry, so this drives the REGISTRY's two + * root-span seams (setRootSpanSeams) rather than the provider's: + * - rootIdResolver: returns the current simulated root id, standing in for the + * native peek_root_span_id() / spl_object_id(\DDTrace\root_span()). + * - rootCloseScheduler: records the one-shot accumulator reset that the + * registry binds to $root->onClose; closeRootSpan() fires it, mirroring * span.c invoking the onClose handlers before the native flush. * - * The provider's real accumulate path runs unchanged; the accumulator IS the - * payload that stageSpanEnrichmentTags() pushes to the native store, so the - * regression tests assert on the accumulator's toSpanTags() (what would be - * staged for the current root). + * The provider's real resolve -> binder -> registry accumulate path runs + * unchanged; the shared accumulator IS the payload stage() pushes to the native + * store, so the regression tests assert on the registry's staged tags (what + * would be staged for the current root). */ final class MultiRootSpanEnrichmentProvider { @@ -705,12 +719,14 @@ public function __construct(DataDogProvider $provider, SpanEnrichmentAccumulator $this->provider = $provider; $this->accumulator = $accumulator; - self::setProp($provider, 'rootIdResolver', function (): ?int { - return $this->currentRootId; - }); - self::setProp($provider, 'rootCloseScheduler', function (int $rootId, callable $reset): void { - $this->onRootClose[$rootId][] = $reset; - }); + SpanEnrichmentRegistry::instance()->setRootSpanSeams( + function (): ?int { + return $this->currentRootId; + }, + function (int $rootId, callable $reset): void { + $this->onRootClose[$rootId][] = $reset; + } + ); } public function enterRootSpan(int $rootId): void @@ -742,7 +758,7 @@ public function resolveStringValue(...$args): void /** * The encoded tag set that would be staged for the CURRENT root span — i.e. - * exactly what stageSpanEnrichmentTags() pushes into the native store. + * exactly what the shared registry's stage() pushes into the native store. * * @return array */ @@ -755,13 +771,4 @@ public function accumulator(): SpanEnrichmentAccumulator { return $this->accumulator; } - - private static function setProp(object $object, string $name, $value): void - { - $property = (new \ReflectionObject($object))->getProperty($name); - if (\PHP_VERSION_ID < 80100) { - $property->setAccessible(true); - } - $property->setValue($object, $value); - } } diff --git a/tests/api/Unit/FeatureFlags/ClientTest.php b/tests/api/Unit/FeatureFlags/ClientTest.php index b2bc83d8321..5df917995fe 100644 --- a/tests/api/Unit/FeatureFlags/ClientTest.php +++ b/tests/api/Unit/FeatureFlags/ClientTest.php @@ -151,6 +151,34 @@ public function invalidDefaultProvider() ); } + public function testGateOffAllocatesNoSpanEnrichmentBinder() + { + // PR-review should-fix: with the span-enrichment gate off, the Client + // must construct NO SpanEnrichmentBinder (DG-005 strict zero-idle). In + // this unit context dd_trace_env_config is unavailable, so the gate + // reads as off; the private $spanEnrichmentBinder must stay null. + $client = new Client(new RecordingLogger()); + + $binder = (function () { + return $this->spanEnrichmentBinder; + })->call($client); + + $this->assertNull($binder); + } + + public function testGateOffEvaluationSkipsEnrichmentWithoutError() + { + // The evaluate() path must short-circuit the enrichment call when no + // binder was constructed; it must not fatal or warn about enrichment. + $evaluator = new ClientTestEvaluator(); + $evaluator->setSuccess('flag', 'on', EvaluationReason::SPLIT, 'treatment', array(), array('serialId' => 7)); + $client = $this->clientForEvaluator($evaluator, new RecordingLogger()); + + $details = $client->getStringDetails('flag', 'fallback', array('targetingKey' => 'user-1')); + + $this->assertSame('on', $details->getValue()); + } + private function clientForEvaluator(Evaluator $evaluator, LoggerInterface $logger) { $client = new Client($logger); diff --git a/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php b/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php new file mode 100644 index 00000000000..eb9073fc8eb --- /dev/null +++ b/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php @@ -0,0 +1,212 @@ +registryBoundToRoot(1); + + $binderA = new SpanEnrichmentBinder(); + $binderB = new SpanEnrichmentBinder(); + + $binderA->accumulate('flag-a', $this->detailsWithSerialId(100), null); + $binderB->accumulate('flag-b', $this->detailsWithSerialId(108), null); + + $decoded = $this->decodeFlags($registry->stagedTags()); + $this->assertSame(array(100, 108), $decoded); + } + + public function testTwoBindersUnderOneRootAggregateSubjects() + { + SpanEnrichmentRegistry::reset(); + $registry = $this->registryBoundToRoot(1); + + $binderA = new SpanEnrichmentBinder(); + $binderB = new SpanEnrichmentBinder(); + + // do_log true + targeting key => a hashed subject is recorded for each. + $binderA->accumulate('flag-a', $this->detailsWithSerialId(100, true), 'subject-a'); + $binderB->accumulate('flag-b', $this->detailsWithSerialId(108, true), 'subject-b'); + + $tags = $registry->stagedTags(); + $this->assertArrayHasKey(SpanEnrichmentAccumulator::TAG_SUBJECTS, $tags); + $subjects = json_decode($tags[SpanEnrichmentAccumulator::TAG_SUBJECTS], true); + $this->assertArrayHasKey(hash('sha256', 'subject-a'), $subjects); + $this->assertArrayHasKey(hash('sha256', 'subject-b'), $subjects); + } + + public function testTwoBindersUnderOneRootAggregateRuntimeDefaults() + { + SpanEnrichmentRegistry::reset(); + $registry = $this->registryBoundToRoot(1); + + $binderA = new SpanEnrichmentBinder(); + $binderB = new SpanEnrichmentBinder(); + + // No serial id + no variant => runtime default. + $binderA->accumulate('flag-a', $this->detailsRuntimeDefault('value-a'), null); + $binderB->accumulate('flag-b', $this->detailsRuntimeDefault('value-b'), null); + + $defaults = json_decode( + $registry->stagedTags()[SpanEnrichmentAccumulator::TAG_RUNTIME_DEFAULTS], + true + ); + $this->assertSame('value-a', $defaults['flag-a']); + $this->assertSame('value-b', $defaults['flag-b']); + } + + public function testNewRootDoesNotInheritPreviousRootIds() + { + // CR-01: crossing a root-span boundary resets the shared accumulator so + // the next root carries only its own evaluations. + SpanEnrichmentRegistry::reset(); + $rootId = 1; + $registry = SpanEnrichmentRegistry::instance(); + $registry->setRootSpanSeams(function () use (&$rootId) { + return $rootId; + }, function ($id, $reset) { + // no-op scheduler: we drive the boundary manually below. + }); + + $binder = new SpanEnrichmentBinder(); + $binder->accumulate('flag-a', $this->detailsWithSerialId(100), null); + $this->assertSame(array(100), $this->decodeFlags($registry->stagedTags())); + + // Enter a new root span; the next accumulation must not see id 100. + $rootId = 2; + $binder->accumulate('flag-b', $this->detailsWithSerialId(200), null); + $this->assertSame(array(200), $this->decodeFlags($registry->stagedTags())); + } + + public function testRootCloseSchedulerIsBoundAtMostOncePerRootAcrossManyBinders() + { + // The lifecycle fix: a long-lived root with many short-lived binders must + // register AT MOST ONE close reset (not one per binder), so binders/ + // accumulators are not retained per instance. + SpanEnrichmentRegistry::reset(); + $registry = SpanEnrichmentRegistry::instance(); + $scheduleCount = 0; + $registry->setRootSpanSeams(function () { + return 7; + }, function ($id, $reset) use (&$scheduleCount) { + $scheduleCount++; + }); + + for ($i = 0; $i < 5; $i++) { + $binder = new SpanEnrichmentBinder(); + $binder->accumulate('flag-' . $i, $this->detailsWithSerialId(10 + $i), null); + } + + $this->assertSame(1, $scheduleCount); + } + + public function testRootCloseResetClearsSharedAccumulator() + { + SpanEnrichmentRegistry::reset(); + $registry = SpanEnrichmentRegistry::instance(); + $captured = array(); + $registry->setRootSpanSeams(function () { + return 3; + }, function ($id, $reset) use (&$captured) { + $captured[] = $reset; + }); + + $binder = new SpanEnrichmentBinder(); + $binder->accumulate('flag', $this->detailsWithSerialId(42), null); + $this->assertSame(array(42), $this->decodeFlags($registry->stagedTags())); + + // Fire the one-shot reset the engine would invoke on root close. + $this->assertCount(1, $captured); + call_user_func($captured[0]); + + $this->assertSame(array(), $registry->stagedTags()); + } + + private function registryBoundToRoot($rootId) + { + $registry = SpanEnrichmentRegistry::instance(); + $registry->setRootSpanSeams(function () use ($rootId) { + return $rootId; + }, function ($id, $reset) { + // no-op: tests that need close behaviour capture it explicitly. + }); + + return $registry; + } + + /** + * @param array $staged + * @return array + */ + private function decodeFlags(array $staged) + { + $flags = isset($staged[SpanEnrichmentAccumulator::TAG_FLAGS]) + ? $staged[SpanEnrichmentAccumulator::TAG_FLAGS] + : null; + if ($flags === null) { + return array(); + } + + return (new SpanEnrichmentAccumulator())->decodeDeltaVarint($flags); + } + + private function detailsWithSerialId($serialId, $doLog = false) + { + return new EvaluationDetails( + 'on', + EvaluationType::STRING, + EvaluationReason::TARGETING_MATCH, + 'on', + null, + null, + array(), + array('serialId' => $serialId, 'doLog' => $doLog), + array() + ); + } + + private function detailsRuntimeDefault($value) + { + // No serial id, no variant => runtime default (Pattern C). + return new EvaluationDetails( + $value, + EvaluationType::STRING, + EvaluationReason::DEFAULT_REASON, + null, + null, + null, + array(), + array(), + array() + ); + } +} diff --git a/tests/ext/ffe/peek_root_span_id_non_creating.phpt b/tests/ext/ffe/peek_root_span_id_non_creating.phpt new file mode 100644 index 00000000000..e9389cb982f --- /dev/null +++ b/tests/ext/ffe/peek_root_span_id_non_creating.phpt @@ -0,0 +1,42 @@ +--TEST-- +FFE span enrichment: DDTrace\Internal\peek_root_span_id() is non-creating and matches spl_object_id(root_span()) +--SKIPIF-- + +--ENV-- +DD_TRACE_GENERATE_ROOT_SPAN=0 +DD_TRACE_CLI_ENABLED=1 +--FILE-- + +--EXPECT-- +peek_fn_exists=true +peek_before=null +active_span_still_null_after_peek=true +peek_is_int=true +peek_matches_spl_object_id=true From 3229c80533be95e3fdb6992918162bb81e11cf10 Mon Sep 17 00:00:00 2001 From: "leo.romanovsky" Date: Wed, 17 Jun 2026 03:19:36 -0400 Subject: [PATCH 11/12] fix(ffe): emit ffe_* JSON as raw UTF-8 with unescaped slashes (Node parity) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit json_encode() without flags escaped non-ASCII to \uXXXX and '/' to '\/', diverging from the frozen Node JSON.stringify contract for ffe_subjects_enc and ffe_runtime_defaults. For object/struct runtime defaults the \uXXXX inflation also pushed the value past the 64-char limit so the truncation cut mid-escape-sequence, yielding invalid JSON inside the tag. Add JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES at all three json_encode sites (toSpanTags subjects + runtime defaults, and stringifyDefault for object/array values) so the emitted bytes match Node exactly. Verified via the ffe-dogfooding unicode scenario: decoded ffe_runtime_defaults is now raw UTF-8 (héllo-wörld-☃-日本語-Ω / こんにちは / 🎉), valid JSON, codepoint-safe 64-char truncation. --- src/api/FeatureFlags/SpanEnrichmentAccumulator.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php index e82061ea352..2bb43ce078f 100644 --- a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php +++ b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php @@ -136,11 +136,13 @@ public function toSpanTags() foreach ($this->subjects as $hashed => $ids) { $encodedSubjects[$hashed] = $this->encodeDeltaVarint(array_keys($ids)); } - $tags[self::TAG_SUBJECTS] = json_encode($encodedSubjects); + // JSON_UNESCAPED_UNICODE + JSON_UNESCAPED_SLASHES match Node JSON.stringify + // byte-for-byte: raw UTF-8 (no \uXXXX) and bare '/' (base64 ids contain '/'). + $tags[self::TAG_SUBJECTS] = json_encode($encodedSubjects, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); } if (count($this->defaults) > 0) { - $tags[self::TAG_RUNTIME_DEFAULTS] = json_encode($this->defaults); + $tags[self::TAG_RUNTIME_DEFAULTS] = json_encode($this->defaults, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); } return $tags; @@ -254,7 +256,10 @@ private function hashTargetingKey($key) private function stringifyDefault($value) { if (is_array($value) || is_object($value)) { - $encoded = json_encode($value); + // Match Node JSON.stringify: raw UTF-8 (no \uXXXX) and bare '/'. Default + // json_encode escapes both, which both breaks byte-parity AND inflates the + // length so the 64-char truncation can cut mid-escape-sequence. + $encoded = json_encode($value, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); $valueStr = $encoded === false ? '' : $encoded; } elseif (is_bool($value)) { // Match the Node String(boolean) form: "true"/"false". From 156726d70cfa22a7658bddff3e7a7450db229155 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Wed, 17 Jun 2026 19:22:53 -0400 Subject: [PATCH 12/12] Fix PHP span enrichment CI compatibility --- src/api/FeatureFlags/Client.php | 3 +++ .../FeatureFlags/SpanEnrichmentAccumulator.php | 18 +++++++++--------- .../FeatureFlags/SpanEnrichmentRegistry.php | 4 ++++ .../ffe/peek_root_span_id_non_creating.phpt | 5 ++++- tests/ext/ffe/system_test_data_evaluate.phpt | 3 +++ 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/api/FeatureFlags/Client.php b/src/api/FeatureFlags/Client.php index d83d76a4ba1..033ef2688b9 100644 --- a/src/api/FeatureFlags/Client.php +++ b/src/api/FeatureFlags/Client.php @@ -29,7 +29,10 @@ public function __construct($logger = null) // construct the binder ONLY when the experimental span-enrichment gate // is on; when it is off $spanEnrichmentBinder stays null and evaluate() // skips the enrichment call entirely. + require_once __DIR__ . '/SpanEnrichmentBinder.php'; if (SpanEnrichmentBinder::gateEnabled()) { + require_once __DIR__ . '/SpanEnrichmentAccumulator.php'; + require_once __DIR__ . '/SpanEnrichmentRegistry.php'; $this->spanEnrichmentBinder = new SpanEnrichmentBinder(); } } diff --git a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php index 2bb43ce078f..dc05a29bd0a 100644 --- a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php +++ b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php @@ -18,15 +18,15 @@ */ final class SpanEnrichmentAccumulator { - public const TAG_FLAGS = 'ffe_flags_enc'; - public const TAG_SUBJECTS = 'ffe_subjects_enc'; - public const TAG_RUNTIME_DEFAULTS = 'ffe_runtime_defaults'; - - private const MAX_SERIAL_IDS = 200; - private const MAX_SUBJECTS = 10; - private const MAX_EXPERIMENTS_PER_SUBJECT = 20; - private const MAX_DEFAULTS = 5; - private const MAX_DEFAULT_VALUE_LENGTH = 64; + const TAG_FLAGS = 'ffe_flags_enc'; + const TAG_SUBJECTS = 'ffe_subjects_enc'; + const TAG_RUNTIME_DEFAULTS = 'ffe_runtime_defaults'; + + const MAX_SERIAL_IDS = 200; + const MAX_SUBJECTS = 10; + const MAX_EXPERIMENTS_PER_SUBJECT = 20; + const MAX_DEFAULTS = 5; + const MAX_DEFAULT_VALUE_LENGTH = 64; /** @var array Set of unique serial ids (dedupe-before-encode). */ private $serialIds = array(); diff --git a/src/api/FeatureFlags/SpanEnrichmentRegistry.php b/src/api/FeatureFlags/SpanEnrichmentRegistry.php index 93addfe3306..060b14b1a3c 100644 --- a/src/api/FeatureFlags/SpanEnrichmentRegistry.php +++ b/src/api/FeatureFlags/SpanEnrichmentRegistry.php @@ -233,6 +233,10 @@ private function currentRootSpanId() return null; } + if (!\function_exists('spl_object_id')) { + return null; + } + $root = \DDTrace\root_span(); return $root !== null ? \spl_object_id($root) : null; } diff --git a/tests/ext/ffe/peek_root_span_id_non_creating.phpt b/tests/ext/ffe/peek_root_span_id_non_creating.phpt index e9389cb982f..5a7706312e1 100644 --- a/tests/ext/ffe/peek_root_span_id_non_creating.phpt +++ b/tests/ext/ffe/peek_root_span_id_non_creating.phpt @@ -1,7 +1,10 @@ --TEST-- FFE span enrichment: DDTrace\Internal\peek_root_span_id() is non-creating and matches spl_object_id(root_span()) --SKIPIF-- - + --ENV-- DD_TRACE_GENERATE_ROOT_SPAN=0 DD_TRACE_CLI_ENABLED=1 diff --git a/tests/ext/ffe/system_test_data_evaluate.phpt b/tests/ext/ffe/system_test_data_evaluate.phpt index a673af26ce1..fcc2e5a784a 100644 --- a/tests/ext/ffe/system_test_data_evaluate.phpt +++ b/tests/ext/ffe/system_test_data_evaluate.phpt @@ -112,6 +112,9 @@ function require_feature_flag_api($root) 'EvaluationReason', 'EvaluationErrorCode', 'EvaluationDetails', + 'SpanEnrichmentAccumulator', + 'SpanEnrichmentRegistry', + 'SpanEnrichmentBinder', ) as $classFile) { require_once $apiRoot . '/' . $classFile . '.php'; }