Commit 05671ce
feat(feature-flagging): FFE APM feature-flag span enrichment (experimental, 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>1 parent 871f5b6 commit 05671ce
30 files changed
Lines changed: 2228 additions & 14 deletions
File tree
- dd-java-agent/agent-bootstrap
- src/main/java/datadog/trace/bootstrap
- dd-trace-api
- src/main/java/datadog/trace/api/config
- metadata
- products/feature-flagging
- feature-flagging-agent/src/main/java/com/datadog/featureflag
- feature-flagging-api
- src
- main/java/datadog/trace/api/openfeature
- test/java/datadog/trace/api/openfeature
- feature-flagging-bootstrap/src
- main/java/datadog/trace/api/featureflag
- ufc/v1
- test/java/datadog/trace/api/featureflag
- feature-flagging-config
- src/main/java/datadog/trace/api/featureflag/config
- feature-flagging-lib
- src
- main/java/com/datadog/featureflag
- test/java/com/datadog/featureflag
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
26 | 27 | | |
27 | 28 | | |
28 | 29 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
37 | | - | |
38 | 37 | | |
39 | 38 | | |
40 | 39 | | |
| |||
44 | 43 | | |
45 | 44 | | |
46 | 45 | | |
| 46 | + | |
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
41 | | - | |
42 | 41 | | |
43 | 42 | | |
44 | 43 | | |
| |||
Lines changed: 0 additions & 6 deletions
This file was deleted.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1505 | 1505 | | |
1506 | 1506 | | |
1507 | 1507 | | |
| 1508 | + | |
| 1509 | + | |
| 1510 | + | |
| 1511 | + | |
| 1512 | + | |
| 1513 | + | |
| 1514 | + | |
| 1515 | + | |
1508 | 1516 | | |
1509 | 1517 | | |
1510 | 1518 | | |
| |||
Lines changed: 13 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
| |||
27 | 28 | | |
28 | 29 | | |
29 | 30 | | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
30 | 39 | | |
31 | 40 | | |
32 | 41 | | |
33 | 42 | | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
34 | 47 | | |
35 | 48 | | |
36 | 49 | | |
| |||
Lines changed: 3 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| 47 | + | |
| 48 | + | |
47 | 49 | | |
48 | 50 | | |
49 | 51 | | |
| 52 | + | |
50 | 53 | | |
51 | 54 | | |
52 | 55 | | |
| |||
Lines changed: 21 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
49 | 59 | | |
50 | 60 | | |
51 | 61 | | |
| |||
392 | 402 | | |
393 | 403 | | |
394 | 404 | | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
395 | 416 | | |
396 | 417 | | |
397 | 418 | | |
| |||
Lines changed: 56 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| 19 | + | |
19 | 20 | | |
20 | 21 | | |
21 | 22 | | |
| |||
28 | 29 | | |
29 | 30 | | |
30 | 31 | | |
| 32 | + | |
31 | 33 | | |
32 | 34 | | |
33 | 35 | | |
34 | 36 | | |
35 | 37 | | |
36 | 38 | | |
37 | 39 | | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
38 | 45 | | |
39 | 46 | | |
40 | 47 | | |
| |||
45 | 52 | | |
46 | 53 | | |
47 | 54 | | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
48 | 66 | | |
49 | 67 | | |
50 | 68 | | |
| |||
58 | 76 | | |
59 | 77 | | |
60 | 78 | | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
61 | 107 | | |
62 | 108 | | |
63 | 109 | | |
| |||
167 | 213 | | |
168 | 214 | | |
169 | 215 | | |
170 | | - | |
171 | | - | |
172 | | - | |
173 | | - | |
| 216 | + | |
174 | 217 | | |
175 | 218 | | |
176 | 219 | | |
177 | 220 | | |
178 | 221 | | |
179 | 222 | | |
180 | 223 | | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
181 | 227 | | |
182 | 228 | | |
183 | 229 | | |
184 | 230 | | |
185 | 231 | | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
186 | 237 | | |
187 | 238 | | |
188 | 239 | | |
| |||
223 | 274 | | |
224 | 275 | | |
225 | 276 | | |
226 | | - | |
| 277 | + | |
227 | 278 | | |
228 | 279 | | |
229 | 280 | | |
| |||
Lines changed: 25 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
0 commit comments