Skip to content

Split TagMap into id and name stores, owned directly by the span - #12918

Draft
dougqh wants to merge 8 commits into
dougqh/tag-registry-direction-resolutionfrom
dougqh/tagmap-split-stores
Draft

dougqh wants to merge 8 commits into
dougqh/tag-registry-direction-resolutionfrom
dougqh/tagmap-split-stores

Conversation

@dougqh

@dougqh dougqh commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

Splits tag storage into strict id and name stores, and has the span own them directly. TagMap becomes a thin view for everyone else. No behavior change.

The idea: classify a tag name once, at the boundary (known tag → id, custom tag → name, direction-scoped alias → the span's call), then keep the two kinds apart inside.

Layers

Layer Responsibility
KnownTagCodec Facts: ids, names, aliases, direction-dependence
HashStore The name-keyed hash. Through the 64-bit tag-hash scheme it also holds known tags by id, so in hash mode it plays both roles: one object, no extra allocation
IdStore Known tags by id. The hash-backed adapter (HashIdStore) now; a dense store slots in here
TagRouting The one place a name is classified and routed to a store. An alias is returned to the caller to decide
DDSpanContext Owns its HashStore (also its one lock) and, outside hash mode, an IdStore. All setting logic lives here, with DirectionalTags and TagInterceptor
TagMap A thin, final view over a store pair. Standalone maps (tracer tags, DD_TAGS, prototypes, baggage) have their own stores; a span's view is direction-aware

Notable changes

  • TagMap.Internals is gone. The strict store API is the direct path, so core no longer needs a trusted side door around TagMap. Its tests move to TagStoresTest.
  • HASH_MODE is a compile-time constant (currently true). Each id operation is HASH_MODE ? hash.x(id) : ids.x(id), so hash mode has no adapter object and no extra load.
  • Internal readers (post-processors, serialization) get a cached, live, direction-aware TagMap view over the span's stores. getTags() is still a snapshot copy.
  • Read-through is per store. Known and custom tags never share a key, so precedence never crosses stores. Semantics are unchanged.
  • Entry sharing is preserved. Copies share Entry objects and clone only BucketGroup chains, as before.

Motivation

TagMap had accreted name canonicalization, known-versus-custom classification and, with #12731, direction awareness. That forced core to reach around it through Internals, and every write route repeated the known-versus-custom and direction logic. Most of #12731's review findings were one write route missing a rule another route had.

This puts policy back in the span layer, keeps storage simple and fast, and gives a dense store a clean place to replace the known-tag half. That matters for the biggest remaining allocation sites: point reads building an Entry per read, primitive metrics stored as boxed Entrys, and the dense store's id arrays.

Additional Notes

Performance (JDK 17, -f 2, -prof gc; base is #12731's tip 6aa6d4f1e2):

Benchmark Base This PR
SetTag.knownTag_byId 13.48 ns, 48 B 13.44 ns, 48 B
SetTag.knownTag_byName 13.30 ns, 48 B 13.46 ns, 48 B
SetTag.customTag_byName 13.42 ns, 48 B 13.40 ns, 48 B
GetTag.knownTag_unsafeGetTag 5.21 ns, 0 B 5.19 ns, 0 B
GetTag.knownTag_getTag 9.15 ns, 0 B 9.12 ns, 0 B
SpanCreation.webServerSpan 1.343 ops/µs, 1912 B 1.315 ops/µs, 1952 B
SpanCreation.bareStartSpan (4 forks) 992 B 1032 B

Throughput is within noise. With the adapter instead of the folded HASH_MODE path, unsafeGetTag was +6% and bare id writes +4%.

Known cost: +40 B per span. The span's view, handed to the tag post-processor chain, escapes there and isn't scalar-replaced, so the view is cached per span (32 B plus a field). This is a stopgap: the post-processors are due for rework, and once they read the span's stores directly, the view and the field go away. Without the cache it was +32 B on the app thread and another +32 B at serialization.

Semantic note: name-keyed reads through a span's view resolve aliases by the span's direction, where today's raw unsafeTags reads don't. No current post-processor reads an alias name.

Draft: stacked on #12731's branch, so this diff shows only the split. A follow-up commit moves pending aliases into a small lazily created holder: the custom store then holds only custom tags, and the pending-alias check becomes a null check that resets after resolution.

🤖 Generated with Claude Code

dougqh and others added 7 commits October 16, 2026 08:37
TagMap now holds its tags in two internal stores: an IdStore for known
tags by id and a name store for custom tags by name. In hash mode, the
only mode so far, one HashStore serves as both, so the split adds no
object per tag. The bucket structure, read-through to frozen parents and
removal tombstones move into HashStore unchanged; TagMap delegates to it.

The public API and behavior are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TagRouting resolves a tag name once, by KnownTagCodec.keyOrAliasOf, and
routes the operation to the store that holds it: a known tag to the id
store by its id, anything else to the name store by its name. TagMap's
name-keyed getters, setters, removals and entry factories, and its
resolved variants, all go through it, so name-to-id resolution lives in
one place. A direction-scoped alias is still stored as a custom tag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DDSpanContext now owns a HashStore (and, outside hash mode, an IdStore)
instead of a TagMap, created when the span is constructed and read
through to the frozen tracer-level tags. Every tag write and read goes
through TagRouting under one lock, the HashStore itself. Direction,
alias, interception and pending-alias handling stay in DDSpanContext and
DirectionalTags, now applied to the stores.

HashStore is no longer an IdStore: it is the name-keyed hash, which also
holds known tags by id, and exposes an IdStore adapter over them. The
static HASH_MODE flag lets hash mode call the hash directly, so a span
needs no adapter object.

Internal readers (tag post-processors, the serializer) get a short-lived
TagMap view over the span's stores whose name-keyed reads honor the
span's direction; getTags() is still a snapshot copy. TagMap.Internals
is gone: the store API is the direct path. Its tests move to
TagStoresTest.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TagRouting.putAll built a TagMap view to reuse TagMap.putAll, but putAll
is too big to inline, so the view escaped and cost an allocation on the
span's tag-seeding path. In hash mode it now copies into the HashStore
directly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The view handed to the post-processor chain escapes, so building one per
use cost an allocation at trace write and another at serialization. The
span now creates one lazily, under its tag lock, and reuses it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dougqh dougqh added comp: core Tracer core tag: no release notes Changes to exclude from release notes type: refactoring tag: ai generated Largely based on code generated by an AI or LLM labels Oct 16, 2026
A direction-scoped alias written before the span has a kind is no
longer stored in the span's HashStore. It goes into a small holder that
DirectionalTags owns, which stays null until the first such write, so
the HashStore holds only custom tags. Kind-set resolution walks only the
holder, and once every alias has resolved the holder is dropped, so the
span returns to the fast path rather than staying flagged for life.

The id setters' routing gate now tests the holder for null instead of
reading a sticky flag. Reads of a tag with an alias probe the holder
only when there is one. Last-write-wins and the removal rules are
unchanged. Emitting the span's tags (trace-write and serialization
processing, and the getTags snapshot) copies the holder's entries in
under their literal names, so a span that never learns its kind still
emits them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

🎯 Code Coverage (details)
• Patch Coverage: 82.70%
• Overall Coverage: 59.95% (+0.10%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 2d0ef38 | Docs | Give us feedback!

@dd-octo-sts

dd-octo-sts Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

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

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 13.99 s 13.93 s [-0.2%; +1.0%] (no difference)
startup:insecure-bank:tracing:Agent 12.92 s 12.88 s [-0.5%; +1.1%] (no difference)
startup:petclinic:appsec:Agent 17.69 s 17.58 s [-0.2%; +1.5%] (no difference)
startup:petclinic:iast:Agent 17.53 s 17.61 s [-1.4%; +0.5%] (no difference)
startup:petclinic:profiling:Agent 17.25 s 17.21 s [-0.7%; +1.2%] (no difference)
startup:petclinic:sca:Agent 17.73 s 17.66 s [-0.6%; +1.5%] (no difference)
startup:petclinic:tracing:Agent 16.56 s 16.65 s [-1.5%; +0.4%] (no difference)

Commit: 2d0ef382 · CI Pipeline · Benchmarking Platform UI


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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant