Skip to content

Route tags by registry id instead of by name (otel/otlp) - #12398

Closed
dougqh wants to merge 8 commits into
dougqh/tag-registry-otelfrom
dougqh/tag-interceptor-ids
Closed

Route tags by registry id instead of by name (otel/otlp)#12398
dougqh wants to merge 8 commits into
dougqh/tag-registry-otelfrom
dougqh/tag-interceptor-ids

Conversation

@dougqh

@dougqh dougqh commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Does This Do

Stacked on #12354 — review that first; this PR's diff is the last two commits.

Retires TagInterceptor's two parallel string switches in favour of the tag registry's ids.

The Java overlay arrives. tag-conventions.yaml is the language-agnostic domain spec, and its header defers set-path routing to "a per-language overlay alongside this file". tag-conventions.java.yaml is that overlay. It has two sections:

  • intercepted: — names of domain tags this tracer also routes. Named, not redeclared: they already have ids.
  • reserved: — keys accepted by setTag but diverted to a span field or a trace directive, existing only because this tracer routes them (manual.keep, _dd.origin, sampling.priority, …). These get ids of their own.

Both sections' keys carry a new INTERCEPTED flag (bit 3) in their id. Domain serials are assigned first, so a Java-only reserved key cannot renumber the shared spec's generated output.

Routing moves onto ids. The pre-screen becomes a mask test on that flag; the dispatch becomes an int switch over dense serials — a tableswitch, where the name switch was a lookupswitch on string hashes plus an equals() per hit. DDSpanContext resolves the id once and hands it to both, so a routed tag is never looked up twice, and a TagMap entry already carries its own id, so the per-bundle screens in CoreTracer cost one mask per entry with no name comparison at all.

splitServiceTags stays a name lookup — it is user configuration and may name a custom tag with no id — but is now skipped outright when unset.

Motivation

TagInterceptor kept two hand-maintained string switches — one to decide whether a tag needs routing at all, one to dispatch it — listing the same twenty-odd tags twice. The registry already knows which tags exist and what they are called; asking it removes the duplication, and the id it returns is reusable by everything downstream (dispatch, storage, outbound naming) where a name has to be re-examined at each step.

It also removes a class of silent drift. KnownTagCodec records that an earlier classification bit was deleted because it could disagree with the interceptor's switch, with no error when it did. The bit returns here only because that agreement is now asserted by a test.

Both matter for the follow-on OpenTelemetry naming work (#12230), which needs one identity per tag across namespaces rather than a name the interceptor happens to recognise.

Additional Notes

Behaviour change worth a look

keyOf is many→one, so a tag now routes under every name it is known by. That collapses the hand-maintained "service.name"/"service" pair of case labels into the one service serial, which is pure cleanup. But it also means OpenTelemetry names now route rather than merely store:

tag set as now also does
url.full sets the resource name (as http.url does)
db.query.text sets the resource name (as db.statement does)
http.response.status_code sets the span's status field

This is the registry's premise — one identity across namespaces — and it is what the follow-on OTel work wants. It is called out here because it is a semantic expansion, not a refactor, and is the one thing in this PR that a reviewer might want gated behind a flag instead.

Namespace handling is a deliberate choice, not a side effect. The registry can emit more than one name→id table -- a Datadog-names table, an OpenTelemetry-names table, and a combined one -- and each entry point can resolve against whichever it should. This PR uses the combined table, so a tag routes under every name it is known by. That is the simple option and it is chosen on purpose.

The alternative considered was restricting the Datadog set path to Datadog spellings, which would have made this PR strictly behaviour-preserving. It was not taken because the resulting behaviour is the one customers want: an OpenTelemetry user setting url.full today gets a span with no resource name, while a Datadog user setting http.url gets one -- the same span, worse product, decided only by which SDK the call came through. The combined table closes that gap.

What is deferred, not decided here: whether the OTel bridge should translate names on the way in (arguably the layer this belongs in), and whether the default table per entry point should be declared in the shared conventions rather than chosen per language. Both become cheap once the per-namespace tables exist; neither blocks this PR.

Why the flag is allowed back

KnownTagCodec records that an earlier classification bit was deleted because it could disagree with TagInterceptor's switch, silently. It returns only because the agreement is now asserted: TagInterceptorRoutingTest walks every serial the registry has assigned and checks, behaviourally, whether the tag reaches the switch's default branch — configuring every known tag as a split-service tag makes that branch, and only that branch, call setServiceName(value, SPLIT_BY_TAGS). The declared set must equal the handled set in both directions. Verified to have teeth: deleting one case label fails it with the right message.

The generator also fails the build on two ways the overlay can be wrong: a reserved: key the domain spec already declares (that would mint a second identity for one tag), and an intercepted: name that matches no domain tag (a typo there would silently flag nothing and stop the pre-screen recognising a key the interceptor still handles).

Benchmark

TagInterceptorScreenBenchmark (new, dd-trace-core/src/jmh). Both implementations live in it as arms: the old name switch is deleted by this PR, so there is no one-binary flag to A/B, and a two-jar master-vs-branch run would conflate master drift. The ByName arms are a frozen verbatim copy of the pre-change 22-label switch.

-f3, 5×5 iterations, @Threads(8), -prof gc. Throughput, ops/us, higher is better.

Where the id is already in hand. This is the path that matters: CoreTracer screens defaultSpanTags / localRootSpanTags / mergedTracerTags as bundles, and DDSpanContext.setTag(EntryReader) reads the entry's own id. No keyOf at all. Bundle is a 7-entry web-shaped TagMap with nothing routed in it, so the scan runs to completion.

arm splitByTags ById ByName Δ
bundle screen (7 entries) off 201.6 ± 6.2 117.7 ± 3.2 +71%
bundle screen (7 entries) on 160.4 ± 7.7 99.7 ± 4.7 +61%
dispatch switch off 1481.4 ± 57.9 1397.2 ± 33.0 +6.0%
dispatch switch on 1551.5 ± 22.9 1375.6 ± 21.8 +12.8%

Where the name must be resolved first. Mixed with the shipped config, and clearly negative once split-by-tags is populated:

arm splitByTags ById ByName Δ
screen, known-but-unrouted tag off 2003.4 ± 68.4 1701.8 ± 43.1 +18%
screen, known-but-unrouted tag on 986.2 ± 12.6 1374.6 ± 31.8 −28%
screen, routed tag off 1574.5 ± 18.9 1680.8 ± 5.4 −6%
screen, routed tag on 1630.1 ± 75.5 1677.3 ± 10.2 ≈0
screen, custom tag off 3046.7 ± 146.1 1670.3 ± 37.1 +82%
screen, custom tag on 738.7 ± 2.6 1165.6 ± 14.6 −37%

That is the keyOf tax: at a String call site the id path pays an open-addressed string lookup the lookupswitch did not, and on a miss neither path can then skip the splitServiceTags set probe.

Three things to read alongside those rows, in order of weight:

  • The arm over-charges the id path. screenById_* calls needsIntercept(String), which resolves the id internally and discards it. Production does not: DDSpanContext.setTag resolves once and hands the same id to both the screen and interceptTag. The arm pays keyOf and gets none of the dispatch saving back, so −28% is a worst-case bound on a call shape the tracer does not have, not the production delta.
  • splitByTags ships empty, so off is the default-config column.
  • screenById_custom off at +82% is the one number with no mechanism offered for it. It is reported, not relied on.

Allocation is flat and ≈0 on all twenty rows (gc.alloc.rate.norm 10⁻⁵–10⁻⁷ B/op, gc.count ≈ 0). This is a CPU lever, so that is the correctness check passing, not the result.

Directional only. The micro over-states its own share of a real span; PetClinic is the acceptance number.

Not in this PR

  • kind: structural|directive and field: from the earlier overlay draft. That is the id→handler dispatch table's payload; it lands with the PR that actually builds the table, so the overlay never carries a field nothing reads.
  • The cross-tag lookups inside the http resource rule (unsafeGetTag(HTTP_URL), which still asks by Datadog name).
  • Anything dense-store.

Testing

:dd-trace-core:test — 4293 tests, 3 failures, all pre-existing and environmental in this checkout: two TracerConnectionReliabilityTest cases fail on Could not find a valid Docker environment (Testcontainers, no local Docker) and PendingTraceBufferTest.testingTracerFlareDumpWithMultipleTraces is the known timing flake. spotbugsMain and spotlessJavaCheck clean.

New tests: the routing drift sweep, the pre-screen/flag agreement (including a custom split tag with no id), alternate-name routing, and the bundle screen.

Contributor Checklist

🤖 Generated with Claude Code

dougqh and others added 7 commits September 2, 2026 12:57
@dougqh @claude
Add the tag registry: generated tag ids + name resolution
1042303 
Introduces a language-agnostic tag registry. tag-conventions.yaml
declares each known tag once; a buildSrc Gradle plugin generates
KnownTags.java from it into internal-api/src/generated, committed and on
the main compile path, with a verifyKnownTags task wired into check so a
stale checkout fails the build rather than drifting.

A tag id is IDENTITY, not storage: a globally unique 16-bit serial in
bits [63-48] plus the trace/span level bit. It says what a tag is, never
how it is stored or set. Bits [47-32] are documented and held vacant for
the co-occurrence slot the dense tag store assigns by graph coloring,
so that store lands as a purely additive change.

KnownTagCodec is the hand-written half of the registry -- bit layout and
naming policy -- and generated KnownTags is the data half. Java has no
partial classes, so the codec's Installed holder names KnownTags.RESOLVER
directly: reading a tag name is what initializes the registry, with no
registration call and no ordering to get wrong. The holder keeps the
resolver in a static final of an initialized class, which the JIT
constant-folds to an exact klass, so keyOf/nameOf devirtualize and inline
with no lock, volatile read, null check or virtual call. The nesting is
load-bearing: KnownTags calls back into the codec, so a field on the
codec itself would re-enter a half-initialized class.

keyOf is many-to-one -- a Datadog name or an OpenTelemetry name both
resolve to the one id -- and datadogNameOf/openTelemetryNameOf take the
name back out per namespace. openTelemetryTagOf is the single home for
the pass-through policy (declared rename, else the Datadog name), so no
serializer re-decides it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dougqh @claude
Give TagMap.EntryReader its tag id and OpenTelemetry name
e6d5962 
tagId() resolves the entry's key through the registry, and
openTelemetryTag() layers the namespace projection on top: the tag's
declared OpenTelemetry rename when it has one, otherwise its own key.
The reader supplies that last fallback because a custom tag has no
registry name, and only the holder of the key can name it.

Entry.tagId() is deliberately computed rather than memoized, unlike
hash(). TagMap$Entry is the tracer's largest allocation source -- one per
tag per span, on the app thread -- and it packs to 40 bytes with only 3
bytes of padding, so a long memo widens every entry to 48 and adds a
putfield per construction. The only caller is serialization: background
thread, once per entry, where keyOf is a single open-addressed probe over
a static final table keyed on an already-cached String hash.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dougqh @claude
Emit OTLP attributes under OpenTelemetry tag names
c655e96 
OTLP is the OpenTelemetry wire format, so each known tag is rendered
under its OpenTelemetry name: http.method as http.request.method,
http.useragent as user_agent.original, and so on, falling back to the
Datadog name for a tag that declares no rename and to the entry's own key
for a custom tag the registry does not know.

Tags the tracer intercepts into first-class Metadata fields never reach
the per-entry projection, so their keys resolve through the same registry
policy once each at class init -- otherwise service and http.status_code
would keep emitting under Datadog names while every other tag moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dougqh @claude
Cover the tag registry: name resolution, namespaces, and id partitioning
e2c723c 
KnownTagsTest pins the registry itself: keyOf/nameOf round-trip for every
known tag, many-to-one resolution from either namespace, serial encoding
and the trace-level bit, and unknown names resolving to 0.
TagMapNamespaceNamesTest covers EntryReader.openTelemetryTag() -- rename,
pass-through, and the custom-tag fallback the codec cannot supply.
OtlpTraceProtoTest asserts attributes now land under OpenTelemetry names.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dougqh @claude
Register every declared tag, not only layout-reachable ones
8d60761 
The registry was built by resolving concrete span types, so a tag reached
the union only through a type that could carry it. A mixin attaches via
its applies: targets, and ci_visibility applies to `test`, which the
conventions do not model yet -- so test.name, test.suite, test.status and
test.framework were dropped, silently. keyOf reported live CI Visibility
tags as unknown, and nothing in the generated output showed the four
declarations had gone anywhere.

An id is identity, and identity does not depend on layout: a declared tag
gets an id whether or not a modeled span type can carry it today. So the
registry is now built from declarationGroups() -- trace-level, span types
and every mixin -- which is also what makes mixins behave uniformly
rather than working only when their target happens to be modeled.

The layout gap is real, just not fatal, so the generator reports it:
resolved-tags.txt names each mixin whose applies: target is unmodeled and
says its tags are registered but occupy no per-type slot. Declaring tags
ahead of the span type that will carry them is a legitimate intermediate
state; being unable to see it is not.

Registry grows 47 -> 51 tags. Serials are assigned over the sorted name
list, so the four insertions shift the ids of tags sorting after them --
fine, since ids are in-process with no cross-release stability.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dougqh
Add the Java overlay: intercepted + reserved tag classification
d7973fe 
The domain spec (tag-conventions.yaml) deliberately models only what a tag IS,
and defers set-path routing to "a per-language overlay alongside this file".
This adds that overlay for Java, restoring the intercepted/reserved taxonomy
from the pre-OTLP-split generator work.

  intercepted:  domain tags this tracer also routes (named, not redeclared --
                they already have an identity; this only flags it)
  reserved:     keys accepted by setTag but diverted to a span field or a trace
                directive, which exist only because this tracer routes them and
                so have no place in a cross-language contract

Both get bit 3 of the tag id, INTERCEPTED. The point is speed: a TagMap entry
carries its own tag id, so screening a bundle for anything the interceptor cares
about is a mask test on an id already in hand -- no name lookup, no side table.

An earlier version of this bit was deleted because it could disagree with
TagInterceptor's switch. It returns because the agreement becomes a test rather
than a convention; that test lands with the dispatch rework that consumes it.

Nothing here records whether a routed tag is also STORED. That is decided per
call from the value -- http.url is routed and stored, manual.keep is consumed
only when its value coerces to a boolean -- so it is not a property of the tag,
and a static flag mirroring it would be the same drift in a new place.

Reserved serials are assigned after every domain serial, so a Java-only key
cannot renumber the domain block: of the 51 existing tags, only the 8 named in
`intercepted` change at all, and only in that flag bit.
@dougqh @claude
Route tags by registry id instead of by name
989a3e9 
TagInterceptor's two parallel string switches become one id lookup: the
pre-screen is a mask test on the INTERCEPTED flag, and the dispatch is an
int switch over dense serials (a tableswitch, where the name switch was a
lookupswitch on string hashes plus an equals() per hit). DDSpanContext
resolves the id once and hands it to both, so a routed tag is never looked
up twice; a TagMap entry already carries its own id, so the bundle screens
cost a mask per entry.

Because keyOf is many->one, a tag now routes under every name it is known
by. The hand-maintained "service.name"/"service" pair of case labels
collapses into the one `service` serial, and OpenTelemetry names route
without a second label.

splitServiceTags stays a name lookup -- it is user configuration and may
name a custom tag with no id -- but is now skipped outright when unset.

TagInterceptorRoutingTest asserts the declared flag set is exactly the set
the switch handles. That test is what licenses the flag: an earlier version
of it was deleted because the declaration and the switch could drift apart
silently.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dougqh dougqh added tag: ai generated Largely based on code generated by an AI or LLM comp: core Tracer core type: refactoring tag: no release notes Changes to exclude from release notes labels Sep 3, 2026
@datadog-official

This comment has been minimized.

Sign in to view
@dd-octo-sts

dd-octo-sts Bot commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 14.77 s 14.66 s [-0.2%; +1.7%] (no difference)
startup:insecure-bank:tracing:Agent 13.65 s 13.62 s [-0.7%; +1.2%] (no difference)
startup:petclinic:appsec:Agent 17.02 s 16.96 s [-0.5%; +1.2%] (no difference)
startup:petclinic:iast:Agent 16.93 s 16.99 s [-1.2%; +0.4%] (no difference)
startup:petclinic:profiling:Agent 16.71 s 16.86 s [-1.9%; +0.2%] (no difference)
startup:petclinic:sca:Agent 17.06 s 16.26 s [+0.3%; +9.5%] (maybe worse)
startup:petclinic:tracing:Agent 16.08 s 16.20 s [-1.9%; +0.4%] (no difference)

Commit: 1e1ff99b · CI Pipeline · Benchmarking Platform UI

Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@dougqh @claude
Add TagInterceptorScreenBenchmark
1e1ff99 
Both the id-based screen/dispatch and a frozen copy of the pre-change
name switch live here as arms: the old switch is deleted, so there is no
one-binary flag, and a two-jar A/B would conflate master drift.

This is a CPU-not-allocation lever, so flat gc.alloc.rate.norm across
every arm is the correctness check, not the result.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dougqh dougqh changed the title Route tags by registry id instead of by name Route tags by registry id instead of by name (otel/otlp - priority) Sep 15, 2026
@dougqh dougqh changed the title Route tags by registry id instead of by name (otel/otlp - priority) Route tags by registry id instead of by name (otel/otlp) Sep 15, 2026
@bric3
bric3 added this pull request to stack #12687 September 29, 2026 14:58
@dougqh
dougqh force-pushed the dougqh/tag-registry-otel branch from c3c341e to e49a183 Compare October 7, 2026 12:50
@dougqh

dougqh commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Closing for now: it was stacked on #12354 and blocked its stack merge requirements, and its base (dougqh/tag-registry-otel) has since been squash-rebased. The id-keyed TagInterceptor routing it explored will be redone on the current tag-registry stack; this branch stays available as reference.

@dougqh dougqh closed this Oct 7, 2026
@dougqh
dougqh removed this pull request from stack #12687 October 7, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp: core Tracer core tag: ai generated Largely based on code generated by an AI or LLM tag: no release notes Changes to exclude from release notes type: refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@dougqh