Remove containerTagsHash/processTags from DSM primary pathway hash by arjunguhaswe · Pull Request #12573 · DataDog/dd-trace-java

arjunguhaswe · 2026-09-18T20:54:46Z

Summary

Fixes DSM2-335: DSM's primary pathway hash was seeded from BaseHash.getBaseHash(), a global static shared with DBM that folds in serviceName + env + primaryTag + processTags + containerTagsHash. containerTagsHash is derived by the local Agent from kube_replica_set and changes on every rolling deploy; processTags is enabled by default. Since DDAgentFeaturesDiscovery.processInfoResponseHeaders() recalculates BaseHash unconditionally whenever the Agent reports a new container-tags hash, the same logical DSM edge (same service, same topic, same direction) was producing a new pathway hash on every rolling deploy — permanently inflating block_on_hashes cardinality with no real fan-out.

Testing evidence

Ran the repro against the Data Streams Sandbox account (metrics collection happens in the Datadog HQ / org2 account).

Before (unpatched — primary hash fragments on rolling deploy)
Testing Run (Data Streams Prod)
Dropped Payload Metrics (Org2)

After (with this fix — primary hash stable across rolling deploy)
Testing Run (Data Streams Prod)
Dropped Payload Metrics (Org2)

Test plan

BaseHashTest: identity hash tracks service/env/primaryTag but is unaffected by container-tags hash or process tags
DataStreamsTagsTest: container-tags hash / process tag changes affect aggregationHash/completeHash but not the primary hash (the DSM2-335 regression tests)
DefaultPathwayContextTest updated to seed via the new BaseHash.updateIdentityHash() test hook
Existing SQLCommenterTest suite (DBM, 91 tests) passes unmodified — confirms BaseHash.getBaseHash() behavior for DBM is untouched
Manual verification against org 1300002025 (Huuuge Games) once available — recommend first confirming they're on dd-trace-java >= 1.58.0, since the already-fixed datasetName bug is an equally plausible explanation for versions before that

🤖 Generated with Claude Code

DataStreamsTags seeded its primary pathway hash from BaseHash.getBaseHash(), a global static shared with DBM that folds in serviceName+env+primaryTag plus process tags and the Agent-reported containerTagsHash. The latter two are DBM-oriented (per-container SQL attribution) and change on every rolling deploy, so the same logical DSM edge was producing a new pathway hash on every deploy, inflating block_on_hashes cardinality with no real fan-out. BaseHash now also exposes an identity-only hash (service+env+primaryTag), which DSM's primary hash is seeded from; getBaseHash() is untouched so DBM's SQL-comment injection keeps its existing per-container behavior. containerTagsHash and process tags are instead folded into DataStreamsTags.aggregationHash/completeHash, independently of each other, mirroring the datasetName fix in bd3f6f5. DSM2-335 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

linear-code · 2026-09-18T20:55:00Z

DSM2-335

They were never decoded into discrete tags by the backend, so folding them into aggregationHash only added opaque cardinality with no user-visible benefit. Leaves a TODO to tag a DSM backend owner before this ships, since raphaelgavache flagged a related concern in PR #9282 that was never answered. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

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

dd-octo-sts · 2026-09-18T21:19:24Z

🟢 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.00 s	13.94 s	[-0.5%; +1.3%] (no difference)
startup:insecure-bank:tracing:Agent	12.95 s	13.00 s	[-1.1%; +0.4%] (no difference)
startup:petclinic:appsec:Agent	16.99 s	16.86 s	[-0.1%; +1.7%] (no difference)
startup:petclinic:iast:Agent	16.93 s	17.01 s	[-1.3%; +0.4%] (no difference)
startup:petclinic:profiling:Agent	16.69 s	16.73 s	[-1.2%; +0.8%] (no difference)
startup:petclinic:sca:Agent	17.02 s	16.90 s	[-0.2%; +1.7%] (no difference)
startup:petclinic:tracing:Agent	16.17 s	16.20 s	[-0.9%; +0.5%] (no difference)

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

…ntainertagshashprocesstags-from-dsm

arjunguhaswe · 2026-09-21T19:20:30Z

Testing evidence

Ran the repro against the Data Streams Sandbox account (metrics collection happens in the Datadog HQ / org2 account).

Before (unpatched — primary hash fragments on rolling deploy)

After (with this fix — primary hash stable across rolling deploy)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ac2f088e8

ℹ️ 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-prod-us1-6

More details

The identity hash follows the existing base-hash update path. DBM keeps its current hash behavior.

Was this helpful? React 👍 or 👎

_{Open Bits AI session}

_{🤖 Bits Code Review · Commit 1ac2f08 · @DataDog review to ask questions}

AlexeyKuznetsov-DD

Left minor comments.

internal-api unit tests should be JUnit 5 per AGENTS.md; the Spock suites in BaseHashTest.groovy/DataStreamsTagsTest.groovy predate that convention, so leave them as-is and add the new coverage in new JUnit 5 files instead of extending them further. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

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

service/env/primaryTag are fixed for the JVM's lifetime once Config is built, so recomputing identityHash inside recalc() on every containerTagsHash/processTags change was redundant. Compute it once at class-load instead; calcIdentity is now package-private so it can be exercised directly in tests without forcing a Config-driven recompute. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

AlexeyKuznetsov-DD

LGTM from LP side, but I think it make sense to get an approval from DSM/IDM too?

…ckpoint BaseHash's class initializer (and therefore identityHash's snapshot of Config.get().getServiceName()/getEnv()/getPrimaryTag()) can run early as a side effect of unrelated static initialization - DefaultDataStreamsMonitoring's REPORT/POISON_PILL fields reference DataStreamsTags.EMPTY, which forces BaseHash to load. Since identityHash is now calculated once, that snapshot could be permanently stale if it's taken before Config settles. Add BaseHash.ensureIdentityHash(), a one-time guard that recalculates identityHash from the current Config, and call it right before the first outbound DSM checkpoint's tags are created in DefaultDataStreamsMonitoring.setProduceCheckpoint. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Concurrent callers all recompute from the same Config, so a race just means a few redundant, identical writes - not worth the lock contention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…inel No need for a separate boolean guard - identityHash is either "not yet computed" (0) or holds a real value, so getIdentityHash() can just check for that directly. This also removes the need for an explicit BaseHash.ensureIdentityHash() call site in DefaultDataStreamsMonitoring.setProduceCheckpoint, since every checkpoint (inbound or outbound) already reads getIdentityHash() via DataStreamsTags. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ntainertagshashprocesstags-from-dsm

dougqh · 2026-09-22T17:48:19Z

+
+    // once non-zero, later reads don't recalculate - a caller that needs a fresh value can
+    // still force one directly (e.g. tests via updateIdentityHash)
+    BaseHash.updateIdentityHash(42L);


getIdentityHashRecalculatesFromConfigWhenUnset ends by calling BaseHash.updateIdentityHash(42L) and never resets it afterward. Since this class isn't forked per-method and JUnit 5 doesn't guarantee method order, if this test runs before identityHashIsUnaffectedByContainerTagsHashOrProcessTags, the latter's getIdentityHash() calls will return the stale 42 (the identityHash == 0 lazy-init guard never fires) — so that test would pass even if a real regression reintroduced container-tags-hash/process-tags into the identity hash, which is the exact bug this PR fixes. Consider resetting identityHash (e.g. via a test hook that sets it back to 0 or recomputes via calcIdentity()) in an @AfterEach.

ditto, great catch, will fix before merge.

dougqh · 2026-09-22T17:48:19Z

    def two = getTags(0)

-    BaseHash.updateBaseHash(12)
+    BaseHash.updateIdentityHash(12)


Same root cause as in BaseHashIdentityTest: 'test service name override and global hash' calls BaseHash.updateIdentityHash(12), but cleanup() only resets baseHash/lastContainerTagsHash via recalcBaseHash(null) — identityHash is left stuck at 12 for the rest of the test JVM. Doesn't break current assertions since they're relative, but it's a latent leak that could mask failures in a future test asserting an exact identity-hash value. Worth resetting identityHash in cleanup() too.

Great catch, will fix before merge.

piochelepiotr · 2026-09-22T18:31:40Z

Just looking at description of the PR, but containerTagsHash is not supposed to change on every restart. Do you know what container tag is causing that?
If we remove containerTagsHash from the data streams cache, it will break service renaming for Data Stream pathways.

arjunguhaswe · 2026-09-22T18:47:29Z

@piochelepiotr -- the tag causing that is kube_replica_set, but I imagine kube_job could cause it as well. I may have been wrong in my interpretation of this code, but is service overriding not done in the DataStreamTags? (This is right before edge tag calc)

331:    this.hash = BaseHash.getIdentityHash();
332:
333:    if (DataStreamsTags.serviceNameOverride != null) {
334:      String val = DataStreamsTags.serviceNameOverride.get();
335:      if (val != null) {
336:        this.hash = FNV64Hash.continueHash(this.hash, val, FNV64Hash.Version.v1);
337:      }
338:    }

arjunguhaswe added type: bug fix Bug fix comp: data streams Data Streams Monitoring tag: ai generated Largely based on code generated by an AI or LLM labels Sep 18, 2026

This comment has been minimized.

Sign in to view

arjunguhaswe and others added 2 commits September 18, 2026 17:15

Simplify DSM2-335 removal comment

d123fca

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

Merge branch 'master' into arjunguha/dsm2-335-dd-trace-java-remove-co…

1ac2f08

…ntainertagshashprocesstags-from-dsm

arjunguhaswe marked this pull request as ready for review September 21, 2026 19:36

arjunguhaswe requested review from a team as code owners September 21, 2026 19:36

arjunguhaswe requested review from AlexeyKuznetsov-DD and removed request for a team September 21, 2026 19:36

chatgpt-codex-connector Bot reviewed Sep 21, 2026

View reviewed changes

Comment thread internal-api/src/test/groovy/datadog/trace/api/BaseHashTest.groovy Outdated

datadog-prod-us1-6 Bot reviewed Sep 21, 2026

View reviewed changes

ericfirth reviewed Sep 21, 2026

View reviewed changes

Comment thread internal-api/src/main/java/datadog/trace/api/BaseHash.java

AlexeyKuznetsov-DD reviewed Sep 21, 2026

View reviewed changes

Comment thread internal-api/src/main/java/datadog/trace/api/BaseHash.java Outdated

Comment thread internal-api/src/main/java/datadog/trace/api/BaseHash.java Outdated

arjunguhaswe and others added 3 commits September 21, 2026 16:37

Add braces around single-line if in BaseHash.calcIdentity

47634e9

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

AlexeyKuznetsov-DD approved these changes Sep 21, 2026

View reviewed changes

arjunguhaswe and others added 3 commits September 21, 2026 20:37

Drop synchronization from BaseHash.ensureIdentityHash

d43081e

Concurrent callers all recompute from the same Config, so a race just means a few redundant, identical writes - not worth the lock contention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

ericfirth approved these changes Sep 22, 2026

View reviewed changes

arjunguhaswe enabled auto-merge September 22, 2026 17:13

Merge branch 'master' into arjunguha/dsm2-335-dd-trace-java-remove-co…

b3548ee

…ntainertagshashprocesstags-from-dsm

dougqh reviewed Sep 22, 2026

View reviewed changes

dougqh approved these changes Sep 22, 2026

View reviewed changes

Conversation

arjunguhaswe commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing evidence

Test plan

Uh oh!

linear-code Bot commented Sep 18, 2026

Uh oh!

This comment has been minimized.

dd-octo-sts Bot commented Sep 18, 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!

arjunguhaswe commented Sep 21, 2026

Testing evidence

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

datadog-prod-us1-6 Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AlexeyKuznetsov-DD left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlexeyKuznetsov-DD left a comment

Choose a reason for hiding this comment

Uh oh!

dougqh Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

arjunguhaswe Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

dougqh Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

arjunguhaswe Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

piochelepiotr commented Sep 22, 2026

Uh oh!

arjunguhaswe commented Sep 22, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

arjunguhaswe commented Sep 18, 2026 •

edited

Loading

dd-octo-sts Bot commented Sep 18, 2026 •

edited

Loading