Make the TagMap Entry pathway null-tolerant by dougqh · Pull Request #11963 · DataDog/dd-trace-java

dougqh · 2026-07-15T14:57:14Z

What Does This Do

Makes the TagMap Entry pathway null-tolerant, so a null/empty value flows through as "no tag" without any caller guarding:

Entry.create(Object) / Entry.create(CharSequence) return null for a null or empty value (@Nullable). create(Object) now applies the empty-CharSequence check by runtime type, so an empty String passed as Object skips the same as via the CharSequence overload — behavior no longer depends on the static type at the call site.
The Entry sinks — getAndSet(Entry) and set(EntryReader) — treat a null Entry as a no-op (@Nullable params). Two guards because set(EntryReader) derefs .entry() before delegating.
The strict (key, value) setters keep their contract: their values (set(String, Object) / set(String, CharSequence)) are @Nonnull.
Keys stay strict everywhere they're written: all tag keys on the write/create surface (put / set / getAndSet / create) are @Nonnull — a null key is a bug, not "no tag". So null-tolerance is scoped to values and Entries on the Entry pathway; keys and strict-setter values are non-null. The annotations make the whole split self-describing.

Motivation

Make handling easier - and avoid NPEs

The Entry pathway is the prebuilt/producer path (create(...) → set(Entry)); a null Entry is the designed "nothing to add" value, mirroring DDSpanContext.setTag's null/empty ⇒ absent convention. Centralizing the tolerance in the primitive means callers needn't each remember a guard — which they don't: this fixes a latent NPE by construction in RemoteHostnameAdder, which sets create(TRACER_HOST, hostname) guarding only null (not empty), so an empty hostname previously NPE'd on set(null).

Notes

getAndSet's null check is the first statement, ahead of checkWriteAccess() and any field read, so it dead-code-eliminates when inlined at a provably-non-null call site (e.g. set(String,value) → getAndSet(newAnyEntry(...))) and stays only where the arg can actually be null. A null Entry is thus a complete no-op and does not assert write access.
Scope: primitive + test only. Caller-side guard cleanup — dropping the now-redundant guards (incl. InternalTagAdder: defensively guard against null prebuilt tagmap entry #11958) and simplifying InternalTagsAdder — is a fast follow-up.
The @Nonnull keys cover the write/create surface; read/lookup keys (getString, remove, getXxxOrDefault) are intentionally left unannotated here — same "null key = bug" logic, but a broader, Map-contract-adjacent sweep worth doing on its own.

Test

TagMapNullToleranceTest pins the contract: create(...) → null for null/empty (incl. empty-as-Object); set/getAndSet null no-op; end-to-end create()→set() with null/empty leaves no tag. Existing TagMapTest / TagMapEntryTest green (no behavior regression).

tag: ai generated
tag: no release note

🤖 Generated with Claude Code

@nonnull

create(Object)/create(CharSequence) may return null for a null or empty value, and the Entry sinks -- getAndSet(Entry) / set(EntryReader) -- treat a null Entry as a no-op, so a null/empty value flows through the Entry pathway as "no tag" without any caller guarding. create(Object) now applies the empty-CharSequence check by runtime type, so the null/empty => absent convention holds regardless of the static type at the call site. The strict (key,value) setters keep their contract -- their values are now @nonnull -- so null tolerance is scoped to the Entry pathway. The annotations make the split self-describing. Fixes a latent NPE by construction: RemoteHostnameAdder sets create(TRACER_HOST, hostname) guarding only null, not empty, so an empty hostname previously NPE'd on set(null). Caller-side guard cleanup (incl. the redundant #11958 guard) is left to a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

datadog-prod-us1-5 · 2026-07-15T15:14:50Z

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 56.74% (-0.45%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: c451336 | Docs | Datadog PR Page | Give us feedback!}

dd-octo-sts · 2026-07-15T15:21:03Z

🟢 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	14.01 s	13.98 s	[-0.5%; +0.9%] (no difference)
startup:insecure-bank:tracing:Agent	12.85 s	13.06 s	[-2.5%; -0.7%] (maybe better)
startup:petclinic:appsec:Agent	16.88 s	16.97 s	[-1.4%; +0.4%] (no difference)
startup:petclinic:iast:Agent	16.39 s	16.95 s	[-7.6%; +1.0%] (no difference)
startup:petclinic:profiling:Agent	16.77 s	16.61 s	[-0.3%; +2.2%] (no difference)
startup:petclinic:sca:Agent	17.06 s	16.88 s	[+0.0%; +2.1%] (maybe worse)
startup:petclinic:tracing:Agent	16.13 s	16.14 s	[-1.3%; +1.2%] (no difference)

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

@nonnull

A tag has no valid null key, so put/set/getAndSet/create now take @nonnull tag. This completes the null contract alongside the value/Entry side: keys are strict (null = a bug), values/Entries on the Entry pathway are tolerant (null = no tag). Scoped to the write/create surface; read/lookup keys (getString, remove, getXxxOrDefault) are left for a possible follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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

datadog-prod-us1-5

More details

The null guard in getAndSet(Entry) is placed before checkWriteAccess(), which means a null entry on a frozen map silently returns null instead of throwing an AccessException — this is intentional and documented. All new and existing tests pass (4 + 181 + 112 = 297 tests); adversarial coverage confirmed the empty-StringBuilder-as-Object path, frozen-map no-op contract, and no-existing-tag-removal invariant all hold correctly.

Was this helpful? React 👍 or 👎

_{📊 Validated against 20 scenarios · Open Bits AI session}

_{🤖 Datadog Autotest · Commit fc7bc19 · What is Autotest? · Any feedback? Reach out in #autotest}

@nonnull

The interface -> final-class fold removed the TagMap interface decls, which carried the @Nullable/@nonnull param annotations from #11963. Re-home them onto the now-concrete methods: @nonnull tag keys and strict-setter values, @nullable on the set(EntryReader)/getAndSet(Entry) sinks (+ the getAndSet contract javadoc). The null-tolerance behavior was already preserved by the fold; this restores the self-describing contract on the write surface. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

A fresh, mutable TagMap can read through to a frozen parent on local misses, so a span can layer its own tags over a shared, immutable set (e.g. merged tracer tags) without copying them. - createFromParent(parent): the only way to attach a parent; the parent must be frozen and is fixed at construction (no re-parenting), so read-through can treat it as stable. Single-parent by design in phase 1. - Reads resolve local-first, then the parent; a local entry shadows the parent's (local-wins). Removing a parent key locally records a lazy tombstone (removedFromParent) so it stops reading through; the tombstone set is null until first needed, keeping the hot paths untouched. - size()/isEmpty() are exact (Map contract) and resolve the parent; isDefinitelyEmpty()/estimateSize() are the cheap conservative variants for the hot path. copy() preserves the parent and tombstones; forEach walks local then parent. Built on the folded final-class TagMap (#11967); composes cleanly with the null-tolerant Entry pathway (#11963). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

dd-octo-sts · 2026-07-15T19:23:15Z

/merge

gh-worker-devflow-routing-ef8351 · 2026-07-15T19:23:19Z

View all feedbacks in Devflow UI.

2026-07-15 19:23:19 UTC ℹ️ Start processing command /merge

2026-07-15 19:23:24 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in master is approximately 2h (p90).

2026-07-15 20:25:12 UTC ℹ️ MergeQueue: This merge request was merged

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 Jul 15, 2026

dougqh marked this pull request as ready for review July 15, 2026 16:40

dougqh requested a review from a team as a code owner July 15, 2026 16:40

dougqh requested a review from ygree July 15, 2026 16:40

dougqh and others added 2 commits July 15, 2026 12:46

Brace single-statement null-check ifs per style convention

c451336

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

datadog-prod-us1-5 Bot reviewed Jul 15, 2026

View reviewed changes

mhdatie approved these changes Jul 15, 2026

View reviewed changes

dougqh mentioned this pull request Jul 15, 2026

Fold OptimizedTagMap into a final class TagMap #11967

Open

dougqh added this pull request to the merge queue Jul 15, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 15, 2026

gh-worker-dd-mergequeue-cf854d Bot merged commit d80a30e into master Jul 15, 2026
586 checks passed

gh-worker-dd-mergequeue-cf854d Bot deleted the dougqh/tagmap-null-entry-tolerance branch July 15, 2026 20:25

github-actions Bot added this to the 1.65.0 milestone Jul 15, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Make the TagMap Entry pathway null-tolerant#11963

Make the TagMap Entry pathway null-tolerant#11963
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
masterfrom
dougqh/tagmap-null-entry-tolerance

dougqh commented Jul 15, 2026 •

edited

Loading

Uh oh!

datadog-prod-us1-5 Bot commented Jul 15, 2026 •

edited by datadog-official Bot

Loading

Uh oh!

dd-octo-sts Bot commented Jul 15, 2026 •

edited

Loading

Uh oh!

datadog-prod-us1-5 Bot left a comment

Uh oh!

dd-octo-sts Bot commented Jul 15, 2026

Uh oh!

gh-worker-devflow-routing-ef8351 Bot commented Jul 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

dougqh commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Does This Do

Motivation

Notes

Test

Uh oh!

datadog-prod-us1-5 Bot commented Jul 15, 2026 • edited by datadog-official Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dd-octo-sts Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟢 Java Benchmark SLOs — All performance SLOs passed

Uh oh!

datadog-prod-us1-5 Bot left a comment

Choose a reason for hiding this comment

Uh oh!

dd-octo-sts Bot commented Jul 15, 2026

Uh oh!

gh-worker-devflow-routing-ef8351 Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dougqh commented Jul 15, 2026 •

edited

Loading

datadog-prod-us1-5 Bot commented Jul 15, 2026 •

edited by datadog-official Bot

Loading

dd-octo-sts Bot commented Jul 15, 2026 •

edited

Loading

gh-worker-devflow-routing-ef8351 Bot commented Jul 15, 2026 •

edited

Loading