Skip to content

Emit additional metric tags on the OTLP stats export path (client-side stats) #12069

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dougqh wants to merge 4 commits into
base: master
Choose a base branch
from
+177 −10
Open

Emit additional metric tags on the OTLP stats export path (client-side stats) #12069

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
49f7a64
Emit additional metric tags on the OTLP stats export path
dougqh Jul 24, 2026
ad8e42a
Emit empty additional-tag values on the OTLP stats export path
dougqh Jul 27, 2026
9994e8f
Assert empty additional-tag value is emitted as key="" in OTLP
dougqh Jul 27, 2026
94e310e
Include additional tags in AggregateEntry test equality contract
dougqh Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions dd-trace-core/src/main/java/datadog/trace/common/metrics/ClientStatsAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -165,6 +161,7 @@ public ClientStatsAggregator(
OtlpStatsMetricWriter metricWriter) {
this(
config.getMetricsIgnoredResources(),
additionalTagsSchemaFrom(config),
sharedCommunicationObjects.featuresDiscovery(config),
healthMetrics,
NoOpSink.INSTANCE,
Expand All @@ -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<String> ignoredResources,
Expand Down
22 changes: 22 additions & 0 deletions dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

@mhlidd mhlidd Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Discussed w/ Munir - let's keep the raw key instead of datadog.*

for (UTF8BytesString additionalTag : entry.getAdditionalTags()) {

@datadog-datadog-us1-prod datadog-datadog-us1-prod Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Additional tags can overwrite standard OTLP attributes

OTLP client-side metric points can have corrupted HTTP semantic attributes or be rejected when a valid user configuration overlaps a standard OTLP key.

Assertion details
  • Input: Set DD_TRACE_STATS_ADDITIONAL_TAGS to http.route and process a span that has an HTTP route tag of /users/{id} but an additional-tag value such as custom-route.
  • Expected: The emitted data point has one unambiguous http.route semantic attribute representing the standard HTTP route, while the user-configured dimension is represented without colliding with reserved OTLP keys.
  • Actual: emitDataPointAttributes writes the standard http.route attribute first, then emitAdditionalTag writes another http.route attribute from the packed user tag. The protobuf and JSON collectors append both entries; there is no collision check or namespace, so consumers that collapse attributes by key can observe custom-route as the HTTP route, and consumers that reject duplicate keys can reject the point.

Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

@mhlidd mhlidd Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dougqh Is this a valid concern? Should we handle this the same way that native CSS handles it?

@dougqh dougqh Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had Claude do an analysis of this for both the Datadog & OTLP serialization. Here are the findings...

Yes, it's a valid concern — but it's specific to the OTLP path, and native doesn't have it by design.

On the native msgpack path (SerializingMetricWriter), additional tags are written into a dedicated AdditionalMetricTags array, structurally separate from the built-in dimensions (Resource, HTTPStatusCode, PeerTags, …). So configuring http.route as an additional tag lands in its own section and never touches the built-in route — no collision possible.

OTLP data-point attributes are a single flat KeyValue bag. We emit the built-ins first (span.name, span.kind, service.name, http.request.method, http.response.status_code, http.route, rpc.response.status_code, status.code, plus datadog.* in default mode), then loop the user's additional tags with raw keys. So a user tag whose name equals a built-in produces two entries for the same key, and a receiver may keep either or reject the point. That's exactly what the P1 describes.

The two ways to reproduce native's separation on a flat bag are (a) a key namespace like datadog.*, or (b) don't let a user tag overwrite a built-in. Since we've settled on raw keys (a) is out, so the fix has to be collision-avoidance at emit time: reserve the built-in attribute keys and skip any additional tag that collides — built-in wins. Corrupting or duplicating a standard semantic attribute is worse than dropping a user tag that's almost certainly a misconfiguration (the name is already emitted as a first-class dimension).

One implementation note: this reservation must live in OtlpStatsMetricWriter, not in the shared AdditionalTagsSchema. On the native path those same names are legitimate and non-colliding, so rejecting them at schema construction would wrongly break native. I'll compute the collision set once at writer construction (intersect the configured tag names with the built-in key set, warn once), and skip those slots at emit — no per-point cost.

I'll push that change; happy to revisit if we ever reintroduce a namespace.

emitAdditionalTag(metric, additionalTag);
}
// Default (Datadog) mode: emit datadog.* per-point attributes
if (!otelSemanticsMode) {
emitStringAttribute(metric, DATADOG_OPERATION_NAME, entry.getOperationName());
Expand All @@ -239,6 +246,21 @@ 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 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();

@dougqh dougqh Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not thrilled with the need to split here, but I'm leaving client-side stats optimized for interaction with Datadog agent and our protocol for the moment.

The problems also run deeper than just the split.
There is also an issue with encoding to UTF8 as well.

That's probably solvable with the our UTF8 cache, but I'm leaving that to another PR.

int separator = packed.indexOf(':');
if (separator <= 0) {
return;
}
metric.visitAttribute(
STRING_ATTRIBUTE, packed.substring(0, separator), packed.substring(separator + 1));
Comment on lines +260 to +261

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Prevent additional tags from duplicating built-in attributes

When DD_TRACE_STATS_ADDITIONAL_TAGS contains a name already emitted by this writer, such as span.name, status.code, or http.response.status_code, this raw key creates two KeyValue entries for the same OTLP attribute; the latter example can even have both integer and string values. Receivers may retain different occurrences, so built-in dimensions and their types become ambiguous. Reserve these names, skip duplicates, or settle the namespace before emitting user tags.

Useful? React with 👍 / 👎.

@mhlidd mhlidd Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as the above Datadog comment.

}

// accepts both String literals and UTF8BytesString (both CharSequence); skips null values
private static void emitStringAttribute(
OtlpMetricVisitor metric, String key, @Nullable CharSequence value) {
Expand Down
52 changes: 47 additions & 5 deletions dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -45,6 +46,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<UTF8BytesString> 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);
Expand All @@ -56,7 +94,8 @@ public static AggregateEntry of(
UTF8BytesString grpcUtf = AggregateEntry.createUtf8(grpcStatusCode);
List<UTF8BytesString> 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,
Expand All @@ -73,8 +112,8 @@ public static AggregateEntry of(
traceRoot,
peerTagsArr,
peerTagsArr.length,
emptyAdditional,
0);
additionalTagsArr,
additionalTagsArr.length);
return new AggregateEntry(
keyHash,
resourceUtf,
Expand All @@ -90,7 +129,7 @@ public static AggregateEntry of(
synthetic,
traceRoot,
peerTagsList,
emptyAdditional);
additionalTagsArr);
}

/**
Expand Down Expand Up @@ -136,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());
}

/**
Expand Down
98 changes: 98 additions & 0 deletions dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -427,6 +428,103 @@ 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<String, Object> 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<String, Object> 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 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",
"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<String, Object> 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");
assertTrue(attrs.containsKey("emptyvalue"), "empty-value slot emitted");
assertEquals("", attrs.get("emptyvalue"), "empty value emitted as empty string");
}

@Test
void serviceNameEmittedOnlyForNonDefaultService() throws IOException {
CapturingSender sender = new CapturingSender();
Expand Down