Add FFE evaluation metrics#3911
Merged
Merged
Conversation
|
✨ Fix all issues with BitsAI or with Cursor
|
Benchmarks [ tracer ]Benchmark execution time: 2026-06-05 15:00:06 Comparing candidate commit 867c3c5 in PR branch Found 3 performance improvements and 1 performance regressions! Performance is the same for 189 metrics, 1 unstable metrics.
|
leoromanovsky
requested review from
dd-oleksii and
typotter
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f021234c39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
leoromanovsky
added a commit
to DataDog/libdatadog
that referenced
this pull request
May 23, 2026
Adds a parallel pathway for PHP feature-flag evaluation metrics mirroring the FfeExposures forwarder. dd-trace-php encodes `feature_flag.evaluations` counters as OTLP/protobuf in PHP (via its existing PHP 7-safe `OtlpMetricEncoder`) and ships the encoded bytes to the sidecar, which POSTs them to the user-configured OTLP HTTP metrics intake. Why a sibling action instead of reusing FfeExposures: - The OTLP collector is not the Datadog Agent. It's user-configurable via OTEL_EXPORTER_OTLP_METRICS_ENDPOINT (default http://localhost:4318/v1/metrics), so the endpoint travels with the payload rather than being derived from the sidecar session's agent base URL. - Content type differs (application/x-protobuf vs application/json). - No EVP subdomain header. - The payload is binary protobuf, not a JSON string. dd-trace-php side (PR DataDog/dd-trace-php#3911) will refactor its existing `OtlpHttpMetricTransport` (which currently does PHP-side HTTP I/O, violating the architectural rule "no I/O outside the sidecar") to call this new FFI. Validation: - `cargo test -p datadog-sidecar ffe` passes 7 tests (3 exposures + 4 metrics). - `cargo check -p datadog-sidecar-ffi` clean.
This was referenced May 23, 2026
leoromanovsky
added a commit
that referenced
this pull request
May 23, 2026
Per Bob's PR review (2026-05-22), the tracer extension must perform no I/O outside the sidecar. Replaces the raw-socket `AgentExposureTransport` with `SidecarExposureTransport`, which forwards exposure batches to the libdatadog sidecar via a new native PHP function `\DDTrace\send_ffe_exposures` that calls the `ddog_sidecar_send_ffe_exposures` FFI added in DataDog/libdatadog#2026. PHP side: - Delete `Internal/Exposure/AgentExposureTransport.php` (raw socket POST to the Agent EVP proxy). - Add `Internal/Exposure/SidecarExposureTransport.php` that JSON-encodes the batch and calls `\DDTrace\send_ffe_exposures()`. Fire-and-forget; the sidecar handles retries. - Update `ExposureWriter::createDefault()` to instantiate the sidecar transport. - Drop the obsolete `testAgentTransportBuildsAgentEvpRequest` PHPUnit test (HTTP construction now lives in libdatadog, covered by `cargo test -p datadog-sidecar ffe_flusher`). - Add `Internal/DefaultEvaluationCompletedHook` and `Internal/CompositeEvaluationCompletedHook` so production callers go through a composite hook factory. In this PR the composite contains only `ExposureHook`; the metrics PR (#3911) contributes `EvaluationMetricHook` and the file conflict at merge resolves by combining both. Update `Client::create()` to call `DefaultEvaluationCompletedHook::create()`. C/Rust bridge: - Declare `ddog_ByteSlice` (and underlying `ddog_Slice_U8`) in `components-rs/common.h` for the metrics path; declare both `ddog_sidecar_send_ffe_exposures` and `ddog_sidecar_send_ffe_metrics` in `components-rs/sidecar.h`. - Add C wrappers `ddtrace_sidecar_send_ffe_exposures(zend_string *)` and `ddtrace_sidecar_send_ffe_metrics(zend_string *endpoint, zend_string *payload_bytes)` in `ext/sidecar.{h,c}` that call the FFI with the current sidecar transport + instance id + queue id. - Declare native PHP functions `\DDTrace\send_ffe_exposures(string): bool` and `\DDTrace\send_ffe_metrics(string, string): bool` in `ext/ddtrace.stub.php`; add corresponding arginfo entries and `ZEND_FUNCTION` registrations in `ext/ddtrace_arginfo.h`; implement `PHP_FUNCTION(DDTrace_send_ffe_exposures)` and `PHP_FUNCTION(DDTrace_send_ffe_metrics)` in `ext/ddtrace.c`. - Bump `libdatadog` submodule to FFE branch tip `29762335c` (which provides both FFIs). The submodule will be bumped to the libdatadog main commit once #2026 merges. Docs: - Add `docs/php-ffe-stack/{stack,system}-pr3910.{mmd,png}` for this PR. Validation: - `php vendor/bin/phpunit --config phpunit.xml tests/api/Unit/FeatureFlags` → 41 tests, 174 assertions, OK. - libdatadog sidecar tests (`cargo test -p datadog-sidecar ffe_flusher`) → 3 passed, on the pinned submodule commit. - Mermaid PNGs regenerate via `npx @mermaid-js/mermaid-cli`. `make test_featureflags` and `make test_c TESTS=tests/ext/ffe/...` will run in CI; running them locally requires rebuilding the extension which is gated behind libdatadog #2026 merging.
Adds the M3 evaluation-metrics layer on top of the hook PR (#3909) as a sibling of the EVP exposures PR (#3910). Records `feature_flag.evaluations` for both PHP 7 (DD Client hook) and PHP 8 (OpenFeature SDK hook); both paths share `EvaluationMetricHook::sharedWriter()` for unified aggregation. OTLP/protobuf payloads are encoded in PHP via the existing `OtlpMetricEncoder` and delivered to the user-configured OTLP HTTP metrics intake through the libdatadog sidecar (`ddog_sidecar_send_ffe_metrics` FFI added in DataDog/libdatadog#2026). This branch is force-pushed (user-authorized one-time exception to the no-force-push rule, 2026-05-23) to restructure history away from being linearly stacked on the M2 exposures PR (#3910). The PR now stacks directly on the hook PR (#3909) as a sibling of the EVP PR. PHP side: - Add `Internal/Metric/EvaluationMetricWriter` with bounded series aggregation, drop accounting, and shutdown flush. - Add `Internal/Metric/EvaluationMetricHook` (DD Client hook) and `OtlpMetricEncoder` (PHP 7-safe protobuf encoding). - Add `Internal/Metric/SidecarOtlpMetricsTransport` that calls `\DDTrace\send_ffe_metrics()` (FFI declared in #3910). Endpoint resolution: `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT`, falling back to `OTEL_EXPORTER_OTLP_ENDPOINT + /v1/metrics`, default `http://localhost:4318/v1/metrics`. - Add `DDTrace\OpenFeature\EvalMetricsHook` implementing `OpenFeature\interfaces\hooks\Hook` (after + error stages), registered on `DataDogProvider` via `setHooks()`. - `DataDogProvider` constructs its internal DD `Client` with `DefaultEvaluationCompletedHook::createWithoutMetric()` so the OpenFeature path records the metric via the OpenFeature hook (PR 3911 scope) and NOT via the DD Client hook — preventing double-counting. PHP 7 path keeps recording via the DD Client hook. - Add `Internal/CompositeEvaluationCompletedHook` and `Internal/DefaultEvaluationCompletedHook` (metric-only composite). This is the merge-conflict point with PR #3910's `[ExposureHook]` composite — second merge resolves by combining both hooks. - Update `Client::create()` to call `DefaultEvaluationCompletedHook::create()`. - Drop the obsolete `testOtlpTransportBuildsHttpProtobufRequest` PHPUnit test (HTTP construction now lives in libdatadog, covered by `cargo test -p datadog-sidecar ffe_metrics_flusher`). - Add `_files_openfeature.php` entry for `EvalMetricsHook.php`. C/Rust bridge: the `\DDTrace\send_ffe_metrics()` native function, its C wrapper `ddtrace_sidecar_send_ffe_metrics()`, and the `ddog_sidecar_send_ffe_metrics` FFI declaration in `components-rs/sidecar.h` were already added in #3910. This PR's branch picks up those changes once #3910 merges (or via the same libdatadog submodule pin during review). For development locally the libdatadog submodule is pinned to the FFE branch tip (`29762335c`). Docs: - Add `docs/php-ffe-stack/{stack,system}-pr3911.{mmd,png}` per the 4-PR documentation convention. Validation: - `php vendor/bin/phpunit --config phpunit.xml tests/api/Unit/FeatureFlags` → 40 tests, 160 assertions, OK. - Mermaid PNGs regenerate via `npx @mermaid-js/mermaid-cli`. `make test_featureflags`, OpenFeature PHPUnit, and ffe-dogfooding end-to-end validation will run in CI / are validated separately by FOLLOW-05 Steps 4–5.
leoromanovsky
force-pushed
the
leo.romanovsky/m3-ffe-evaluation-metrics
branch
from
f021234 to
e74b050
Compare
leoromanovsky
changed the base branch from
leo.romanovsky/m2-ffe-exposures
to
leo.romanovsky/m2-m3-evaluation-completed-base
The SidecarOtlpMetricsTransport::resolveEndpoint() default ("http://localhost:4318/v1/metrics")
doesn't match the system-tests parametric setup, where the PHP test
client receives DD_AGENT_HOST=<test_agent_container> but no
OTEL_EXPORTER_OTLP_METRICS_ENDPOINT. The previous OtlpHttpMetricTransport
(replaced by this transport) derived the OTLP endpoint from DD_AGENT_HOST
+ port 4318. Restore that fallback so system-tests
test_php_ffe_evaluation_metric finds the test-agent OTLP intake.
Resolution order (now matches the old transport):
1. OTEL_EXPORTER_OTLP_METRICS_ENDPOINT (explicit)
2. OTEL_EXPORTER_OTLP_ENDPOINT + /v1/metrics
3. DD_AGENT_HOST + :4318/v1/metrics
4. localhost:4318/v1/metrics
The C/Rust bridge for the new native PHP functions \DDTrace\send_ffe_exposures() and \DDTrace\send_ffe_metrics() lives on the M2 EVP exposures PR (#3910) because that PR introduced the bridge when refactoring the exposure transport. PR #3911 (this PR) needs the same bridge for its OTLP metrics transport — without it the SidecarOtlpMetricsTransport silently drops batches because function_exists('\\DDTrace\\send_ffe_metrics') is false. Adds the same bridge files here so the M3 branch is independently compilable. At merge time the two PRs will conflict at the file level on these bridge files; resolution is deduplication (the bridge is identical in both PRs by design). Files added/modified: - components-rs/sidecar.h: declares ddog_sidecar_send_ffe_exposures and ddog_sidecar_send_ffe_metrics FFIs. - components-rs/common.h: declares ddog_ByteSlice typedef for the metrics payload. - ext/sidecar.h, ext/sidecar.c: C wrappers ddtrace_sidecar_send_ffe_exposures() and ddtrace_sidecar_send_ffe_metrics(). - ext/ddtrace.stub.php, ext/ddtrace_arginfo.h, ext/ddtrace.c: declares the native PHP functions and the PHP_FUNCTION implementations.
Pulls in libdatadog commit `875ec8f0e` ("fix(sidecar): dispatch FFE
actions before application-entry check"). Without this fix, the
`SidecarOtlpMetricsTransport::send()` call from PHP would silently
no-op for short-lived processes: the sidecar received the
`FfeMetrics` action but dropped it because the `Entry::Occupied`
gate on the application metadata had not yet fired.
This unblocks the parametric system-test
`Test_Feature_Flag_Parametric_Evaluation_Metrics::test_php_ffe_evaluation_metric`
which exercises the full PHP -> sidecar -> OTLP-HTTP-intake path
end-to-end. Local result: 26/27 FFE-scoped parametric tests pass
(remaining failure is the EVP exposure test, which lives on the
M2 PR #3910 branch).
leoromanovsky
added a commit
that referenced
this pull request
May 23, 2026
Pulls in libdatadog commit `875ec8f0e` ("fix(sidecar): dispatch FFE
actions before application-entry check") so the EVP exposure batch sent
via `ddog_sidecar_send_ffe_exposures` is no longer silently dropped
when the PHP runtime hasn't yet registered the application against the
sidecar's `QueueId`. Same fix is on the sibling PR #3911 (`2a48c4987")
for the OTLP metric path.
Without this submodule bump, `Test_Feature_Flag_Parametric_Exposures::test_php_ffe_exposure_event`
sees zero EVP POSTs at the test-agent because the sidecar's
`enqueue_actions` handler discards the `FfeExposures` action under
the `Entry::Occupied` gate.
leoromanovsky
added a commit
that referenced
this pull request
May 23, 2026
…macOS+colima
`build-debug-artifact` produces a `/output` bind mount via `-v
${TMP_OUT}:/output`. On macOS+colima only paths under $HOME are
mounted into the Linux VM; the macOS default `/var/folders/...` temp
dir is not, so writes from inside the container to `/output` land in
the VM and never propagate back to the host. The script exits without
error and the user's binaries dir is left with whatever stale artifact
was there before.
Pin TMP_OUT/TMP_PKG under $OUTPUT_DIR (which the user just passed as
an absolute, usable destination) so the bind mount is always on a
host-visible path. Inherits cleanup via the existing EXIT trap.
The same fix is independently on the M3 branch (#3911) as part of
e74b050; bringing it to the M2 branch (#3910) so both branches are
buildable on macOS without a `TMPDIR=$HOME/...` override.
…gh rez Three fixes to the per-PR FFE diagrams: 1. **Titles were truncated to "PHP FFE 4-PR stack — current =" with the PR number missing.** Mermaid uses the YAML frontmatter for the title block, and YAML treats unquoted `#` as the start of a comment, so `title: PHP FFE 4-PR stack — current = #3911 (OTLP metrics)` got parsed as just `PHP FFE 4-PR stack — current =`. Quoting the title keeps the `#PR-number` portion intact. 2. **System diagrams switched from `flowchart LR` to `flowchart TD`.** LR forced the PHP-process / host / backend lanes into a single very-wide row that rendered as an unreadable horizontal strip on PR pages. TD stacks them vertically and keeps the per-lane subgraphs readable. 3. **Re-rendered at 2400×2400 with `--scale 3`** (~1800×2000 stack, ~3000×4500 system) instead of the default ~600px width. PR-page thumbnails render legibly and zoomed-in detail stays sharp. README's regeneration recipe updated with all three knobs and a note on why the `title:` quoting matters.
leoromanovsky
added a commit
that referenced
this pull request
May 23, 2026
…gh rez Same three fixes as the parallel commit on the M3 (PR #3911) branch: 1. Quote the YAML `title:` so the `#PR-number` is not parsed as a comment (previous render had the title truncated at "current ="). 2. `flowchart LR` → `flowchart TD` on system diagrams so vertical PHP-process → host → backend lanes stack vertically instead of getting squeezed into one wide row. 3. Render with `-w 2400 -H 2400 --scale 3 -b white` (~1900×2000 stack, ~3000×4600 system) instead of ~600px default.
leoromanovsky
added a commit
that referenced
this pull request
May 23, 2026
…gh rez Same three fixes as on the M2 (#3910) and M3 (#3911) sibling branches: 1. Quote the YAML `title:` so the `#PR-number` survives parsing (otherwise YAML treats the `#` as a comment and the title renders as "PHP FFE 4-PR stack — current =" with the rest missing). 2. `flowchart LR` → `flowchart TD` on the system diagram so the PHP-process / host-sidecar / backend lanes stack vertically. 3. Render at 2400×2400 `--scale 3` instead of ~600px default.
leoromanovsky
added a commit
that referenced
this pull request
May 23, 2026
Brings the PHP FFE diagram convention to the M1 PR. Each subsequent PR in the stack (#3909, #3910, #3911) already carried its own stack + system diagram; #3906 was missing them. Mirrors the format used by the rest of the stack: - `stack-pr3906.mmd` — the 4-PR stack with #3906 badged as current and the downstream layers shown as "future". - `system-pr3906.mmd` — the target end-to-end architecture with M1's scope (UserCode, OpenFeature Client, DataDogProvider, DDTrace FeatureFlags Client, NativeEvaluator, Remote Config client) highlighted, and everything from the Hook layer onward dashed. All conventions match the other branches: quoted YAML titles (to keep `#PR-number` out of the YAML comment parser), `flowchart TD` orientation, rendered with `-w 2400 -H 2400 --scale 3 -b white`.
`AbstractProvider::setHooks(array $hooks)` REPLACES the hook list, so registering our `EvalMetricsHook` via `setHooks([$metricsHook])` in the constructor would silently drop our metric emission as soon as a user configures their own provider-level hooks. Override `getHooks()` to always prepend the Datadog metric hook to the caller-supplied list. The user can register their own provider hooks freely (`$provider->setHooks($theirHooks)`) and we still record `feature_flag.evaluations` on every OpenFeature evaluation. Adds a unit test that constructs a DataDogProvider, calls `$provider->setHooks([$userHook])`, then asserts `$provider->getHooks()` returns `[EvalMetricsHook, $userHook]`.
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
to DataDog/libdatadog
that referenced
this pull request
Jun 4, 2026
## Motivation Companion PHP evaluation-metrics work needs FFE metrics to follow the sidecar architecture used by the rest of the tracer: configure endpoints once on the session, then send typed sidecar actions without carrying endpoint strings on every flush. The previous FFE metrics path accepted an OTLP endpoint alongside each metrics batch. That diverged from the normal `Endpoint` flow, made test-session-token propagation easy to miss, and forced PHP to keep fetching endpoint configuration during request flushes. Design context: https://docs.google.com/document/d/1NvMfTpZWLBlFmEFNjdnlMyeVpy5l7KD8qujGFco6w2w/edit?tab=t.0 Companion PHP PR: DataDog/dd-trace-php#3911 ## Changes - Add an optional OTLP metrics `Endpoint` to sidecar session configuration. - Thread that endpoint through `ddog_sidecar_session_set_config` and session state. - Dispatch `SidecarAction::FfeEvaluationMetrics` through the session's configured OTLP metrics endpoint instead of an endpoint passed with each metrics batch. - Preserve request-replayer test-session routing by inheriting the agent endpoint `test_token` when the OTLP metrics endpoint does not already have one. - Update runtime test-token changes so the configured OTLP metrics endpoint is updated with the rest of the session endpoints. - Extend the sidecar FFE metrics dispatch test to assert the outgoing OTLP request carries `x-datadog-test-session-token` through `Endpoint` standard headers. ## Decisions FFE metrics remain caller-driven. This PR does not make shared evaluator calls auto-emit metrics, so SDKs that still log metrics in host language implementations can continue to coexist without double counting. The sidecar owns metrics transport, aggregation, OTLP/protobuf encoding, and endpoint/header handling. PHP remains responsible for constructing the appropriate OTLP metrics endpoint from its existing configuration resolver and supplying it once during sidecar session setup. The test-session-token behavior is intentionally implemented through `Endpoint`, not by manually adding FFE-specific headers. Production endpoints normally have no test token, so production behavior is unchanged. ## Validation Local targeted test: ```sh cargo test -p datadog-sidecar ffe_metric_actions_dispatch_without_registered_application ``` Result: passed locally. PHP integration smoke test: Companion PHP PR: DataDog/dd-trace-php#3911 The local PHP branch backing that PR was updated to this libdatadog PR head (`091c98df6`): PHP passes the OTLP metrics `Endpoint` once during sidecar session setup, and FFE metric flushes send only context plus metric payloads. ```sh docker-compose run --no-deps 8.3-bookworm bash -lc \ 'export PATH=/rust/cargo/bin:$PATH TMPDIR=/home/circleci/app/tmp TMP=/home/circleci/app/tmp TEMP=/home/circleci/app/tmp; make MAX_TEST_PARALLELISM=1 test_c TESTS=tests/ext/ffe/evaluation_metrics_unix_agent_endpoint.phpt' ``` Result: `tests/ext/ffe/evaluation_metrics_unix_agent_endpoint.phpt` passed locally under Docker on PHP 8.3/Linux aarch64: `Tests passed 1 (100.0%)`. That PHPT validates the PHP sidecar integration over `DD_TRACE_AGENT_URL=unix://...`: the request reaches `/v1/metrics`, uses `application/x-protobuf`, and carries the request-replayer test-session token through the configured endpoint path. Negative validation during review: - Removing only the mock header matcher still allows the request through, but stops proving token propagation. - Keeping the matcher and removing `Endpoint.test_token` fails with a missing `x-datadog-test-session-token` header. - Restoring both passes, confirming the request uses `Endpoint` standard headers. CI on commit `091c98d` reports all tests passed and no new flaky tests. ## End-to-End Evidence The companion PHP branch for DataDog/dd-trace-php#3911 was validated locally against this libdatadog PR head (`091c98df6`). That path exercises the full PHP metrics flow: `PHP API -> tracer request buffer -> sidecar session-configured OTLP metrics Endpoint -> sidecar FFE metrics action -> request-replayer UDS intake` Docker command used from the PHP repo: ```sh docker-compose run --no-deps 8.3-bookworm bash -lc \ 'export PATH=/rust/cargo/bin:$PATH TMPDIR=/home/circleci/app/tmp TMP=/home/circleci/app/tmp TEMP=/home/circleci/app/tmp; make MAX_TEST_PARALLELISM=1 test_c TESTS=tests/ext/ffe/evaluation_metrics_unix_agent_endpoint.phpt' ``` Observed result: ```text Number of tests : 1 1 Tests failed : 0 ( 0.0%) ( 0.0%) Tests passed : 1 (100.0%) (100.0%) ``` The passing PHPT confirms PHP can record and flush an FFE evaluation metric through the sidecar over `DD_TRACE_AGENT_URL=unix://...`, and that the captured request reaches `/v1/metrics` with `Content-Type: application/x-protobuf` plus the request-replayer test-session token propagated through the configured `Endpoint`. Co-authored-by: leo.romanovsky <leo.romanovsky@datadoghq.com>
…-metrics' into leo.romanovsky/m3-ffe-evaluation-metrics
sameerank
approved these changes
Jun 4, 2026
sameerank
left a comment
sameerank
left a comment
There was a problem hiding this comment.
Looked at this mainly checking for parity with the other server SDKs, and I think overall it looks right. Nice work!
| ): ResolutionDetailsInterface { | ||
| $details = $this->evaluate($flagKey, $expectedType, $defaultValue, $this->normalizeContext($context)); | ||
| $this->warnIfNonProductionRuntime($details); | ||
| $this->recordEvaluationMetric($flagKey, $details); |
There was a problem hiding this comment.
The other SDKs have been using the finally hook to record evaluations. E.g.
Ruby: https://github.com/DataDog/dd-trace-rb/blob/6cf558f959f5edff37b6a243d25754687032efc9/lib/datadog/open_feature/hooks/flag_eval_hook.rb#L29
Go: https://github.com/DataDog/dd-trace-go/blob/f7dde0ce6bcea5417dc6663cf8b089d8e5555129/openfeature/flageval_metrics.go#L61
Java: https://github.com/DataDog/dd-trace-java/blob/81bb30521d96519d510b0446c5ca3eefb631e0b9/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/FlagEvalHook.java#L36
Explanation for why we use finally in JS: https://github.com/DataDog/dd-trace-js/blob/41789b40877c2072f8022fd3d854749741103d26/packages/dd-trace/src/openfeature/eval-metrics-hook.js#L14-L20
And I see that our reason for not doing that here is that ResolutionDetails aren't included in OpenFeature's implementation https://github.com/open-feature/php-sdk/blob/af1de424cbe9dcaae0c8b17d92754ffcd28d5f2f/src/implementation/hooks/HookExecutor.php#L81-L88 so it's not possible to use the finally hook here.
It took a bit of cross-referencing to understand why this differed from the other SDKs, so worth adding a comment explaining why we do this differently
Otherwise I think this is a good approach given the limitations 👍 and hopefully at some point we can update the ^2.1 version constraint
| private function allocationKey(EvaluationDetails $details): ?string | ||
| { | ||
| $exposure = $details->getExposureData(); | ||
| if (!is_array($exposure) || !isset($exposure['allocationKey']) || !is_string($exposure['allocationKey'])) { |
There was a problem hiding this comment.
I'm recalling that in the other SDKs we wanted to indicate that this was an internal key in the name DataDog/dd-trace-rb#5599 (comment)
But I guess that's not possible here without a concept of flagMetadata
Comment on lines
+106
to
+110
| metric->flag_key = zend_string_init(flag_key, flag_key_len, 0); | ||
| metric->variant = zend_string_init(variant ? variant : "", variant ? variant_len : 0, 0); | ||
| metric->reason = zend_string_init(reason ? reason : "", reason ? reason_len : 0, 0); | ||
| metric->error_type = zend_string_init(error_type ? error_type : "", error_type ? error_type_len : 0, 0); | ||
| metric->allocation_key = zend_string_init(allocation_key ? allocation_key : "", allocation_key ? allocation_key_len : 0, 0); |
There was a problem hiding this comment.
So on a successful eval, this sends error_type:"" and allocation_key:"", but the sidecar omits them in ffe_metric_from_ffi, which matches what the other SDKs do
Libdatadog integrations are a bit opaque so this looked like an accident and took cross-referencing to realize that it wasn't. A comment would be helpful here too
…fe-evaluation-metrics
Also build the endpoint fully in rust code for simplicity
This reverts commit c5934af.
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
bwoebi
force-pushed
the
leo.romanovsky/m3-ffe-evaluation-metrics
branch
from
c722e14 to
f69000f
Compare
bwoebi
approved these changes
Jun 5, 2026
bwoebi
left a comment
bwoebi
left a comment
Collaborator
There was a problem hiding this comment.
Looks like all non-flaky tests pass now
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
5 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
PHP FFE evaluation metrics need to follow the native runtime architecture from the shared design doc: https://docs.google.com/document/d/1NvMfTpZWLBlFmEFNjdnlMyeVpy5l7KD8qujGFco6w2w/edit?tab=t.0
#3906 gave PHP a real Remote Config-backed evaluator. This PR adds metric delivery without a PHP OTLP writer, encoder, or transport. PHP records a typed final evaluation result; native/libdatadog sidecar code owns buffering, aggregation, encoding, and OTLP submission.
Local system-test validation also exposed a runtime bug in the evaluator config store: RC could be delivered and ACKed on one native thread while a PHP request evaluated on another thread and still saw
provider_not_ready. The UFC config now lives in synchronized process-wide Rust state instead ofthread_local!, so RC-loaded config is visible to later evaluations in the same process. This is the right shape for PHP-FPM/ZTS because the stored value is RustConfigurationdata, not PHP request-local state.Changes
This PR targets current
masterand advances thelibdatadogsubmodule to091c98df6from DataDog/libdatadog#2076. The companion libdatadog change lets the sidecar session own the OTLP metrics endpoint and preserves request-replayer test behavior through libdatadog endpoint handling.Native
DDTrace\ffe_evaluaterecords evaluation metrics by default. The PHP 8 OpenFeature path disables automatic native recording for the raw evaluation call and then records the final OpenFeature-aware outcome once through the typedEvaluationMetricRecorderadapter.FFE metric buffering and flushing live in
tracer/ffe.c/tracer/ffe.h, matching the tracer/common-extension split requested in review. The flush path sends metric batches to the sidecar without passing an endpoint per flush; the sidecar session is configured once with the resolved OTLP metrics endpoint.Endpoint resolution now goes through
ext/otel_config.*. UnixDD_TRACE_AGENT_URLvalues are normalized through libdatadog's UDS helper so/var/run/datadog/apm.socketbecomes a validunix://<hex-socket-path>/v1/metricsendpoint.The FFE evaluator config store in
components-rs/ffe.rsis now process-wideLazyLock<RwLock<...>>state. This fixes the observed RC ACK vs evaluation-thread split and adds a regression test asserting config loaded on one thread is visible on another.Decisions
Metric delivery remains separate from exposure logging. This PR intentionally expects OTLP evaluation metrics and no EVP exposure stream; #3910 owns exposure delivery and DataDog/libdatadog#2026 owns the exposure-only sidecar work.
The PHP layer is a typed adapter only. It does not aggregate metrics, encode OTLP, or submit HTTP payloads. Users do not need to call anything extra; metrics are recorded by the Datadog provider/evaluator path.
The evaluator config is intentionally not request-local or PHP module-global. FFE config comes from RC and must persist across requests and be visible across native/PHP threads in the same process. The shared state is protected by Rust
RwLockand stores no PHPzval,zend_string, or request-owned memory.Validation
Code style:
Result: passed locally.
Focused Rust regression tests:
RUSTC_BOOTSTRAP=1 cargo test -p datadog-php ffe::tests -- --nocaptureResult:
2 passed; includesconfiguration_state_is_process_wideandempty_targeting_key_is_not_dropped.ZTS build proof:
Result: passed locally; produced
dd-library-php-1.20.0-aarch64-linux-gnu.tar.gzfrom the ZTS target.Local system-tests validation through DataDog/system-tests#7033:
Before the process-wide FFE config fix, the active run failed
12/17with emitted metrics tagged aserror.type:provider_not_readyeven though RC delivery was ACKed. After the fix, result was:FFE dogfooding transport proof:
cd ffe-dogfooding FLAG_KEY=php-metrics-dogfood-1780520626 \ SUBJECT_ID=php-metrics-validator-1780520626 \ REQUESTS=5 \ WAIT_SECONDS=120 \ EXPECT_OTLP=1 \ EXPECT_EVP=0 \ scripts/validate-local-php-telemetry.shResult:
Dogfooding did not prove live non-default RC values in this run because the local agent was rejecting the configured key with
API Key invalid (403 response). The system-tests run above is the config-backed validation evidence for this PR.Related PRs: #3906, #3910, DataDog/libdatadog#2026, DataDog/libdatadog#2076, DataDog/system-tests#7033.