Repository navigation
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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tag-registry generator now assigns each known tag a slot in id bits [47-32] by graph coloring: each concrete span type's resolved tags, and the trace-level tags, form a clique and get distinct slots. So 1L << slotOf(id) is an exact presence bit for a span's tags, and one long holds them all. Tags on no modeled span type get NO_SLOT, which a long shift maps onto bit 63; real slots are capped below it. KnownTagCodec gains SLOT_SHIFT, NO_SLOT, slotOf, hasSlot and slotCount. Nothing in the tracer reads slots yet: this is split out of the dense store (#12856) so the store, the span builder's tag ledger and a map-level routing summary can each build on it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ledger stays an ordered log of sets and removals, but no longer stores an Entry or EntryRemoval per change: a known tag is recorded under its id, a custom tag under its customHash and name, and the value (or a REMOVED marker) beside them. Recording allocates nothing once the arrays are sized, and a reused builder ledger allocates nothing at all. - Consumers read changes through EntryChangeReader, an EntryReader plus isRemoval(), via iterator() or an allocation-free forEach(thisObj, consumer) that reuses one reader. - set(long, ...), remove(long) and smartRemove(long) take ids; name-keyed calls resolve the id once. - occupancy ORs 1L << slotOf(id) per recorded known tag (bit 63 for custom tags), so smartRemove of a tag never recorded is a one-bit miss. - idFlags ORs recorded ids' flag bits. setAllTags(Ledger) skips per-change interception when no recorded id is intercepted and no split-by-tags entry is configured (TagInterceptor.hasSplitServiceTags), and stores known tags through Internals.setKnown by id. - SpanBuilder gains withTag(long, Object); unknown ids are ignored. Values are kept as objects. Every production caller already passes objects (withTag boxes; propagation passes strings), so no new boxing on those paths; the ledger's primitive set overloads now box. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Builder ledgers record mostly known tags, which need no name: their name is nameOf(id). The names array is now allocated on the first custom tag, saving 48 bytes per fresh ledger of known tags. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Contributor
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
dougqh
force-pushed
the
dougqh/tag-slot-coloring
branch
from
October 15, 2026 19:28
1a63fa0 to
06bac77
Compare
#12882 was rebased onto master after #12817 merged, and gained two commits (a custom tag's zero id is unslotted; a slot bit is only a may-be-present signal). This merge's tree is the rebased #12882 plus exactly this branch's own changes (0faed2d..8610d72), which apply to it without conflicts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Does This Do
Rebuilds
TagMap.Ledger, the ordered log of span-builder and propagation tag changes, on parallel arrays keyed by tag id, and addsSpanBuilder.withTag(long, Object).Meant to land after the dense store (#12856). With the flag off and a fresh builder per span, it allocates more than today (numbers below); the win comes from the dense store, where a known tag goes from the ledger into the span with no
Entryat all.EntryorEntryRemovalper change. A known tag is recorded under its id, a custom tag under itscustomHashand name, and the value (or aREMOVEDmarker) beside them. The names array is allocated only on the first custom tag.EntryChangeReader(anEntryReaderplusisRemoval()) is how consumers read changes, throughiterator()or an allocation-freeforEach(thisObj, consumer)that reuses one reader.setAllTags(Ledger),build()andfill()use it.Entry/EntryChange/EntryRemovalthemselves are unchanged.Ledger.set(long, ...),remove(long),smartRemove(long), andSpanBuilder.withTag(long, Object)(unknown ids are ignored, like the span's id setters). Name-keyed calls resolve the id once.smartRemoveuses the slot coloring (Color known tags into co-occurrence slots in their ids (phase 3a - groundwork - dense tag storage) #12882). The ledger ORs1L << slotOf(id)per recorded known tag, with bit 63 for custom tags. Removing a tag the ledger never recorded, the usualwithTag(x, null), is a one-bit miss instead of canonicalizing the name and scanning.setAllTags(Ledger)skips per-change interception when no recorded id is intercepted and no split-by-tags entry is configured (TagInterceptor.hasSplitServiceTags(), the usual case), and stores known tags by id throughInternals.setKnown.Measurements
SpanCreationBenchmark(@Threads(8),-f 2 -wi 3 -i 5, 2 s iterations,-prof gc, Zulu 17), dense store off, against #12882:webServerSpanViaBuilder(7 builder tags by name)webServerSpanViaBuilderById(same, by id; new arm)TagMapAccessBenchmark.insert_via_ledger(18 tags, one-shot ledger →build())Throughput differences are within this run's noise; read B/op. The builder path is +88 B per span, the one-shot ledger +768 B.
Why it costs more today:
buildSpan()creates a new builder per span (only the opt-inReusableSingleSpanBuilderreuses its ledger), so the ledger's arrays are allocated per span: along[8]and anObject[8]where the old ledger had oneEntryChange[8].Entrydoesn't go away, it moves: the old ledger built it atwithTagtime and stored that object; nowsetKnownbuilds it at apply time.setKnownbuilds noEntry, so about 7 entries per span disappear. That's where this pays off; I'll measure it once it's merged with Store known span tags densely, behind an experimental flag (phase 3b - dense tag storage) #12856.Additional Notes
withTagboxesNumber/boolean; propagation passes strings). The ledger's primitivesetoverloads now box; nothing in production calls them. If a hot primitive caller appears, lazily allocated prim/type arrays can mirrorEntry's primitive slot.forEachcaches oneLedgerReaderper ledger (about 24 B per fresh ledger). MakingLedgerimplementEntryChangeReaderwith an internal cursor would save that, at the cost of reader methods onLedger's public API that are only valid duringforEach. I left it out.testTypesnow checks resolved types. The ledger keeps values, not typed entries, so it asserts what a reader sees (type(), viaTagValueConversions/Entry's ANY resolution) rather than anEntry's raw storage type.🤖 Generated with Claude Code