From 49f7a64bce4dfd17402adefeb835ff9bb4963ef8 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 24 Jul 2026 12:17:15 -0400 Subject: [PATCH 1/4] Emit additional metric tags on the OTLP stats export path The native SerializingMetricWriter path already carries user-configured span-derived additional metric tags (DD_TRACE_STATS_ADDITIONAL_TAGS), but the OTLP export path dropped them twice: ClientStatsAggregator built the OTLP writer through the schema-less (EMPTY) constructor, and OtlpStatsMetricWriter never read entry.getAdditionalTags(). Route the OTLP aggregator ctor through the schema-carrying ctor (capture is already unconditional once the schema is real) and split each packed "key:value" additional tag into a plain OTLP string attribute keyed by the tag name, in both default and otel-semantics modes. Co-Authored-By: Claude Opus 4.8 --- .../common/metrics/ClientStatsAggregator.java | 15 ++- .../otlp/metrics/OtlpStatsMetricWriter.java | 20 ++++ .../metrics/AggregateEntryTestUtils.java | 46 ++++++++- .../metrics/OtlpStatsMetricWriterTest.java | 95 +++++++++++++++++++ 4 files changed, 167 insertions(+), 9 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/ClientStatsAggregator.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/ClientStatsAggregator.java index c5781b7220c..8a0c69cbed0 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/ClientStatsAggregator.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/ClientStatsAggregator.java @@ -133,11 +133,7 @@ public ClientStatsAggregator( this( config.getWellKnownTags(), config.getMetricsIgnoredResources(), - AdditionalTagsSchema.from( - config.getTraceStatsAdditionalTags(), - config.getTraceStatsCardinalityLimit( - "additional_tags", MetricCardinalityLimits.ADDITIONAL_TAG_VALUE), - MetricCardinalityLimits.USE_BLOCKED_SENTINEL), + additionalTagsSchemaFrom(config), sharedCommunicationObjects.featuresDiscovery(config), healthMetrics, new OkHttpSink( @@ -165,6 +161,7 @@ public ClientStatsAggregator( OtlpStatsMetricWriter metricWriter) { this( config.getMetricsIgnoredResources(), + additionalTagsSchemaFrom(config), sharedCommunicationObjects.featuresDiscovery(config), healthMetrics, NoOpSink.INSTANCE, @@ -176,6 +173,14 @@ public ClientStatsAggregator( true); } + private static AdditionalTagsSchema additionalTagsSchemaFrom(Config config) { + return AdditionalTagsSchema.from( + config.getTraceStatsAdditionalTags(), + config.getTraceStatsCardinalityLimit( + "additional_tags", MetricCardinalityLimits.ADDITIONAL_TAG_VALUE), + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + } + ClientStatsAggregator( WellKnownTags wellKnownTags, Set ignoredResources, diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java index a5a21302877..60598b80c21 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java @@ -228,6 +228,13 @@ private void emitDataPointAttributes( if (entry.hasGrpcStatusCode()) { emitStringAttribute(metric, RPC_RESPONSE_STATUS_CODE, entry.getGrpcStatusCode()); } + // Additional metric tags: user-configured span-derived dimensions, carried as packed + // "key:value" UTF8 strings in schema order. Emitted in both modes as plain OTLP string + // attributes keyed by the tag name. NOTE: the attribute-key representation (raw tag name vs a + // datadog.* namespace) is an open cross-team question with the OTLP/agent side -- see the PR. + for (UTF8BytesString additionalTag : entry.getAdditionalTags()) { + emitAdditionalTag(metric, additionalTag); + } // Default (Datadog) mode: emit datadog.* per-point attributes if (!otelSemanticsMode) { emitStringAttribute(metric, DATADOG_OPERATION_NAME, entry.getOperationName()); @@ -239,6 +246,19 @@ private void emitDataPointAttributes( } } + // Splits a packed "key:value" additional-tag string at the first ':' (keys cannot contain ':', + // values may) and emits it as an OTLP string attribute. Skips malformed slots (no ':', empty key + // or empty value) defensively. + private static void emitAdditionalTag(OtlpMetricVisitor metric, UTF8BytesString additionalTag) { + String packed = additionalTag.toString(); + int separator = packed.indexOf(':'); + if (separator <= 0 || separator == packed.length() - 1) { + return; + } + metric.visitAttribute( + STRING_ATTRIBUTE, packed.substring(0, separator), packed.substring(separator + 1)); + } + // accepts both String literals and UTF8BytesString (both CharSequence); skips null values private static void emitStringAttribute( OtlpMetricVisitor metric, String key, @Nullable CharSequence value) { diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java index 76c72c240e2..29adc3a6d91 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java @@ -45,6 +45,43 @@ public static AggregateEntry of( @Nullable CharSequence httpMethod, @Nullable CharSequence httpEndpoint, @Nullable CharSequence grpcStatusCode) { + return of( + resource, + service, + operationName, + serviceSource, + type, + httpStatusCode, + synthetic, + traceRoot, + spanKind, + peerTags, + httpMethod, + httpEndpoint, + grpcStatusCode, + null); + } + + /** + * Same as {@link #of} but also carries pre-packed {@code "key:value"} additional metric tags (in + * schema order), letting the OTLP/serializing writer tests exercise the additional-tags path + * without driving a full {@code AggregateTable}/{@code AdditionalTagsSchema} canonicalization. + */ + public static AggregateEntry of( + CharSequence resource, + CharSequence service, + CharSequence operationName, + @Nullable CharSequence serviceSource, + CharSequence type, + int httpStatusCode, + boolean synthetic, + boolean traceRoot, + CharSequence spanKind, + @Nullable List peerTags, + @Nullable CharSequence httpMethod, + @Nullable CharSequence httpEndpoint, + @Nullable CharSequence grpcStatusCode, + @Nullable UTF8BytesString[] additionalTags) { UTF8BytesString resourceUtf = AggregateEntry.createUtf8(resource); UTF8BytesString serviceUtf = AggregateEntry.createUtf8(service); UTF8BytesString operationNameUtf = AggregateEntry.createUtf8(operationName); @@ -56,7 +93,8 @@ public static AggregateEntry of( UTF8BytesString grpcUtf = AggregateEntry.createUtf8(grpcStatusCode); List peerTagsList = peerTags == null ? Collections.emptyList() : peerTags; UTF8BytesString[] peerTagsArr = peerTagsList.toArray(new UTF8BytesString[0]); - UTF8BytesString[] emptyAdditional = new UTF8BytesString[0]; + UTF8BytesString[] additionalTagsArr = + additionalTags == null ? new UTF8BytesString[0] : additionalTags; long keyHash = AggregateEntry.hashOf( resourceUtf, @@ -73,8 +111,8 @@ public static AggregateEntry of( traceRoot, peerTagsArr, peerTagsArr.length, - emptyAdditional, - 0); + additionalTagsArr, + additionalTagsArr.length); return new AggregateEntry( keyHash, resourceUtf, @@ -90,7 +128,7 @@ public static AggregateEntry of( synthetic, traceRoot, peerTagsList, - emptyAdditional); + additionalTagsArr); } /** diff --git a/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java b/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java index 49e252459ac..17e80c0ce72 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java @@ -13,6 +13,7 @@ import com.google.protobuf.WireFormat; import datadog.metrics.api.Histograms; import datadog.metrics.impl.DDSketchHistograms; +import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.common.metrics.AggregateEntry; import datadog.trace.common.metrics.AggregateEntryTestUtils; import datadog.trace.common.writer.RemoteApi; @@ -427,6 +428,100 @@ void httpAndGrpcAttributesAppearOnlyWhenSet() throws IOException { assertFalse(bareAttrs.containsKey("rpc.response.status_code")); } + @Test + void additionalMetricTagsEmittedAsStringAttributes() throws IOException { + // Additional tags arrive on the entry pre-packed as "key:value" UTF8 strings in schema order; + // the writer splits each at the first ':' and emits it as a plain OTLP string attribute keyed + // by the tag name, in both semantics modes. + AggregateEntry e = + AggregateEntryTestUtils.of( + "GET /users", + "web", + "servlet.request", + null, + "web", + 0, + false, + true, + "server", + null, + null, + null, + null, + new UTF8BytesString[] { + UTF8BytesString.create("region:us-east-1"), + UTF8BytesString.create("tenant_id:acme:corp") + }); + AggregateEntryTestUtils.recordOk(e, SECONDS.toNanos(1)); + + Map attrs = writeAndDecode(false, e).dataPoints.get(0).attributes; + assertEquals("us-east-1", attrs.get("region")); + // value may itself contain ':' — only the first ':' separates key from value + assertEquals("acme:corp", attrs.get("tenant_id")); + } + + @Test + void additionalMetricTagsEmittedInOtelSemanticsMode() throws IOException { + // Unlike datadog.* attributes, additional tags are user-configured dimensions and are emitted + // in otel-semantics mode too. + AggregateEntry e = + AggregateEntryTestUtils.of( + "GET /users", + "web", + "servlet.request", + null, + "web", + 0, + false, + true, + "server", + null, + null, + null, + null, + new UTF8BytesString[] {UTF8BytesString.create("region:us-east-1")}); + AggregateEntryTestUtils.recordOk(e, SECONDS.toNanos(1)); + + Map attrs = writeAndDecode(true, e).dataPoints.get(0).attributes; + assertEquals("us-east-1", attrs.get("region")); + assertFalse( + attrs.containsKey("datadog.operation.name"), "datadog.* still absent in otel-semantics"); + } + + @Test + void malformedAdditionalTagsAreSkipped() throws IOException { + // Defensive: a slot with no ':', an empty key, or an empty value is dropped rather than emitted + // as a malformed attribute. + AggregateEntry e = + AggregateEntryTestUtils.of( + "GET /users", + "web", + "servlet.request", + null, + "web", + 0, + false, + true, + "server", + null, + null, + null, + null, + new UTF8BytesString[] { + UTF8BytesString.create("noseparator"), + UTF8BytesString.create(":emptykey"), + UTF8BytesString.create("emptyvalue:"), + UTF8BytesString.create("region:us-east-1") + }); + AggregateEntryTestUtils.recordOk(e, SECONDS.toNanos(1)); + + Map attrs = writeAndDecode(false, e).dataPoints.get(0).attributes; + assertEquals("us-east-1", attrs.get("region"), "well-formed tag still emitted"); + assertFalse(attrs.containsKey("noseparator"), "no-separator slot skipped"); + assertFalse(attrs.containsKey(""), "empty-key slot skipped"); + assertFalse(attrs.containsKey("emptyvalue"), "empty-value slot skipped"); + } + @Test void serviceNameEmittedOnlyForNonDefaultService() throws IOException { CapturingSender sender = new CapturingSender(); From ad8e42ad61130e518ce14357e55f78fe10fd0d9c Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 27 Jul 2026 12:22:06 -0400 Subject: [PATCH 2/4] Emit empty additional-tag values on the OTLP stats export path Aggregation treats an explicitly-empty configured tag ("key:") as a distinct dimension from an absent one, so it forms a separate aggregate row. The OTLP writer dropped the empty-value slot, which meant an absent-tag row and an empty-value row exported identical attribute sets -- two separately-aggregated points indistinguishable to the backend. Emit the empty value as key="" so the OTLP attributes stay faithful to the aggregate key; only truly-malformed slots (no ':' or empty key) are still skipped. Co-Authored-By: Claude Opus 4.8 --- .../trace/core/otlp/metrics/OtlpStatsMetricWriter.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java index 60598b80c21..d7b7f56ac94 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java @@ -247,12 +247,14 @@ private void emitDataPointAttributes( } // Splits a packed "key:value" additional-tag string at the first ':' (keys cannot contain ':', - // values may) and emits it as an OTLP string attribute. Skips malformed slots (no ':', empty key - // or empty value) defensively. + // values may) and emits it as an OTLP string attribute. Skips only malformed slots with no ':' or + // an empty key. An empty value ("key:") is emitted as key="": the aggregation path treats an + // explicitly-empty tag as a distinct dimension from an absent one, so dropping it here would + // export two separately-aggregated rows with identical OTLP attribute sets. private static void emitAdditionalTag(OtlpMetricVisitor metric, UTF8BytesString additionalTag) { String packed = additionalTag.toString(); int separator = packed.indexOf(':'); - if (separator <= 0 || separator == packed.length() - 1) { + if (separator <= 0) { return; } metric.visitAttribute( From 9994e8f74341a7da5170d307c1f8d2745f8a4b21 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 27 Jul 2026 12:22:08 -0400 Subject: [PATCH 3/4] Assert empty additional-tag value is emitted as key="" in OTLP Co-Authored-By: Claude Opus 4.8 --- .../core/otlp/metrics/OtlpStatsMetricWriterTest.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java b/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java index 17e80c0ce72..3d5eee87b60 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java @@ -489,9 +489,11 @@ void additionalMetricTagsEmittedInOtelSemanticsMode() throws IOException { } @Test - void malformedAdditionalTagsAreSkipped() throws IOException { - // Defensive: a slot with no ':', an empty key, or an empty value is dropped rather than emitted - // as a malformed attribute. + void emptyValueEmittedButMalformedSlotsSkipped() throws IOException { + // A slot with no ':' or an empty key is dropped as malformed. An empty value ("key:") is NOT + // malformed -- aggregation treats an explicitly-empty tag as a distinct dimension from an + // absent + // one, so it is emitted as key="" to keep the OTLP attributes faithful to the aggregate key. AggregateEntry e = AggregateEntryTestUtils.of( "GET /users", @@ -519,7 +521,8 @@ void malformedAdditionalTagsAreSkipped() throws IOException { assertEquals("us-east-1", attrs.get("region"), "well-formed tag still emitted"); assertFalse(attrs.containsKey("noseparator"), "no-separator slot skipped"); assertFalse(attrs.containsKey(""), "empty-key slot skipped"); - assertFalse(attrs.containsKey("emptyvalue"), "empty-value slot skipped"); + assertTrue(attrs.containsKey("emptyvalue"), "empty-value slot emitted"); + assertEquals("", attrs.get("emptyvalue"), "empty value emitted as empty string"); } @Test From 94e310e7c42eee583ba9b7382f02ec94d1a0af51 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 27 Jul 2026 12:22:09 -0400 Subject: [PATCH 4/4] Include additional tags in AggregateEntry test equality contract The helper's equals() omitted getAdditionalTags() while keyHash folds it in, so entries differing only in additional tags compared equal yet produced different hash codes -- letting matcher-based tests silently accept missing or wrong additional dimensions. Compare the arrays positionally (schema order) to match the key identity. Co-Authored-By: Claude Opus 4.8 --- .../trace/common/metrics/AggregateEntryTestUtils.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java index 29adc3a6d91..221d23909d8 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java @@ -1,6 +1,7 @@ package datadog.trace.common.metrics; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Objects; @@ -174,7 +175,10 @@ public static boolean equals(AggregateEntry a, AggregateEntry b) { && a.getPeerTags().equals(b.getPeerTags()) && Objects.equals(a.getHttpMethod(), b.getHttpMethod()) && Objects.equals(a.getHttpEndpoint(), b.getHttpEndpoint()) - && Objects.equals(a.getGrpcStatusCode(), b.getGrpcStatusCode()); + && Objects.equals(a.getGrpcStatusCode(), b.getGrpcStatusCode()) + // Additional tags are part of the key (folded into keyHash in schema order), so entries + // that differ only in additional tags must not compare equal. + && Arrays.equals(a.getAdditionalTags(), b.getAdditionalTags()); } /**