Skip to content

Commit 764277e

Browse files
dougqhclaude
andcommitted
Always emit AdditionalMetricTags when the feature is configured
The Span-Derived Primary Tags RFC specifies the additional_metric_tags field is present whenever the feature is configured -- as an empty array for spans that matched no configured key -- not gated on whether a given entry carried any tags. SerializingMetricWriter previously omitted the field whenever the entry's packed array was empty, which conflated "feature off" with "feature on, no match". It now takes a configured flag (threaded from AdditionalTagsSchema.size() > 0) and emits the field, empty array included, whenever configured; when the feature is off the field is omitted entirely so non-users pay zero payload overhead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 70fe2a8 commit 764277e

3 files changed

Lines changed: 85 additions & 17 deletions

File tree

dd-trace-core/src/main/java/datadog/trace/common/metrics/ClientStatsAggregator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ public ClientStatsAggregator(
218218
features,
219219
healthMetric,
220220
sink,
221-
new SerializingMetricWriter(wellKnownTags, sink),
221+
new SerializingMetricWriter(wellKnownTags, sink, additionalTagsSchema.size() > 0),
222222
maxAggregates,
223223
queueSize,
224224
reportingInterval,

dd-trace-core/src/main/java/datadog/trace/common/metrics/SerializingMetricWriter.java

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ public final class SerializingMetricWriter implements MetricWriter {
6363
private final WellKnownTags wellKnownTags;
6464
private final WritableFormatter writer;
6565
private final Sink sink;
66+
67+
/**
68+
* Whether the span-derived additional-tags feature is configured (at least one key). When {@code
69+
* true}, the {@code AdditionalMetricTags} field is always emitted -- as an empty array for
70+
* entries that matched no configured key -- so the field is present whenever the feature is on,
71+
* per the Span-Derived Primary Tags RFC. When {@code false} (the common case: feature off) the
72+
* field is omitted entirely, so non-users pay zero payload overhead.
73+
*/
74+
private final boolean additionalTagsConfigured;
75+
6676
private final GrowableBuffer buffer;
6777
private final DDCache<GitInfo, UTF8BytesString> gitInfoCache =
6878
DDCaches.newFixedSizeWeakKeyCache(4);
@@ -76,7 +86,12 @@ public final class SerializingMetricWriter implements MetricWriter {
7686
private byte[] emptyHistogramBytesCache;
7787

7888
public SerializingMetricWriter(WellKnownTags wellKnownTags, Sink sink) {
79-
this(wellKnownTags, sink, 512 * 1024);
89+
this(wellKnownTags, sink, false);
90+
}
91+
92+
public SerializingMetricWriter(
93+
WellKnownTags wellKnownTags, Sink sink, boolean additionalTagsConfigured) {
94+
this(wellKnownTags, sink, 512 * 1024, GitInfoProvider.INSTANCE, additionalTagsConfigured);
8095
}
8196

8297
public SerializingMetricWriter(WellKnownTags wellKnownTags, Sink sink, int initialCapacity) {
@@ -88,11 +103,21 @@ public SerializingMetricWriter(
88103
Sink sink,
89104
int initialCapacity,
90105
final GitInfoProvider gitInfoProvider) {
106+
this(wellKnownTags, sink, initialCapacity, gitInfoProvider, false);
107+
}
108+
109+
public SerializingMetricWriter(
110+
WellKnownTags wellKnownTags,
111+
Sink sink,
112+
int initialCapacity,
113+
final GitInfoProvider gitInfoProvider,
114+
boolean additionalTagsConfigured) {
91115
this.wellKnownTags = wellKnownTags;
92116
this.buffer = new GrowableBuffer(initialCapacity);
93117
this.writer = new MsgPackWriter(buffer);
94118
this.sink = sink;
95119
this.gitInfoProvider = gitInfoProvider;
120+
this.additionalTagsConfigured = additionalTagsConfigured;
96121
}
97122

98123
@Override
@@ -159,14 +184,15 @@ public void add(AggregateEntry entry) {
159184
final boolean hasServiceSource = entry.hasServiceSource();
160185
final boolean hasGrpcStatusCode = entry.hasGrpcStatusCode();
161186
final UTF8BytesString[] additionalTags = entry.getAdditionalTags();
162-
final boolean hasAdditionalTags = additionalTags.length > 0;
187+
// When the feature is configured the field is always emitted (empty array for entries that
188+
// matched no key); when it is off the field is omitted entirely so non-users pay nothing.
163189
final int mapSize =
164190
15
165191
+ (hasServiceSource ? 1 : 0)
166192
+ (hasHttpMethod ? 1 : 0)
167193
+ (hasHttpEndpoint ? 1 : 0)
168194
+ (hasGrpcStatusCode ? 1 : 0)
169-
+ (hasAdditionalTags ? 1 : 0);
195+
+ (additionalTagsConfigured ? 1 : 0);
170196

171197
writer.startMap(mapSize);
172198

@@ -203,9 +229,10 @@ public void add(AggregateEntry entry) {
203229
}
204230

205231
// Emit AdditionalMetricTags as a packed array of pre-built "key:value" UTF8BytesStrings, in
206-
// schema (alphabetical-by-key) order. The field is omitted entirely when the entry carries no
207-
// additional tags, so spans that set none pay zero payload overhead.
208-
if (hasAdditionalTags) {
232+
// schema (alphabetical-by-key) order. Present whenever the feature is configured -- an empty
233+
// array for entries that matched no key -- and omitted entirely when the feature is off, so
234+
// spans in non-using deployments pay zero payload overhead.
235+
if (additionalTagsConfigured) {
209236
writer.writeUTF8(ADDITIONAL_METRIC_TAGS);
210237
writer.startArray(additionalTags.length);
211238
for (UTF8BytesString slot : additionalTags) {

dd-trace-core/src/test/java/datadog/trace/common/metrics/SerializingMetricWriterTest.java

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -489,8 +489,9 @@ void additionalMetricTagsEmittedWhenSet() {
489489
}
490490

491491
@Test
492-
void additionalMetricTagsFieldOmittedWhenNoneSet() {
493-
// Schema configured, but the span doesn't set any of the configured tags.
492+
void additionalMetricTagsEmittedAsEmptyArrayWhenConfiguredButNoneSet() {
493+
// Schema configured, but the span doesn't set any of the configured tags. Per the Span-Derived
494+
// Primary Tags RFC the field is still present (as an empty array) whenever the feature is on.
494495
AdditionalTagsSchema schema =
495496
AdditionalTagsSchema.from(new LinkedHashSet<>(Arrays.asList("region")));
496497
AggregateTable table = newTable(schema);
@@ -499,12 +500,37 @@ void additionalMetricTagsFieldOmittedWhenNoneSet() {
499500

500501
List<AggregateEntry> content = contentOf(table);
501502
assertEquals(1, content.size());
502-
// No slots populated -> empty packed array -> AdditionalMetricTags omitted from the payload.
503+
// No slots populated -> empty packed array, but the field is still emitted since configured.
503504
assertEquals(0, content.get(0).getAdditionalTags().length);
504505

505506
serializeAndValidate(content);
506507
}
507508

509+
@Test
510+
void additionalMetricTagsFieldOmittedWhenFeatureOff() {
511+
// Feature off (no schema): the writer omits AdditionalMetricTags entirely so non-users pay no
512+
// payload overhead.
513+
long startTime = MILLISECONDS.toNanos(System.currentTimeMillis());
514+
long duration = SECONDS.toNanos(10);
515+
WellKnownTags wellKnownTags =
516+
new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language");
517+
AggregateTable table = newTable(AdditionalTagsSchema.EMPTY);
518+
table.findOrInsert(snapshot(AdditionalTagsSchema.EMPTY)).recordOneDuration(1L);
519+
List<AggregateEntry> content = contentOf(table);
520+
assertEquals(1, content.size());
521+
assertEquals(0, content.get(0).getAdditionalTags().length);
522+
523+
// Default (unconfigured) sink + writer: the field must be absent.
524+
ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content);
525+
SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128);
526+
writer.startBucket(content.size(), startTime, duration);
527+
for (AggregateEntry entry : content) {
528+
writer.add(entry);
529+
}
530+
writer.finishBucket();
531+
assertTrue(sink.validatedInput());
532+
}
533+
508534
@Test
509535
void additionalMetricTagsSkipsNullSlots() {
510536
AdditionalTagsSchema schema =
@@ -579,8 +605,11 @@ private static void serializeAndValidate(List<AggregateEntry> content) {
579605
long duration = SECONDS.toNanos(10);
580606
WellKnownTags wellKnownTags =
581607
new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language");
582-
ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content);
583-
SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128);
608+
// These helpers exercise the configured-feature path, so the writer is built configured and the
609+
// sink expects the AdditionalMetricTags field present (possibly empty) on every entry.
610+
ValidatingSink sink =
611+
new ValidatingSink(wellKnownTags, startTime, duration, content, null, true);
612+
SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, true);
584613

585614
writer.startBucket(content.size(), startTime, duration);
586615
for (AggregateEntry entry : content) {
@@ -599,6 +628,7 @@ static final class ValidatingSink implements Sink {
599628
private boolean validated = false;
600629
private final List<AggregateEntry> content;
601630
private final String expectedGitCommitSha;
631+
private final boolean additionalTagsConfigured;
602632

603633
ValidatingSink(
604634
WellKnownTags wellKnownTags,
@@ -614,11 +644,22 @@ static final class ValidatingSink implements Sink {
614644
long duration,
615645
List<AggregateEntry> content,
616646
String expectedGitCommitSha) {
647+
this(wellKnownTags, startTimeNanos, duration, content, expectedGitCommitSha, false);
648+
}
649+
650+
ValidatingSink(
651+
WellKnownTags wellKnownTags,
652+
long startTimeNanos,
653+
long duration,
654+
List<AggregateEntry> content,
655+
String expectedGitCommitSha,
656+
boolean additionalTagsConfigured) {
617657
this.wellKnownTags = wellKnownTags;
618658
this.startTimeNanos = startTimeNanos;
619659
this.duration = duration;
620660
this.content = content;
621661
this.expectedGitCommitSha = expectedGitCommitSha;
662+
this.additionalTagsConfigured = additionalTagsConfigured;
622663
}
623664

624665
@Override
@@ -684,14 +725,13 @@ private void validate(ByteBuffer buffer) throws IOException {
684725
boolean hasServiceSource = entry.hasServiceSource();
685726
boolean hasGrpcStatusCode = entry.hasGrpcStatusCode();
686727
UTF8BytesString[] additionalTags = entry.getAdditionalTags();
687-
boolean hasAdditionalTags = additionalTags.length > 0;
688728
int expectedMapSize =
689729
15
690730
+ (hasServiceSource ? 1 : 0)
691731
+ (hasHttpMethod ? 1 : 0)
692732
+ (hasHttpEndpoint ? 1 : 0)
693733
+ (hasGrpcStatusCode ? 1 : 0)
694-
+ (hasAdditionalTags ? 1 : 0);
734+
+ (additionalTagsConfigured ? 1 : 0);
695735
assertEquals(expectedMapSize, metricMapSize);
696736
int elementCount = 0;
697737
assertEquals("Name", unpacker.unpackString());
@@ -729,9 +769,10 @@ private void validate(ByteBuffer buffer) throws IOException {
729769
}
730770
++elementCount;
731771
// AdditionalMetricTags is a packed array of pre-built "key:value" strings in schema
732-
// (alphabetical-by-key) order. Emitted immediately after PeerTags and omitted entirely
733-
// when the entry set none, mirroring the writer's optional-field handling above.
734-
if (hasAdditionalTags) {
772+
// (alphabetical-by-key) order. Emitted immediately after PeerTags whenever the feature is
773+
// configured -- as an empty array for entries that matched no key -- and omitted entirely
774+
// when the feature is off, mirroring the writer's field handling above.
775+
if (additionalTagsConfigured) {
735776
assertEquals("AdditionalMetricTags", unpacker.unpackString());
736777
int additionalTagsLength = unpacker.unpackArrayHeader();
737778
assertEquals(additionalTags.length, additionalTagsLength);

0 commit comments

Comments
 (0)