From 053f9d83c1e7aa0627565ad250a4290994825980 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Mon, 15 Jun 2026 23:30:45 -0400 Subject: [PATCH 01/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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'; } From 265422ffd0a43d4647acf40562070aa3fa8efed2 Mon Sep 17 00:00:00 2001 From: Pavel Date: Wed, 15 Jul 2026 15:02:42 +0300 Subject: [PATCH 13/26] fix(ffe): fix span-enrichment tag leak, truncation, and reset-order bugs from review --- .../SpanEnrichmentAccumulator.php | 45 ++++++++++++++----- .../FeatureFlags/SpanEnrichmentRegistry.php | 30 ++++++++++++- .../SpanEnrichmentAccumulatorTest.php | 35 +++++++++++++++ .../SpanEnrichmentRegistryTest.php | 41 +++++++++++++++++ .../ext/ffe/root_drop_clears_staged_tags.phpt | 38 ++++++++++++++++ tracer/span.c | 4 ++ 6 files changed, 181 insertions(+), 12 deletions(-) create mode 100644 tests/ext/ffe/root_drop_clears_staged_tags.phpt diff --git a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php index dc05a29bd0a..db488770bc7 100644 --- a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php +++ b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php @@ -287,21 +287,46 @@ private function truncateUtf8($value, $maxLength) return mb_substr($value, 0, $maxLength, 'UTF-8'); } + return $this->truncateUtf8ByteFallback($value, $maxLength); + } + + /** + * mbstring-free fallback for truncateUtf8(). Walks codepoint-by-codepoint + * (via UTF-8 leading-byte length) so the $maxLength cutoff counts + * characters, not bytes -- a byte cutoff would truncate multi-byte text + * (e.g. CJK, emoji) far below $maxLength chars. Kept as its own method so + * it is directly testable regardless of whether ext-mbstring is loaded in + * the environment running the tests. + */ + private function truncateUtf8ByteFallback($value, $maxLength) + { 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--; + $len = strlen($value); + $offset = 0; + $count = 0; + while ($offset < $len && $count < $maxLength) { + $byte = ord($value[$offset]); + if ($byte < 0x80) { + $seqLen = 1; + } elseif (($byte & 0xE0) === 0xC0) { + $seqLen = 2; + } elseif (($byte & 0xF0) === 0xE0) { + $seqLen = 3; + } elseif (($byte & 0xF8) === 0xF0) { + $seqLen = 4; + } else { + $seqLen = 1; + } + if ($offset + $seqLen > $len) { + break; + } + $offset += $seqLen; + $count++; } - return substr($truncated, 0, $i); + return substr($value, 0, $offset); } } diff --git a/src/api/FeatureFlags/SpanEnrichmentRegistry.php b/src/api/FeatureFlags/SpanEnrichmentRegistry.php index 060b14b1a3c..9bf7fba1c5a 100644 --- a/src/api/FeatureFlags/SpanEnrichmentRegistry.php +++ b/src/api/FeatureFlags/SpanEnrichmentRegistry.php @@ -282,9 +282,35 @@ private function scheduleResetOnRootClose($rootId) return; } - $root->onClose[] = static function () use ($reset) { + $root->onClose = self::prependOnCloseReset($root->onClose, $reset); + } + + /** + * Prepend, not append, the reset closure to $onClose: the native close + * path (tracer/span.c) runs $onClose in REVERSE registration order (last + * registered runs first), so prepending guarantees this reset is the LAST + * callback to run for this root regardless of how many other callbacks get + * registered after it. If it instead ran before an earlier-registered + * callback that itself evaluates a flag during root close, this reset would + * look like a root-boundary crossing to that evaluation and wipe out the + * union already staged for this root (PR review: reset ordering vs. other + * onClose callbacks). Kept as its own method so the ordering guarantee is + * directly testable without a real root span / native extension. + * + * @param mixed $onClose + * @param callable $reset + * @return array + */ + private static function prependOnCloseReset($onClose, $reset) + { + if (!\is_array($onClose)) { + $onClose = array(); + } + \array_unshift($onClose, static function () use ($reset) { $reset(); - }; + }); + + return $onClose; } /** diff --git a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php index 4a3f4804d30..5d7374c491f 100644 --- a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php +++ b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php @@ -194,6 +194,41 @@ public function testDefaultValueTruncationIsUtf8Safe(): void self::assertSame(64, mb_strlen($defaults['flag'], 'UTF-8')); } + /** + * Regression for the mbstring-free fallback path (PR review: byte vs. + * character truncation): without ext-mbstring, a naive substr($value, 0, + * 64) cuts at 64 BYTES, which truncates multi-byte text far below 64 + * characters (e.g. ~21 chars for 3-byte CJK). Invoked directly via + * Reflection so this test exercises the fallback regardless of whether + * ext-mbstring happens to be loaded in the environment running the suite. + * + * @dataProvider utf8FallbackTruncationCases + */ + public function testTruncateUtf8ByteFallbackCountsCharactersNotBytes(string $input, int $expectedChars): void + { + $acc = new SpanEnrichmentAccumulator(); + $method = new \ReflectionMethod(SpanEnrichmentAccumulator::class, 'truncateUtf8ByteFallback'); + $method->setAccessible(true); + + $truncated = $method->invoke($acc, $input, 64); + + self::assertTrue(mb_check_encoding($truncated, 'UTF-8'), 'fallback must not split a multi-byte sequence'); + self::assertSame($expectedChars, mb_strlen($truncated, 'UTF-8')); + } + + /** + * @return array + */ + public static function utf8FallbackTruncationCases(): array + { + return [ + 'ascii (1 byte/char)' => [str_repeat('a', 100), 64], + 'CJK (3 bytes/char)' => [str_repeat("\u{3042}", 100), 64], + 'emoji (4 bytes/char)' => [str_repeat("\u{1F600}", 100), 64], + 'shorter than limit unchanged' => [str_repeat("\u{3042}", 10), 10], + ]; + } + // ---- Case 5: JSON / object default ------------------------------------ public function testObjectDefaultIsJsonStringified(): void diff --git a/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php b/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php index eb9073fc8eb..608a71238e3 100644 --- a/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php +++ b/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php @@ -151,6 +151,47 @@ public function testRootCloseResetClearsSharedAccumulator() $this->assertSame(array(), $registry->stagedTags()); } + /** + * Regression for the reset-ordering bug (PR review): tracer/span.c fires + * $root->onClose in REVERSE registration order. If the registry's reset + * were simply appended, it would run BEFORE an earlier-registered onClose + * callback that itself evaluates a flag during root close -- wiping the + * PHP-side bookkeeping mid-close and losing that evaluation from the + * union. prependOnCloseReset() must guarantee the reset is the LAST + * onClose callback to fire regardless of registration order. + */ + public function testPrependOnCloseResetRunsLastUnderReverseIteration() + { + $method = new \ReflectionMethod(SpanEnrichmentRegistry::class, 'prependOnCloseReset'); + $method->setAccessible(true); + + $order = array(); + $appCallback = function () use (&$order) { + $order[] = 'app'; + }; + $reset = function () use (&$order) { + $order[] = 'reset'; + }; + + // Simulate: app code registers its onClose callback BEFORE the first + // flag evaluation schedules the registry's reset. + $onClose = array($appCallback); + $onClose = $method->invoke(null, $onClose, $reset); + + // A later integration registers another callback after ours is bound. + $laterCallback = function () use (&$order) { + $order[] = 'later'; + }; + $onClose[] = $laterCallback; + + // Mirror tracer/span.c's ZEND_HASH_REVERSE_FOREACH_VAL: last element first. + foreach (array_reverse($onClose) as $callback) { + $callback(); + } + + $this->assertSame(array('later', 'app', 'reset'), $order); + } + private function registryBoundToRoot($rootId) { $registry = SpanEnrichmentRegistry::instance(); diff --git a/tests/ext/ffe/root_drop_clears_staged_tags.phpt b/tests/ext/ffe/root_drop_clears_staged_tags.phpt new file mode 100644 index 00000000000..2db0ef8eb66 --- /dev/null +++ b/tests/ext/ffe/root_drop_clears_staged_tags.phpt @@ -0,0 +1,38 @@ +--TEST-- +FFE span enrichment: dropping a root span clears staged tags so they cannot leak onto the next root +--SKIPIF-- + +--INI-- +datadog.trace.generate_root_span=0 +--ENV-- +DD_TRACE_AUTO_FLUSH_ENABLED=0 +DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED=1 +--FILE-- +onClose[] = function ($span) { + \DDTrace\try_drop_span($span); +}; +\DDTrace\close_span(); + +// A second, unrelated root opens and closes normally. Pre-fix, the tags +// staged for the dropped root above would still be sitting in the native +// globals and would get flushed onto THIS span instead of being discarded. +\DDTrace\start_span(); +\DDTrace\close_span(); + +foreach (dd_trace_serialize_closed_spans() as $span) { + var_dump(array_key_exists("ffe_flags_enc", $span["meta"])); +} +?> +--EXPECT-- +bool(false) diff --git a/tracer/span.c b/tracer/span.c index 27bc67efea3..209426e4ad8 100644 --- a/tracer/span.c +++ b/tracer/span.c @@ -1089,6 +1089,10 @@ void ddtrace_drop_span(ddtrace_span_data *span) { --DDTRACE_G(open_spans_count); if (&stack->root_span->span == span) { + // A dropped root never reaches ddtrace_ffe_flush_span_enrichment_tags(), so any + // staged ffe_* tags for this root must be discarded here -- otherwise they would + // be flushed onto whichever root span closes next (PR review: FFE span leakage). + ddtrace_ffe_clear_span_enrichment_tags(); ddtrace_switch_span_stack(stack->parent_stack); stack->root_span = NULL; } else if (!stack->active || SPANDATA(stack->active)->stack != stack) { From fcda2fadc3f43204a2d6468896bf2a23e6e5878f Mon Sep 17 00:00:00 2001 From: Pavel Date: Wed, 15 Jul 2026 15:48:52 +0300 Subject: [PATCH 14/26] fix(ffe): align root-drop test with proven refcount shape so it hits the real drop path --- tests/ext/ffe/root_drop_clears_staged_tags.phpt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/ext/ffe/root_drop_clears_staged_tags.phpt b/tests/ext/ffe/root_drop_clears_staged_tags.phpt index 2db0ef8eb66..8174adf8205 100644 --- a/tests/ext/ffe/root_drop_clears_staged_tags.phpt +++ b/tests/ext/ffe/root_drop_clears_staged_tags.phpt @@ -6,8 +6,8 @@ if (getenv('PHP_PEAR_RUNTESTS') === '1') die("skip: pecl run-tests does not supp ?> --INI-- datadog.trace.generate_root_span=0 +datadog.trace.auto_flush_enabled=1 --ENV-- -DD_TRACE_AUTO_FLUSH_ENABLED=0 DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED=1 --FILE-- 2 safe-reject-via-normal-close +// fallback. \DDTrace\Internal\set_ffe_span_enrichment_tags("ZAgUAg==", null, null); -$extraRef = $droppedRoot = \DDTrace\start_span(); -$droppedRoot->onClose[] = function ($span) { - \DDTrace\try_drop_span($span); +$extraRef = $span = \DDTrace\start_span(); +$span->onClose[] = function ($span) { + var_dump(\DDTrace\try_drop_span($span)); }; \DDTrace\close_span(); @@ -35,4 +38,5 @@ foreach (dd_trace_serialize_closed_spans() as $span) { } ?> --EXPECT-- +bool(true) bool(false) From 82ffc6a1e871f3be72ec697c1d21511f0cc5189d Mon Sep 17 00:00:00 2001 From: Pavel Date: Wed, 15 Jul 2026 16:27:21 +0300 Subject: [PATCH 15/26] Fix --- tests/ext/ffe/root_drop_clears_staged_tags.phpt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/ext/ffe/root_drop_clears_staged_tags.phpt b/tests/ext/ffe/root_drop_clears_staged_tags.phpt index 8174adf8205..00b4d0dd05a 100644 --- a/tests/ext/ffe/root_drop_clears_staged_tags.phpt +++ b/tests/ext/ffe/root_drop_clears_staged_tags.phpt @@ -30,12 +30,15 @@ $span->onClose[] = function ($span) { // A second, unrelated root opens and closes normally. Pre-fix, the tags // staged for the dropped root above would still be sitting in the native // globals and would get flushed onto THIS span instead of being discarded. -\DDTrace\start_span(); +// Inspect ->meta directly on the held object rather than via +// dd_trace_serialize_closed_spans(): auto_flush_enabled=1 (required above so +// try_drop_span() takes the real-drop branch rather than the refcount>2 +// safe-reject-via-normal-close fallback) drains the closed-span buffer as +// soon as this span closes, so serialize_closed_spans() would see nothing. +$secondRoot = \DDTrace\start_span(); \DDTrace\close_span(); -foreach (dd_trace_serialize_closed_spans() as $span) { - var_dump(array_key_exists("ffe_flags_enc", $span["meta"])); -} +var_dump(array_key_exists("ffe_flags_enc", $secondRoot->meta)); ?> --EXPECT-- bool(true) From 92f1b6ffee8e1a67ca1665ab7fa2b38911f3f950 Mon Sep 17 00:00:00 2001 From: Pavel Date: Thu, 16 Jul 2026 17:31:19 +0300 Subject: [PATCH 16/26] fix(ffe): skip staging without a root, use span_id for root identity, match Node float formatting, flush ffe_* tags before sampling --- .../SpanEnrichmentAccumulator.php | 25 +++++++++++++ .../FeatureFlags/SpanEnrichmentRegistry.php | 37 ++++++++++++++++++- .../SpanEnrichmentAccumulatorTest.php | 9 ++++- .../SpanEnrichmentRegistryTest.php | 18 +++++++++ .../ffe/peek_root_span_id_non_creating.phpt | 22 +++++++---- tracer/ddtrace.stub.php | 4 +- tracer/functions.c | 10 +++-- tracer/span.c | 21 +++++++---- 8 files changed, 123 insertions(+), 23 deletions(-) diff --git a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php index db488770bc7..eb685589238 100644 --- a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php +++ b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php @@ -266,6 +266,8 @@ private function stringifyDefault($value) $valueStr = $value ? 'true' : 'false'; } elseif ($value === null) { $valueStr = 'null'; + } elseif (is_float($value)) { + $valueStr = $this->stringifyFloat($value); } else { $valueStr = (string) $value; } @@ -273,6 +275,29 @@ private function stringifyDefault($value) return $this->truncateUtf8($valueStr, self::MAX_DEFAULT_VALUE_LENGTH); } + /** + * Match Node's String(number) form for floats. PHP's plain (string) cast + * diverges from it in three ways: uppercase "E" exponents (Node: lowercase + * "e"), a signed zero that renders "-0" (Node: "0"), and (via json_encode, + * used below to get a round-trip-shortest mantissa like Node's) a + * trailing ".0" before the exponent that Node's formatter omits. + */ + private function stringifyFloat($value) + { + if (is_nan($value)) { + return 'NaN'; + } + if (is_infinite($value)) { + return $value > 0 ? 'Infinity' : '-Infinity'; + } + if ($value === 0.0) { + return '0'; + } + + $encoded = json_encode($value); + return preg_replace('/\.0(?=e)/', '', $encoded); + } + /** * 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 diff --git a/src/api/FeatureFlags/SpanEnrichmentRegistry.php b/src/api/FeatureFlags/SpanEnrichmentRegistry.php index 9bf7fba1c5a..242734dd84f 100644 --- a/src/api/FeatureFlags/SpanEnrichmentRegistry.php +++ b/src/api/FeatureFlags/SpanEnrichmentRegistry.php @@ -30,6 +30,19 @@ * gate is off -- the accumulator is allocated lazily on first use and only when * the gate is on, so there is no idle per-evaluation or per-span overhead. * + * KNOWN LIMITATION (PR review, not fixed here): this registry tracks exactly + * ONE active root at a time via a flat $rootId/$accumulator pair, and the + * native staging bridge (tracer/ffe.c set_ffe_span_enrichment_tags) mirrors + * that with a single set of request-global slots. When two roots are + * genuinely open concurrently (fibers, multiple span stacks) and evaluations + * interleave across them, switching the active root discards the other + * root's accumulated data, and flushing can attach staged tags to the wrong + * root. Fixing this properly requires per-root-keyed storage on BOTH sides + * (PHP accumulators keyed by root id, AND the native slots keyed the same + * way) -- a native wire-contract-adjacent change, not a PHP-only patch. + * Deferred as a follow-up given the feature is experimental and gated off by + * default. + * * PHP 7 compatible (lives in src/api alongside Client / SpanEnrichmentAccumulator * and is consumed by the PHP 8-only DataDogProvider). */ @@ -43,7 +56,7 @@ final class SpanEnrichmentRegistry /** @var SpanEnrichmentAccumulator|null Allocated lazily, only when the gate is on. */ private $accumulator = null; - /** @var int|null Identity (spl_object_id) of the root span currently bound. */ + /** @var int|null Identity (root span_id) of the root span currently bound. */ private $rootId = null; /** * Identity of the root span we have already bound a one-shot close reset to. @@ -129,6 +142,12 @@ public function accumulate($flagKey, $details, $targetingKey) try { $this->resetForRootBoundary(); + if ($this->rootId === null) { + // No active root span: nothing to attach this evaluation to, and + // staging it now would leak into whichever root opens next. + return; + } + if ($this->accumulator === null) { $this->accumulator = new SpanEnrichmentAccumulator(); } @@ -163,6 +182,12 @@ public function accumulate($flagKey, $details, $targetingKey) * 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. + * + * KNOWN LIMITATION: this is where the single-active-root assumption bites. + * If root A is still open when root B becomes active (concurrent roots via + * fibers / multiple span stacks), this discards A's accumulated data on + * the A->B transition, and switching back to A later starts it fresh + * rather than resuming. See the class docblock. */ private function resetForRootBoundary() { @@ -215,6 +240,16 @@ private function resetStaging() * the (creating) DDTrace\root_span() only on older extensions that predate * the peek helper, preserving behaviour there. * + * peek_root_span_id() returns the root's own span_id, not spl_object_id(): + * object handles are recycled by the engine once a span object is + * destroyed (e.g. a dropped root), so a handle-based identity could alias a + * later, unrelated root and make resetForRootBoundary() wrongly skip its + * reset (PR review, handle-reuse finding). span_id is a random 64-bit value + * that is never reused within the process, so it does not have this + * problem. The spl_object_id() fallback below (old extensions without the + * peek helper) keeps the theoretical handle-reuse exposure, but that path + * only exists for pre-peek-helper extensions. + * * @return int|null */ private function currentRootSpanId() diff --git a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php index 5d7374c491f..3ba95a80efa 100644 --- a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php +++ b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php @@ -274,6 +274,13 @@ public static function nodeStringValueRenderings(): array 'float => string' => [4.5, '4.5'], 'string passthrough' => ['plain', 'plain'], 'stdClass => JSON' => [(object) ['a' => 1], '{"a":1}'], + // PHP's plain (string) cast diverges from Node's String(number) for + // these: uppercase "E" exponents, a "-0" for negative zero, and (via + // (string)'s default precision) a differently-rounded mantissa. + 'float exponent lowercase e' => [1.0e-7, '1e-7'], + 'float exponent with sign' => [1.5e21, '1.5e+21'], + 'negative zero => "0"' => [-0.0, '0'], + 'whole float => no decimal' => [100.0, '100'], ]; } @@ -725,7 +732,7 @@ private function typeForValue($value): string * 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()). + * native peek_root_span_id() (the root span's own span_id). * - 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. diff --git a/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php b/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php index 608a71238e3..854979b0d73 100644 --- a/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php +++ b/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php @@ -107,6 +107,24 @@ public function testNewRootDoesNotInheritPreviousRootIds() $this->assertSame(array(200), $this->decodeFlags($registry->stagedTags())); } + public function testAccumulateWithoutAnActiveRootIsSkippedRatherThanStaged() + { + // PR review: an evaluation made with no active root span (rootIdResolver + // returns null) must not be staged -- otherwise it would leak onto + // whichever root span opens next. + SpanEnrichmentRegistry::reset(); + $registry = SpanEnrichmentRegistry::instance(); + $registry->setRootSpanSeams(function () { + return null; + }, function ($id, $reset) { + }); + + $binder = new SpanEnrichmentBinder(); + $binder->accumulate('flag-a', $this->detailsWithSerialId(100), null); + + $this->assertSame(array(), $registry->stagedTags()); + } + public function testRootCloseSchedulerIsBoundAtMostOncePerRootAcrossManyBinders() { // The lifecycle fix: a long-lived root with many short-lived binders must 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 5a7706312e1..7689007b25a 100644 --- a/tests/ext/ffe/peek_root_span_id_non_creating.phpt +++ b/tests/ext/ffe/peek_root_span_id_non_creating.phpt @@ -1,9 +1,8 @@ --TEST-- -FFE span enrichment: DDTrace\Internal\peek_root_span_id() is non-creating and matches spl_object_id(root_span()) +FFE span enrichment: DDTrace\Internal\peek_root_span_id() is non-creating and stable/unique per root --SKIPIF-- --ENV-- DD_TRACE_GENERATE_ROOT_SPAN=0 @@ -26,14 +25,20 @@ show('peek_fn_exists', function_exists('DDTrace\\Internal\\peek_root_span_id')); show('peek_before', \DDTrace\Internal\peek_root_span_id()); show('active_span_still_null_after_peek', \DDTrace\active_span() === null); -// Now open a real span. peek_root_span_id() must return its identity, and that -// identity must equal spl_object_id(\DDTrace\root_span()) — the value the -// PHP-side accumulator previously used to detect a root-span boundary. +// Now open a real span. peek_root_span_id() must return an identity that is +// stable for the lifetime of that root (not derived from a recyclable zend +// object handle, which a dropped-and-replaced root could alias). $span = \DDTrace\start_span(); $peeked = \DDTrace\Internal\peek_root_span_id(); -$root = \DDTrace\root_span(); +$peekedAgain = \DDTrace\Internal\peek_root_span_id(); show('peek_is_int', is_int($peeked)); -show('peek_matches_spl_object_id', $peeked === \spl_object_id($root)); +show('peek_stable_within_same_root', $peeked === $peekedAgain); +\DDTrace\close_span(); + +// A second, distinct root must get a distinct identity. +$span2 = \DDTrace\start_span(); +$peeked2 = \DDTrace\Internal\peek_root_span_id(); +show('peek_distinct_across_roots', $peeked !== $peeked2); \DDTrace\close_span(); ?> @@ -42,4 +47,5 @@ peek_fn_exists=true peek_before=null active_span_still_null_after_peek=true peek_is_int=true -peek_matches_spl_object_id=true +peek_stable_within_same_root=true +peek_distinct_across_roots=true diff --git a/tracer/ddtrace.stub.php b/tracer/ddtrace.stub.php index 9d0a4401f7d..e274d33c4fb 100644 --- a/tracer/ddtrace.stub.php +++ b/tracer/ddtrace.stub.php @@ -1106,8 +1106,8 @@ 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 + * Identity (the root span's own span_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. diff --git a/tracer/functions.c b/tracer/functions.c index 6c817d410dd..a22c84ad29e 100644 --- a/tracer/functions.c +++ b/tracer/functions.c @@ -2873,14 +2873,16 @@ PHP_FUNCTION(DDTrace_Internal_peek_root_span_id) { // 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. + // root's own span_id (a random 64-bit id, never reused within the process) + // rather than the zend object handle: handles are recycled by the engine's + // object store once a span object is destroyed (e.g. a dropped root), so a + // handle-based identity could alias a later, unrelated root and make the + // PHP-side accumulator wrongly treat it as still bound to the old one. 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); + RETURN_LONG((zend_long) DDTRACE_G(active_stack)->root_span->span_id); } /* {{{ proto array generate_distributed_tracing_headers() */ diff --git a/tracer/span.c b/tracer/span.c index 209426e4ad8..10300363df5 100644 --- a/tracer/span.c +++ b/tracer/span.c @@ -968,6 +968,20 @@ void ddtrace_close_top_span_without_stack_swap(ddtrace_span_data *span) { stack->closed_ring = span; } + if (span->std.ce == ddtrace_ce_root_span_data) { + ddtrace_root_span_data *root = ROOTSPANDATA(&span->std); + + // APM feature-flag span enrichment (PHP-01): flush any staged ffe_* + // tags into the root's meta BEFORE the sampling decision below runs, + // so span-level sampling rules that match against ffe_* tags can + // actually see them (PR review: this used to run after the sampling + // decision, so such rules could never match). 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)); + } + ddtrace_decide_on_closed_span_sampling(span); if (span->notify_user_req_end) { ddtrace_user_req_notify_finish(span); @@ -977,13 +991,6 @@ 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 bedfcde1497a6befd8c84ff3cd3e4cac0da349dc Mon Sep 17 00:00:00 2001 From: Pavel Date: Sat, 18 Jul 2026 10:37:41 +0300 Subject: [PATCH 17/26] fix(ffe): drop redundant require_once in Client and correct misleading serialId comment --- src/api/FeatureFlags/Client.php | 8 ++++---- src/api/FeatureFlags/Internal/ResultMapper.php | 8 ++++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/api/FeatureFlags/Client.php b/src/api/FeatureFlags/Client.php index 033ef2688b9..81f7b45c3fb 100644 --- a/src/api/FeatureFlags/Client.php +++ b/src/api/FeatureFlags/Client.php @@ -28,11 +28,11 @@ public function __construct($logger = null) // 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. - require_once __DIR__ . '/SpanEnrichmentBinder.php'; + // skips the enrichment call entirely. SpanEnrichmentBinder/Accumulator/ + // Registry live in this same PSR-4 autoloaded namespace as Client + // itself, so referencing them here loads nothing more than autoloading + // Client already did -- no explicit require_once needed. if (SpanEnrichmentBinder::gateEnabled()) { - require_once __DIR__ . '/SpanEnrichmentAccumulator.php'; - require_once __DIR__ . '/SpanEnrichmentRegistry.php'; $this->spanEnrichmentBinder = new SpanEnrichmentBinder(); } } diff --git a/src/api/FeatureFlags/Internal/ResultMapper.php b/src/api/FeatureFlags/Internal/ResultMapper.php index d717d24b9db..3694700b1e8 100644 --- a/src/api/FeatureFlags/Internal/ResultMapper.php +++ b/src/api/FeatureFlags/Internal/ResultMapper.php @@ -276,8 +276,12 @@ private function exposureData($rawResult) // 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. + // result actually carried one; a null/absent value must be left out + // entirely rather than defaulted to 0/false. Absence alone does NOT + // mean a runtime default was used -- a waterfall-assigned variant can + // also have no serial id. SpanEnrichmentRegistry::accumulate() only + // treats an evaluation as a runtime default when BOTH serialId AND the + // variant are absent. $serialId = $this->read($rawResult, array('serial_id', 'serialId'), null); if ($serialId !== null) { $exposureData['serialId'] = (int) $serialId; From 8096369c978bd83841c6a98a4b4d08c79e3f693a Mon Sep 17 00:00:00 2001 From: Pavel Date: Thu, 23 Jul 2026 10:47:55 +0300 Subject: [PATCH 18/26] fix(config): bump ZAI_CONFIG_ENTRIES_COUNT_MAX to 320 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merging master pushed the total config-entry count to the 300 cap, failing the NUMBER_OF_CONFIGURATIONS static assert in ext/configuration.c and breaking the extension build. Bump the cap (same pattern as prior 200→220→255→300 bumps) to restore headroom. --- zend_abstract_interface/config/config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zend_abstract_interface/config/config.h b/zend_abstract_interface/config/config.h index d7598cf35fd..3a9c8d34f50 100644 --- a/zend_abstract_interface/config/config.h +++ b/zend_abstract_interface/config/config.h @@ -18,7 +18,7 @@ typedef uint16_t zai_config_id; #include "config_ini.h" #include "config_stable_file.h" -#define ZAI_CONFIG_ENTRIES_COUNT_MAX 300 +#define ZAI_CONFIG_ENTRIES_COUNT_MAX 320 #define ZAI_CONFIG_NAMES_COUNT_MAX 4 #define ZAI_CONFIG_NAME_BUFSIZ 72 From fea9fd5fc170637ed66da8362dd6ad7ec56dcdb2 Mon Sep 17 00:00:00 2001 From: Pavel Date: Thu, 23 Jul 2026 16:40:40 +0300 Subject: [PATCH 19/26] fix(ffe): narrow FfeResult.serial_id from i64 to i32 The source field (ffe::Assignment.serial_id) is Option, so the value half never needs more than 32 bits. Only has_serial_id is required to represent the Option; the i64 width was an unnecessary widening with no documented rationale. Narrow serial_id to i32 in the Rust struct and the cbindgen-generated common.h ABI, and drop the `as i64` cast. The PHP boundary still widens to zend_long in functions.c ((zend_long) promotes int32_t cleanly), so no C reader change is needed. Addresses review feedback from @bwoebi. Co-Authored-By: Claude Opus 4.8 (1M context) --- components-rs/common.h | 2 +- components-rs/ffe.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components-rs/common.h b/components-rs/common.h index 05e80d3b71c..75832653b9e 100644 --- a/components-rs/common.h +++ b/components-rs/common.h @@ -484,7 +484,7 @@ typedef struct ddog_FfeResult { _zend_string * allocation_key; int32_t reason; int32_t error_code; - int64_t serial_id; + int32_t serial_id; bool has_serial_id; bool do_log; bool valid; diff --git a/components-rs/ffe.rs b/components-rs/ffe.rs index d2f487e7767..4964bc7b0b9 100644 --- a/components-rs/ffe.rs +++ b/components-rs/ffe.rs @@ -103,7 +103,7 @@ pub struct FfeResult { // 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 serial_id: i32, pub has_serial_id: bool, pub do_log: bool, pub valid: bool, @@ -228,7 +228,7 @@ fn result_from_assignment(assignment: Result) AssignmentReason::Default => REASON_DEFAULT, }, error_code: ERROR_NONE, - serial_id: assignment.serial_id.unwrap_or(0) as i64, + serial_id: assignment.serial_id.unwrap_or(0), has_serial_id: assignment.serial_id.is_some(), do_log: assignment.do_log, valid: true, From e2468b767a6951bf95e146f43c910d98eea85be6 Mon Sep 17 00:00:00 2001 From: Pavel Date: Thu, 23 Jul 2026 17:53:20 +0300 Subject: [PATCH 20/26] refactor(ffe): attach span enrichment to root span via ObjectKVStore --- src/DDTrace/OpenFeature/DataDogProvider.php | 34 +- src/api/FeatureFlags/Client.php | 22 - .../FeatureFlags/Internal/NativeEvaluator.php | 13 +- src/api/FeatureFlags/SpanEnrichmentBinder.php | 60 --- .../FeatureFlags/SpanEnrichmentRegistry.php | 365 ++----------- .../SpanEnrichmentAccumulatorTest.php | 510 +----------------- tests/api/Unit/FeatureFlags/ClientTest.php | 28 - .../SpanEnrichmentRegistryTest.php | 271 ---------- tests/ext/ffe/system_test_data_evaluate.phpt | 1 - 9 files changed, 78 insertions(+), 1226 deletions(-) delete mode 100644 src/api/FeatureFlags/SpanEnrichmentBinder.php delete mode 100644 tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php diff --git a/src/DDTrace/OpenFeature/DataDogProvider.php b/src/DDTrace/OpenFeature/DataDogProvider.php index cd4ca2493fa..05c7e771f6d 100644 --- a/src/DDTrace/OpenFeature/DataDogProvider.php +++ b/src/DDTrace/OpenFeature/DataDogProvider.php @@ -12,7 +12,6 @@ use DDTrace\FeatureFlags\Internal\Metric\EvaluationMetric; use DDTrace\FeatureFlags\Internal\Metric\EvaluationMetricRecorder; use DDTrace\FeatureFlags\Internal\NativeEvaluator; -use DDTrace\FeatureFlags\SpanEnrichmentBinder; use DDTrace\Log\LoggerInterface; use DDTrace\Log\TriggerErrorLogger; use OpenFeature\implementation\provider\AbstractProvider; @@ -34,16 +33,6 @@ final class DataDogProvider extends AbstractProvider private LoggerInterface $warningLogger; private bool $warnedAboutNonProductionRuntime = false; private EvaluationMetricRecorder $metricRecorder; - /** - * 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 ?SpanEnrichmentBinder $spanEnrichmentBinder = null; public function __construct(?LoggerInterface $logger = null) { @@ -52,14 +41,6 @@ public function __construct(?LoggerInterface $logger = null) $this->evaluator = NativeEvaluator::create(false); $this->warningLogger = $logger ?: new TriggerErrorLogger(); $this->metricRecorder = EvaluationMetricRecorder::createDefault(); - - // 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(); - } } /** @@ -137,19 +118,8 @@ private function resolve( // 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. 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 - ); - } + // APM span enrichment is recorded inside NativeEvaluator::evaluate() + // (the shared choke point), so there is nothing to do here. $builder = (new ResolutionDetailsBuilder()) ->withValue($details->getValue()) diff --git a/src/api/FeatureFlags/Client.php b/src/api/FeatureFlags/Client.php index 81f7b45c3fb..b42bbfcbcdd 100644 --- a/src/api/FeatureFlags/Client.php +++ b/src/api/FeatureFlags/Client.php @@ -11,8 +11,6 @@ final class Client private $evaluator; private $logger; private $warnedAboutNonProductionRuntime = false; - /** @var SpanEnrichmentBinder|null Null unless the span-enrichment gate is on. */ - private $spanEnrichmentBinder = null; public function __construct($logger = null) { @@ -22,19 +20,6 @@ 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. 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. SpanEnrichmentBinder/Accumulator/ - // Registry live in this same PSR-4 autoloaded namespace as Client - // itself, so referencing them here loads nothing more than autoloading - // Client already did -- no explicit require_once needed. - if (SpanEnrichmentBinder::gateEnabled()) { - $this->spanEnrichmentBinder = new SpanEnrichmentBinder(); - } } /** @@ -131,13 +116,6 @@ private function evaluate($flagKey, $expectedType, $defaultValue, array $context ); $this->warnIfNonProductionRuntime($details); - // 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/Internal/NativeEvaluator.php b/src/api/FeatureFlags/Internal/NativeEvaluator.php index 2d9c3674101..8588f694e8e 100644 --- a/src/api/FeatureFlags/Internal/NativeEvaluator.php +++ b/src/api/FeatureFlags/Internal/NativeEvaluator.php @@ -3,6 +3,7 @@ namespace DDTrace\FeatureFlags\Internal; use DDTrace\FeatureFlags\EvaluationType; +use DDTrace\FeatureFlags\SpanEnrichmentRegistry; final class NativeEvaluator implements Evaluator { @@ -50,7 +51,17 @@ public function evaluate( $rawResult = $this->withProviderState($rawResult); } - return $this->mapper->map($rawResult, $expectedType, $defaultValue); + $details = $this->mapper->map($rawResult, $expectedType, $defaultValue); + + // APM feature-flag span enrichment. This is the single choke point both + // the native Client and the OpenFeature DataDogProvider evaluate through, + // and it is only reachable with the extension loaded (UnavailableEvaluator + // is returned otherwise), so enrichment is recorded here once rather than + // duplicated in each caller. No-op when the gate is off or there is no + // active root span; never throws into evaluation. + SpanEnrichmentRegistry::record($flagKey, $details, $targetingKey); + + return $details; } private function typeId($expectedType) diff --git a/src/api/FeatureFlags/SpanEnrichmentBinder.php b/src/api/FeatureFlags/SpanEnrichmentBinder.php deleted file mode 100644 index fe667def3ec..00000000000 --- a/src/api/FeatureFlags/SpanEnrichmentBinder.php +++ /dev/null @@ -1,60 +0,0 @@ -accumulate($flagKey, $details, $targetingKey); - } -} diff --git a/src/api/FeatureFlags/SpanEnrichmentRegistry.php b/src/api/FeatureFlags/SpanEnrichmentRegistry.php index 242734dd84f..86957999263 100644 --- a/src/api/FeatureFlags/SpanEnrichmentRegistry.php +++ b/src/api/FeatureFlags/SpanEnrichmentRegistry.php @@ -2,154 +2,82 @@ namespace DDTrace\FeatureFlags; +use DDTrace\Util\ObjectKVStore; + /** - * Request-scoped, single shared accumulator + lifecycle for APM feature-flag - * span enrichment, used by ALL PHP evaluation paths (the native - * DDTrace\FeatureFlags\Client and the OpenFeature DataDogProvider). + * APM feature-flag span enrichment: attach evaluated-flag metadata to the root + * span so APM can filter traces/errors by active flag variant. * - * Why a single shared registry (PR review blocker, tracer/ffe.c - * set_ffe_span_enrichment_tags): the native staging API REPLACES the three - * request-global tag slots on every call. If each Client / each provider owned - * its own accumulator and staged independently, two clients, two providers, or - * a mixed OpenFeature + native-client evaluation under the SAME root span would - * OVERWRITE one another's serial ids / hashed subjects / runtime defaults - * instead of AGGREGATING them into one root-span payload. Routing every PHP - * evaluation path through one request-scoped accumulator makes the staged tag - * set the union of all evaluations seen on the active root span, matching the - * frozen Node contract. + * State lives ON the root span (weak-keyed via ObjectKVStore), matching the + * cross-SDK design principle "state on the span/trace, not a central store" + * and every other SDK (JS WeakMap / Python WeakKeyDictionary / Ruby WeakMap / + * Java WeakHashMap keyed by root span / dotnet field on TraceContext). Because + * the accumulator is bound to the span object itself, its identity and lifetime + * ARE the span's: multiple evaluations under one root aggregate into the same + * accumulator, concurrently-open roots each keep their own, and everything is + * released with the span -- no manual root-id tracking or reset lifecycle. * - * Lifecycle is centralized here (PR review should-fix, per-binder onClose - * retention): the registry binds AT MOST ONE root-close reset per root span - * (tracked by $rootCloseBoundRootId), so an arbitrary number of short-lived - * Client instances under one long-lived root span do not each retain a closure - * / accumulator. The accumulator is also reset on any root-span boundary - * transition (CR-01) so a dropped/abandoned root cannot leak into the next root - * or, in persistent SAPIs, the next request. + * The encoded ffe_* tags are written directly onto the root span's meta (the + * same mechanism the rest of the tracer uses to set tags); there is no native + * staging layer. * - * DG-005: the registry stays fully inert when the experimental span-enrichment - * gate is off -- the accumulator is allocated lazily on first use and only when - * the gate is on, so there is no idle per-evaluation or per-span overhead. + * Gated by DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED (off by + * default): when the gate is off, record() is a cheap no-op. * - * KNOWN LIMITATION (PR review, not fixed here): this registry tracks exactly - * ONE active root at a time via a flat $rootId/$accumulator pair, and the - * native staging bridge (tracer/ffe.c set_ffe_span_enrichment_tags) mirrors - * that with a single set of request-global slots. When two roots are - * genuinely open concurrently (fibers, multiple span stacks) and evaluations - * interleave across them, switching the active root discards the other - * root's accumulated data, and flushing can attach staged tags to the wrong - * root. Fixing this properly requires per-root-keyed storage on BOTH sides - * (PHP accumulators keyed by root id, AND the native slots keyed the same - * way) -- a native wire-contract-adjacent change, not a PHP-only patch. - * Deferred as a follow-up given the feature is experimental and gated off by - * default. + * Only ever reached from NativeEvaluator::evaluate(), i.e. with the tracer + * extension loaded (UnavailableEvaluator is used otherwise), so \DDTrace\root_span() + * and dd_trace_env_config() are always available here. * - * PHP 7 compatible (lives in src/api alongside Client / SpanEnrichmentAccumulator - * and is consumed by the PHP 8-only DataDogProvider). + * PHP 7 compatible (lives in src/api alongside Client / SpanEnrichmentAccumulator, + * consumed by the PHP 8-only DataDogProvider via the shared NativeEvaluator). */ final class SpanEnrichmentRegistry { + const ACCUMULATOR_KEY = 'ffe_accumulator'; const SERIAL_ID_METADATA_KEY = 'serialId'; const DO_LOG_METADATA_KEY = 'doLog'; + const CONFIG_KEY = 'DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED'; - /** @var SpanEnrichmentRegistry|null Request-scoped singleton. */ - private static $instance = null; - - /** @var SpanEnrichmentAccumulator|null Allocated lazily, only when the gate is on. */ - private $accumulator = null; - /** @var int|null Identity (root span_id) of the root span currently bound. */ - private $rootId = null; /** - * Identity of the root span we have already bound a one-shot close reset to. - * Ensures AT MOST ONE onClose closure per root span regardless of how many - * evaluation paths route through the registry under that root. + * Whether the experimental span-enrichment gate is on. Process-level env + * config; cannot toggle mid-request. * - * @var int|null + * @return bool */ - private $rootCloseBoundRootId = 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; - - private function __construct() + public static function gateEnabled() { + return \dd_trace_env_config(self::CONFIG_KEY) === true; } /** - * The request-scoped shared registry. Callers MUST gate-check before using - * it (the registry itself does no gate read so it stays inert when unused). + * Record one evaluation's enrichment onto the current root span, then write + * the encoded ffe_* tags onto the root's meta. No-op when the gate is off or + * there is no active root span. Never throws into flag evaluation. * - * @return self - */ - public static function instance() - { - if (self::$instance === null) { - self::$instance = new self(); - } - - return self::$instance; - } - - /** - * Drop the request-scoped singleton. Test-only seam so each test starts with - * a clean shared accumulator (in production the registry is recreated per - * request because PHP statics are reset on request shutdown). - */ - public static function reset() - { - self::$instance = null; - } - - /** - * Inject the root-span lifecycle test seams. Null in production, where the - * registry reads the active root span and binds to its $onClose directly. - * - * @param callable|null $rootIdResolver function(): ?int - * @param callable|null $rootCloseScheduler function(int $rootId, callable $reset): void - */ - public function setRootSpanSeams($rootIdResolver, $rootCloseScheduler) - { - $this->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. + * 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 serial id and no variant is a runtime default. * * @param string $flagKey * @param EvaluationDetails $details * @param string|null $targetingKey */ - public function accumulate($flagKey, $details, $targetingKey) + public static function record($flagKey, $details, $targetingKey) { try { - $this->resetForRootBoundary(); + if (!self::gateEnabled()) { + return; + } - if ($this->rootId === null) { - // No active root span: nothing to attach this evaluation to, and - // staging it now would leak into whichever root opens next. + $root = \DDTrace\root_span(); + if ($root === null) { return; } - if ($this->accumulator === null) { - $this->accumulator = new SpanEnrichmentAccumulator(); + $accumulator = ObjectKVStore::get($root, self::ACCUMULATOR_KEY); + if (!$accumulator instanceof SpanEnrichmentAccumulator) { + $accumulator = new SpanEnrichmentAccumulator(); + ObjectKVStore::put($root, self::ACCUMULATOR_KEY, $accumulator); } $exposure = $details->getExposureData(); @@ -159,208 +87,25 @@ public function accumulate($flagKey, $details, $targetingKey) $doLog = is_array($exposure) && !empty($exposure[self::DO_LOG_METADATA_KEY]); if ($serialId !== null) { - $this->accumulator->addSerialId((int) $serialId); + $accumulator->addSerialId((int) $serialId); if ($doLog && $targetingKey !== null && $targetingKey !== '') { - $this->accumulator->addSubject($targetingKey, (int) $serialId); + $accumulator->addSubject($targetingKey, (int) $serialId); } } else { $variant = $details->getVariant(); if ($variant === null || $variant === '') { - $this->accumulator->addDefault((string) $flagKey, $details->getValue()); + $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. - * - * KNOWN LIMITATION: this is where the single-active-root assumption bites. - * If root A is still open when root B becomes active (concurrent roots via - * fibers / multiple span stacks), this discards A's accumulated data on - * the A->B transition, and switching back to A later starts it fresh - * rather than resuming. See the class docblock. - */ - 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. - * - * peek_root_span_id() returns the root's own span_id, not spl_object_id(): - * object handles are recycled by the engine once a span object is - * destroyed (e.g. a dropped root), so a handle-based identity could alias a - * later, unrelated root and make resetForRootBoundary() wrongly skip its - * reset (PR review, handle-reuse finding). span_id is a random 64-bit value - * that is never reused within the process, so it does not have this - * problem. The spl_object_id() fallback below (old extensions without the - * peek helper) keeps the theoretical handle-reuse exposure, but that path - * only exists for pre-peek-helper extensions. - * - * @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; - } - - if (!\function_exists('spl_object_id')) { - 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; + // Write (overwrite) the encoded union onto the root span's meta. Safe + // to do on every evaluation: toSpanTags() re-encodes the accumulator's + // full state, so the latest write is always the complete union. + foreach ($accumulator->toSpanTags() as $key => $value) { + $root->meta[$key] = $value; } - 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 = self::prependOnCloseReset($root->onClose, $reset); - } - - /** - * Prepend, not append, the reset closure to $onClose: the native close - * path (tracer/span.c) runs $onClose in REVERSE registration order (last - * registered runs first), so prepending guarantees this reset is the LAST - * callback to run for this root regardless of how many other callbacks get - * registered after it. If it instead ran before an earlier-registered - * callback that itself evaluates a flag during root close, this reset would - * look like a root-boundary crossing to that evaluation and wipe out the - * union already staged for this root (PR review: reset ordering vs. other - * onClose callbacks). Kept as its own method so the ordering guarantee is - * directly testable without a real root span / native extension. - * - * @param mixed $onClose - * @param callable $reset - * @return array - */ - private static function prependOnCloseReset($onClose, $reset) - { - if (!\is_array($onClose)) { - $onClose = array(); - } - \array_unshift($onClose, static function () use ($reset) { - $reset(); - }); - - return $onClose; - } - - /** - * 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(); + } catch (\Throwable $e) { + // Enrichment must never break flag evaluation. } - - return $this->accumulator->toSpanTags(); } } diff --git a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php index 3ba95a80efa..3f463bdfa26 100644 --- a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php +++ b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php @@ -4,29 +4,20 @@ namespace DDTrace\Tests\OpenFeature; -use DDTrace\FeatureFlags\EvaluationDetails; -use DDTrace\FeatureFlags\EvaluationReason; -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; -use OpenFeature\implementation\flags\Attributes; -use OpenFeature\implementation\flags\EvaluationContext; use PHPUnit\Framework\TestCase; /** - * L0 suite for the PHP APM feature-flag span enrichment (PHP-01). + * Unit suite for the frozen APM feature-flag span-enrichment codec. * - * These tests exercise the frozen contract logic that does NOT require the - * native extension: the codec, the accumulator limits/shapes, and the DG-004 - * inline accumulation in DataDogProvider::resolve(). The native close-span - * write (ffe_* onto root meta) and the C-bridge serial_id passthrough are - * covered by tests/ext/ffe/serial_id_passthrough.phpt against the built - * extension. + * These tests exercise only the SpanEnrichmentAccumulator contract logic that + * does NOT require the native extension: the delta-varint/base64 codec, the + * limits/dedupe rules, runtime-default stringification, and tag shapes. + * + * The end-to-end recording path (SpanEnrichmentRegistry::record() attaching the + * accumulator to the root span and writing ffe_* onto the root meta) and the + * C-bridge serial_id passthrough are covered by the .phpt ext tests against the + * built extension. */ final class SpanEnrichmentAccumulatorTest extends TestCase { @@ -330,487 +321,4 @@ 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) - ); - // 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); - } - - // ---- 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 testGateOffAllocatesNoBinder(): void - { - // 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)); - - $binderProp = self::accessibleProperty($provider, 'spanEnrichmentBinder'); - - 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 spanEnrichmentBinder null. - $provider = DataDogProvider::createWithDependencies( - $evaluator, - new NullLogger(LogLevel::EMERGENCY) - ); - - $provider->resolveStringValue('flag', 'fallback', new EvaluationContext('user-123')); - - // 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 ---------------------------------------------------------- - - /** - * 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 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; - } -} - -/** - * 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; - } -} - -/** - * Pure-PHP simulator of the native root-span lifecycle so the CR-01 regression - * 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() (the root span's own span_id). - * - 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 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 -{ - /** @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; - - SpanEnrichmentRegistry::instance()->setRootSpanSeams( - function (): ?int { - return $this->currentRootId; - }, - 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 the shared registry's stage() pushes into the native store. - * - * @return array - */ - public function lastStagedTags(): array - { - return $this->accumulator->toSpanTags(); - } - - public function accumulator(): SpanEnrichmentAccumulator - { - return $this->accumulator; - } } diff --git a/tests/api/Unit/FeatureFlags/ClientTest.php b/tests/api/Unit/FeatureFlags/ClientTest.php index 5df917995fe..b2bc83d8321 100644 --- a/tests/api/Unit/FeatureFlags/ClientTest.php +++ b/tests/api/Unit/FeatureFlags/ClientTest.php @@ -151,34 +151,6 @@ 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 deleted file mode 100644 index 854979b0d73..00000000000 --- a/tests/api/Unit/FeatureFlags/SpanEnrichmentRegistryTest.php +++ /dev/null @@ -1,271 +0,0 @@ -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 testAccumulateWithoutAnActiveRootIsSkippedRatherThanStaged() - { - // PR review: an evaluation made with no active root span (rootIdResolver - // returns null) must not be staged -- otherwise it would leak onto - // whichever root span opens next. - SpanEnrichmentRegistry::reset(); - $registry = SpanEnrichmentRegistry::instance(); - $registry->setRootSpanSeams(function () { - return null; - }, function ($id, $reset) { - }); - - $binder = new SpanEnrichmentBinder(); - $binder->accumulate('flag-a', $this->detailsWithSerialId(100), null); - - $this->assertSame(array(), $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()); - } - - /** - * Regression for the reset-ordering bug (PR review): tracer/span.c fires - * $root->onClose in REVERSE registration order. If the registry's reset - * were simply appended, it would run BEFORE an earlier-registered onClose - * callback that itself evaluates a flag during root close -- wiping the - * PHP-side bookkeeping mid-close and losing that evaluation from the - * union. prependOnCloseReset() must guarantee the reset is the LAST - * onClose callback to fire regardless of registration order. - */ - public function testPrependOnCloseResetRunsLastUnderReverseIteration() - { - $method = new \ReflectionMethod(SpanEnrichmentRegistry::class, 'prependOnCloseReset'); - $method->setAccessible(true); - - $order = array(); - $appCallback = function () use (&$order) { - $order[] = 'app'; - }; - $reset = function () use (&$order) { - $order[] = 'reset'; - }; - - // Simulate: app code registers its onClose callback BEFORE the first - // flag evaluation schedules the registry's reset. - $onClose = array($appCallback); - $onClose = $method->invoke(null, $onClose, $reset); - - // A later integration registers another callback after ours is bound. - $laterCallback = function () use (&$order) { - $order[] = 'later'; - }; - $onClose[] = $laterCallback; - - // Mirror tracer/span.c's ZEND_HASH_REVERSE_FOREACH_VAL: last element first. - foreach (array_reverse($onClose) as $callback) { - $callback(); - } - - $this->assertSame(array('later', 'app', 'reset'), $order); - } - - 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/system_test_data_evaluate.phpt b/tests/ext/ffe/system_test_data_evaluate.phpt index fcc2e5a784a..43398f7e5c6 100644 --- a/tests/ext/ffe/system_test_data_evaluate.phpt +++ b/tests/ext/ffe/system_test_data_evaluate.phpt @@ -114,7 +114,6 @@ function require_feature_flag_api($root) 'EvaluationDetails', 'SpanEnrichmentAccumulator', 'SpanEnrichmentRegistry', - 'SpanEnrichmentBinder', ) as $classFile) { require_once $apiRoot . '/' . $classFile . '.php'; } From dd06af9dca2184aea7cb54c2adf98d568aada73c Mon Sep 17 00:00:00 2001 From: Pavel Date: Thu, 23 Jul 2026 18:43:19 +0300 Subject: [PATCH 21/26] refactor(ffe): remove native span-enrichment staging layer Enrichment now writes ffe_* directly to root_span()->meta from PHP (previous commit), so the native staging path is dead code. Remove the request-global staging slots, the set_ffe_span_enrichment_tags / peek_root_span_id internal functions, and the close-span flush / dropped-root / rshutdown clear hooks. Drop the DD_TEST_HELPERS guard around await_ffe_config (kept, like the other dd_trace_internal_fn helpers). Delete the .phpt tests that covered the removed native surface. --- config.m4 | 9 --- .../ffe/peek_root_span_id_non_creating.phpt | 51 ------------- .../ext/ffe/root_drop_clears_staged_tags.phpt | 45 ----------- tests/ext/ffe/serial_id_passthrough.phpt | 4 - tracer/ddtrace.c | 4 - tracer/ddtrace.stub.php | 23 ------ tracer/ddtrace_arginfo.h | 13 ---- tracer/ddtrace_globals.h | 10 --- tracer/ffe.c | 76 ------------------- tracer/ffe.h | 7 -- tracer/functions.c | 48 ++---------- tracer/span.c | 19 ----- 12 files changed, 5 insertions(+), 304 deletions(-) delete mode 100644 tests/ext/ffe/peek_root_span_id_non_creating.phpt delete mode 100644 tests/ext/ffe/root_drop_clears_staged_tags.phpt diff --git a/config.m4 b/config.m4 index d34c8627821..aa69d309a43 100644 --- a/config.m4 +++ b/config.m4 @@ -262,15 +262,6 @@ 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/tests/ext/ffe/peek_root_span_id_non_creating.phpt b/tests/ext/ffe/peek_root_span_id_non_creating.phpt deleted file mode 100644 index 7689007b25a..00000000000 --- a/tests/ext/ffe/peek_root_span_id_non_creating.phpt +++ /dev/null @@ -1,51 +0,0 @@ ---TEST-- -FFE span enrichment: DDTrace\Internal\peek_root_span_id() is non-creating and stable/unique per root ---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_stable_within_same_root=true -peek_distinct_across_roots=true diff --git a/tests/ext/ffe/root_drop_clears_staged_tags.phpt b/tests/ext/ffe/root_drop_clears_staged_tags.phpt deleted file mode 100644 index 00b4d0dd05a..00000000000 --- a/tests/ext/ffe/root_drop_clears_staged_tags.phpt +++ /dev/null @@ -1,45 +0,0 @@ ---TEST-- -FFE span enrichment: dropping a root span clears staged tags so they cannot leak onto the next root ---SKIPIF-- - ---INI-- -datadog.trace.generate_root_span=0 -datadog.trace.auto_flush_enabled=1 ---ENV-- -DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED=1 ---FILE-- -2 safe-reject-via-normal-close -// fallback. -\DDTrace\Internal\set_ffe_span_enrichment_tags("ZAgUAg==", null, null); - -$extraRef = $span = \DDTrace\start_span(); -$span->onClose[] = function ($span) { - var_dump(\DDTrace\try_drop_span($span)); -}; -\DDTrace\close_span(); - -// A second, unrelated root opens and closes normally. Pre-fix, the tags -// staged for the dropped root above would still be sitting in the native -// globals and would get flushed onto THIS span instead of being discarded. -// Inspect ->meta directly on the held object rather than via -// dd_trace_serialize_closed_spans(): auto_flush_enabled=1 (required above so -// try_drop_span() takes the real-drop branch rather than the refcount>2 -// safe-reject-via-normal-close fallback) drains the closed-span buffer as -// soon as this span closes, so serialize_closed_spans() would see nothing. -$secondRoot = \DDTrace\start_span(); -\DDTrace\close_span(); - -var_dump(array_key_exists("ffe_flags_enc", $secondRoot->meta)); -?> ---EXPECT-- -bool(true) -bool(false) diff --git a/tests/ext/ffe/serial_id_passthrough.phpt b/tests/ext/ffe/serial_id_passthrough.phpt index 25d65e2f06c..5c8ffc6b30a 100644 --- a/tests/ext/ffe/serial_id_passthrough.phpt +++ b/tests/ext/ffe/serial_id_passthrough.phpt @@ -65,9 +65,6 @@ 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 @@ -78,4 +75,3 @@ 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 67cdffe5e0a..7f5e9681d92 100644 --- a/tracer/ddtrace.c +++ b/tracer/ddtrace.c @@ -612,10 +612,6 @@ 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 e274d33c4fb..adac5b1f10c 100644 --- a/tracer/ddtrace.stub.php +++ b/tracer/ddtrace.stub.php @@ -1093,29 +1093,6 @@ 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 {} - - /** - * Identity (the root span's own span_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 adb4fed6c80..afa0f62d9f7 100644 --- a/tracer/ddtrace_arginfo.h +++ b/tracer/ddtrace_arginfo.h @@ -258,15 +258,6 @@ 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_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") @@ -454,8 +445,6 @@ 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(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); @@ -558,8 +547,6 @@ 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("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/ddtrace_globals.h b/tracer/ddtrace_globals.h index 19f44dcbea2..3865b2e48f9 100644 --- a/tracer/ddtrace_globals.h +++ b/tracer/ddtrace_globals.h @@ -100,16 +100,6 @@ 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 d4fe28e7670..13653a2fab0 100644 --- a/tracer/ffe.c +++ b/tracer/ffe.c @@ -250,79 +250,3 @@ 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 717763f26fc..0b61ab9a210 100644 --- a/tracer/ffe.h +++ b/tracer/ffe.h @@ -11,11 +11,4 @@ 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 a22c84ad29e..44f4478ed41 100644 --- a/tracer/functions.c +++ b/tracer/functions.c @@ -2121,14 +2121,11 @@ 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. + // await_ffe_config is a test/debug helper (like the other + // dd_trace_internal_fn helpers): it actively pumps Remote Config and can + // block for up to 5s, so long-running CLI test servers (the system-tests + // parametric app / ffe-dogfooding harness) can deterministically wait for + // the pushed UFC before evaluating. } 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 @@ -2149,7 +2146,6 @@ 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..."]] @@ -2851,40 +2847,6 @@ 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); -} - -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 - // root's own span_id (a random 64-bit id, never reused within the process) - // rather than the zend object handle: handles are recycled by the engine's - // object store once a span object is destroyed (e.g. a dropped root), so a - // handle-based identity could alias a later, unrelated root and make the - // PHP-side accumulator wrongly treat it as still bound to the old one. - 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->span_id); -} - /* {{{ 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 10300363df5..a744e020cc2 100644 --- a/tracer/span.c +++ b/tracer/span.c @@ -25,7 +25,6 @@ #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 @@ -968,20 +967,6 @@ void ddtrace_close_top_span_without_stack_swap(ddtrace_span_data *span) { stack->closed_ring = span; } - if (span->std.ce == ddtrace_ce_root_span_data) { - ddtrace_root_span_data *root = ROOTSPANDATA(&span->std); - - // APM feature-flag span enrichment (PHP-01): flush any staged ffe_* - // tags into the root's meta BEFORE the sampling decision below runs, - // so span-level sampling rules that match against ffe_* tags can - // actually see them (PR review: this used to run after the sampling - // decision, so such rules could never match). 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)); - } - ddtrace_decide_on_closed_span_sampling(span); if (span->notify_user_req_end) { ddtrace_user_req_notify_finish(span); @@ -1096,10 +1081,6 @@ void ddtrace_drop_span(ddtrace_span_data *span) { --DDTRACE_G(open_spans_count); if (&stack->root_span->span == span) { - // A dropped root never reaches ddtrace_ffe_flush_span_enrichment_tags(), so any - // staged ffe_* tags for this root must be discarded here -- otherwise they would - // be flushed onto whichever root span closes next (PR review: FFE span leakage). - ddtrace_ffe_clear_span_enrichment_tags(); ddtrace_switch_span_stack(stack->parent_stack); stack->root_span = NULL; } else if (!stack->active || SPANDATA(stack->active)->stack != stack) { From 1738ca1054aa376d44f1f026422cb8a827ac6e9e Mon Sep 17 00:00:00 2001 From: Pavel Date: Thu, 23 Jul 2026 18:51:59 +0300 Subject: [PATCH 22/26] test(ffe): cover span-enrichment recording against real root spans Add .phpt tests driving SpanEnrichmentRegistry::record() through real root spans: aggregation onto root_span()->meta (flags + hashed subjects + runtime defaults), per-root isolation across concurrent stacks (the case the old single-active-root design couldn't handle), and gate-off no-op. --- .../ffe/span_enrichment_concurrent_roots.phpt | 61 +++++++++++++++++ tests/ext/ffe/span_enrichment_gate_off.phpt | 45 +++++++++++++ tests/ext/ffe/span_enrichment_root_meta.phpt | 67 +++++++++++++++++++ 3 files changed, 173 insertions(+) create mode 100644 tests/ext/ffe/span_enrichment_concurrent_roots.phpt create mode 100644 tests/ext/ffe/span_enrichment_gate_off.phpt create mode 100644 tests/ext/ffe/span_enrichment_root_meta.phpt diff --git a/tests/ext/ffe/span_enrichment_concurrent_roots.phpt b/tests/ext/ffe/span_enrichment_concurrent_roots.phpt new file mode 100644 index 00000000000..26d61d00c25 --- /dev/null +++ b/tests/ext/ffe/span_enrichment_concurrent_roots.phpt @@ -0,0 +1,61 @@ +--TEST-- +FFE span enrichment: concurrently-open root spans each keep their own tags +--INI-- +datadog.trace.generate_root_span=0 +datadog.experimental_flagging_provider_span_enrichment_enabled=1 +--FILE-- + 100, 'doLog' => false)); +SpanEnrichmentRegistry::record('flag.a', $a, null); + +// Root B on a second, independent stack (models concurrent roots / fibers). +\DDTrace\create_stack(); +\DDTrace\start_span(); +$rootB = \DDTrace\root_span(); +$b = new EvaluationDetails('off', EvaluationType::STRING, EvaluationReason::SPLIT, 'b', null, null, array(), array('serialId' => 200, 'doLog' => false)); +SpanEnrichmentRegistry::record('flag.b', $b, null); + +// Each root carries only its own evaluation; keying the accumulator on the span +// object makes this correct without any manual root tracking. +show('rootA_flags', $codec->decodeDeltaVarint($rootA->meta['ffe_flags_enc'])); +show('rootB_flags', $codec->decodeDeltaVarint($rootB->meta['ffe_flags_enc'])); +show('rootA_has_b', in_array(200, $codec->decodeDeltaVarint($rootA->meta['ffe_flags_enc']), true)); +show('rootB_has_a', in_array(100, $codec->decodeDeltaVarint($rootB->meta['ffe_flags_enc']), true)); +?> +--EXPECT-- +rootA_flags=[100] +rootB_flags=[200] +rootA_has_b=false +rootB_has_a=false diff --git a/tests/ext/ffe/span_enrichment_gate_off.phpt b/tests/ext/ffe/span_enrichment_gate_off.phpt new file mode 100644 index 00000000000..c0c8a9c8200 --- /dev/null +++ b/tests/ext/ffe/span_enrichment_gate_off.phpt @@ -0,0 +1,45 @@ +--TEST-- +FFE span enrichment: gate off writes no ffe_* tags onto the root span +--INI-- +datadog.experimental_flagging_provider_span_enrichment_enabled=0 +--FILE-- + 100, 'doLog' => true)); +SpanEnrichmentRegistry::record('flag.a', $a, 'user-1'); + +$meta = \DDTrace\root_span()->meta; +show('has_flags', isset($meta['ffe_flags_enc'])); +show('has_subjects', isset($meta['ffe_subjects_enc'])); +show('has_defaults', isset($meta['ffe_runtime_defaults'])); +?> +--EXPECT-- +has_flags=false +has_subjects=false +has_defaults=false diff --git a/tests/ext/ffe/span_enrichment_root_meta.phpt b/tests/ext/ffe/span_enrichment_root_meta.phpt new file mode 100644 index 00000000000..8b6daf70a73 --- /dev/null +++ b/tests/ext/ffe/span_enrichment_root_meta.phpt @@ -0,0 +1,67 @@ +--TEST-- +FFE span enrichment: evaluations aggregate onto the active root span's meta +--INI-- +datadog.experimental_flagging_provider_span_enrichment_enabled=1 +--FILE-- + 100, 'doLog' => true)); +SpanEnrichmentRegistry::record('flag.a', $a, 'user-1'); + +$b = new EvaluationDetails('blue', EvaluationType::STRING, EvaluationReason::SPLIT, 'blue', null, null, array(), array('serialId' => 108, 'doLog' => false)); +SpanEnrichmentRegistry::record('flag.b', $b, 'user-1'); + +$c = new EvaluationDetails('fallback', EvaluationType::STRING, EvaluationReason::DEFAULT_REASON, null, null, null, array(), array()); +SpanEnrichmentRegistry::record('flag.c', $c, null); + +$meta = \DDTrace\root_span()->meta; +$codec = new SpanEnrichmentAccumulator(); + +show('flags', $codec->decodeDeltaVarint($meta['ffe_flags_enc'])); + +$subjects = json_decode($meta['ffe_subjects_enc'], true); +$subjectKeys = array_keys($subjects); +$subjectVals = array_values($subjects); +show('subject_count', count($subjects)); +show('subject_key_is_sha256', (bool) preg_match('/^[0-9a-f]{64}$/', (string) $subjectKeys[0])); +show('subject_ids', $codec->decodeDeltaVarint($subjectVals[0])); + +show('defaults', json_decode($meta['ffe_runtime_defaults'], true)); +?> +--EXPECT-- +flags=[100,108] +subject_count=1 +subject_key_is_sha256=true +subject_ids=[100] +defaults={"flag.c":"fallback"} From 2b928325d398e430f3e1eb1179021f60a416f925 Mon Sep 17 00:00:00 2001 From: Pavel Date: Thu, 23 Jul 2026 18:57:47 +0300 Subject: [PATCH 23/26] docs(ffe): refresh span-enrichment comments for the ObjectKVStore design Drop stale design-doc artifacts (DG-/CR-/Pattern labels) and references to the removed native-staging / close-span path and the old accumulate() method. --- src/api/FeatureFlags/Internal/ResultMapper.php | 2 +- src/api/FeatureFlags/SpanEnrichmentAccumulator.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/api/FeatureFlags/Internal/ResultMapper.php b/src/api/FeatureFlags/Internal/ResultMapper.php index 3694700b1e8..d263d64e085 100644 --- a/src/api/FeatureFlags/Internal/ResultMapper.php +++ b/src/api/FeatureFlags/Internal/ResultMapper.php @@ -279,7 +279,7 @@ private function exposureData($rawResult) // result actually carried one; a null/absent value must be left out // entirely rather than defaulted to 0/false. Absence alone does NOT // mean a runtime default was used -- a waterfall-assigned variant can - // also have no serial id. SpanEnrichmentRegistry::accumulate() only + // also have no serial id. SpanEnrichmentRegistry::record() only // treats an evaluation as a runtime default when BOTH serialId AND the // variant are absent. $serialId = $this->read($rawResult, array('serial_id', 'serialId'), null); diff --git a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php index eb685589238..ee95ce1c6c6 100644 --- a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php +++ b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php @@ -12,9 +12,9 @@ * system-tests assert exact parity, so any divergence (signed varint, wrong * sort, `[object Object]` defaults, bare-vs-JSON tag shape) breaks the chain. * - * Lifecycle: one instance per root span, request-scoped. The DataDogProvider - * constructs it lazily only when the span-enrichment gate is on (DG-005), and - * the native close-span path flushes + clears it on root-span finish. + * Lifecycle: one instance per root span. SpanEnrichmentRegistry::record() + * lazily attaches it to the root span (via ObjectKVStore) on first evaluation + * and encodes its state onto the root span's meta; it is released with the span. */ final class SpanEnrichmentAccumulator { @@ -55,8 +55,8 @@ public function addSerialId($id) /** * 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. + * SHA256-hashed before storage (privacy: raw targeting keys are never + * emitted) and is only recorded when `do_log` authorizes it. */ public function addSubject($targetingKey, $id) { @@ -114,7 +114,7 @@ public function hasData() /** * Encode the accumulated state into the frozen `ffe_*` span tag set. * - * Output-shape contract (Pattern F): + * Output-shape contract (frozen): * - ffe_flags_enc => BARE base64 string * - ffe_subjects_enc => JSON-stringified object {sha256hex: base64} * - ffe_runtime_defaults => JSON-stringified object {flagKey: valueStr} From 2cb81e17204bd02efd8ab0dc9f798042f1135a9f Mon Sep 17 00:00:00 2001 From: Pavel Date: Thu, 23 Jul 2026 19:45:03 +0300 Subject: [PATCH 24/26] test(ffe): fix span-enrichment .phpt for pecl + concurrent roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skip the src/-dependent tests in the PECL package (src/ isn't shipped there, matching system_test_data_evaluate). Rebuild the concurrent-roots test with start_trace_span() — create_stack() inherits the parent trace's root, so both stacks shared one root; start_trace_span() creates a genuinely independent root. --- .../ffe/span_enrichment_concurrent_roots.phpt | 18 +++++++++++------- tests/ext/ffe/span_enrichment_gate_off.phpt | 6 ++++++ tests/ext/ffe/span_enrichment_root_meta.phpt | 6 ++++++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/tests/ext/ffe/span_enrichment_concurrent_roots.phpt b/tests/ext/ffe/span_enrichment_concurrent_roots.phpt index 26d61d00c25..55abce3443d 100644 --- a/tests/ext/ffe/span_enrichment_concurrent_roots.phpt +++ b/tests/ext/ffe/span_enrichment_concurrent_roots.phpt @@ -1,5 +1,11 @@ --TEST-- FFE span enrichment: concurrently-open root spans each keep their own tags +--SKIPIF-- + --INI-- datadog.trace.generate_root_span=0 datadog.experimental_flagging_provider_span_enrichment_enabled=1 @@ -34,16 +40,14 @@ function show($label, $value) { $codec = new SpanEnrichmentAccumulator(); -// Root A on the initial stack. -\DDTrace\start_span(); -$rootA = \DDTrace\root_span(); +// Trace A: start_trace_span() creates a NEW root span on its own stack. +$rootA = \DDTrace\start_trace_span(); $a = new EvaluationDetails('on', EvaluationType::STRING, EvaluationReason::SPLIT, 'a', null, null, array(), array('serialId' => 100, 'doLog' => false)); SpanEnrichmentRegistry::record('flag.a', $a, null); -// Root B on a second, independent stack (models concurrent roots / fibers). -\DDTrace\create_stack(); -\DDTrace\start_span(); -$rootB = \DDTrace\root_span(); +// Trace B: a second, concurrently-open root on its own stack (A stays open +// underneath). Models the fibers / multiple-span-stacks case. +$rootB = \DDTrace\start_trace_span(); $b = new EvaluationDetails('off', EvaluationType::STRING, EvaluationReason::SPLIT, 'b', null, null, array(), array('serialId' => 200, 'doLog' => false)); SpanEnrichmentRegistry::record('flag.b', $b, null); diff --git a/tests/ext/ffe/span_enrichment_gate_off.phpt b/tests/ext/ffe/span_enrichment_gate_off.phpt index c0c8a9c8200..726a7a77aa1 100644 --- a/tests/ext/ffe/span_enrichment_gate_off.phpt +++ b/tests/ext/ffe/span_enrichment_gate_off.phpt @@ -1,5 +1,11 @@ --TEST-- FFE span enrichment: gate off writes no ffe_* tags onto the root span +--SKIPIF-- + --INI-- datadog.experimental_flagging_provider_span_enrichment_enabled=0 --FILE-- diff --git a/tests/ext/ffe/span_enrichment_root_meta.phpt b/tests/ext/ffe/span_enrichment_root_meta.phpt index 8b6daf70a73..a36fd936595 100644 --- a/tests/ext/ffe/span_enrichment_root_meta.phpt +++ b/tests/ext/ffe/span_enrichment_root_meta.phpt @@ -1,5 +1,11 @@ --TEST-- FFE span enrichment: evaluations aggregate onto the active root span's meta +--SKIPIF-- + --INI-- datadog.experimental_flagging_provider_span_enrichment_enabled=1 --FILE-- From 92f0115d5d9427bc61c2aedc64ebbd0f4f7616f5 Mon Sep 17 00:00:00 2001 From: Pavel Date: Fri, 24 Jul 2026 14:06:38 +0300 Subject: [PATCH 25/26] =?UTF-8?q?refactor(ffe):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20typed=20args,=20dirty-guarded=20tag=20writes,=20sim?= =?UTF-8?q?pler=20UTF-8=20truncation,=20drop=20dead=20clear()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../SpanEnrichmentAccumulator.php | 54 ++++++++++--------- .../FeatureFlags/SpanEnrichmentRegistry.php | 20 +++---- .../SpanEnrichmentAccumulatorTest.php | 13 ----- 3 files changed, 40 insertions(+), 47 deletions(-) diff --git a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php index ee95ce1c6c6..7bfa9c5c41b 100644 --- a/src/api/FeatureFlags/SpanEnrichmentAccumulator.php +++ b/src/api/FeatureFlags/SpanEnrichmentAccumulator.php @@ -37,13 +37,15 @@ final class SpanEnrichmentAccumulator /** @var array flagKey => stringified default value. */ private $defaults = array(); + /** @var bool Whether the state changed since the last toSpanTags() encode. */ + private $dirty = false; + /** * 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) + public function addSerialId(int $id) { - $id = (int) $id; if (isset($this->serialIds[$id])) { return; } @@ -51,6 +53,7 @@ public function addSerialId($id) return; } $this->serialIds[$id] = true; + $this->dirty = true; } /** @@ -58,10 +61,9 @@ public function addSerialId($id) * SHA256-hashed before storage (privacy: raw targeting keys are never * emitted) and is only recorded when `do_log` authorizes it. */ - public function addSubject($targetingKey, $id) + public function addSubject(string $targetingKey, int $id) { - $id = (int) $id; - $hashed = $this->hashTargetingKey((string) $targetingKey); + $hashed = $this->hashTargetingKey($targetingKey); if (isset($this->subjects[$hashed])) { if (isset($this->subjects[$hashed][$id])) { @@ -71,6 +73,7 @@ public function addSubject($targetingKey, $id) return; } $this->subjects[$hashed][$id] = true; + $this->dirty = true; return; } @@ -78,6 +81,7 @@ public function addSubject($targetingKey, $id) return; } $this->subjects[$hashed] = array($id => true); + $this->dirty = true; } /** @@ -88,9 +92,8 @@ public function addSubject($targetingKey, $id) * * @param mixed $value */ - public function addDefault($flagKey, $value) + public function addDefault(string $flagKey, $value) { - $flagKey = (string) $flagKey; if (array_key_exists($flagKey, $this->defaults)) { return; } @@ -99,6 +102,19 @@ public function addDefault($flagKey, $value) } $this->defaults[$flagKey] = $this->stringifyDefault($value); + $this->dirty = true; + } + + /** + * Whether the accumulated state changed since the last call, consuming the + * flag. Lets the caller skip re-encoding and re-writing the span tags when + * an evaluation added nothing new. + */ + public function consumeDirty(): bool + { + $wasDirty = $this->dirty; + $this->dirty = false; + return $wasDirty; } /** @@ -148,17 +164,6 @@ public function toSpanTags() 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). * @@ -300,16 +305,15 @@ private function stringifyFloat($value) /** * 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. + * UTF-8 sequence. Matches up to $maxLength code points from the start: /u + * treats the subject as UTF-8 and /s lets '.' span newlines. If the subject + * is not valid UTF-8 (preg_match returns false), falls back to a + * codepoint-safe byte walk. */ 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 (preg_match('/\A.{0,' . (int) $maxLength . '}/su', $value, $m)) { + return $m[0]; } return $this->truncateUtf8ByteFallback($value, $maxLength); diff --git a/src/api/FeatureFlags/SpanEnrichmentRegistry.php b/src/api/FeatureFlags/SpanEnrichmentRegistry.php index 86957999263..4194d9243e7 100644 --- a/src/api/FeatureFlags/SpanEnrichmentRegistry.php +++ b/src/api/FeatureFlags/SpanEnrichmentRegistry.php @@ -39,8 +39,7 @@ final class SpanEnrichmentRegistry const CONFIG_KEY = 'DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED'; /** - * Whether the experimental span-enrichment gate is on. Process-level env - * config; cannot toggle mid-request. + * Whether the experimental span-enrichment gate is on. * * @return bool */ @@ -80,11 +79,12 @@ public static function record($flagKey, $details, $targetingKey) ObjectKVStore::put($root, self::ACCUMULATOR_KEY, $accumulator); } + // getExposureData() always returns an array (EvaluationDetails types it). $exposure = $details->getExposureData(); - $serialId = is_array($exposure) && array_key_exists(self::SERIAL_ID_METADATA_KEY, $exposure) + $serialId = 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]); + $doLog = !empty($exposure[self::DO_LOG_METADATA_KEY]); if ($serialId !== null) { $accumulator->addSerialId((int) $serialId); @@ -98,11 +98,13 @@ public static function record($flagKey, $details, $targetingKey) } } - // Write (overwrite) the encoded union onto the root span's meta. Safe - // to do on every evaluation: toSpanTags() re-encodes the accumulator's - // full state, so the latest write is always the complete union. - foreach ($accumulator->toSpanTags() as $key => $value) { - $root->meta[$key] = $value; + // Only re-encode + rewrite the tags when this evaluation actually + // changed the accumulated state; a repeated/duplicate evaluation adds + // nothing new and the meta already holds the current union. + if ($accumulator->consumeDirty()) { + foreach ($accumulator->toSpanTags() as $key => $value) { + $root->meta[$key] = $value; + } } } catch (\Throwable $e) { // Enrichment must never break flag evaluation. diff --git a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php index 3f463bdfa26..56615a3bb9f 100644 --- a/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php +++ b/tests/OpenFeature/SpanEnrichmentAccumulatorTest.php @@ -308,17 +308,4 @@ public function testFlagsTagIsBareBase64NotJson(): void 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()); - } - } From bf321e601707a228626dda77b2d3f78fdd50b4b4 Mon Sep 17 00:00:00 2001 From: Pavel Date: Fri, 24 Jul 2026 16:40:01 +0300 Subject: [PATCH 26/26] test(ffe): run span-enrichment .phpt under the PHPUnit featureflags suite instead of ext --- .../span_enrichment_concurrent_roots.phpt | 17 +++++------------ .../span_enrichment_gate_off.phpt | 17 +++++------------ .../span_enrichment_root_meta.phpt | 17 +++++------------ tests/phpunit.xml | 1 + 4 files changed, 16 insertions(+), 36 deletions(-) rename tests/{ext/ffe => OpenFeature}/span_enrichment_concurrent_roots.phpt (83%) rename tests/{ext/ffe => OpenFeature}/span_enrichment_gate_off.phpt (75%) rename tests/{ext/ffe => OpenFeature}/span_enrichment_root_meta.phpt (85%) diff --git a/tests/ext/ffe/span_enrichment_concurrent_roots.phpt b/tests/OpenFeature/span_enrichment_concurrent_roots.phpt similarity index 83% rename from tests/ext/ffe/span_enrichment_concurrent_roots.phpt rename to tests/OpenFeature/span_enrichment_concurrent_roots.phpt index 55abce3443d..f80927c2996 100644 --- a/tests/ext/ffe/span_enrichment_concurrent_roots.phpt +++ b/tests/OpenFeature/span_enrichment_concurrent_roots.phpt @@ -1,11 +1,5 @@ --TEST-- FFE span enrichment: concurrently-open root spans each keep their own tags ---SKIPIF-- - --INI-- datadog.trace.generate_root_span=0 datadog.experimental_flagging_provider_span_enrichment_enabled=1 @@ -18,11 +12,10 @@ use DDTrace\FeatureFlags\EvaluationType; use DDTrace\FeatureFlags\SpanEnrichmentAccumulator; use DDTrace\FeatureFlags\SpanEnrichmentRegistry; -$root = getenv('TEST_PHP_SRCDIR'); -if (!is_string($root) || $root === '') { - $root = dirname(dirname(dirname(__DIR__))); -} -require_once $root . '/src/DDTrace/Util/ObjectKVStore.php'; +// Runs under the PHPUnit `featureflags` suite (PHP 8.0+), CWD = repo root. The +// DDTrace API classes are plain PHP under src/; load them explicitly (the +// OpenFeature composer autoload does not map the DDTrace namespace). +require_once './src/DDTrace/Util/ObjectKVStore.php'; foreach (array( 'EvaluationType', 'EvaluationReason', @@ -31,7 +24,7 @@ foreach (array( 'SpanEnrichmentAccumulator', 'SpanEnrichmentRegistry', ) as $classFile) { - require_once $root . '/src/api/FeatureFlags/' . $classFile . '.php'; + require_once './src/api/FeatureFlags/' . $classFile . '.php'; } function show($label, $value) { diff --git a/tests/ext/ffe/span_enrichment_gate_off.phpt b/tests/OpenFeature/span_enrichment_gate_off.phpt similarity index 75% rename from tests/ext/ffe/span_enrichment_gate_off.phpt rename to tests/OpenFeature/span_enrichment_gate_off.phpt index 726a7a77aa1..517d0880f56 100644 --- a/tests/ext/ffe/span_enrichment_gate_off.phpt +++ b/tests/OpenFeature/span_enrichment_gate_off.phpt @@ -1,11 +1,5 @@ --TEST-- FFE span enrichment: gate off writes no ffe_* tags onto the root span ---SKIPIF-- - --INI-- datadog.experimental_flagging_provider_span_enrichment_enabled=0 --FILE-- @@ -16,11 +10,10 @@ use DDTrace\FeatureFlags\EvaluationReason; use DDTrace\FeatureFlags\EvaluationType; use DDTrace\FeatureFlags\SpanEnrichmentRegistry; -$root = getenv('TEST_PHP_SRCDIR'); -if (!is_string($root) || $root === '') { - $root = dirname(dirname(dirname(__DIR__))); -} -require_once $root . '/src/DDTrace/Util/ObjectKVStore.php'; +// Runs under the PHPUnit `featureflags` suite (PHP 8.0+), CWD = repo root. The +// DDTrace API classes are plain PHP under src/; load them explicitly (the +// OpenFeature composer autoload does not map the DDTrace namespace). +require_once './src/DDTrace/Util/ObjectKVStore.php'; foreach (array( 'EvaluationType', 'EvaluationReason', @@ -29,7 +22,7 @@ foreach (array( 'SpanEnrichmentAccumulator', 'SpanEnrichmentRegistry', ) as $classFile) { - require_once $root . '/src/api/FeatureFlags/' . $classFile . '.php'; + require_once './src/api/FeatureFlags/' . $classFile . '.php'; } function show($label, $value) { diff --git a/tests/ext/ffe/span_enrichment_root_meta.phpt b/tests/OpenFeature/span_enrichment_root_meta.phpt similarity index 85% rename from tests/ext/ffe/span_enrichment_root_meta.phpt rename to tests/OpenFeature/span_enrichment_root_meta.phpt index a36fd936595..6f23b6cd03c 100644 --- a/tests/ext/ffe/span_enrichment_root_meta.phpt +++ b/tests/OpenFeature/span_enrichment_root_meta.phpt @@ -1,11 +1,5 @@ --TEST-- FFE span enrichment: evaluations aggregate onto the active root span's meta ---SKIPIF-- - --INI-- datadog.experimental_flagging_provider_span_enrichment_enabled=1 --FILE-- @@ -17,11 +11,10 @@ use DDTrace\FeatureFlags\EvaluationType; use DDTrace\FeatureFlags\SpanEnrichmentAccumulator; use DDTrace\FeatureFlags\SpanEnrichmentRegistry; -$root = getenv('TEST_PHP_SRCDIR'); -if (!is_string($root) || $root === '') { - $root = dirname(dirname(dirname(__DIR__))); -} -require_once $root . '/src/DDTrace/Util/ObjectKVStore.php'; +// Runs under the PHPUnit `featureflags` suite (PHP 8.0+), CWD = repo root. The +// DDTrace API classes are plain PHP under src/; load them explicitly (the +// OpenFeature composer autoload does not map the DDTrace namespace). +require_once './src/DDTrace/Util/ObjectKVStore.php'; foreach (array( 'EvaluationType', 'EvaluationReason', @@ -30,7 +23,7 @@ foreach (array( 'SpanEnrichmentAccumulator', 'SpanEnrichmentRegistry', ) as $classFile) { - require_once $root . '/src/api/FeatureFlags/' . $classFile . '.php'; + require_once './src/api/FeatureFlags/' . $classFile . '.php'; } function show($label, $value) { diff --git a/tests/phpunit.xml b/tests/phpunit.xml index 2a064f1b294..13e70fe2a27 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -143,6 +143,7 @@ ./OpenFeature/ + ./OpenFeature/