From 19b4dfffe12a5bd08698e4b0c06cf41d55585002 Mon Sep 17 00:00:00 2001 From: Arjun Guha Date: Fri, 18 Sep 2026 16:53:48 -0400 Subject: [PATCH 1/9] Remove containerTagsHash/processTags from DSM primary pathway hash 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 bd3f6f5c89. DSM2-335 Co-Authored-By: Claude Sonnet 5 --- .../DefaultPathwayContextTest.java | 6 +-- .../main/java/datadog/trace/api/BaseHash.java | 41 ++++++++++++++++-- .../api/datastreams/DataStreamsTags.java | 25 ++++++++++- .../datadog/trace/api/BaseHashTest.groovy | 29 +++++++++++++ .../datastreams/DataStreamsTagsTest.groovy | 43 ++++++++++++++++++- 5 files changed, 134 insertions(+), 10 deletions(-) diff --git a/dd-trace-core/src/test/java/datadog/trace/core/datastreams/DefaultPathwayContextTest.java b/dd-trace-core/src/test/java/datadog/trace/core/datastreams/DefaultPathwayContextTest.java index 8860cf1571c..576ae28165c 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/datastreams/DefaultPathwayContextTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/datastreams/DefaultPathwayContextTest.java @@ -515,7 +515,7 @@ void checkContextExtractorDecoratorBehavior(boolean dynamicConfigEnabled) throws payloadWriter, DEFAULT_BUCKET_DURATION_NANOS); - BaseHash.updateBaseHash(BASE_HASH); + BaseHash.updateIdentityHash(BASE_HASH); DefaultPathwayContext context = new DefaultPathwayContext(timeSource, null); timeSource.advance(MILLISECONDS.toNanos(50)); context.setCheckpoint( @@ -575,7 +575,7 @@ void checkContextExtractorDecoratorBehaviorWhenTraceDataIsNull(boolean globalDsm payloadWriter, DEFAULT_BUCKET_DURATION_NANOS); - BaseHash.updateBaseHash(BASE_HASH); + BaseHash.updateIdentityHash(BASE_HASH); DefaultPathwayContext context = new DefaultPathwayContext(timeSource, null); timeSource.advance(MILLISECONDS.toNanos(50)); context.setCheckpoint( @@ -635,7 +635,7 @@ void checkContextExtractorDecoratorBehaviorWhenLocalTraceConfigIsNull(boolean gl payloadWriter, DEFAULT_BUCKET_DURATION_NANOS); - BaseHash.updateBaseHash(BASE_HASH); + BaseHash.updateIdentityHash(BASE_HASH); DefaultPathwayContext context = new DefaultPathwayContext(timeSource, null); timeSource.advance(MILLISECONDS.toNanos(50)); context.setCheckpoint( diff --git a/internal-api/src/main/java/datadog/trace/api/BaseHash.java b/internal-api/src/main/java/datadog/trace/api/BaseHash.java index 8aa17c4a0ff..6373f6738ec 100644 --- a/internal-api/src/main/java/datadog/trace/api/BaseHash.java +++ b/internal-api/src/main/java/datadog/trace/api/BaseHash.java @@ -6,16 +6,24 @@ public final class BaseHash { private static volatile long baseHash; private static volatile String baseHashStr; private static volatile String lastContainerTagsHash; + private static volatile long identityHash; private BaseHash() {} public static void recalcBaseHash(String containerTagsHash) { lastContainerTagsHash = containerTagsHash; - updateBaseHash(calc(containerTagsHash)); + recalc(); } static void recalcBaseHash() { + recalc(); + } + + private static void recalc() { updateBaseHash(calc(lastContainerTagsHash)); + identityHash = + calcIdentity( + Config.get().getServiceName(), Config.get().getEnv(), Config.get().getPrimaryTag()); } public static void updateBaseHash(long hash) { @@ -31,6 +39,26 @@ public static String getBaseHashStr() { return baseHashStr; } + /** + * DSM topology-identity hash: service + env + primary tag only. Unlike {@link #getBaseHash()} + * (which also folds in process tags and the agent-reported container-tags hash for DBM's + * per-container SQL attribution use case), this is stable across pod restarts / rolling deploys, + * so it's safe to use as the seed for DSM's cardinality-sensitive pathway hash. + */ + public static long getIdentityHash() { + return identityHash; + } + + /** The most recent container-tags hash reported by the Agent, or {@code null}/empty if none. */ + public static String getLastContainerTagsHash() { + return lastContainerTagsHash; + } + + /** Test-only: lets tests set the identity hash without going through {@link Config}. */ + public static void updateIdentityHash(long hash) { + identityHash = hash; + } + public static long calc(String containerTagsHash) { return calc( Config.get().getServiceName(), @@ -40,15 +68,20 @@ public static long calc(String containerTagsHash) { containerTagsHash); } + private static long calcIdentity(CharSequence serviceName, CharSequence env, String primaryTag) { + long hash = FNV64Hash.generateHash(serviceName.toString(), FNV64Hash.Version.v1); + hash = FNV64Hash.continueHash(hash, env.toString(), FNV64Hash.Version.v1); + if (primaryTag != null) hash = FNV64Hash.continueHash(hash, primaryTag, FNV64Hash.Version.v1); + return hash; + } + private static long calc( CharSequence serviceName, CharSequence env, String primaryTag, CharSequence processTags, String containerTagsHash) { - long hash = FNV64Hash.generateHash(serviceName.toString(), FNV64Hash.Version.v1); - hash = FNV64Hash.continueHash(hash, env.toString(), FNV64Hash.Version.v1); - if (primaryTag != null) hash = FNV64Hash.continueHash(hash, primaryTag, FNV64Hash.Version.v1); + long hash = calcIdentity(serviceName, env, primaryTag); if (processTags != null) { hash = FNV64Hash.continueHash(hash, processTags.toString(), FNV64Hash.Version.v1); if (containerTagsHash != null && !containerTagsHash.isEmpty()) { diff --git a/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java b/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java index df3eb214813..8b88724bfd8 100644 --- a/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java +++ b/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java @@ -1,6 +1,7 @@ package datadog.trace.api.datastreams; import datadog.trace.api.BaseHash; +import datadog.trace.api.ProcessTags; import datadog.trace.util.FNV64Hash; import java.util.Objects; @@ -325,7 +326,10 @@ public DataStreamsTags( kafkaClusterId != null ? KAFKA_CLUSTER_ID_TAG + ":" + kafkaClusterId : null; this.partition = partition != null ? PARTITION_TAG + ":" + partition : null; - this.hash = BaseHash.getBaseHash(); + // seeded from service+env+primaryTag only: process tags and the agent-reported + // container-tags hash vary per-pod/per-rollout and must not fragment pathway identity + // (see DSM2-335) — they're folded into aggregationHash below instead. + this.hash = BaseHash.getIdentityHash(); if (DataStreamsTags.serviceNameOverride != null) { String val = DataStreamsTags.serviceNameOverride.get(); @@ -343,8 +347,25 @@ public DataStreamsTags( } } - // aggregation tags are 7-11: datasetName, datasetNamespace, isManual, group, consumerGroup this.aggregationHash = this.hash; + + // process tags and container-tags hash are decorative/volatile: they belong in the + // aggregation tier, not the primary pathway-identity hash. Applied independently of + // each other (unlike BaseHash.getBaseHash(), which is DBM-oriented and nests one under + // the other). + CharSequence processTags = ProcessTags.getTagsForSerialization(); + if (processTags != null) { + this.aggregationHash = + FNV64Hash.continueHash( + this.aggregationHash, processTags.toString(), FNV64Hash.Version.v1); + } + String containerTagsHash = BaseHash.getLastContainerTagsHash(); + if (containerTagsHash != null && !containerTagsHash.isEmpty()) { + this.aggregationHash = + FNV64Hash.continueHash(this.aggregationHash, containerTagsHash, FNV64Hash.Version.v1); + } + + // aggregation tags are 7-11: datasetName, datasetNamespace, isManual, group, consumerGroup for (int i = 7; i < 12; i++) { String tag = this.tagByIndex(i); if (tag != null) { diff --git a/internal-api/src/test/groovy/datadog/trace/api/BaseHashTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/BaseHashTest.groovy index 1deb7d97103..80b21667eaf 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/BaseHashTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/BaseHashTest.groovy @@ -108,6 +108,35 @@ class BaseHashTest extends DDSpecification { BaseHash.getBaseHash() == hashBefore } + def "Identity hash tracks service/env/primaryTag like base hash"() { + when: + BaseHash.recalcBaseHash(null) + def firstIdentityHash = BaseHash.getIdentityHash() + + injectSysConfig(SERVICE_NAME, "service-1") + BaseHash.recalcBaseHash(null) + def secondIdentityHash = BaseHash.getIdentityHash() + + then: + firstIdentityHash != secondIdentityHash + } + + def "Identity hash is unaffected by container tags hash or process tags"() { + when: + BaseHash.recalcBaseHash(null) + def baseIdentityHash = BaseHash.getIdentityHash() + + BaseHash.recalcBaseHash("some-container-tags-hash") + def withContainerTagsHash = BaseHash.getIdentityHash() + + ProcessTags.addTag("foo", "bar") + def withProcessTags = BaseHash.getIdentityHash() + + then: "DSM2-335: identity hash must not be perturbed by per-pod/per-rollout inputs" + baseIdentityHash == withContainerTagsHash + baseIdentityHash == withProcessTags + } + def "ContainerTagsHash used in hash calculation when provided"() { when: injectSysConfig(EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED, propagateTagsEnabled.toString()) diff --git a/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy index aa26cb9ebbb..93d1ac89ceb 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy @@ -1,11 +1,18 @@ package datadog.trace.api.datastreams import datadog.trace.api.BaseHash +import datadog.trace.api.Config +import datadog.trace.api.ProcessTags import spock.lang.Specification import java.nio.ByteBuffer class DataStreamsTagsTest extends Specification { + def cleanup() { + BaseHash.recalcBaseHash(null) + ProcessTags.reset(Config.get()) + } + def getTags(int idx) { return new DataStreamsTags("bus" + idx, DataStreamsTags.Direction.OUTBOUND, "exchange" + idx, "topic" + idx, "type" + idx, "subscription" + idx, "dataset_name" + idx, "dataset_namespace" + idx, true, "group" + idx, "consumer_group" + idx, true, @@ -80,7 +87,7 @@ class DataStreamsTagsTest extends Specification { DataStreamsTags.setServiceNameOverride(serviceName) def two = getTags(0) - BaseHash.updateBaseHash(12) + BaseHash.updateIdentityHash(12) def three = getTags(0) expect: @@ -222,6 +229,40 @@ class DataStreamsTagsTest extends Specification { base != withRoutingKey } + def 'test container tags hash does not fragment primary pathway hash (DSM2-335)'() { + setup: "simulate the Agent reporting the pod/container's tags hash at startup" + BaseHash.recalcBaseHash("container-tags-hash-1") + def base = getTags(0) + + when: "a rolling deploy changes the container-tags hash the Agent reports" + BaseHash.recalcBaseHash("container-tags-hash-2") + def afterRollingDeploy = getTags(0) + + then: "the primary pathway hash is unchanged, so block_on_hashes cardinality doesn't grow" + base.getHash() == afterRollingDeploy.getHash() + + and: "the aggregation/complete hashes do still reflect the container-tags hash change" + base.getAggregationHash() != afterRollingDeploy.getAggregationHash() + base != afterRollingDeploy + } + + def 'test process tags do not fragment primary pathway hash (DSM2-335)'() { + setup: + BaseHash.recalcBaseHash(null) + def base = getTags(0) + + when: "a process tag is added (e.g. cluster.name discovered after startup)" + ProcessTags.addTag("cluster.name", "new-cluster") + def withProcessTag = getTags(0) + + then: "the primary pathway hash is unchanged" + base.getHash() == withProcessTag.getHash() + + and: "the aggregation/complete hashes do still reflect the process tag change" + base.getAggregationHash() != withProcessTag.getAggregationHash() + base != withProcessTag + } + def 'test all three hash levels are different when appropriate tags change'() { setup: def base = new DataStreamsTags("bus", DataStreamsTags.Direction.OUTBOUND, null, "topic", From e11b5ba19492d1e584b7cd9a6b2b4744c5d1b835 Mon Sep 17 00:00:00 2001 From: Arjun Guha Date: Fri, 18 Sep 2026 17:15:58 -0400 Subject: [PATCH 2/9] Drop containerTagsHash/processTags from DSM hashing entirely 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 --- .../main/java/datadog/trace/api/BaseHash.java | 5 ---- .../api/datastreams/DataStreamsTags.java | 23 ++++++------------- .../datastreams/DataStreamsTagsTest.groovy | 20 +++++++--------- 3 files changed, 15 insertions(+), 33 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/BaseHash.java b/internal-api/src/main/java/datadog/trace/api/BaseHash.java index 6373f6738ec..1870942c7a0 100644 --- a/internal-api/src/main/java/datadog/trace/api/BaseHash.java +++ b/internal-api/src/main/java/datadog/trace/api/BaseHash.java @@ -49,11 +49,6 @@ public static long getIdentityHash() { return identityHash; } - /** The most recent container-tags hash reported by the Agent, or {@code null}/empty if none. */ - public static String getLastContainerTagsHash() { - return lastContainerTagsHash; - } - /** Test-only: lets tests set the identity hash without going through {@link Config}. */ public static void updateIdentityHash(long hash) { identityHash = hash; diff --git a/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java b/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java index 8b88724bfd8..c99395ac237 100644 --- a/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java +++ b/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java @@ -1,7 +1,6 @@ package datadog.trace.api.datastreams; import datadog.trace.api.BaseHash; -import datadog.trace.api.ProcessTags; import datadog.trace.util.FNV64Hash; import java.util.Objects; @@ -349,21 +348,13 @@ public DataStreamsTags( this.aggregationHash = this.hash; - // process tags and container-tags hash are decorative/volatile: they belong in the - // aggregation tier, not the primary pathway-identity hash. Applied independently of - // each other (unlike BaseHash.getBaseHash(), which is DBM-oriented and nests one under - // the other). - CharSequence processTags = ProcessTags.getTagsForSerialization(); - if (processTags != null) { - this.aggregationHash = - FNV64Hash.continueHash( - this.aggregationHash, processTags.toString(), FNV64Hash.Version.v1); - } - String containerTagsHash = BaseHash.getLastContainerTagsHash(); - if (containerTagsHash != null && !containerTagsHash.isEmpty()) { - this.aggregationHash = - FNV64Hash.continueHash(this.aggregationHash, containerTagsHash, FNV64Hash.Version.v1); - } + // DSM2-335: process tags and the agent-reported container-tags hash used to be folded in + // here (per-pod/per-process metadata inherited from DBM's BaseHash, see BaseHash.getBaseHash() + // and PR #9282). They're dropped entirely rather than moved to the aggregation tier: the + // backend never decodes them into discrete tags today, so they only added opaque cardinality + // with no user-visible benefit. @TODO tag a DSM backend owner to confirm there's no hidden + // reliance on aggregationHash/completeHash changing when these values change before this + // ships (see PR #9282 review thread, raphaelgavache's unanswered comment on this line). // aggregation tags are 7-11: datasetName, datasetNamespace, isManual, group, consumerGroup for (int i = 7; i < 12; i++) { diff --git a/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy index 93d1ac89ceb..5412565a394 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy @@ -229,7 +229,7 @@ class DataStreamsTagsTest extends Specification { base != withRoutingKey } - def 'test container tags hash does not fragment primary pathway hash (DSM2-335)'() { + def 'test container tags hash does not affect any hash tier (DSM2-335)'() { setup: "simulate the Agent reporting the pod/container's tags hash at startup" BaseHash.recalcBaseHash("container-tags-hash-1") def base = getTags(0) @@ -238,15 +238,13 @@ class DataStreamsTagsTest extends Specification { BaseHash.recalcBaseHash("container-tags-hash-2") def afterRollingDeploy = getTags(0) - then: "the primary pathway hash is unchanged, so block_on_hashes cardinality doesn't grow" + then: "no hash tier is affected - container-tags hash is dropped entirely from DSM" base.getHash() == afterRollingDeploy.getHash() - - and: "the aggregation/complete hashes do still reflect the container-tags hash change" - base.getAggregationHash() != afterRollingDeploy.getAggregationHash() - base != afterRollingDeploy + base.getAggregationHash() == afterRollingDeploy.getAggregationHash() + base == afterRollingDeploy } - def 'test process tags do not fragment primary pathway hash (DSM2-335)'() { + def 'test process tags do not affect any hash tier (DSM2-335)'() { setup: BaseHash.recalcBaseHash(null) def base = getTags(0) @@ -255,12 +253,10 @@ class DataStreamsTagsTest extends Specification { ProcessTags.addTag("cluster.name", "new-cluster") def withProcessTag = getTags(0) - then: "the primary pathway hash is unchanged" + then: "no hash tier is affected - process tags are dropped entirely from DSM" base.getHash() == withProcessTag.getHash() - - and: "the aggregation/complete hashes do still reflect the process tag change" - base.getAggregationHash() != withProcessTag.getAggregationHash() - base != withProcessTag + base.getAggregationHash() == withProcessTag.getAggregationHash() + base == withProcessTag } def 'test all three hash levels are different when appropriate tags change'() { From d123fcad847cef3c57e685ac51afabf2863662ff Mon Sep 17 00:00:00 2001 From: Arjun Guha Date: Fri, 18 Sep 2026 17:17:55 -0400 Subject: [PATCH 3/9] Simplify DSM2-335 removal comment Co-Authored-By: Claude Sonnet 5 --- .../java/datadog/trace/api/datastreams/DataStreamsTags.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java b/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java index c99395ac237..fccc9a407e1 100644 --- a/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java +++ b/internal-api/src/main/java/datadog/trace/api/datastreams/DataStreamsTags.java @@ -352,9 +352,7 @@ public DataStreamsTags( // here (per-pod/per-process metadata inherited from DBM's BaseHash, see BaseHash.getBaseHash() // and PR #9282). They're dropped entirely rather than moved to the aggregation tier: the // backend never decodes them into discrete tags today, so they only added opaque cardinality - // with no user-visible benefit. @TODO tag a DSM backend owner to confirm there's no hidden - // reliance on aggregationHash/completeHash changing when these values change before this - // ships (see PR #9282 review thread, raphaelgavache's unanswered comment on this line). + // with no user-visible benefit. // aggregation tags are 7-11: datasetName, datasetNamespace, isManual, group, consumerGroup for (int i = 7; i < 12; i++) { From e9736918a0c5e560ee2bbe31900733e604264fba Mon Sep 17 00:00:00 2001 From: Arjun Guha Date: Mon, 21 Sep 2026 16:36:41 -0400 Subject: [PATCH 4/9] Move new DSM2-335 test coverage to JUnit 5 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 --- .../datadog/trace/api/BaseHashTest.groovy | 29 -------- .../datastreams/DataStreamsTagsTest.groovy | 30 --------- .../trace/api/BaseHashIdentityTest.java | 54 +++++++++++++++ ...taStreamsTagsContainerProcessTagsTest.java | 67 +++++++++++++++++++ 4 files changed, 121 insertions(+), 59 deletions(-) create mode 100644 internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java create mode 100644 internal-api/src/test/java/datadog/trace/api/datastreams/DataStreamsTagsContainerProcessTagsTest.java diff --git a/internal-api/src/test/groovy/datadog/trace/api/BaseHashTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/BaseHashTest.groovy index 80b21667eaf..1deb7d97103 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/BaseHashTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/BaseHashTest.groovy @@ -108,35 +108,6 @@ class BaseHashTest extends DDSpecification { BaseHash.getBaseHash() == hashBefore } - def "Identity hash tracks service/env/primaryTag like base hash"() { - when: - BaseHash.recalcBaseHash(null) - def firstIdentityHash = BaseHash.getIdentityHash() - - injectSysConfig(SERVICE_NAME, "service-1") - BaseHash.recalcBaseHash(null) - def secondIdentityHash = BaseHash.getIdentityHash() - - then: - firstIdentityHash != secondIdentityHash - } - - def "Identity hash is unaffected by container tags hash or process tags"() { - when: - BaseHash.recalcBaseHash(null) - def baseIdentityHash = BaseHash.getIdentityHash() - - BaseHash.recalcBaseHash("some-container-tags-hash") - def withContainerTagsHash = BaseHash.getIdentityHash() - - ProcessTags.addTag("foo", "bar") - def withProcessTags = BaseHash.getIdentityHash() - - then: "DSM2-335: identity hash must not be perturbed by per-pod/per-rollout inputs" - baseIdentityHash == withContainerTagsHash - baseIdentityHash == withProcessTags - } - def "ContainerTagsHash used in hash calculation when provided"() { when: injectSysConfig(EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED, propagateTagsEnabled.toString()) diff --git a/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy index 5412565a394..16dc93ad321 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/datastreams/DataStreamsTagsTest.groovy @@ -229,36 +229,6 @@ class DataStreamsTagsTest extends Specification { base != withRoutingKey } - def 'test container tags hash does not affect any hash tier (DSM2-335)'() { - setup: "simulate the Agent reporting the pod/container's tags hash at startup" - BaseHash.recalcBaseHash("container-tags-hash-1") - def base = getTags(0) - - when: "a rolling deploy changes the container-tags hash the Agent reports" - BaseHash.recalcBaseHash("container-tags-hash-2") - def afterRollingDeploy = getTags(0) - - then: "no hash tier is affected - container-tags hash is dropped entirely from DSM" - base.getHash() == afterRollingDeploy.getHash() - base.getAggregationHash() == afterRollingDeploy.getAggregationHash() - base == afterRollingDeploy - } - - def 'test process tags do not affect any hash tier (DSM2-335)'() { - setup: - BaseHash.recalcBaseHash(null) - def base = getTags(0) - - when: "a process tag is added (e.g. cluster.name discovered after startup)" - ProcessTags.addTag("cluster.name", "new-cluster") - def withProcessTag = getTags(0) - - then: "no hash tier is affected - process tags are dropped entirely from DSM" - base.getHash() == withProcessTag.getHash() - base.getAggregationHash() == withProcessTag.getAggregationHash() - base == withProcessTag - } - def 'test all three hash levels are different when appropriate tags change'() { setup: def base = new DataStreamsTags("bus", DataStreamsTags.Direction.OUTBOUND, null, "topic", diff --git a/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java b/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java new file mode 100644 index 00000000000..a162f885498 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java @@ -0,0 +1,54 @@ +package datadog.trace.api; + +import static datadog.trace.api.config.GeneralConfig.SERVICE_NAME; +import static datadog.trace.test.junit.utils.config.WithConfigExtension.injectSysConfig; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +import datadog.trace.test.junit.utils.config.WithConfigExtension; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(WithConfigExtension.class) +class BaseHashIdentityTest { + + @BeforeEach + void setup() { + ProcessTags.reset(); + } + + @AfterEach + void cleanup() { + ProcessTags.reset(); + } + + @Test + void identityHashTracksServiceEnvPrimaryTagLikeBaseHash() { + BaseHash.recalcBaseHash(null); + long firstIdentityHash = BaseHash.getIdentityHash(); + + injectSysConfig(SERVICE_NAME, "service-1"); + BaseHash.recalcBaseHash(null); + long secondIdentityHash = BaseHash.getIdentityHash(); + + assertNotEquals(firstIdentityHash, secondIdentityHash); + } + + @Test + void identityHashIsUnaffectedByContainerTagsHashOrProcessTags() { + BaseHash.recalcBaseHash(null); + long baseIdentityHash = BaseHash.getIdentityHash(); + + BaseHash.recalcBaseHash("some-container-tags-hash"); + long withContainerTagsHash = BaseHash.getIdentityHash(); + + ProcessTags.addTag("foo", "bar"); + long withProcessTags = BaseHash.getIdentityHash(); + + // DSM2-335: identity hash must not be perturbed by per-pod/per-rollout inputs + assertEquals(baseIdentityHash, withContainerTagsHash); + assertEquals(baseIdentityHash, withProcessTags); + } +} diff --git a/internal-api/src/test/java/datadog/trace/api/datastreams/DataStreamsTagsContainerProcessTagsTest.java b/internal-api/src/test/java/datadog/trace/api/datastreams/DataStreamsTagsContainerProcessTagsTest.java new file mode 100644 index 00000000000..cc01a741ce1 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/datastreams/DataStreamsTagsContainerProcessTagsTest.java @@ -0,0 +1,67 @@ +package datadog.trace.api.datastreams; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import datadog.trace.api.BaseHash; +import datadog.trace.api.Config; +import datadog.trace.api.ProcessTags; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +class DataStreamsTagsContainerProcessTagsTest { + + @AfterEach + void cleanup() { + BaseHash.recalcBaseHash(null); + ProcessTags.reset(Config.get()); + } + + private static DataStreamsTags getTags(int idx) { + return new DataStreamsTags( + "bus" + idx, + DataStreamsTags.Direction.OUTBOUND, + "exchange" + idx, + "topic" + idx, + "type" + idx, + "subscription" + idx, + "dataset_name" + idx, + "dataset_namespace" + idx, + true, + "group" + idx, + "consumer_group" + idx, + true, + "kafka_cluster_id" + idx, + "partition" + idx); + } + + @Test + void containerTagsHashDoesNotAffectAnyHashTier() { + // simulate the Agent reporting the pod/container's tags hash at startup + BaseHash.recalcBaseHash("container-tags-hash-1"); + DataStreamsTags base = getTags(0); + + // a rolling deploy changes the container-tags hash the Agent reports + BaseHash.recalcBaseHash("container-tags-hash-2"); + DataStreamsTags afterRollingDeploy = getTags(0); + + // DSM2-335: no hash tier is affected - container-tags hash is dropped entirely from DSM + assertEquals(base.getHash(), afterRollingDeploy.getHash()); + assertEquals(base.getAggregationHash(), afterRollingDeploy.getAggregationHash()); + assertEquals(base, afterRollingDeploy); + } + + @Test + void processTagsDoNotAffectAnyHashTier() { + BaseHash.recalcBaseHash(null); + DataStreamsTags base = getTags(0); + + // a process tag is added (e.g. cluster.name discovered after startup) + ProcessTags.addTag("cluster.name", "new-cluster"); + DataStreamsTags withProcessTag = getTags(0); + + // DSM2-335: no hash tier is affected - process tags are dropped entirely from DSM + assertEquals(base.getHash(), withProcessTag.getHash()); + assertEquals(base.getAggregationHash(), withProcessTag.getAggregationHash()); + assertEquals(base, withProcessTag); + } +} From 47634e92fcbcd512e43c142befcc3bedfc23184e Mon Sep 17 00:00:00 2001 From: Arjun Guha Date: Mon, 21 Sep 2026 16:46:57 -0400 Subject: [PATCH 5/9] Add braces around single-line if in BaseHash.calcIdentity Co-Authored-By: Claude Sonnet 5 --- internal-api/src/main/java/datadog/trace/api/BaseHash.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/api/BaseHash.java b/internal-api/src/main/java/datadog/trace/api/BaseHash.java index 1870942c7a0..ffb7bfa5a7e 100644 --- a/internal-api/src/main/java/datadog/trace/api/BaseHash.java +++ b/internal-api/src/main/java/datadog/trace/api/BaseHash.java @@ -66,7 +66,9 @@ public static long calc(String containerTagsHash) { private static long calcIdentity(CharSequence serviceName, CharSequence env, String primaryTag) { long hash = FNV64Hash.generateHash(serviceName.toString(), FNV64Hash.Version.v1); hash = FNV64Hash.continueHash(hash, env.toString(), FNV64Hash.Version.v1); - if (primaryTag != null) hash = FNV64Hash.continueHash(hash, primaryTag, FNV64Hash.Version.v1); + if (primaryTag != null) { + hash = FNV64Hash.continueHash(hash, primaryTag, FNV64Hash.Version.v1); + } return hash; } From 8527d0d47aa03dc71fcedd06cbcfb5d4b345e7d5 Mon Sep 17 00:00:00 2001 From: Arjun Guha Date: Mon, 21 Sep 2026 17:46:08 -0400 Subject: [PATCH 6/9] Calculate BaseHash's identity hash once instead of on every recalc 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 --- .../main/java/datadog/trace/api/BaseHash.java | 12 ++++++----- .../trace/api/BaseHashIdentityTest.java | 21 +++++++++---------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/BaseHash.java b/internal-api/src/main/java/datadog/trace/api/BaseHash.java index ffb7bfa5a7e..84ca5951fa0 100644 --- a/internal-api/src/main/java/datadog/trace/api/BaseHash.java +++ b/internal-api/src/main/java/datadog/trace/api/BaseHash.java @@ -6,7 +6,12 @@ public final class BaseHash { private static volatile long baseHash; private static volatile String baseHashStr; private static volatile String lastContainerTagsHash; - private static volatile long identityHash; + + // service/env/primaryTag are fixed for the JVM's lifetime once Config is built, so this only + // needs to be calculated once rather than every time recalcBaseHash()/recalc() runs. + private static volatile long identityHash = + calcIdentity( + Config.get().getServiceName(), Config.get().getEnv(), Config.get().getPrimaryTag()); private BaseHash() {} @@ -21,9 +26,6 @@ static void recalcBaseHash() { private static void recalc() { updateBaseHash(calc(lastContainerTagsHash)); - identityHash = - calcIdentity( - Config.get().getServiceName(), Config.get().getEnv(), Config.get().getPrimaryTag()); } public static void updateBaseHash(long hash) { @@ -63,7 +65,7 @@ public static long calc(String containerTagsHash) { containerTagsHash); } - private static long calcIdentity(CharSequence serviceName, CharSequence env, String primaryTag) { + static long calcIdentity(CharSequence serviceName, CharSequence env, String primaryTag) { long hash = FNV64Hash.generateHash(serviceName.toString(), FNV64Hash.Version.v1); hash = FNV64Hash.continueHash(hash, env.toString(), FNV64Hash.Version.v1); if (primaryTag != null) { diff --git a/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java b/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java index a162f885498..8527e45d3f3 100644 --- a/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java +++ b/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java @@ -1,7 +1,5 @@ package datadog.trace.api; -import static datadog.trace.api.config.GeneralConfig.SERVICE_NAME; -import static datadog.trace.test.junit.utils.config.WithConfigExtension.injectSysConfig; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -25,15 +23,16 @@ void cleanup() { } @Test - void identityHashTracksServiceEnvPrimaryTagLikeBaseHash() { - BaseHash.recalcBaseHash(null); - long firstIdentityHash = BaseHash.getIdentityHash(); - - injectSysConfig(SERVICE_NAME, "service-1"); - BaseHash.recalcBaseHash(null); - long secondIdentityHash = BaseHash.getIdentityHash(); - - assertNotEquals(firstIdentityHash, secondIdentityHash); + void identityHashDependsOnServiceEnvAndPrimaryTag() { + // identityHash is calculated once (service/env/primaryTag are fixed for the JVM's + // lifetime), so this exercises the underlying hashing function directly rather than + // via Config + recalcBaseHash. + long base = BaseHash.calcIdentity("service", "env", "region-1"); + + assertNotEquals(base, BaseHash.calcIdentity("service-2", "env", "region-1")); + assertNotEquals(base, BaseHash.calcIdentity("service", "env-2", "region-1")); + assertNotEquals(base, BaseHash.calcIdentity("service", "env", "region-2")); + assertEquals(base, BaseHash.calcIdentity("service", "env", "region-1")); } @Test From 2c92565ee3ac64e7bfca7831e33fd7023976d1d4 Mon Sep 17 00:00:00 2001 From: Arjun Guha Date: Mon, 21 Sep 2026 20:37:48 -0400 Subject: [PATCH 7/9] Ensure identityHash reflects settled Config before first outbound checkpoint 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 --- .../DefaultDataStreamsMonitoring.java | 5 +++ .../main/java/datadog/trace/api/BaseHash.java | 33 +++++++++++++++++++ .../trace/api/BaseHashIdentityTest.java | 17 ++++++++++ 3 files changed, 55 insertions(+) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultDataStreamsMonitoring.java b/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultDataStreamsMonitoring.java index 61b403a22e4..cddce3c8edf 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultDataStreamsMonitoring.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultDataStreamsMonitoring.java @@ -17,6 +17,7 @@ import datadog.communication.ddagent.DDAgentFeaturesDiscovery; import datadog.communication.ddagent.SharedCommunicationObjects; import datadog.context.propagation.Propagator; +import datadog.trace.api.BaseHash; import datadog.trace.api.Config; import datadog.trace.api.TraceConfig; import datadog.trace.api.datastreams.Backlog; @@ -357,6 +358,10 @@ public void setProduceCheckpoint( log.warn("SetProduceCheckpoint is called with no active span"); return; } + // BaseHash.identityHash may have been calculated prematurely as a side effect of unrelated + // static initialization (e.g. DataStreamsTags.EMPTY); give it one chance to recompute from a + // settled Config before it's baked into this outbound checkpoint's tags. + BaseHash.ensureIdentityHash(); DataStreamsTags tags; if (manualCheckpoint) { tags = createManual(type, OUTBOUND, target); diff --git a/internal-api/src/main/java/datadog/trace/api/BaseHash.java b/internal-api/src/main/java/datadog/trace/api/BaseHash.java index 84ca5951fa0..b897219aa82 100644 --- a/internal-api/src/main/java/datadog/trace/api/BaseHash.java +++ b/internal-api/src/main/java/datadog/trace/api/BaseHash.java @@ -13,6 +13,12 @@ public final class BaseHash { calcIdentity( Config.get().getServiceName(), Config.get().getEnv(), Config.get().getPrimaryTag()); + // Guards ensureIdentityHash(): this class can be loaded as a side effect of unrelated static + // initialization (e.g. DataStreamsTags.EMPTY) well before Config's values have settled, so + // identityHash's field initializer above may capture a premature snapshot. ensureIdentityHash() + // gets one chance to recalculate it from a (hopefully by-then-settled) Config. + private static volatile boolean identityHashEnsured; + private BaseHash() {} public static void recalcBaseHash(String containerTagsHash) { @@ -51,11 +57,38 @@ public static long getIdentityHash() { return identityHash; } + /** + * Recalculates {@link #identityHash} from the current {@link Config}, but only the first time + * it's called. Callers should invoke this right before the first outbound DSM checkpoint is set, + * so that if {@link #identityHash}'s field initializer ran prematurely (before {@link Config}'s + * values settled), it gets one chance to pick up the settled values before any pathway hash is + * actually reported. + */ + public static void ensureIdentityHash() { + if (!identityHashEnsured) { + synchronized (BaseHash.class) { + if (!identityHashEnsured) { + identityHash = + calcIdentity( + Config.get().getServiceName(), + Config.get().getEnv(), + Config.get().getPrimaryTag()); + identityHashEnsured = true; + } + } + } + } + /** Test-only: lets tests set the identity hash without going through {@link Config}. */ public static void updateIdentityHash(long hash) { identityHash = hash; } + /** Test-only: lets tests re-exercise {@link #ensureIdentityHash()}'s one-time guard. */ + static void resetIdentityHashEnsuredForTesting() { + identityHashEnsured = false; + } + public static long calc(String containerTagsHash) { return calc( Config.get().getServiceName(), diff --git a/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java b/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java index 8527e45d3f3..e2c3399ad16 100644 --- a/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java +++ b/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java @@ -20,6 +20,7 @@ void setup() { @AfterEach void cleanup() { ProcessTags.reset(); + BaseHash.resetIdentityHashEnsuredForTesting(); } @Test @@ -50,4 +51,20 @@ void identityHashIsUnaffectedByContainerTagsHashOrProcessTags() { assertEquals(baseIdentityHash, withContainerTagsHash); assertEquals(baseIdentityHash, withProcessTags); } + + @Test + void ensureIdentityHashRecalculatesFromConfigOnlyOnce() { + // simulate identityHash's field initializer having captured a stale/premature snapshot + BaseHash.updateIdentityHash(0L); + + BaseHash.ensureIdentityHash(); + long ensured = BaseHash.getIdentityHash(); + assertNotEquals(0L, ensured); + + // a later caller mutating the hash directly (e.g. via BaseHash.updateIdentityHash) must not + // be clobbered by a second ensureIdentityHash() call - it only ever recalculates once + BaseHash.updateIdentityHash(42L); + BaseHash.ensureIdentityHash(); + assertEquals(42L, BaseHash.getIdentityHash()); + } } From d43081e8c5e23be9e365c64efe24349d0168f44f Mon Sep 17 00:00:00 2001 From: Arjun Guha Date: Mon, 21 Sep 2026 22:22:10 -0400 Subject: [PATCH 8/9] Drop synchronization from BaseHash.ensureIdentityHash 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 --- .../main/java/datadog/trace/api/BaseHash.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/BaseHash.java b/internal-api/src/main/java/datadog/trace/api/BaseHash.java index b897219aa82..cacb5d92b95 100644 --- a/internal-api/src/main/java/datadog/trace/api/BaseHash.java +++ b/internal-api/src/main/java/datadog/trace/api/BaseHash.java @@ -63,19 +63,17 @@ public static long getIdentityHash() { * so that if {@link #identityHash}'s field initializer ran prematurely (before {@link Config}'s * values settled), it gets one chance to pick up the settled values before any pathway hash is * actually reported. + * + *

Deliberately unsynchronized: concurrent callers all recompute from the same (by-then + * settled) Config, so a race just means a few redundant, identical writes rather than a + * correctness issue. */ public static void ensureIdentityHash() { if (!identityHashEnsured) { - synchronized (BaseHash.class) { - if (!identityHashEnsured) { - identityHash = - calcIdentity( - Config.get().getServiceName(), - Config.get().getEnv(), - Config.get().getPrimaryTag()); - identityHashEnsured = true; - } - } + identityHash = + calcIdentity( + Config.get().getServiceName(), Config.get().getEnv(), Config.get().getPrimaryTag()); + identityHashEnsured = true; } } From ed4f05a55038c685c795f7f809a77726037dae8f Mon Sep 17 00:00:00 2001 From: Arjun Guha Date: Mon, 21 Sep 2026 22:45:47 -0400 Subject: [PATCH 9/9] Simplify identityHash lazy init: drop the ensured flag, use 0 as sentinel 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 --- .../DefaultDataStreamsMonitoring.java | 5 --- .../main/java/datadog/trace/api/BaseHash.java | 42 ++++--------------- .../trace/api/BaseHashIdentityTest.java | 15 +++---- 3 files changed, 15 insertions(+), 47 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultDataStreamsMonitoring.java b/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultDataStreamsMonitoring.java index cddce3c8edf..61b403a22e4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultDataStreamsMonitoring.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultDataStreamsMonitoring.java @@ -17,7 +17,6 @@ import datadog.communication.ddagent.DDAgentFeaturesDiscovery; import datadog.communication.ddagent.SharedCommunicationObjects; import datadog.context.propagation.Propagator; -import datadog.trace.api.BaseHash; import datadog.trace.api.Config; import datadog.trace.api.TraceConfig; import datadog.trace.api.datastreams.Backlog; @@ -358,10 +357,6 @@ public void setProduceCheckpoint( log.warn("SetProduceCheckpoint is called with no active span"); return; } - // BaseHash.identityHash may have been calculated prematurely as a side effect of unrelated - // static initialization (e.g. DataStreamsTags.EMPTY); give it one chance to recompute from a - // settled Config before it's baked into this outbound checkpoint's tags. - BaseHash.ensureIdentityHash(); DataStreamsTags tags; if (manualCheckpoint) { tags = createManual(type, OUTBOUND, target); diff --git a/internal-api/src/main/java/datadog/trace/api/BaseHash.java b/internal-api/src/main/java/datadog/trace/api/BaseHash.java index cacb5d92b95..73910b8867f 100644 --- a/internal-api/src/main/java/datadog/trace/api/BaseHash.java +++ b/internal-api/src/main/java/datadog/trace/api/BaseHash.java @@ -7,17 +7,11 @@ public final class BaseHash { private static volatile String baseHashStr; private static volatile String lastContainerTagsHash; - // service/env/primaryTag are fixed for the JVM's lifetime once Config is built, so this only - // needs to be calculated once rather than every time recalcBaseHash()/recalc() runs. - private static volatile long identityHash = - calcIdentity( - Config.get().getServiceName(), Config.get().getEnv(), Config.get().getPrimaryTag()); - - // Guards ensureIdentityHash(): this class can be loaded as a side effect of unrelated static - // initialization (e.g. DataStreamsTags.EMPTY) well before Config's values have settled, so - // identityHash's field initializer above may capture a premature snapshot. ensureIdentityHash() - // gets one chance to recalculate it from a (hopefully by-then-settled) Config. - private static volatile boolean identityHashEnsured; + // 0 means "not yet computed". service/env/primaryTag are fixed for the JVM's lifetime once + // Config is built, so this only needs to be calculated once, lazily, on first read - computing + // it eagerly in a field initializer would risk capturing a premature Config snapshot if this + // class gets loaded (e.g. via DataStreamsTags.EMPTY) before Config settles. + private static volatile long identityHash; private BaseHash() {} @@ -54,27 +48,14 @@ public static String getBaseHashStr() { * so it's safe to use as the seed for DSM's cardinality-sensitive pathway hash. */ public static long getIdentityHash() { - return identityHash; - } - - /** - * Recalculates {@link #identityHash} from the current {@link Config}, but only the first time - * it's called. Callers should invoke this right before the first outbound DSM checkpoint is set, - * so that if {@link #identityHash}'s field initializer ran prematurely (before {@link Config}'s - * values settled), it gets one chance to pick up the settled values before any pathway hash is - * actually reported. - * - *

Deliberately unsynchronized: concurrent callers all recompute from the same (by-then - * settled) Config, so a race just means a few redundant, identical writes rather than a - * correctness issue. - */ - public static void ensureIdentityHash() { - if (!identityHashEnsured) { + if (identityHash == 0) { + // Deliberately unsynchronized: concurrent callers all recompute from the same Config, so a + // race just means a few redundant, identical writes rather than a correctness issue. identityHash = calcIdentity( Config.get().getServiceName(), Config.get().getEnv(), Config.get().getPrimaryTag()); - identityHashEnsured = true; } + return identityHash; } /** Test-only: lets tests set the identity hash without going through {@link Config}. */ @@ -82,11 +63,6 @@ public static void updateIdentityHash(long hash) { identityHash = hash; } - /** Test-only: lets tests re-exercise {@link #ensureIdentityHash()}'s one-time guard. */ - static void resetIdentityHashEnsuredForTesting() { - identityHashEnsured = false; - } - public static long calc(String containerTagsHash) { return calc( Config.get().getServiceName(), diff --git a/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java b/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java index e2c3399ad16..4ed33972f9a 100644 --- a/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java +++ b/internal-api/src/test/java/datadog/trace/api/BaseHashIdentityTest.java @@ -20,7 +20,6 @@ void setup() { @AfterEach void cleanup() { ProcessTags.reset(); - BaseHash.resetIdentityHashEnsuredForTesting(); } @Test @@ -53,18 +52,16 @@ void identityHashIsUnaffectedByContainerTagsHashOrProcessTags() { } @Test - void ensureIdentityHashRecalculatesFromConfigOnlyOnce() { - // simulate identityHash's field initializer having captured a stale/premature snapshot + void getIdentityHashRecalculatesFromConfigWhenUnset() { + // 0 means "not yet computed" - simulate that state, e.g. before this class is ever touched BaseHash.updateIdentityHash(0L); - BaseHash.ensureIdentityHash(); - long ensured = BaseHash.getIdentityHash(); - assertNotEquals(0L, ensured); + long recalculated = BaseHash.getIdentityHash(); + assertNotEquals(0L, recalculated); - // a later caller mutating the hash directly (e.g. via BaseHash.updateIdentityHash) must not - // be clobbered by a second ensureIdentityHash() call - it only ever recalculates once + // 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); - BaseHash.ensureIdentityHash(); assertEquals(42L, BaseHash.getIdentityHash()); } }