Repository navigation
Register APM tag constants on their span types (otlp - tag registry) - #12893
Conversation
Adds id-keyed setters, getters and removal to TagMap, TagMap.Entry and spans, so a writer that knows the tag skips the name lookup. Squashed from the review history of #12715. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
span.setTag(0L, "") or a null value reached TagMap.getAndRemove(long), which rejects unknown ids, so clearing threw IllegalArgumentException while every id-keyed setter ignores an unknown id. removeTag(long) now ignores it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The long-valued setters prechecked with needsIntercept, but when the interceptor declined the tag they discarded the box it was given and stored the primitive. Follow the same precheckIntercept -> setBox shape as the other primitive setters, so TagMap keeps the box. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
peer.port is declared per direction in the registry (#12713), so it has no direction-free id: BaseDecorator, shared by client and server decorators, sets it by name, and the set-by-id tests use an unshared int tag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Set-path routing is a per-language concern, so it lives in a Java overlay next to tag-conventions.yaml rather than in the language-agnostic conventions. The overlay declares the keys that exist only to be routed (resource.name, error, sampling directives, ...) and lists every tag TagInterceptor may route. Each listed tag's id carries the INTERCEPTED bit (bit 1), so a setter called with a constant id can fold the interception test away. Serial numbers become public so TagInterceptor can switch on them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TagInterceptor switches on the tag's serial rather than its name, so a known tag is matched under any of its names and the *_OTEL_NAME cases go away. needsIntercept(long) tests the INTERCEPTED bit first, which folds away for a constant id. split-by-tags entries resolve to serials at construction (each direction for a name declared per direction); only custom tags are still matched by name. A test checks that exactly the tags carrying the bit have a case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A name setter resolves a known tag's name to its id once and sets it as the id-keyed setters do; only a custom tag is still handled by name. The id-keyed setters no longer look up the name to feed the interceptor, so with a constant id the interception test folds to the INTERCEPTED bit. Entry paths (builder ledger, prototypes, default tags) route by the entry's id and precheck before boxing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The generated nameOf switched over every tag's serial: 491 bytes of bytecode, too big for C2 to inline, so each id-keyed set paid two out-of-line calls into it (the span's unknown-id guard and the entry's name). Read a NAMES_BY_SERIAL array instead; nameOf is now 23 bytes and inlines at every caller. Also adds SetTagBenchmark (span setTag by constant id, non-constant id, name, and custom name, against a bare and a synchronized TagMap store). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The table is always allocated and never resized, so the split check -- the only run-time check left on a non-intercepted constant-id set -- is a single load, with no null or length test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The span's unknown-id guard called nameOf; KnownTagCodec.isKnown answers the same question from the serial and a generated SERIAL_LIMIT, so it folds away for a constant id. Also note that serials are not stable across releases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DDSpanContext.setTag calls interceptTag only for intercepted tags. Kept out of line (over FreqInlineSize, 325 bytes of bytecode), it never brings the profiled handler bodies into setTag's compiled code, which would push setTag past InlineSmallCode and stop callers inlining it -- the inlining a constant id needs to fold its interception test. A test now fails if the switch shrinks below the limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An overlay tag exists only to be routed, so the generator now marks it intercepted without a second listing; intercepted: lists only the tags declared in tag-conventions.yaml that the tracer also routes. Also restore the doc comment and @Suppress that the overlay helper displaced, read the serial limit one way, inline isUnknownTag, and drop an unused default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A tag declared once per direction (peer.port) has two ids but one shared Datadog name, and keyOf resolves that name to neither. An entry keyed by one of those ids hashed by the id while name-based access hashed the name as a custom tag, so get/remove/containsKey by name missed it and a later set by name produced a duplicate key. Encode a SHARED_NAME flag in the reserved id bit 0 so the check folds for a constant id. TagMap's id-keyed create/get/remove reject such ids, and DDSpanContext's id-keyed setters ignore them, as they do unknown ids, until name resolution knows the span's direction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Entry's package-private name factories are now custom-only: they skip the registry and assert the name is not a known tag. Callers that start from a name resolve it once -- TagMap.set(String, ...) forwards a known tag to set(long, ...), while getAndSet, put, Ledger.set, putAll and the public Entry.create(String, ...) API go through anyEntryFor and friends -- so a custom tag pays one registry lookup instead of two. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # .github/CODEOWNERS
TagMap.internals() returns a small Internals view whose setKnown(...) skips the id validation the checked set(long, ...) repeats; DDSpanContext's id-keyed setters, which have already checked isKeyableById, now use it. Internals is created per call, so escape analysis removes it. internals() is marked with a new @restricted(allowedIn = ...) annotation, and gradle/forbiddenApiFilters/instrumentation.txt bans it in instrumentation modules, so calling it there fails forbiddenApisMain. Internals is also where the batched setTagsFrom/TagSink writes will go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It exists to be created, used within the expression, and scalar-replaced; storing one would force a real allocation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Restricted lives in the new datadog.apicontract package, which had no owner. Give it the same owner as its sibling datadog.perfcontract. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
peer.port was modeled as one tag per direction: PEER_PORT_INBOUND_ID and
PEER_PORT_OUTBOUND_ID, sharing the Datadog name and marked with a
SHARED_NAME id bit. Two ids under one name kept producing lookup and
supersede bugs: keyOf("peer.port") resolved to neither id, so a value set
by name and one set by id landed in different entries, and tombstones and
getAndSet by name could not find what an id had stored.
The generator now merges a name's per-direction declarations into one tag
with one id (PEER_PORT_ID), and only its OpenTelemetry name depends on the
span's direction: openTelemetryNameOf(id, direction) returns client.port
inbound and server.port outbound. The SHARED_NAME bit, hasSharedName and
isKeyableById go away (bit 0 is left unused), and keyOf("peer.port")
returns PEER_PORT_ID.
KnownTagCodec gains the DIRECTION_* constants and
openTelemetryTagOf(id, direction). Direction-scoped OpenTelemetry names
are still not resolved when written by name.
This is the direction-neutral-id part of #12731's redesign; its
direction-scoped aliases and read-time resolution are left to that PR.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With peer.port now one direction-neutral tag, its OpenTelemetry name depends on the span: server.port on a client span, client.port on a server span. The OTLP JSON and protobuf writers compute the span's direction once from its span.kind and name each tag for it, so direction-scoped renames apply too: peer.hostname exports as server.address on a client span, and network.client.ip as network.peer.address on a server span. KnownTagCodec gains directionOf(TagMap) / directionOfKind(String), and TagMap.EntryReader gains openTelemetryTag(direction), standing in for the SpanTags helpers #12731 introduces. Ported from #12731's redesign. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Entry's full constructor and the Internals and Ledger constructors were
private, so javac (targeting Java 8, without nestmates) gave each one a
synthetic access constructor taking the never-loaded class TagMap$1. C2
refuses to inline a call whose signature names an unloaded class
("unloaded signature classes"). So the Internals view the core creates
for each id write could not be scalar-replaced, and every Entry built
through that constructor went through a real call. Making the
constructors package-private removes the accessors and TagMap$1.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Several private members were used across TagMap's nested classes, so javac (targeting Java 8) generated access$ methods for them: Entry's *2Prim helpers and requireKnownName, TagMap.parentEntryVisible, TagMap.putEntry and the TagMap.buckets field. Each accessor adds a level of inlining depth. putEntry's accessor sits on Internals.setKnown, the core's trusted id-write path. All of these are now package-private. TagMapSyntheticAccessorTest fails if TagMap or any of its nested classes gains a synthetic accessor again. It is the prototype for the @NoSyntheticAccess contract (APMLP-1976). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Master now has #12715 (set-by-id, squashed) and #12037 (decorator SpanPrototype). Conflicts resolve to this branch's newer id-keyed code, plus: - getAndSet finds its inherited prior by the entry's id (#12715's last fix); the shared-name test for it is dropped, since peer.port now has one id. - #12037's resolveSplitByTagsPrecedence checks component by id and language by name against the serial-indexed split-by-tags table. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings the three classes back over jacoco's thresholds: the trusted setters' Object/float/double overloads, isIntercepted, isKnown's range edges, canonicalTagName, span-kind direction, and an entry's directional OpenTelemetry name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
build-logic is a separate Gradle build, so the root spotlessApply never formats it. Three generator lines were wrapped where its Kotlin formatter keeps them on one line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
The two most actionable issues are: messaging span type declarations (producer, consumer, client) omit peer address/hostname/service tag mixins that RabbitMQ instrumentation already writes, and web.handler spans are excluded from the code-origin tag applies list, causing Spring controller spans to omit _dd.code_origin.type from their resolved tag sets.
🤖 Bits Code Review · Commit 20a0148
Two review findings, both confirmed in the code: - RabbitChannelInstrumentation calls onPeerConnection on its client, producer and consumer decorators. messaging.producer and messaging.client now include peer, peer_address and outbound_peer (peer.service is computed for client and producer spans, as for http.client and rpc.client); messaging.consumer includes peer_address and inbound_peer. Consumers also set peer.hostname, which outbound_peer declares for client spans only: a TODO, as for websocket spans. - Code origin's entry capture instruments annotated controller methods, whose active span is the framework's handler span, so the code_origin mixin also applies to web.handler. Slots unchanged at 47 with #12882's coloring. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sarahchen6
left a comment
There was a problem hiding this comment.
Changes look reasonable, though I'm not familiar with OTLP tag expectations TBH... Probably worth another check from the SDK Cap team
Clean merge. With the merged coloring, this branch's 212 registered tags color into 47 slots (SLOT_COUNT 47, cap 63), and no tag shares a slot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…registry-civis Clean merge; the generator now matches #12893's, with no trace of the reverted slot marker. With the merged coloring the 319 tags color into 63 slots: the CI test span types overflow, and 66 optional tags share slots (CI Visibility's occasional tags plus _dd.span_links, _dd.svc_src, error.debug_info_captured and events), on those span types only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for f80c7d6: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
- OtelSpanBuilder, OpenTelemetry14DirectionalAttributesTest, and KnownTagCodec.directionalKeyOf: master's versions, carrying #12731's late fixes (held attributes resolved at setSpanKind, null keys ignored). - tag-conventions.yaml: #12893 already declares process_id and _dd.trace_span_attribute_schema as required local-root trace tags, so this branch's duplicate declarations go; _dd.agent_psr joins that group. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…12900) Set known tags by id on TagMap and spans Adds id-keyed setters, getters and removal to TagMap, TagMap.Entry and spans, so a writer that knows the tag skips the name lookup. Squashed from the review history of #12715. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Ignore unknown ids when clearing a tag by id; fix a comment wrap span.setTag(0L, "") or a null value reached TagMap.getAndRemove(long), which rejects unknown ids, so clearing threw IllegalArgumentException while every id-keyed setter ignores an unknown id. removeTag(long) now ignores it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Precheck and reuse the box in long-valued setters The long-valued setters prechecked with needsIntercept, but when the interceptor declined the tag they discarded the box it was given and stored the primitive. Follow the same precheckIntercept -> setBox shape as the other primitive setters, so TagMap keeps the box. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Keep peer.port off the id path peer.port is declared per direction in the registry (#12713), so it has no direction-free id: BaseDecorator, shared by client and server decorators, sets it by name, and the set-by-id tests use an unshared int tag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Merge set-by-id (#12715) onto master Mark intercepted tags with an id bit, via a tracer overlay Set-path routing is a per-language concern, so it lives in a Java overlay next to tag-conventions.yaml rather than in the language-agnostic conventions. The overlay declares the keys that exist only to be routed (resource.name, error, sampling directives, ...) and lists every tag TagInterceptor may route. Each listed tag's id carries the INTERCEPTED bit (bit 1), so a setter called with a constant id can fold the interception test away. Serial numbers become public so TagInterceptor can switch on them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Dispatch TagInterceptor on tag ids TagInterceptor switches on the tag's serial rather than its name, so a known tag is matched under any of its names and the *_OTEL_NAME cases go away. needsIntercept(long) tests the INTERCEPTED bit first, which folds away for a constant id. split-by-tags entries resolve to serials at construction (each direction for a name declared per direction); only custom tags are still matched by name. A test checks that exactly the tags carrying the bit have a case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Set tags on spans by id first A name setter resolves a known tag's name to its id once and sets it as the id-keyed setters do; only a custom tag is still handled by name. The id-keyed setters no longer look up the name to feed the interceptor, so with a constant id the interception test folds to the INTERCEPTED bit. Entry paths (builder ledger, prototypes, default tags) route by the entry's id and precheck before boxing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Look up tag names by serial in an array, not a switch The generated nameOf switched over every tag's serial: 491 bytes of bytecode, too big for C2 to inline, so each id-keyed set paid two out-of-line calls into it (the span's unknown-id guard and the entry's name). Read a NAMES_BY_SERIAL array instead; nameOf is now 23 bytes and inlines at every caller. Also adds SetTagBenchmark (span setTag by constant id, non-constant id, name, and custom name, against a bare and a synchronized TagMap store). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Point stale comments at the tracer overlay and the intercepted bit Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Index the split-by-tags table by serial, sized to every known tag The table is always allocated and never resized, so the split check -- the only run-time check left on a non-intercepted constant-id set -- is a single load, with no null or length test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Check for a known tag id with a range check, not a name lookup The span's unknown-id guard called nameOf; KnownTagCodec.isKnown answers the same question from the serial and a generated SERIAL_LIMIT, so it folds away for a constant id. Also note that serials are not stable across releases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Pin interceptTag(long) above C2's inlining size DDSpanContext.setTag calls interceptTag only for intercepted tags. Kept out of line (over FreqInlineSize, 325 bytes of bytecode), it never brings the profiled handler bodies into setTag's compiled code, which would push setTag past InlineSmallCode and stop callers inlining it -- the inlining a constant id needs to fold its interception test. A test now fails if the switch shrinks below the limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Tighten comments on the id-keyed interceptor Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Treat overlay tags as intercepted; tidy from review An overlay tag exists only to be routed, so the generator now marks it intercepted without a second listing; intercepted: lists only the tags declared in tag-conventions.yaml that the tracer also routes. Also restore the doc comment and @Suppress that the overlay helper displaced, read the serial limit one way, inline isUnknownTag, and drop an unused default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Reject shared-name tag ids on the id-keyed TagMap and span paths A tag declared once per direction (peer.port) has two ids but one shared Datadog name, and keyOf resolves that name to neither. An entry keyed by one of those ids hashed by the id while name-based access hashed the name as a custom tag, so get/remove/containsKey by name missed it and a later set by name produced a duplicate key. Encode a SHARED_NAME flag in the reserved id bit 0 so the check folds for a constant id. TagMap's id-keyed create/get/remove reject such ids, and DDSpanContext's id-keyed setters ignore them, as they do unknown ids, until name resolution knows the span's direction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Test rejection of shared-name tag ids Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Build custom tags by name and known tags by id Entry's package-private name factories are now custom-only: they skip the registry and assert the name is not a known tag. Callers that start from a name resolve it once -- TagMap.set(String, ...) forwards a known tag to set(long, ...), while getAndSet, put, Ledger.set, putAll and the public Entry.create(String, ...) API go through anyEntryFor and friends -- so a custom tag pays one registry lookup instead of two. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Merge master into id-tag-interceptor (#12817) # Conflicts: # .github/CODEOWNERS Give the tracer core a restricted, trusted path into TagMap TagMap.internals() returns a small Internals view whose setKnown(...) skips the id validation the checked set(long, ...) repeats; DDSpanContext's id-keyed setters, which have already checked isKeyableById, now use it. Internals is created per call, so escape analysis removes it. internals() is marked with a new @restricted(allowedIn = ...) annotation, and gradle/forbiddenApiFilters/instrumentation.txt bans it in instrumentation modules, so calling it there fails forbiddenApisMain. Internals is also where the batched setTagsFrom/TagSink writes will go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Mark TagMap.Internals @NoEscape It exists to be created, used within the expression, and scalar-replaced; storing one would force a real allocation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Add datadog.apicontract to CODEOWNERS Restricted lives in the new datadog.apicontract package, which had no owner. Give it the same owner as its sibling datadog.perfcontract. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Merge branch 'master' into dougqh/id-tag-interceptor Give peer.port one direction-neutral id peer.port was modeled as one tag per direction: PEER_PORT_INBOUND_ID and PEER_PORT_OUTBOUND_ID, sharing the Datadog name and marked with a SHARED_NAME id bit. Two ids under one name kept producing lookup and supersede bugs: keyOf("peer.port") resolved to neither id, so a value set by name and one set by id landed in different entries, and tombstones and getAndSet by name could not find what an id had stored. The generator now merges a name's per-direction declarations into one tag with one id (PEER_PORT_ID), and only its OpenTelemetry name depends on the span's direction: openTelemetryNameOf(id, direction) returns client.port inbound and server.port outbound. The SHARED_NAME bit, hasSharedName and isKeyableById go away (bit 0 is left unused), and keyOf("peer.port") returns PEER_PORT_ID. KnownTagCodec gains the DIRECTION_* constants and openTelemetryTagOf(id, direction). Direction-scoped OpenTelemetry names are still not resolved when written by name. This is the direction-neutral-id part of #12731's redesign; its direction-scoped aliases and read-time resolution are left to that PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Name peer.port by the span's kind in OTLP export With peer.port now one direction-neutral tag, its OpenTelemetry name depends on the span: server.port on a client span, client.port on a server span. The OTLP JSON and protobuf writers compute the span's direction once from its span.kind and name each tag for it, so direction-scoped renames apply too: peer.hostname exports as server.address on a client span, and network.client.ip as network.peer.address on a server span. KnownTagCodec gains directionOf(TagMap) / directionOfKind(String), and TagMap.EntryReader gains openTelemetryTag(direction), standing in for the SpanTags helpers #12731 introduces. Ported from #12731's redesign. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Let C2 inline the constructors of TagMap's nested classes Entry's full constructor and the Internals and Ledger constructors were private, so javac (targeting Java 8, without nestmates) gave each one a synthetic access constructor taking the never-loaded class TagMap$1. C2 refuses to inline a call whose signature names an unloaded class ("unloaded signature classes"). So the Internals view the core creates for each id write could not be scalar-replaced, and every Entry built through that constructor went through a real call. Making the constructors package-private removes the accessors and TagMap$1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Remove TagMap's remaining synthetic accessors and guard against them Several private members were used across TagMap's nested classes, so javac (targeting Java 8) generated access$ methods for them: Entry's *2Prim helpers and requireKnownName, TagMap.parentEntryVisible, TagMap.putEntry and the TagMap.buckets field. Each accessor adds a level of inlining depth. putEntry's accessor sits on Internals.setKnown, the core's trusted id-write path. All of these are now package-private. TagMapSyntheticAccessorTest fails if TagMap or any of its nested classes gains a synthetic accessor again. It is the prototype for the @NoSyntheticAccess contract (APMLP-1976). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Merge master into id-tag-interceptor Master now has #12715 (set-by-id, squashed) and #12037 (decorator SpanPrototype). Conflicts resolve to this branch's newer id-keyed code, plus: - getAndSet finds its inherited prior by the entry's id (#12715's last fix); the shared-name test for it is dropped, since peer.port now has one id. - #12037's resolveSplitByTagsPrecedence checks component by id and language by name against the serial-indexed split-by-tags table. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Cover TagMap.Internals, KnownTagCodec and EntryReader paths Brings the three classes back over jacoco's thresholds: the trusted setters' Object/float/double overloads, isIntercepted, isKnown's range edges, canonicalTagName, span-kind direction, and an entry's directional OpenTelemetry name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Format the tag-registry generator to its spotless rules build-logic is a separate Gradle build, so the root spotlessApply never formats it. Three generator lines were wrapped where its Kotlin formatter keeps them on one line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Order the tag-id carve bits by position Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Merge master into id-tag-interceptor Brings #12814 (split-by-tags by OpenTelemetry name). Id matching already covers OpenTelemetry names, so this PR's TagInterceptor wins and #12814's withCanonicalNames goes, as planned. #12814's tests stay; one expectation changes: with db.type configured, setTag("db.system", ...) now splits too, since a known tag matches under any of its names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Re-apply split-by-tags language by its id resolveSplitByTagsPrecedence checked language through the custom-tag overload, which only matches names that resolve to no known tag. language is known, so the check was always false and a prototype's language was never re-applied over another split-by-tags candidate the prototype set. Check and read it by LANGUAGE_ID, as for component. Found by Codex, via a review comment from AlexeyKuznetsov-DD. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Say what a tag id's intercepted bit does and doesn't decide Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Ratchet the tag constants toward full registry coverage Every tag-name constant in Tags, InstrumentationTags and DDTags must be in the tag registry, so instrumentation can set it by id, or be listed: - NOT_TAG_NAMES: 19 constants that are tag values (span kinds, LLM Observability span kinds, the language value) or a name prefix. - NOT_YET_REGISTERED: the 270 tag names not registered yet. Registering one fails the test until its entry is removed, so the list only shrinks; a new constant must be registered or listed. 62 of the 352 constants are registered today. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Let the tracer overlay declare mixins on shared span types The Java overlay could only declare routing-only keys, which belong to no span type. It can now declare `mixins`, parsed like the shared file's, for tags only this tracer sets (servlet's, Spring's, JMS's, ...). Since a shared span type can't `include` a tracer's mixin by name, an overlay mixin must push itself onto shared types with `applies`, and may not reuse a shared mixin's name. Its tags resolve onto those types like any other, so they get slots, and `intercepted` may list them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Register HTTP, DB, websocket and core internal tags Each tag goes on the span types the code sets it on (production set sites checked for every tag), so it gets a slot: - http: http.endpoint, http.fragment.string (server and client). - http.server: the http.forwarded.* headers and the security-testing request headers. - db.client: db.schema, db.warehouse, the Cassandra/Couchbase/Redis specifics, db.command.*, and the DBM propagation tags. - base: _dd.span_links and the debugger's _dd.code_origin.* (frame 0 only, the only frame produced). - trace_level: _dd.apm.enabled (process-static) and the local root span's _dd.trace_span_attribute_schema and _dd.profiling.ctx. Its comment now says it holds both kinds. - New span types: web.handler (framework controller spans), and websocket receive/send/close with their message and close tags; the _dd.dm.* inherited-sampling tags are a mixin on receive and close. - Java overlay mixins: servlet (servlet.context and servlet.path, moved out of the shared file), spring_webmvc (view.type on view.render), spring_webflux (handler.type, request.predicate on web.handler). The coverage test's never-registered list becomes a map with a reason per constant, gaining servlet.dispatch (an operation name), sql.query (unused public API) and the two names only the serializers write. Tags.DB_HOST was unreferenced, as constant and as literal, and goes. 43 more constants are registered (105 of 352); 221 remain. With #12882's coloring the 109 tags use 41 slots, the size of http.server's clique. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Register messaging, RPC and Data Streams tags New shared span types, one per span kind, each messaging system adding its tags with a mixin rather than a type of its own: - messaging: producer, consumer (record.queue_time_ms, record.e2e_duration_ms), broker (time in queue; no direction, which none of these tags needs), client (AMQP commands that neither publish nor consume), and poll (kafka.records_count). - Mixins: kafka (destination, bootstrap servers, cluster id, group, partition, offset, tombstone, Kafka Streams' processor.name) and amqp (command, exchange, routing key, queue, delivery mode, message.size). - rpc: client and server, with the HTTP types' peer mixins; rpc.service, rpc.grpc.status_code, and message.type on gRPC message spans. - Data Streams as mixins, not on base: pathway.hash on http, messaging and rpc spans; dsm.transaction.* on http and messaging producer and consumer spans. - internal: an internal span no other type models. The Java overlay's tibco and mule mixins apply to it. - bucket goes on db.client (Couchbase); the Java overlay's spring_webflux_client mixin puts message (why a request was cancelled) on http.client. jms.produce was never set as a tag (JMSDecorator's operation name has the same string), so it goes. message_bus.destination is unused in production but TagInterceptorTest splits by it as a custom tag, so it is listed as never registered. 29 more constants are registered; 190 remain, mostly CI Visibility and AWS. With #12882's coloring the 138 tags use 44 slots. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Register AWS SDK tags, with a span type per AWS service AWS SDK calls are HTTP client spans, so aws.client extends http.client and each modeled service extends it: aws.s3, aws.sqs, aws.sns, aws.kinesis, aws.dynamodb, aws.sfn and aws.lambda, each with its own tags (bucket and object, topic, stream, table, state machine and execution, function). aws.client itself covers the other services. A type per service, not a mixin per service: one aws.client holding every service's tags would be 67 tags, over the 63-slot cap, though a span never carries two services' tags. Mixins stay for what cuts across span families: aws_request (agent, service, operation, endpoint, request id) and aws_sqs_queue are also on the SQS consumer and broker spans, which are messaging spans. Also registered: the S3 Data Streams payload sizes (http.request/response_content_length), and the span pointer inputs (s3.eTag, dynamodb.primary_key_*), which SpanPointersProcessor removes before serialization. 32 more constants are registered; 158 remain, mostly CI Visibility. With #12882's coloring the 170 tags use 48 slots (aws.s3). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Let the tracer overlay declare span types Tags that extend one span concept belong on a span type; mixins are for concerns that cut across span types or kinds (Data Streams, a messaging system). The Java overlay could only declare mixins, so a Java-only extension of a shared type (servlet's request, say) had to be one. It can now declare `span_types`, parsed like the shared file's; each must extend a span type and may not reuse a shared type's name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Model Java-only extensions as span types, not mixins Mixins are for concerns that cut across span types or kinds. Each Java overlay mixin extended exactly one shared type, so each becomes a Java span type extending it: servlet.request (http.server), spring.view.render (view.render), spring.webflux.handler (web.handler), spring.webflux.client (http.client), tibco.process and mule.action (internal). The registered tags are unchanged. With #12882's coloring this saves a slot (47): servlet's and WebFlux's tags are now in their subtypes' cliques only, not in every server and client span's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Register code origin as a product mixin, without its frame names Code origin is the debugger's add-on, set on entry spans (and their local root) and @trace spans, not on every span. It moves from base to a code_origin mixin on http.server, rpc.server, messaging.consumer and internal, which also takes it out of every client, DB and AWS clique. Only _dd.code_origin.type is registered. The frame tags are _dd.code_origin.frames.<N>.<field>, a name pattern: an unbounded family can't have ids ahead of time, so it can't use the dense store, and the registry doesn't model patterns yet. Frame 0's six constants are listed as never registered, with that reason. (Only frame 0 is produced today.) With #12882's coloring the 164 tags use 40 slots, down from 47. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Register Elasticsearch, Twilio, JSP, Quartz and Hystrix tags; match patterns in the coverage test These integrations' tags were thought unused, but only their constants were: Elasticsearch, Twilio and JSP set them as string literals, and Quartz and Hystrix use the constants. Each becomes a Java span type (set sites checked per tag): - elasticsearch.rest and elasticsearch.transport (db.client): the transport client's request, response and shard tags; the REST client's body and params, and its http.method and http.url. - twilio.sdk (http.client), jsp.compile (internal), jsp.render (view.render), quartz.job and hystrix.command (internal). Six names have no constant and are registered anyway: elasticsearch.action, .request, .body, .params, .shard.replication.successful, and jsp.requestURL. Tags set only by literal are outside the coverage test's reach; APMLP-2046 proposes auditing them in the instrumentation tests. The coverage test now matches entries as patterns (`*`): - NEVER_REGISTERED: a pattern excludes all it matches (Tags.SPAN_KIND_*, Tags.LLMOBS_*_SPAN_KIND, the code-origin frames). - NOT_YET_REGISTERED_FAMILIES: a family, with a reason, excusing only its unregistered constants (Tags.TEST_*, Tags.GIT_*, Tags.CI_*, DDTags.CI_*, DDTags.TEST_*: CI Visibility, in a follow-up PR); stale once none of its constants is unregistered. - NOT_YET_REGISTERED: the remaining single constants, still a strict ratchet. 26 single constants remain, plus the CI Visibility families. With #12882's coloring the 203 tags use 55 slots (elasticsearch.transport). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Register the remaining APM tag constants Set sites checked per tag: - trace_level: process_id (a local root tag) and ai_guard.event (set on the local root of a trace holding an AI Guard evaluation span). - base: events, the span events as JSON when the writer can't encode them natively, alongside _dd.span_links. - exception_replay, a product mixin on every span: error.debug_info_captured. - rpc: status.code and status.description (gRPC, set by literal). - db.client: couchbase.operation_id (set by literal). - Java servlet.request: user.principal (servlet containers) and timeout (Servlet 3 async timeouts); spring.webflux.client: event. Never registered, each with its reason: thread.id and thread.name are span fields the serializers write, never stored as tags; runtime_version is a profiling and telemetry tag; t0 is a baggage key; dd.env, dd.service and dd.version are log correlation (MDC) keys. The CI Visibility constants among them (os.*, runtime.*, _dd.host.vcpu_count, itr_correlation_id, pr.number, and library_version, a span tag only on CI test traces) join the CI Visibility families. That leaves no single unregistered constant: what remains is CI Visibility, for a follow-up PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Split db.client into relational and per-system NoSQL types db.client carried system-specific tags in every DB span's clique. System-specific tags extend one concept, so they become types: - db.client keeps what any database client sets: type, instance, operation, user, statement, and a command's outcome (db.command.cancelled and .results.count, set by Redis and Memcached). - db.sql (JDBC, R2DBC): db.pool.name, db.schema, db.warehouse. - db.cassandra, db.couchbase (seed nodes, bucket, operation id), db.redis (db.redis.dbIndex), and db.mongo. - Database Monitoring is a product add-on, so its propagation tags become a dbm mixin on db.sql and db.mongo, where it injects comments. With #12882's coloring the 213 tags use 47 slots, down from 57 (elasticsearch.transport). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Merge master into tag-registry-coverage Brings #12817 (now merged, squashed as 62e9fe1). Its three conflicts are squash artifacts: master's versions of those files equal #12817's head, which this branch grew from, so this branch's versions stand. The diff against master is exactly this branch's own changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Put span types first in the Java overlay Span types are what instrumentation developers work with most, so they lead the file, ahead of the routing-only keys and the intercepted list. The header now describes both halves of the file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Add peer tags to messaging spans; apply code origin to handler spans Two review findings, both confirmed in the code: - RabbitChannelInstrumentation calls onPeerConnection on its client, producer and consumer decorators. messaging.producer and messaging.client now include peer, peer_address and outbound_peer (peer.service is computed for client and producer spans, as for http.client and rpc.client); messaging.consumer includes peer_address and inbound_peer. Consumers also set peer.hostname, which outbound_peer declares for client spans only: a TODO, as for websocket spans. - Code origin's entry capture instruments annotated controller methods, whose active span is the framework's handler span, so the code_origin mixin also applies to web.handler. Slots unchanged at 47 with #12882's coloring. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Let a tag be marked as sharing a dense-store slot `slot: shared` marks a tag as rarely set. When the coloring honors it (with #12882), a shared tag still gets a slot apart from every tag that isn't shared in its cliques, so tags that are always present keep exact presence bits, but shared tags may share slots with each other. Rare groups then pack into the slots the always-present tags leave free, instead of falling back to the unslotted bit 63 that every slotless tag shares. The generator parses the marker (the only value is `shared`), requires a name's declarations per direction to agree on it, records it on the registry tag, and shows it in each generated id's comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Register the CI Visibility tags on test span types CI Visibility creates one span per test session, module, suite and test (span.kind test_session_end, test_module_end, test_suite_end and test, none of which has a direction). Set sites checked per tag. New shared span types: - ci.span (abstract): what standard enrichment (TestDecoratorImpl.afterStart) puts on every CI span -- the session name, test type, status, framework, session id, and the CI provider and commit (ci.*, git.*) -- plus library_version and the host's vCPU count. - ci.aggregate (abstract, sessions and modules): command, coverage, ITR skipping counts, early flake detection, test management. - test.session, test.module, test.suite and test, sharing tags across levels by ref (test.module, test.suite, the ids, the source location). - test.browser (Selenium). - Java overlay: test.gradle.session, test.gradle.module, test.maven.module, test.android (Robolectric), test.junit5, and junit.callback for JUnit's before/after spans. A test span carries about 100 tags, more than the dense store's 63 slots. So tags set only some of the time (pull request and head commit info, retry, ITR, test management, configuration errors, coverage, browser and Android details, and the os/runtime fields the CI intake mapper normally writes itself) are marked `slot: shared`. With shared-aware coloring (to follow in #12882), a simulation colors the 309 tags into 56 slots, with the always-set tags on exact slots. The old ci_visibility mixin goes: its tags are on the types. The coverage test's CI Visibility families are now all registered, so no family or single constant remains unregistered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Drop the slot marker; sharing follows the requirement level Reverts db7a674. #12882 now shares slots by requirement level: on a span type with more tags than slots, its optional tags share slots with each other, so the conventions need no slot marker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Mark the CI tags standard enrichment always sets recommended Replaces the `slot: shared` markers with requirement levels. The tags TestDecoratorImpl.afterStart puts on every CI span (the CI provider and commit) and the test source range are recommended rather than optional, so the coloring keeps them on exact slots; the tags set only some of the time stay optional and absorb the overflow on test spans. With #12882's coloring, the 306 tags fit in 63 slots; only the CI test span types, which carry 73 to 104 tags, have tags sharing a slot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Merge branch 'master' into dougqh/tag-registry-coverage Merge master (#12882 slot coloring) into tag-registry-coverage Clean merge. With the merged coloring, this branch's 212 registered tags color into 47 slots (SLOT_COUNT 47, cap 63), and no tag shares a slot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Merge tag-registry-coverage (master's #12882 slot coloring) into tag-registry-civis Clean merge; the generator now matches #12893's, with no trace of the reverted slot marker. With the merged coloring the 319 tags color into 63 slots: the CI test span types overflow, and 66 optional tags share slots (CI Visibility's occasional tags plus _dd.span_links, _dd.svc_src, error.debug_info_captured and events), on those span types only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Merge master (#12731, #12893) into dougqh/tag-registry-civis Master's #12893 is this branch's base, squashed, so this branch's versions of its three files stand; tag-conventions.yaml also takes #12731's later server.port tag and direction-resolution notes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
What Does This Do
Registers the APM tag constants in the tag registry, each on the span types it's actually set on, so instrumentation can migrate to setting tags by
KnownTags.*_IDand every tag gets a dense-store slot. A ratchet test tracks what's left.TagConstantsRegistryCoverageTestrequires everyStringconstant inTags,InstrumentationTagsandDDTagsto be registered, or listed by name or pattern (*):NEVER_REGISTEREDmaps each deliberate exception to its reason: values of a tag (Tags.SPAN_KIND_*), a name prefix, a name pattern, an operation name, names only the serializers write, and unused public API.NOT_YET_REGISTERED_FAMILIESnames whole families with a reason, today the CI Visibility constants (Tags.TEST_*,Tags.GIT_*,Tags.CI_*, ...). A family excuses only its unregistered constants, and is stale once none is left.NOT_YET_REGISTEREDlists single constants and can only shrink: registering one fails the test until its entry is removed. A new constant must be registered or listed.span_typesandmixins. Java-only tags (servlet's, Spring's, TIBCO's, Mule's) live intag-conventions-java.yaml, keeping the shared file language-agnostic. An overlay span type must extend a span type; an overlay mixin must say what itappliesto.servlet.pathandservlet.contextmove out of the shared file into a Javaservlet.requesttype.New span types
messaging.producer,.consumer,.broker(time in queue),.client(AMQP commands),.poll, withkafkaandamqpmixins.rpc.client,rpc.server.db.clientkeeps what any database client sets;db.sql(JDBC, R2DBC),db.cassandra,db.couchbase,db.redisanddb.mongoextend it. Database Monitoring's propagation tags are adbmmixin ondb.sqlanddb.mongo.aws.client(anhttp.client) and one type per service:aws.s3,.sqs,.sns,.kinesis,.dynamodb,.sfn,.lambda. Oneaws.clientholding every service's tags would be 67 tags, over the 63-slot cap, though a span never carries two services' tags.websocket.receive,.send,.close.web.handler(framework controller spans),internal(internal spans no other type models).servlet.request,spring.view.render,spring.webflux.handler,spring.webflux.client,tibco.process,mule.action,elasticsearch.rest,elasticsearch.transport,twilio.sdk,jsp.compile,jsp.render,quartz.job,hystrix.command.Motivation
The registry had 66 tags against about 350 tag constants. The migration to ids needs every tag registered, and the dense store gives a tag a precise slot only when it's on the span types it appears on.
Additional Notes
Tags.DB_HOSTandInstrumentationTags.JMS_PRODUCE(whose string isJMSDecorator's operation name).jsp.requestURL,elasticsearch.action, ...): tags set only by literal are outside this test's reach, and APMLP-2046 proposes auditing them in the instrumentation tests.elasticsearch.transport, which adds 21 tags todb.client.trace_levelholds both process-static facts (the.enabledflags, now including_dd.apm.enabled) and values common to a trace (local-root tags such as the attribute schema version)._dd.code_origin.frames.<N>.<field>has unboundedly many names, so it can't have ids ahead of time and can't use the dense store; its members stay custom tags. Frame 0's constants are listed as never registered for that reason, and only_dd.code_origin.typeis registered. A general registry could later describe patterns (name, type, OpenTelemetry naming) without ids.peer.hostnamefrom the handshake, butoutbound_peerdeclares it for client spans only; left as a TODO in the YAML.🤖 Generated with Claude Code