Skip to content

Make TagMap a facade over an id store and a name store (dense store - part 6) - #13007

Draft
dougqh wants to merge 56 commits into
dougqh/tagmap-store-classesfrom
dougqh/tagmap-store-facade
Draft

dougqh wants to merge 56 commits into
dougqh/tagmap-store-classesfrom
dougqh/tagmap-store-facade

Conversation

@dougqh

@dougqh dougqh commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

What Does This Do

TagMap keeps known tags in an IdStore (by id) and custom tags in a HashStore (by name), instead of one bucket array.

  • A name is classified once, at the boundary, and routed to its store. Reads by id go straight to the id store.
  • Read-through works per store. Known and custom tags never share a key, so precedence never crosses stores.
  • The name store is created lazily, on the first custom write. Until then, a map borrows its read-through parent's store read-only.
  • stream() and spliterator() yield stable Entrys, and getEntryReader gives a momentary reader for typed reads.
  • TagMap.Internals stays for now, writing straight to the id store. It goes away in the next PR, when the span owns its stores.
  • Store access is restricted to core. The forbidden-API filter blocks instrumentation from using the stores and TagMap's new view methods.

Motivation

This is the second of three PRs that replace the span's single TagMap with two stores split by tag kind. TagMap had picked up name canonicalization, known-versus-custom classification and direction awareness. Moving storage into the stores leaves TagMap a thin facade, and lets the span use the same stores directly in the next PR.

Additional Notes

  • No behavior change. The TagMap suites pass as they are, including the Internals tests, which now exercise the shim. Entry objects are still shared across copies.
  • TagStoresTest covers the stores through TagMap, and IdStoreContractTest gains the view cases.
  • Tests: internal-api and dd-trace-core pass. The only failures are the two TracerConnectionReliabilityTest cases, which need Docker.

Where this is going

This PR is one step in replacing the span's single TagMap with storage that knows each tag's kind. The goal is to cut what every span allocates on the application thread, without changing behavior.

The end state

Later, not in this stack: cheaper read-through to parent spans, trace-level tags in a per-trace holder (TraceSegment), and primitive tag values without boxing.

The stack. This replaces #12918, split into pieces that can each be reviewed, and measured, on their own.

PR Step Allocation per 7-span trace (flag off unless noted)
#12999 A null tag value removes the tag in TagMap. This is the stack's only behavior change, kept separate on purpose. unchanged
#12992 Bucket known tags by slot in TagMap, and add TraceShapeBenchmark −96 B vs master
#13001 TagStore for the eager post-processors unchanged
#13002 Extract TagMap's bucket mechanics into HashSupport (a pure move) unchanged
#13006 Add the id and name tag stores, not yet used unchanged
#13007 Make TagMap a facade over the two stores unchanged
#13003 Have the span own its tag stores directly +104 B (+1.3%)
#13004 Dense id store behind trace.experimental.dense.tags.enabled (off by default) flag on: −30.6%
#12926 Turn the dense store on by default, add sizing hints and bulk prototype seeding flag on: about 3% less again

Every PR after #12999 is behavior-preserving.

The pieces' allocation figures come from a deterministic per-thread probe (ThreadMXBean allocated bytes) that builds the same 7-span trace as TraceShapeBenchmark. The #12992 and #12926 figures come from TraceShapeBenchmark itself.

🤖 Generated with Claude Code

dougqh and others added 7 commits October 22, 2026 09:31
TagMap bucketed a known tag by folding its serial, so which tags shared
one of the 16 buckets was luck, and each shared bucket costs a
BucketGroup (48 B). Slot coloring already gives tags that appear
together on a span type distinct slots, so bucketing a known tag by its
slot keeps them in distinct buckets more often. Trace-level tags reuse
span-level slot numbers, so the level bit moves them to the other half;
known tags without a slot and custom tags keep their hashes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SpanScenarioBenchmark builds spans and traces the way today's decorators
do (prototype in afterStart, then request or connection tags by name):
bare spans, a server span, the server span via the builder, a jdbc span,
and a server trace with 1, 5 or 20 jdbc children. TraceShapeBenchmark
builds one web-request-shaped trace three ways: every tag by name, the
decorators' calls, and the newest span APIs.

Both finish what they start, so allocation per op covers whole spans and
traces. Single-span results can hide per-child costs that only the trace
scenarios show.

TraceShapeBenchmark is the same file #12918 adds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TraceShapeBenchmark takes the number of jdbc children (1, 5, 20), so
each arm shows how a trace's cost grows with its span count: single-span
and small-trace results can hide costs that land on every child span.
It covers what SpanScenarioBenchmark's trace scenario added, and the
rest of that benchmark overlapped SpanCreationBenchmark (same scenario
names, different semantics), so SpanScenarioBenchmark goes.

TagMapBucketHashTest pins down the bucket hash: known tags get distinct,
nonzero hashes, a slotted tag lands in its slot's bucket (trace-level in
the other half), and tags without a slot and custom tags keep their
hashes. The inline comment now says the level bit flips bit 3.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HeaderPathBenchmark runs one request per op through the propagation
paths: extract the inbound headers with the default propagator, start
the server span from what was extracted, run a few children, inject
into an outbound call, finish everything. Inbound headers vary: none,
Datadog, W3C (with dd, OpenTelemetry and third-party tracestate
members), both, and both plus baggage.

The existing propagation benchmarks each measure one step, so they
can't see costs the extracted state causes later on every span; this
one can. A setup check fails the run if nothing is extracted or
injected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
setTag(tag, null) already removes a tag from a span, but TagMap's own
setters stored a null-valued entry. Internal writers that go straight
to a TagMap could therefore leave null tags on a span, and the
serializer sent them as a msgpack nil under meta.

TagMap.put / set / getAndSet (by name or id, and putAll / fromMap
through set) now remove the tag on a null value and return its prior
value. A null Entry or reader is still a no-op.

The one writer that relied on null tags was payload tagging: a JSON null
in a request or response body produced a tag with a null value. It now
produces no tag, so such a value also no longer counts toward the
max-tags limit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kets

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

This comment has been minimized.

@dougqh dougqh changed the title Make TagMap a facade over an id store and a name store Make TagMap a facade over an id store and a name store (dense store - part 6) Oct 23, 2026
@dd-octo-sts

dd-octo-sts Bot commented Oct 23, 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.98 s 13.91 s [-0.3%; +1.3%] (no difference)
startup:insecure-bank:tracing:Agent 12.97 s 12.97 s [-0.9%; +1.0%] (no difference)
startup:petclinic:appsec:Agent 17.23 s 17.18 s [-0.6%; +1.2%] (no difference)
startup:petclinic:iast:Agent 17.22 s 17.27 s [-1.1%; +0.6%] (no difference)
startup:petclinic:profiling:Agent 16.87 s 17.01 s [-1.9%; +0.3%] (no difference)
startup:petclinic:sca:Agent 17.21 s 17.22 s [-0.9%; +0.8%] (no difference)
startup:petclinic:tracing:Agent 16.32 s 16.20 s [-0.1%; +1.6%] (no difference)

Commit: faa11220 · 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.

@bric3
bric3 added this pull request to stack #13019 October 23, 2026 16:02
dougqh and others added 2 commits October 23, 2026 12:47
Eager post-processors run on the application thread as a span finishes.
They now take a SpanStorage (a span's tags as stored) instead of a
TagMap, and DDSpanContext implements SpanStorage directly, so no TagMap
view needs to be built for them once span storage stops being a single
TagMap.

SpanStorage has its own verbs, load and store, apart from TagMap's
get/set and the span's intercepted getTag/setTag, so DDSpanContext can
offer the raw and the intercepted operations without a clash. Stores
bypass interception; thread safety is left to each implementation
(DDSpanContext's are unsynchronized, called under the tag lock). TagMap
implements it too, through its own get and set, for standalone maps.

This is the first slice: load, loadString and two stores.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A pure move: the loops over one bucket array (find, put, remove, copy,
merge, the forEach variants, fillMap / fillStringMap, count, isEmpty,
the bucket-position check, toInternalString) become static methods of
HashSupport, moved verbatim. Each reports what it did through its
return value (the entry put replaced, the entry remove removed, how much
merge grew the map), and TagMap keeps its fields, its size counter, and
the read-through and tombstone handling around them.

No new objects or fields, and no behavior change: allocation per trace
in TraceShapeBenchmark matches the base within fork-to-fork noise, and
the TagMap and core span suites pass unchanged. This prepares the split
of known and custom tags into separate stores, which reuse these
mechanics.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dougqh
dougqh force-pushed the dougqh/tagmap-store-facade branch from 75ec723 to 77b8ac8 Compare October 23, 2026 16:57
dougqh and others added 2 commits October 26, 2026 10:50
IdStore keeps known tags by id; its one implementation today is
HashIdStore, built through IdStoreFactory so a JVM loads a single
IdStore subclass and calls on it stay devirtualized. HashStore keeps
custom tags by name. Both reuse HashSupport's bucket mechanics and read
through to a frozen parent's store, hiding an inherited tag with a
tombstone: one shared, immutable Entry per known tag (Entry.tombstoneOf)
in the id store, and a removed-name set in the name store. The hash id
store's buckets are its exact state, so it needs no slot masks.

Nothing uses the stores yet: TagMap and the span keep their current
storage. IdStoreContractTest pins the stores' behavior directly. The
only TagMap changes are to its nested Entry, EntryReader and
BucketGroup, which the stores share: the tombstone flag and a lookup of
a bucket chain by id. HashSupport.checkBuckets now indexes a group's
entries by entry rather than by bucket, so the integrity check really
inspects groups. The tag registry gains _dd.agent_psr as a trace-level
known tag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TagMap keeps known tags in an IdStore (by id) and custom tags in a
HashStore (by name), instead of one bucket array. A name is classified
once, at the boundary, and routed to its store; known and custom tags
never share a key, so read-through and precedence work per store and
never cross between them. The name store is created on the first custom
write; until then a map borrows its read-through parent's store
read-only.

Behavior is unchanged: the TagMap suites pass as they are, and Entry
objects are still shared across copies. The span still holds a single
TagMap; it starts owning the stores directly in the next change.
TagMap.Internals stays for now, writing straight to the id store, and
goes away with that change.

Reads by id go straight to the id store. stream() and spliterator()
yield stable Entries, and getEntryReader gives a momentary reader for
typed reads. TagStoresTest covers the stores through TagMap, and
IdStoreContractTest gains the view cases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dougqh and others added 29 commits October 26, 2026 15:43
Co-authored-by: Sarah Chen <sarah.chen@datadoghq.com>
A span isn't conceptually a TagStore: the store is the storage beneath it,
as a Map was beneath a TagMap. It implements one only so post-processing
can be handed its storage without allocating, since the processor chain's
dispatch loop keeps escape analysis from removing a wrapper. Drop the
TagStore Javadoc's note about this being a first slice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ssors-span-context

# Conflicts:
#	dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/HttpEndpointPostProcessor.java
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EagerTagsPostProcessor and EagerPostProcessorChain still called it
storage after the rename to TagStore, while every processor calls it
unsafeTags.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
parentEntryVisible still masked the hash into a bucket by hand before
calling findInBucket, the one place outside HashSupport that knew the
bucket array's power-of-2 layout. It now calls HashSupport.find, and
findInBucket becomes private.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…asses

# Conflicts:
#	internal-api/src/main/java/datadog/trace/api/HashSupport.java
…acade

# Conflicts:
#	internal-api/src/main/java/datadog/trace/api/TagMap.java
HashSupport.forEach checked every entry's tombstone flag and walked
bucket groups through the _entryAt switch, on the path the serializer
takes for every span, though only HashIdStore ever holds a tombstone.
forEach goes back to the unrolled BucketGroup walk with no tombstone
check, for TagMap and HashStore. HashIdStore uses the new forEachLive,
which skips tombstones, and only while its hasTombstones hint is set.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HashCursor held a separate BucketWalker, so a cursor cost two
allocations where TagMap's iterator costs one. It now extends the
walker, whose step becomes nextEntry so it can't be mistaken for the
cursor's filtering next, and next() returns Entry covariantly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The coverage check flagged IdStoreFactory, which no test built a store
through, and HashSupport.BucketWalker, which only a small cursor test
reached. IdStoreFactoryTest builds stores through the JVM's factory,
checking behavior rather than the implementation, and a contract test
walks a cursor over every bucket shape and a parent level. HashStoreTest
grows to the typed setters, removal, copying, merging, sizing and a
read-through over bucket groups.

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

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