Pre-construct TagMap.Entry objects in InternalTagsAdder by dougqh · Pull Request #11555 · DataDog/dd-trace-java

dougqh · 2026-06-04T00:12:48Z

What Does This Do

InternalTagsAdder sets _dd.base_service / version on spans via TagMap.set(tag, value), which allocates a fresh TagMap.Entry per span. Both values are fixed for the life of the tracer, and TagMap.Entry objects are explicitly safe to share across maps (the OptimizedTagMap collision design via BucketGroup relies on it). This pre-builds the two Entry objects once in the constructor and reuses them via set(entry).

Motivation

A JFR allocation profile of spring-petclinic under load (full agent, 2026-06-03) attributed ~52 allocation samples to InternalTagsAdder.processTags — one Entry allocated per span for base.service. Tag-map handling (TagMap$Entry + OptimizedTagMap) was the single largest non-core tracer allocation theme in that profile, ahead of the CSS metrics system. This change drops the InternalTagsAdder contribution to zero.

Notes

Re-applies the logic from Reuse TagMap.Entry objects in InternalTagsAdder #10965 (dd/pre-construct-tagmap-entries), which had drifted 415 commits behind master and changed processTags's signature; this version sits on current master with the unchanged AppendableSpanLinks signature. Supersedes Reuse TagMap.Entry objects in InternalTagsAdder #10965.
Behavior preserved: Entry.stringValue() for the OBJECT entry resolves to obj.toString() (cached), identical to the prior ddService.toString() in the equalsIgnoreCase comparison. The only edge case — empty ddService now yields an early return — is unreachable in production (Config.getServiceName() is never "").
Existing test InternalTagsAdderTest.groovy passes unchanged.

🤖 Generated with Claude Code

InternalTagsAdder set base.service / version via TagMap.set(tag, value), allocating a fresh TagMap.Entry per span. Both values are fixed for the life of the tracer, and TagMap.Entry objects are safe to share across maps (the OptimizedTagMap collision design relies on it), so build the two Entry objects once in the constructor and reuse them via set(entry). A JFR profile of petclinic (2026-06-03) attributed ~52 allocation samples to InternalTagsAdder.processTags (one Entry per span); this drops them to zero. Re-applies the change from the stale PR #10965 (415 commits behind master, drifted signature) onto current master. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

dd-octo-sts · 2026-06-04T00:14:16Z

Hi! 👋 Thanks for your pull request! 🎉

To help us review it, please make sure to:

Add at least one type, and one component or instrumentation label to the pull request

If you need help, please check our contributing guidelines.

datadog-datadog-prod-us1-2 · 2026-06-04T00:14:42Z

✨ Fix all issues with BitsAI

⚠️ Warnings

🚦 3 Pipeline jobs failed

Run system tests | Check system tests success

Run system tests | main / End-to-end #5 / spring-boot-undertow 5

Run system tests | main / End-to-end #7 / resteasy-netty3 7

Useful? React with 👍 / 👎

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

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

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 934fd5dc3b

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

Addresses the Codex review comment on #11555: pre-building the base.service Entry must not change behavior for an explicitly-empty DD_SERVICE. Entry.create rejects empty values, so baseServiceEntry is null in that case; the processTags branch now falls back to set(BASE_SERVICE, ddService) to preserve byte-identical behavior, and the version branch is still reached when the span service also matches the empty configured service. Migrate InternalTagsAdderTest from Groovy/Spock to JUnit 5 (parameterized with @MethodSource) and add regression coverage for the empty-DD_SERVICE case (9 migrated cases + 2 new = 11). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

dd-octo-sts · 2026-06-04T01:38:31Z

🟢 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.05 s	14.03 s	[-0.8%; +1.0%] (no difference)
startup:insecure-bank:tracing:Agent	12.85 s	13.03 s	[-2.1%; -0.7%] (maybe better)
startup:petclinic:appsec:Agent	16.82 s	16.67 s	[-0.2%; +1.9%] (no difference)
startup:petclinic:iast:Agent	16.43 s	16.94 s	[-7.5%; +1.4%] (no difference)
startup:petclinic:profiling:Agent	16.71 s	16.81 s	[-1.6%; +0.4%] (no difference)
startup:petclinic:sca:Agent	16.87 s	16.61 s	[+0.9%; +2.3%] (maybe worse)
startup:petclinic:tracing:Agent	15.97 s	15.60 s	[-2.0%; +6.7%] (no difference)

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

amarziali

it looks a good idea. I left improvements for the comments specifically that looks a bit too verbosely generated

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@TableTest

…gression test Preserves the @TableTest versions of the two existing tests that landed on master, and adds the empty-DD_SERVICE regression test (from the PR) as a plain @test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Extend the processTags null guard to also exit when ddService.length()==0, which prevents writing _dd.base_service="" via the TagMap.set path that has no empty-value guard (unlike Entry.create). Empty ddService now behaves the same as null/unset. - Remove the manual null+length>0 pre-check before TagMap.Entry.create in the constructor; Entry.create already returns null for null or empty values, so the guard was redundant. - Update the regression test to assert the new early-return behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Handle the per-span "common" tags (base.service / version) via the tag-id fast path. These values are fixed for the tracer's life, so build their TagMap.Entry once and share across every span (Entry is immutable + safe to share) — dropping InternalTagsAdder's per-span Entry allocation to zero (cf. PR #11555, the string-keyed precursor), and making the entries tag-id-bearing so they also land in their positional slot. - TagMap.Entry.create(long, Object)/create(long, CharSequence): tag-id keyed, null/empty-rejecting factories mirroring the String create(). - CoreTagIds.BASE_SERVICE / VERSION (stored range) + resolver entries. - InternalTagsAdder prebuilds baseServiceEntry/versionEntry in its ctor and set()s the shared entry; empty DD_SERVICE early-returns (regression test added). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

amarziali · 2026-06-10T06:58:51Z

  public void processTags(
      TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
-    if (spanContext == null || ddService == null) {
+    if (spanContext == null || ddService == null || ddService.length() == 0) {


when that processor is created (in createEagerChain) the ddService value is taken from Config.getServiceName() and this cannot be neither null neither empty. So it looks that instead adding a check for the size we can also remove the null check. I would suggest to improve the documentation (i.e. also add @NonNull to the constructor and simplify that instructions

This was primarily out of an abundance of caution. Although, both AI and human reviewers had raised this as potential concern, so it was easiest to just be cautious.

If you are confident that empty won't reach here, then we can change it.

Good call — confirmed that createEagerChain() always passes Config.getServiceName() which defaults to "unnamed-java-app" and is never null or empty. Changed the constructor param to @Nonnull, dropped the null check on ddService, and simplified the guard in processTags to just spanContext == null.

amarziali · 2026-06-10T07:03:22Z

+  // Regression: empty DD_SERVICE is treated the same as unset — processTags exits early and
+  // writes no tags, regardless of the span's service name.
+  @Test
+  void emptyDdServiceWritesNoTags() {


This test should not be added. I don't understand why the regression word is used here. And the service name cannot be empty in our tracer. I would suggest to remove it and improve the documentation of this processor as suggested above

It is a change in behavior of the InternalTagsAdder, so this test was guarded against that change/regression being introduced again. Mostly, it just comes down to the contract being unclear before, so the AI had to be cautious.

Agreed — removed the test and the @Test import. The contract is now documented by the @Nonnull annotation on the constructor instead.

@empty

…ession test Config.getServiceName() always returns a non-null non-empty string (defaults to "unnamed-java-app"), so the null/@empty guard in processTags and the corresponding regression test for empty DD_SERVICE are unnecessary. Replaced @nullable with @nonnull on the constructor param to document the actual contract. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dougqh mentioned this pull request Jun 4, 2026

Reuse TagMap.Entry objects in InternalTagsAdder #10965

Closed

dougqh marked this pull request as ready for review June 4, 2026 00:14

dougqh requested a review from a team as a code owner June 4, 2026 00:14

dougqh requested a review from ygree June 4, 2026 00:14

dd-octo-sts Bot added the tag: ai generated Largely based on code generated by an AI or LLM label Jun 4, 2026

chatgpt-codex-connector Bot reviewed Jun 4, 2026

View reviewed changes

Comment thread dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/InternalTagsAdder.java Outdated

dougqh added comp: core Tracer core type: enhancement Enhancements and improvements tag: performance Performance related changes labels Jun 4, 2026

amarziali reviewed Jun 8, 2026

View reviewed changes

dougqh and others added 4 commits June 9, 2026 13:02

Trim verbose generated comments in InternalTagsAdder per review feedback

c773ba7

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Merge branch 'master' into dougqh/preconstruct-internal-tags

7456d26

amarziali reviewed Jun 10, 2026

View reviewed changes

Conversation

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

Uh oh!

What Does This Do

Motivation

Notes

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

⚠️ Warnings

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

dd-octo-sts Bot commented Jun 4, 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!

amarziali left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amarziali Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougqh Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

amarziali Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougqh Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dougqh commented Jun 4, 2026 •

edited

Loading

dd-octo-sts Bot commented Jun 4, 2026 •

edited

Loading

datadog-datadog-prod-us1-2 Bot commented Jun 4, 2026 •

edited by datadog-official Bot

Loading

dd-octo-sts Bot commented Jun 4, 2026 •

edited

Loading

dougqh Jun 10, 2026 •

edited

Loading

dougqh Jun 10, 2026 •

edited

Loading