Decouple FFE from Tracing proposal#11888
Merged
Merged
Conversation
leoromanovsky
approved these changes
Jul 8, 2026
leoromanovsky
left a comment
Contributor
There was a problem hiding this comment.
Works for me, thanks!
AlexeyKuznetsov-DD
approved these changes
Jul 8, 2026
There was a problem hiding this comment.
💡 Codex Review
https://github.com/DataDog/dd-trace-java/blob/d3f54ffd72a758ade9b021dafa6011be5b9147e1/dd-trace-api/src/main/java/datadog/trace/api/config/FeatureFlaggingConfig.java#L1
When a customer or external integration upgrades dd-trace-api and still imports datadog.trace.api.config.FeatureFlaggingConfig for these public config keys, deleting the class makes their source stop compiling; reflective/runtime lookups of the old class also fail. The replacement is in a new product module that dd-trace-api does not publish or depend on, so please keep a deprecated forwarding class in the old package (or provide another compatibility path) while moving internal users to the new module.
ℹ️ 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".
Contributor
|
🎯 Code Coverage (details) 🔗 Commit SHA: 4b4b1c2 | Docs | Datadog PR Page | Give us feedback! |
Contributor
Author
I’m good with that. It’s experimental keys and I don’t want to add |
Contributor
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
PerfectSlayer
merged commit Jul 9, 2026
22255cb
into
leo.romanovsky/ffe-apm-span-enrichment
570 of 578 checks passed
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
that referenced
this pull request
Jul 14, 2026
…ental, gated) (#11658) feat(02-02): surface UFC split serialId in eval metadata + add span-enrichment gate - Add nullable Integer serialId to UFC Split model + constructor (Moshi reflectively deserializes the serialId JSON field; no custom adapter needed) - DDEvaluator surfaces __dd_split_serial_id (when present) and __dd_do_log in OpenFeature eval metadata for APM span enrichment (JAVA-01) - Add FeatureFlaggingConfig.SPAN_ENRICHMENT_ENABLED dot-form constant mapping to DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED (distinct, off by default) - Register the env var in metadata/supported-configurations.json (version A, boolean) - Update DDEvaluatorTest Split call sites for the new constructor arg feat(02-02): ULeb128 codec + accumulator + capture hook + write interceptor (gate-gated) - ULeb128Encoder: ULEB128 delta-varint + base64 codec ported verbatim from the frozen Node reference (dd-trace-js#8343); golden vector {100,108,128,130} -> ZAgUAg==, SHA-256 hex targeting-key hashing; JDK stdlib only (java.util.Base64, MessageDigest) - SpanEnrichmentAccumulator: per-trace state keyed by trace id in a shared ConcurrentHashMap; frozen limits (200/10/20/5/64), UTF-8-safe truncation, object default -> JSON (Pattern F tag shapes: ffe_flags_enc bare base64, ffe_subjects_enc / ffe_runtime_defaults JSON objects) - SpanEnrichmentHook: OpenFeature finally hook (capture) resolving the active local root via AgentTracer and applying the Node branch (serialId -> addSerialId/addSubject; missing variant -> addDefault); error-isolated - SpanEnrichmentInterceptor: TraceInterceptor (write) flushing ffe_* onto the local root onTraceComplete with unique priority 4, then clearing state; disable() for provider-close cleanup - Provider: gate-gated construction via ConfigHelper.env (DG-005 — nothing built/ registered when off), hook added to getProviderHooks, interceptor registered via GlobalTracer, shutdown() disables interceptor + drains state - feature-flagging-api: add compileOnly/testImplementation :internal-api for the tracer/interceptor types (runtime-provided by the agent) test(02-02): JUnit 5 L0 suite for span enrichment (7 required cases + golden round-trip) - Codec golden-vector round-trip {100,108,128,130} -> ZAgUAg== (encode + decode + empty + structural dedupe) - no-span, finished-root (accumulate+dedupe+flush), runtime-default missing-variant (ffe_runtime_defaults JSON object), per-subject cap (10 subj / 20 exp/subj) + doLog gating, max-200 serial ids, object-default JSON + 64-char truncation - gate-off negative control: no hook/interceptor constructed, not in getProviderHooks, no accumulator state (DG-005) - gate-on construction + provider-close cleanup (shutdown disables interceptor + drains state); error isolation; interceptor priority uniqueness - JUnit 5 only (no Groovy); injectable RootSpanResolver + Provider gate override avoid static mocking (project mockito uses the subclass mock maker) fix(02-02): correct span-enrichment lifecycle (partial-flush, leak, reconfig) Gap-closure for the three BLOCKER lifecycle defects in 02-REVIEW-java.md. The frozen contract (codec/limits/gate/tag shapes, golden vector ZAgUAg==) is unchanged — this fixes only how per-trace state is scoped, bounded, and flushed. CR-01 (partial-flush data loss + tag misattribution): CoreTracer.write -> interceptCompleteTrace runs onTraceComplete on EVERY flush, and PendingTrace.write(isPartial) excludes the still-open local root from a partial flush (getRootSpan() is null until the final write). The interceptor previously resolved the root by reference (reachable even when absent from the fragment) and unconditionally removed the accumulator on the first partial flush, dropping all pre-flush flags and tagging a not-yet-finished root. SpanEnrichmentInterceptor now flushes + removes ONLY when the local root is actually present in the flushed collection (the final write); a partial flush leaves the accumulator intact. Also adds the missing getTraceId() null guard (WR-01) and never falls back to tagging a non-root span (WR-02). CR-02 (unbounded state leak): State lived in a static ConcurrentHashMap whose only removal paths were trace-complete and shutdown, so dropped / Noop / never-finishing traces leaked forever (#4844 leak class). State now lives in a per-provider SpanEnrichmentStates store, hard-bounded at MAX_TRACES with FIFO eviction of the oldest in-flight entry, so a never-completing trace cannot grow the map unboundedly. CR-03 (reconfiguration corruption): Provider ignored addTraceInterceptor's return value, so a second gate-on provider (rejected on duplicate priority 4) still wired a hook into shared static state and its shutdown() cleared the first provider's live state. Provider now honors the registration result (wires the hook/interceptor only when registration succeeds) and each provider owns its own state store, so one provider's shutdown can never clear another's. Removes the global static map (review IN-03 root cause). Adds an injectable TraceInterceptorRegistrar test seam (mirrors the existing gate-override seam). Runtime-default rendering (String() parity) and hasData() are intentionally unchanged. L0 suite migrated to the instance-owned store; module 146 tests green. test(02-02): regression tests for span-enrichment lifecycle defects Adds SpanEnrichmentLifecycleRegressionTest exercising the real tracer lifecycle paths the original L0 suite bypassed (single root, no children, one provider). Each test FAILS against the pre-fix code and PASSES after the fix: CR-01 partialFlushExcludingRootPreservesPreFlushFlags / partialFlushNeverWrites- TagsOnAChildSpan: a partial flush carrying only child spans (root excluded, as dd-trace-core does) must not drain state or tag a child; the full pre+post-flush serial-id set {100,108,128,130} -> 'ZAgUAg==' must land on the root at final completion. Fail-before: tags written on the partial flush (NeverWantedButInvoked). CR-02 neverCompletingTracesDoNotLeakUnbounded / boundedStoreEvictsOldestFirst: 3x MAX_TRACES distinct never-flushed traces keep the store bounded; eviction is FIFO. Fail-before: store grew to 3x the cap (expected <=cap but was larger). CR-03 secondProviderRejectedRegistrationDoesNotClearFirstProviderState / eachProviderOwnsADistinctStateStore / hookAndInterceptorOfOneProviderShareThe- SameStore: a second provider whose registration is rejected wires no hook/interceptor and its shutdown does not clear the first provider's in-flight state. Fail-before: second provider kept a non-null interceptor (expected null). Fail-before verified by temporarily reintroducing each defect: 5/7 failed (the remaining 2 are structural-invariant checks that hold by construction). chore(feature-flagging): strip internal review labels from span-enrichment comments Remove leaked workflow labels (e.g. JAVA-01) from production comments so the upstream PR carries only behavioral context. fix(feature-flagging): address span-enrichment code-review findings - Reconfiguration safety: replace the per-provider trace interceptor with a single process-wide delegating interceptor (SpanEnrichmentInterceptor.INSTANCE) registered once and rebound per active provider. A closing provider unbinds only if still active, so provider close/reopen no longer permanently disables enrichment via a duplicate-priority rejection, and a displaced provider's late shutdown cannot clobber the active provider's in-flight state. - Runtime defaults: recursively unwrap OpenFeature Value (structure/list/scalar) to its native form before JSON serialization so ffe_runtime_defaults matches the frozen Node contract instead of emitting Value.toString(). - 128-bit trace ids: key per-trace state by the full trace id hex string rather than DDTraceId.toLong(), so two traces sharing low-order 64 bits no longer merge enrichment state. - Gate-off inertness: precompute the immutable provider-hook list once in the constructor so getProviderHooks() (called per evaluation) allocates nothing. - Add regression tests: reconfiguration, displaced-provider late shutdown, 128-bit low-bit collision, Value structure/list/scalar serialization, and gate-off zero-allocation. Merge master Adjustments + clean up; add FeatureFlaggingConfig.SPAN_ENRICHMENT_ENABLED; add Startup INFO log; clean tech debt Rework FFE span-enrichment state store to weak-keyed-by-span map Merge branch 'master' of github.com:DataDog/dd-trace-java into leo.romanovsky/ffe-apm-span-enrichment # Conflicts: # products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java Enhancements after techdebt skill Refactor: Move span-enrichment write tier out of the published dd-openfeature API Use JsonWriter instead of hand-written solution Fix unicode char Address review nits: gate enrichment metadata, name constants, reuse digest - Gate the __dd_split_serial_id / __dd_do_log evaluation metadata on the span-enrichment flag so an enabled provider with enrichment off attaches nothing extra per evaluation (DDEvaluator). - Extract the metadata keys to named constants (METADATA_SPLIT_SERIAL_ID / METADATA_DO_LOG) as the single source of truth; SpanEnrichmentHook references them. - Rename the config constant to EXPERIMENTAL_SPAN_ENRICHMENT_ENABLED to make its experimental status obvious at call sites. - Reuse a ThreadLocal<MessageDigest> for subject hashing instead of allocating a SHA-256 instance per capture (ULeb128Encoder). feat(ffe): Introduce config module to decouple feature flagging from tracing API (#11888) Fix 3 bugs: SpanEnrichmentWriter is now a process-wide singleton; addTraceInterceptor() false return: ensureInterceptorRegistered() now latches only on a true return; registration ordering: moved ensureInterceptorRegistered() below the root == null check Fix nits: add debug logging; fix comments Merge branch 'master' into leo.romanovsky/ffe-apm-span-enrichment Add gate on registration; adjust - add typed metadata Merge branch 'master' of github.com:DataDog/dd-trace-java into leo.romanovsky/ffe-apm-span-enrichment Merge branch 'leo.romanovsky/ffe-apm-span-enrichment' of github.com:DataDog/dd-trace-java into leo.romanovsky/ffe-apm-span-enrichment Merge branch 'master' into leo.romanovsky/ffe-apm-span-enrichment Raise feature-flagging test coverage to pass the JDK 8 jacoco gate Merge branch 'leo.romanovsky/ffe-apm-span-enrichment' of github.com:DataDog/dd-trace-java into leo.romanovsky/ffe-apm-span-enrichment Merge branch 'master' into leo.romanovsky/ffe-apm-span-enrichment Fix failing tests Merge branch 'master' of github.com:DataDog/dd-trace-java into leo.romanovsky/ffe-apm-span-enrichment Merge branch 'leo.romanovsky/ffe-apm-span-enrichment' of github.com:DataDog/dd-trace-java into leo.romanovsky/ffe-apm-span-enrichment Co-authored-by: pavlokhrebto <pavlo.khrebto@datadoghq.com> Co-authored-by: sarahchen6 <sarah.chen@datadoghq.com> Co-authored-by: PerfectSlayer <PerfectSlayer@users.noreply.github.com> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
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.
What Does This Do
This PR introduces a dedicated module for FFE config.
This would avoid adding
:internal-apior:dd-trace-apiinto the:feature-flagging-apimodule and keep the separation between FFE and Tracing clean.Motivation
Configuration will be generated at some points from the JSON configuration.
This is an intermediate solution that will keep the FFE design clean.
Additional Notes
This PR is a change proposal as part of the review of #11658
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)Jira ticket: [PROJ-IDENT]