Skip to content

Make a null tag value remove the tag in TagMap (dense store - part 1) - #12999

Open
dougqh wants to merge 4 commits into
masterfrom
dougqh/tagmap-null-removes
Open

dougqh wants to merge 4 commits into
masterfrom
dougqh/tagmap-null-removes

Conversation

@dougqh

@dougqh dougqh commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

What Does This Do

A null value given to a TagMap setter now removes the tag, matching what setTag(tag, null) already does on a span.

  • TagMap.put, set and getAndSet, by name or by id, remove the tag and return its prior value. putAll and fromMap go through set, so they follow.
  • A null Entry or EntryReader is still a no-op, as before.
  • In payload tagging, a JSON null in a request or response body now produces no tag. Before, it produced a tag with a null value.

Motivation

TagMap and the span API disagreed about null. Writers that bypass setTag and write to a TagMap directly could leave null-valued tags on a span, and the v0.4 serializer then sent them as a msgpack nil under meta. Payload tagging was the one writer that did this.

This is split out of the TagMap store work (#12918) so that the behavior change is reviewed on its own and the optimization PRs stacked on it change no behavior.

Behavior change

Payload tagging only:

  • A JSON null (for example p.j3.3 from [1, 2, 3.14, null, true]) no longer emits a tag.
  • It also no longer counts toward the max-tags limit, so the next value can take its place. In respectMaxTagsLimit, that's p.j3.4.

Additional Notes

  • New TagMapNullToleranceTest cases cover set by name, set by id, put, getAndSet and fromMap.
  • Local runs: 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

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>
@dougqh dougqh added type: bug fix Bug fix comp: core Tracer core tag: ai generated Largely based on code generated by an AI or LLM labels Oct 23, 2026
@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🎯 Code Coverage (details)
• Patch Coverage: 93.75%
• Overall Coverage: 59.09% (-0.38%)

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

@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.97 s 13.99 s [-0.9%; +0.7%] (no difference)
startup:insecure-bank:tracing:Agent 12.92 s 13.01 s [-1.2%; -0.2%] (maybe better)
startup:petclinic:appsec:Agent 17.76 s 17.61 s [-0.2%; +1.9%] (no difference)
startup:petclinic:iast:Agent 17.53 s 17.68 s [-1.8%; -0.0%] (maybe better)
startup:petclinic:profiling:Agent 17.37 s 17.31 s [-0.9%; +1.5%] (no difference)
startup:petclinic:sca:Agent 17.80 s 17.67 s [+0.1%; +1.4%] (maybe worse)
startup:petclinic:tracing:Agent 16.70 s 16.84 s [-2.1%; +0.3%] (no difference)

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

@dougqh dougqh changed the title Make a null tag value remove the tag in TagMap Make a null tag value remove the tag in TagMap (dense store - part 1) Oct 23, 2026
@bric3
bric3 added this pull request to stack #13019 October 23, 2026 16:02
@dougqh
dougqh marked this pull request as ready for review October 26, 2026 15:10
@dougqh
dougqh requested a review from a team as a code owner October 26, 2026 15:10
@dougqh
dougqh requested review from sarahchen6 and removed request for a team October 26, 2026 15:10
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

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-26T15:17:25.846323Z fa9b441 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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa9b441840

ℹ️ 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".

@datadog-datadog-prod-us1-2 datadog-datadog-prod-us1-2 Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: FAIL

A repeated JSON property ending in null retains its earlier value as a payload tag. Restoring the null setter call clears that stale value through TagMap’s new removal semantics.

Open Bits AI session

🤖 Bits Code Review · Commit fa9b441

With null now meaning remove, a JSON null set no tag at all, so a
duplicate key such as {"x":"old","x":null} left the stale x=old tag.
Setting null again removes it, as overwriting with null did before.

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

@sarahchen6 sarahchen6 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one typo nit but LGTM

dougqh and others added 2 commits October 26, 2026 15:43
@dougqh

dougqh commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-10-26 20:16:41 UTC ℹ️ Start processing command /merge


2026-10-26 20:16:51 UTC ℹ️ MergeQueue: Pull request is not mergeable yet

It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.

  • Run /code blockers to see what is blocking it.
  • Run /remove to cancel it.

2026-10-26 21:02:10 UTC ℹ️ MergeQueue: merge request added to the queue

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


2026-10-26 21:50:10 UTC ❌ MergeQueue: The build pipeline contains failing jobs for this merge request

Build pipeline has failing jobs for 59ff42d:

⚠️ Do NOT retry failed jobs directly (why?).

What to do next?

  • Investigate the failures and when ready, re-add your pull request to the queue!
  • If your PR checks are green, try to rebase/merge. It might be because the CI run is a bit old.
  • Any question, go check the FAQ.
Details

Since those jobs are not marked as being allowed to fail, the pipeline will most likely fail.
Therefore, and to allow other builds to be processed, this merge request has been rejected and the pipeline got canceled.

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 type: bug fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants