Repository navigation
Resolve direction-scoped OpenTelemetry names at the OpenTelemetry boundary (otlp - tag registry - phase 3) - #12731
Conversation
🟢 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. |
f681c6d to
6000c50
Compare
Builds on the registry directions (#12713) and id-keyed setters (#12715, included here): KnownTagCodec resolves names by direction, TagMap finds a shared name's tag whichever direction it holds, and DDSpanContext resolves direction-dependent names by the span's kind. Squashed from the review history of #12731. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bfc66c5 to
653c907
Compare
Builds on the registry directions (#12713) and id-keyed setters (#12715, included here): KnownTagCodec resolves names by direction, TagMap finds a shared name's tag whichever direction it holds, and DDSpanContext resolves direction-dependent names by the span's kind. Squashed from the review history of #12731. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6000c50 to
4aa5e81
Compare
653c907 to
5d44b60
Compare
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>
Builds on the registry directions (#12713) and id-keyed setters (#12715, included here): KnownTagCodec resolves names by direction, TagMap finds a shared name's tag whichever direction it holds, and DDSpanContext resolves direction-dependent names by the span's kind. Squashed from the review history of #12731. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4aa5e81 to
a17c830
Compare
5d44b60 to
885bdb5
Compare
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>
There was a problem hiding this comment.
Deferred OpenTelemetry attributes can overwrite newer values, and changing a builder’s kind leaves earlier attributes mapped to the wrong direction. Null attribute keys also throw instead of being ignored.
🤖 Bits Code Review · Commit d7f0ffc
#12731 now resolves direction-scoped OpenTelemetry names in the OpenTelemetry shim, so the core keeps no direction state. Drop the split stores' copy of that machinery along with it: DirectionalTags and the span's pending-alias holder, TagRouting's direction-aware reads, the view's direction fields (attach takes just the custom store), the alias checks in the id setters, and TagMap's pre-resolved setters. A name resolves to an id or a custom tag, nothing else. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Prose only. Brings comments in line with the code after the #12731 merge and the dense port: no direction-scoped aliases or lookup sentinels in the core, put/getAndPut take an EntryReader, only the hash id store keeps Entry tombstones, the dense store keeps no Entry objects, and the root-span tags' floor precedence covers header tags and prototype defaults too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ution Clean textual merge. Also formats TagRegistryGeneratorTest's holder helper, which build-logic's spotless flagged (the root spotlessApply skips build-logic). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ygree
left a comment
There was a problem hiding this comment.
IDM approved assuming that codex findings will be addressed
Patch coverage was 75%, mostly DenseIdStore: the dense store is behind a flag that is off by default, so the shared TagMap suites only reached the hash store and the dense store ran in one small forked test. - IdStoreContractTest runs the IdStore contract against HashIdStore and DenseIdStore in the normal test JVM: typed setters and readers, put and getAndPut priors, removing and reviving inherited tags, size and emptiness through parents, every iteration form, copy, putAllLocal (including colliding buckets), the map fills, growth, multi-level reads and id validation. - DDSpanContextStoresTest covers the span's routing: custom tags by name with each value type, primitive sets by id (intercepted, declined and plain), setMetric with known names, map writes, and the custom store borrowed from the tracer-level tags until the span's first custom write. - The by-name setters' custom branches write straight to the custom store: they already know keyOf(tag) is 0, so going through routeSet repeated the lookup. routeSet's boolean overloads lost their last callers and are removed. Local JaCoCo patch coverage (vs #12731): 75% -> 98%. The rest is mostly checkIntegrity's error branches. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An attribute set before the kind was replayed at start, over any later write to the same attribute. Resolving the held attributes when the kind is set keeps them in write order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A null key reached the generated directional switch and threw into application code. Resolve it as no known tag, so the span builder drops it as it did before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…12893) 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> 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> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for cc88ac9: 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>
What Does This Do
Phase 3 of the tag registry: OpenTelemetry names whose tag depends on the span's direction.
server.addressishttp.hostnameon a server span andpeer.hostnameon a client span;server.portispeer.porton a client span.The rule: in Datadog terms a tag's name never depends on direction, so the tracer core keeps resolving names without one, exactly as on master. Direction is applied only on the OpenTelemetry side, where it is known:
KnownTags.Outbound.SERVER_ADDRESS_IDisPEER_HOSTNAME_ID,KnownTags.Inbound.SERVER_ADDRESS_IDisHTTP_HOSTNAME_ID. No lookup, no branches on the semantics mode for naming.setSpanKindand resolves it as soon as the kind is set (or at start, where the kind defaults to internal), so writes stay in order: a later write to the same attribute wins. A null attribute key is ignored, as before.Registry changes
server.portis declared onhttp.serveras the server's own port. In every direction no alias claims, it is its own tag; on outbound spans the OpenTelemetry side maps it topeer.port.keyOf("server.port")is theserver.porttag.KnownTags.Inbound/Outbound/Internal, and adirectionalKeyOf(name, direction)switch behindKnownTagCodec.directionalKeyOf.What it deliberately doesn't do
span.kindstill reflects the final kind.peer.porton a client span) is exported as written, not reconciled. A test documents this.An earlier revision of this PR resolved these names inside the span (pending aliases folded in when the kind arrives, last-write-wins between a tag and its alias, an id bit and routing for it). That is replaced here: it cost ~1,000 lines in the core to serve user code only.
Motivation
#12765 (OpenTelemetry HTTP semantics) is the main consumer. With per-direction aliases, its decorators write e.g.
Outbound.SERVER_ADDRESS_ID/Inbound.SERVER_PORT_IDinstead of branching on the semantics mode, and it no longer needs an inventedhttp.porttag for the inbound server port.Additional Notes
KnownTagsTest(direction resolution table, shared names, and a reflection check that every per-direction alias agrees with the codec's naming);OpenTelemetry14DirectionalAttributesTest(client/server/producer/internal, attribute before kind, after kind, after start, a write after the kind winning over one held before it, null attribute keys across everysetAttributeoverload,server.port); OTLP export ofserver.portper direction.build-logic:tag-registry:test,:internal-api:test,:dd-java-agent:instrumentation:opentelemetry:opentelemetry-1.4:test, spotless and spotbugs pass.:dd-trace-core:testfailures are environmental or pre-existing flakes:TracerConnectionReliabilityTest(needs Docker),PendingTraceBufferTest,DDAgentWriterCombinedTest.unixSocketTimeoutKeepsWorkerAliveAndReconnects.Contributor Checklist
build-logic:tag-registry:test:internal-api:test:dd-trace-core:test(see notes):dd-java-agent:instrumentation:opentelemetry:opentelemetry-1.4:testspotlessCheck,spotbugsMain🤖 Generated with Claude Code