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

[dougqh]

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. keyOf classifies a name once
IdStore	Known tags by id. IdStoreFactory picks one implementation per JVM from trace.experimental.dense.tags.enabled (off by default): HashIdStore (flag off) or DenseIdStore (flag on: insertion-ordered parallel arrays with one occupancy word per level, no per-tag Entry, point reads answer with a reused flyweight)
HashStore	Custom tags by name, created lazily on the first custom write; until then the owner borrows its parent's custom store read-only
DDSpanContext	Owns its IdStore (also its lock) and its HashStore. Routes each write to a store (routeSet), and holds all setting logic, with DirectionalTags and TagInterceptor
TagMap	A thin, final view over a store pair (knownStore, nameStore). 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.
• One IdStore implementation per JVM. The factory chooses once, so only one implementation loads and class hierarchy analysis devirtualizes every IdStore call. The dense store replaces Store known span tags densely, behind an experimental flag (phase 3b - dense tag storage) #12856, which is closed; per-operation sizing and bulk prototype seeding follow in Presize dense tag stores and seed span prototypes in bulk #12926.
• 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 the dense store a clean place as 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. These numbers were measured before the store factory replaced the earlier compile-time HASH_MODE constant, and they cover only the flag-off store; a re-run on the current tip, flag off and on, is pending.

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.

Follow-up: a later 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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-18T21:12:45.445926Z	0f1e5ac	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f1e5acd2a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Return stable readers from the public iterator

When DD_TRACE_EXPERIMENTAL_DENSE_TAGS_ENABLED is enabled, TagMap.iterator() exposes this cursor directly, and every advance returns the same mutable DenseCursor. Consumers that retain iterator results—such as collecting map.stream() into a list—therefore receive multiple aliases that all report the last visited tag and value, unlike the stable entries returned in hash mode. Materialize a stable reader or Entry at the public iterator boundary.

Useful? React with 👍 / 👎.

[dougqh]

👍 Good point, the reuse is problematic. I'll address that.

[chatgpt-codex-connector]

Preserve null mappings in the dense store

When dense tags are enabled, writing a known key with a null value through the Map API—for example TagMap.put(name, null) or TagMap.fromMap(...)—stores a slot and increments the map size, but this method then treats that stored value as if the key were absent. The resulting map can report size() == 1 while containsKey(name) is false, and removal returns null despite deleting the slot; hash mode previously retained the null mapping. Use a distinct null-value sentinel or reject null before mutating the store.

Useful? React with 👍 / 👎.

[datadog-datadog-prod-us1]

Two critical correctness gaps exist in the dense-storage TagMap implementation: concurrent reads on a frozen map share a mutable reader, allowing tag values to bleed across threads; and stored null values are indistinguishable from absent entries, breaking containsKey, getOrDefault, and removal semantics relative to hash mode. A third issue affects sequential traversal: the iterator exposes a single reused cursor, so buffering or collecting the stream loses all but the final entry.

_{Open Bits AI session}

_{🤖 Bits Code Review · Commit 0f1e5ac}

[datadog-datadog-prod-us1]

Keep public traversal entries stable in dense mode

With dense storage enabled, TagMap.iterator() exposes the same mutable cursor for every known tag. Collecting a two-tag stream into a list therefore retains two references to the final tag, and buffering operations such as sorted() lose earlier entries. Materialize stable entries at public traversal boundaries, including the iterator and related consumer paths, while keeping flyweights internal.

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means

• 👍 Useful: correct, and you're fixing it or will.
• 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
• 😕 Unclear: you can't tell what it means or why it matters.
• 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

_{🤖 Bits Code Review · Open Bits AI session}

[datadog-datadog-prod-us1]

Distinguish null-valued mappings from absent tags

With dense storage enabled, TagMap.put("http.route", null) stores and counts the mapping, but readerOf() treats its null value as absence. The map consequently reports size 1 and exposes the entry while containsKey("http.route") returns false, unlike the base and hash implementations. Preserve presence separately from the value throughout lookup, replacement, and removal handling.

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means

• 👍 Useful: correct, and you're fixing it or will.
• 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
• 😕 Unclear: you can't tell what it means or why it matters.
• 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

_{🤖 Bits Code Review · Open Bits AI session}

[datadog-datadog-prod-us1]

The two most critical issues are: (1) in OtelSpanBuilder, directional attributes are not consistently deferred and re-resolved against the final span kind, so interleaved setAttribute/setSpanKind calls produce stale or wrong attribute mappings at startSpan(); (2) in DenseIdStore, concurrent reads of a frozen TagMap mutate a shared DenseReader flyweight, creating a race where getObject and typed getters can return another tag's value despite no map writes occurring.

_{Open Bits AI session}

_{🤖 Bits Code Review · Commit ac96cd7}

[datadog-datadog-prod-us1]

Resolve directional attributes against the final span kind

Calling setSpanKind(CLIENT), setAttribute("server.address", "example.com"), then setSpanKind(SERVER) leaves peer.hostname on the server span instead of http.hostname. Updating direction does not remap attributes already sent to the delegate. Retain enough original attribute state to resolve against the final kind while preserving write order; changing this assignment alone cannot repair existing writes.

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means

• 👍 Useful: correct, and you're fixing it or will.
• 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
• 😕 Unclear: you can't tell what it means or why it matters.
• 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

_{🤖 Bits Code Review · Open Bits AI session}

[datadog-datadog-prod-us1]

Preserve concurrent reads of frozen tag maps

With dense tags enabled, concurrent reads of the same frozen TagMap mutate this shared reader. Another lookup can replace its ID or value before TagMap.getEntry() materializes the result, causing getObject() to return another key’s value despite no map writes. The child reproduced this race. Each lookup needs an independent result.

Suggested change

	DenseReader reader = this.reader;
	if (reader == null) {
	reader = new DenseReader();
	this.reader = reader;
	}
	DenseReader reader = new DenseReader();

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means

• 👍 Useful: correct, and you're fixing it or will.
• 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
• 😕 Unclear: you can't tell what it means or why it matters.
• 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

_{🤖 Bits Code Review · Open Bits AI session}