Open
Conversation
|
✨ Fix all issues with BitsAI or with Cursor
|
leoromanovsky
force-pushed
the
feature/ffe-feature-flagging
branch
from
18f7f00 to
508a8c8
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3630 +/- ##
==========================================
+ Coverage 62.32% 62.39% +0.07%
==========================================
Files 142 142
Lines 13586 13586
Branches 1775 1775
==========================================
+ Hits 8467 8477 +10
+ Misses 4311 4304 -7
+ Partials 808 805 -3 see 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
leoromanovsky
force-pushed
the
feature/ffe-feature-flagging
branch
3 times, most recently
from
ff84598 to
9f36eb7
Compare
leoromanovsky
force-pushed
the
feature/ffe-feature-flagging
branch
from
7c43198 to
7bbed08
Compare
Benchmarks [ tracer ]Benchmark execution time: 2026-03-12 20:09:41 Comparing candidate commit c77115f in PR branch Found 2 performance improvements and 38 performance regressions! Performance is the same for 153 metrics, 1 unstable metrics. scenario:ComposerTelemetryBench/benchTelemetryParsing
scenario:ContextPropagationBench/benchExtractHeaders128Bit
scenario:ContextPropagationBench/benchExtractHeaders128Bit-opcache
scenario:ContextPropagationBench/benchExtractHeaders64Bit
scenario:ContextPropagationBench/benchExtractTraceContext128Bit
scenario:ContextPropagationBench/benchExtractTraceContext128Bit-opcache
scenario:ContextPropagationBench/benchExtractTraceContext64Bit
scenario:ContextPropagationBench/benchInject128Bit
scenario:ContextPropagationBench/benchInject64Bit
scenario:EmptyFileBench/benchEmptyFileDdprof
scenario:HookBench/benchHookOverheadInstallHookOnFunction
scenario:HookBench/benchHookOverheadInstallHookOnMethod
scenario:HookBench/benchHookOverheadTraceFunction-opcache
scenario:HookBench/benchWithoutHook
scenario:LaravelBench/benchLaravelDdprof
scenario:LogsInjectionBench/benchLogsInfoBaseline-opcache
scenario:MessagePackSerializationBench/benchMessagePackSerialization
scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache
scenario:PHPRedisBench/benchRedisBaseline
scenario:PHPRedisBench/benchRedisOverhead-opcache
scenario:SamplingRuleMatchingBench/benchGlobMatching1
scenario:SamplingRuleMatchingBench/benchGlobMatching2
scenario:SamplingRuleMatchingBench/benchGlobMatching3
scenario:SamplingRuleMatchingBench/benchGlobMatching4
scenario:SamplingRuleMatchingBench/benchRegexMatching1
scenario:SamplingRuleMatchingBench/benchRegexMatching1-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching2
scenario:SamplingRuleMatchingBench/benchRegexMatching2-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching3
scenario:SamplingRuleMatchingBench/benchRegexMatching3-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching4
scenario:SamplingRuleMatchingBench/benchRegexMatching4-opcache
scenario:SpanBench/benchDatadogAPI
scenario:WordPressBench/benchWordPressBaseline
scenario:WordPressBench/benchWordPressDdprof
scenario:WordPressBench/benchWordPressOverhead
|
gh-worker-dd-mergequeue-cf854d bot
pushed a commit
to DataDog/libdatadog
that referenced
this pull request
Feb 9, 2026
## Motivation Add Feature Flagging and Experimentation (FFE) support to the remote config infrastructure, enabling tracers to subscribe to FFE_FLAGS configurations via the sidecar. WIP: php tracer changes (DataDog/dd-trace-php#3630) ## Changes - Add `FfeFlags` variant to `RemoteConfigProduct` enum - Add `"FFE_FLAGS"` string mapping in Display and FromStr - Add `FfeFlagConfigurationRules = 46` to `RemoteConfigCapabilities` - Add `FfeFlags(Vec<u8>)` variant to `RemoteConfigData` to preserve raw config bytes ## Decisions - Raw bytes are preserved (not parsed) in `FfeFlags(Vec<u8>)` since each tracer handles evaluation with the `datadog-ffe` crate directly - Capability bit 46 matches the server-side FFE capability definition Co-authored-by: leo.romanovsky <leo.romanovsky@datadoghq.com>
leoromanovsky
force-pushed
the
feature/ffe-feature-flagging
branch
from
1990188 to
867337e
Compare
leoromanovsky
changed the title
leoromanovsky
force-pushed
the
feature/ffe-feature-flagging
branch
from
a24b1ae to
24e4304
Compare
This was referenced Feb 11, 2026
leoromanovsky
force-pushed
the
feature/ffe-feature-flagging
branch
from
2a5d688 to
1ec323b
Compare
leoromanovsky
changed the title
Add FFE support to dd-trace-php. Flag evaluation is delegated to libdatadog's datadog-ffe Rust crate via FFI. PHP handles orchestration: config lifecycle, exposure dedup, and HTTP transport. Rust FFI layer (components-rs/ffe.rs): - C-callable bridge to datadog-ffe::rules_based - Global config store behind Mutex<FfeState> - Structured attribute passing (no JSON on hot path) C extension (ext/ddtrace.c): - ffe_evaluate, ffe_has_config, ffe_config_changed, ffe_load_config - Marshals PHP arrays to FfeAttribute structs Remote Config (components-rs/remote_config.rs): - Register FfeFlags product + FfeFlagConfigurationRules capability - Handle add/remove of FFE configs via sidecar PHP Provider (src/DDTrace/FeatureFlags/Provider.php): - Singleton checking RC config state - Calls native evaluate, parses JSON results - Reports exposures via LRU-deduplicated writer Exposure pipeline: - LRU cache (65K entries) with length-prefixed composite keys - Batched writer to /evp_proxy/v2/api/v2/exposures (1000 cap) - Auto-flush via register_shutdown_function OpenFeature adapter (src/DDTrace/OpenFeature/DataDogProvider.php): - Implements AbstractProvider for open-feature/sdk Build: - Add datadog-ffe to RUST_FILES in Makefile for PECL packaging - Cargo.lock: minimal additions only (73 new crates, no gratuitous bumps) - Bump libdatadog to ed316b638 (FFE RC support, libdatadog#1532) Tests: - LRU cache unit tests (11 tests) - Exposure cache unit tests (12 tests) - 220 evaluation correctness tests from JSON fixtures Config: DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED (default: false)
…OpenFeature layers - Add null check for flag_key in ddog_ffe_evaluate Rust FFI - Set variant in OpenFeature ResolutionDetails (was always null) - Replace magic numbers with named constants for reason/error codes - Fix json_decode null ambiguity in JSON type parsing with json_last_error() - Add dropped event tracking and curl failure logging (DD_TRACE_DEBUG) - Guard against missing curl extension in ExposureWriter flush - Fix buildEvent docblock parameter order
Add FlagEvalMetrics OpenFeature hook that records an OTel counter (feature_flag.evaluations) after every flag evaluation. Attributes: feature_flag.key, feature_flag.result.variant, feature_flag.result.reason (lowercase), error.type (on error). Enabled only when DD_METRICS_OTEL_ENABLED=true and open-telemetry/sdk is installed. Is a complete noop otherwise. Register hook in DataDogProvider constructor via setHooks(). Add FlagEvalMetrics.php to _files_tracer.php autoloader.
…-zero error codes - Fix errorCodeToTag() to match actual Rust constants: 1=ERROR_TYPE_MISMATCH→type_mismatch, 2=ERROR_CONFIG_PARSE→parse_error, 3=ERROR_FLAG_UNRECOGNIZED→flag_not_found (was transposed) - Per OpenFeature spec, force reason=ERROR for any non-zero error_code. FlagUnrecognizedOrDisabled returns REASON_DEFAULT from the Rust layer but must be reported as ERROR to callers.
leoromanovsky
force-pushed
the
feature/ffe-feature-flagging
branch
from
f624325 to
6b83643
Compare
- FlagEvalMetrics.php: class constant visibility (private const) requires PHP 7.1+; drop to bare const for PHP 7.0 compatibility. The class is already internal so visibility adds nothing. - metadata/supported-configurations.json: regenerate after adding DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED to configuration.h. Fixes the "Configuration Consistency" CI check.
…tements Windows MSVC compiles PHP extensions in C89 mode (no /std:c11), which requires all variable declarations to precede executable statements within a block scope. The ffe_evaluate handler had three violations: - c_attrs and attrs_count declared after the targeting_key if-statement - idx/key/val declared after the c_attrs = ecalloc() call - val/var/ak declared after array_init(return_value) Moves all declarations to the top of their respective blocks.
leoromanovsky
changed the title
- Add feature_flag.provider.name=datadog to OTel metric attributes - Propagate error codes to OpenFeature ResolutionDetails via withError() - Prefer dd_trace_env_config over getenv in isFeatureFlagEnabled() - Log non-2xx HTTP status codes in ExposureWriter debug mode - Clarify continue-in-switch comment in ddtrace.c
leoromanovsky
requested review from
a team,
dd-oleksii and
greghuels
and removed request for
a team
Adds EvaluationTest.php with a data-provider that scans all JSON fixture files under tests/FeatureFlags/fixtures/evaluation-cases/ and drives ffe_evaluate() for each case, asserting value and reason. Fixture files cover boolean, integer, numeric, string, JSON, kill-switch, disabled, regex, date, comparator, null-operator, and special-character flag types. JSON variation fixtures use float values (1.0/2.0) matching the raw JSON returned by the FFE engine (php json_decode preserves the decimal point from the UFC config). Also fixes a minor LRUCache put() return-value doc inconsistency.
dd-oleksii
approved these changes
Mar 13, 2026
Member
dd-oleksii
left a comment
dd-oleksii
left a comment
There was a problem hiding this comment.
Looks good overall. I think the only major piece is that we shouldn't default targeting key to empty string. oh, and a suspicious const-cast that probably needs some comments/justification
Not sure if that would speed up reviews or not but this PR seems pretty splittable:
- general setup returning default values, OF integration, public API
- evaluation, rust pieces
- exposures, lru cache, etc.
- evaluation metrics
| /// contention is not a practical concern. Keeping a Mutex for simplicity. | ||
| struct FfeState { | ||
| config: Option<Configuration>, | ||
| changed: bool, |
Member
There was a problem hiding this comment.
minor: not sure how changed is used by the other side but having a global changed flag seems error-prone. What do you think about replacing it with version: u64?
Comment on lines
+75
to
+77
| let was_changed = state.changed; | ||
| state.changed = false; | ||
| was_changed |
Member
There was a problem hiding this comment.
nitpick: not necessarily better but there's a function for this:
Suggested change
| let was_changed = state.changed; | |
| state.changed = false; | |
| was_changed | |
| std::mem::relpace(&mut state.changed, false) |
Comment on lines
+115
to
+120
| pub value_json: CString, | ||
| pub variant: Option<CString>, | ||
| pub allocation_key: Option<CString>, | ||
| pub reason: i32, | ||
| pub error_code: i32, | ||
| pub do_log: bool, |
Member
There was a problem hiding this comment.
minor: shall fields be non-public if it's supposed to be opaque?
Suggested change
| pub value_json: CString, | |
| pub variant: Option<CString>, | |
| pub allocation_key: Option<CString>, | |
| pub reason: i32, | |
| pub error_code: i32, | |
| pub do_log: bool, | |
| value_json: CString, | |
| variant: Option<CString>, | |
| allocation_key: Option<CString>, | |
| reason: i32, | |
| error_code: i32, | |
| do_log: bool, |
| attributes_count: usize, | ||
| ) -> *mut FfeResult { | ||
| if flag_key.is_null() { | ||
| return std::ptr::null_mut(); |
Member
There was a problem hiding this comment.
minor: consider returning an error result on errors?
| }; | ||
|
|
||
| // Build targeting key | ||
| let tk = if targeting_key.is_null() { |
Member
There was a problem hiding this comment.
nitpick:
Suggested change
| let tk = if targeting_key.is_null() { | |
| let targeting_key = if targeting_key.is_null() { |
| return rtrim($agentUrl, '/'); | ||
| } | ||
|
|
||
| $host = $this->getConfigValue('DD_AGENT_HOST', 'localhost'); |
Member
There was a problem hiding this comment.
just checking — is this name correct? The other two start with DD_TRACE_AGENT and this one is DD_AGENT
Comment on lines
+14
to
+29
| private static $REASON_MAP = [ | ||
| 0 => 'STATIC', | ||
| 1 => 'DEFAULT', | ||
| 2 => 'TARGETING_MATCH', | ||
| 3 => 'SPLIT', | ||
| 4 => 'DISABLED', | ||
| 5 => 'ERROR', | ||
| ]; | ||
|
|
||
| private static $TYPE_MAP = [ | ||
| 'STRING' => 0, | ||
| 'INTEGER' => 1, | ||
| 'NUMERIC' => 2, | ||
| 'BOOLEAN' => 3, | ||
| 'JSON' => 4, | ||
| ]; |
Member
There was a problem hiding this comment.
minor: does it make sense to keep these translations in C, so we can use enums vs. having to sync maps manually?
| { | ||
| $this->writer = new ExposureWriter(); | ||
| $this->exposureCache = new ExposureCache(65536); | ||
| $this->enabled = $this->isFeatureFlagEnabled(); |
Member
There was a problem hiding this comment.
nitpick: misleading name for the function
| * Flag configurations are received via Remote Config through the sidecar. | ||
| * Reports exposure events to the EVP proxy for analytics. | ||
| */ | ||
| class Provider |
Member
There was a problem hiding this comment.
Doesn't look like it actually implements openfeature provider, so the name is misleading
Comment on lines
+100
to
+104
| $targetingKey = ''; | ||
| $attributes = []; | ||
|
|
||
| if ($context !== null) { | ||
| $targetingKey = $context->getTargetingKey() ?? ''; |
Member
There was a problem hiding this comment.
major: don't default targeting key to empty string. Default value should be null
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Add FFE (Feature Flags & Experimentation) support to dd-trace-php. PHP applications can evaluate feature flags delivered via Remote Config using the same
datadog-ffeRust engine used by Ruby and Python. Per the RFC "Flag evaluations tracking for APM tracers", we also emit afeature_flag.evaluationsOTel counter metric on each evaluation so flag usage can be tracked in Datadog.Changes
Core FFE Implementation
components-rs/ffe.rs): C-callable bridge todatadog-ffe::rules_based— config store, evaluate, result accessorsext/ddtrace.c):ffe_evaluate,ffe_has_config,ffe_config_changed,ffe_load_configinternal functions that marshal PHP arrays toFfeAttributestructscomponents-rs/remote_config.rs): RegisterFfeFlagsproduct +FfeFlagConfigurationRulescapability; handle add/remove of FFE configssrc/DDTrace/FeatureFlags/Provider.php): Singleton that checks RC config state, calls native evaluate, parses JSON results, reports exposures; enforcesreason=ERRORfor any non-zero error code per OpenFeature spec/evp_proxy/v2/api/v2/exposures(1000 event buffer cap)src/DDTrace/OpenFeature/DataDogProvider.php): ImplementsAbstractProviderfor theopen-feature/sdkcomposer packagedatadog-ffetoRUST_FILESin Makefile for PECL packagingDD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLEDgating via X-macro inext/configuration.hOTel Flag Evaluation Metrics (new)
src/DDTrace/FeatureFlags/FlagEvalMetrics.php: OTel counter hook that emitsfeature_flag.evaluations(count=1) after each evaluation. Tags:feature_flag.key,feature_flag.provider.name=datadog,feature_flag.result.variant,feature_flag.result.reason,feature_flag.result.allocation_key, and on error:error.type(type_mismatch/parse_error/flag_not_found).errorCodeToTag()now maps Rust constants correctly:1=ERROR_TYPE_MISMATCH→type_mismatch,2=ERROR_CONFIG_PARSE→parse_error,3=ERROR_FLAG_UNRECOGNIZED→flag_not_found(was transposed in initial implementation).Provider.php: Forcesreason=ERRORwhenerror_code != 0. The Rust layer returnsREASON_DEFAULTforFlagUnrecognizedOrDisabledbut the OpenFeature spec requiresERRORto be surfaced to callers.Decisions
Evaluation in Rust, not PHP. All flag evaluation (UFC parsing, targeting rules, shard hashing, allocation resolution) happens in
libdatadog'sdatadog-ffecrate via FFI. PHP only handles orchestration (config lifecycle, exposure dedup, HTTP transport). This matches Ruby and Python — no language re-implements evaluation logic.Global config behind
Mutex<FfeState>. The Rust FFE config is stored in alazy_staticglobal with aMutex. PHP is single-threaded per process, soRwLockwould be unnecessary complexity.Reuses existing RC pipeline. FFE configs flow through the same sidecar →
ddog_process_remote_configs()path as APM Tracing and Live Debugger. No new polling mechanism.Structured attributes, not JSON blobs. The C extension converts PHP arrays into
FfeAttributestructs (typed: string/number/bool) before calling Rust, avoiding JSON encode/decode overhead on the hot path.ExposureWriter caps at 1000 events per request. Matches Ruby and Python. Flush via
register_shutdown_function.FlagEvalMetrics via OTel SDK. Uses
open-telemetry/api+ OTLP exporter. Recorded after the type-specific evaluation method returns (not inside coreevaluate()) so type mismatch errors are captured withreason=errorrather than the intermediatetargeting_match.ERROR reason forced for non-zero error codes.
FlagUnrecognizedOrDisabledreturnsREASON_DEFAULTfrom Rust (the default value was served) but the OpenFeature spec requiresreason=ERRORwhenever an error code is set.Provider.phpoverrides the Rust-reported reason in this case.Test Results
Unit tests
System tests — all FFE tests pass (PHP 8.0-apache)
The 1 xfail is
test_cross_request_dedup— expected because PHP's shared-nothing architecture prevents cross-request LRU state.OTel metrics tests (all 5 pass)
LOC (excluding Cargo.lock + JSON fixtures)
Companion PRs