diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java index aa87269f964..a32f997cebe 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java @@ -87,6 +87,7 @@ public final class GeneralConfig { public static final String TRACER_METRICS_MAX_PENDING = "trace.tracer.metrics.max.pending"; public static final String TRACER_METRICS_IGNORED_RESOURCES = "trace.tracer.metrics.ignored.resources"; + public static final String TRACE_STATS_ADDITIONAL_TAGS = "trace.stats.additional.tags"; public static final String AZURE_APP_SERVICES = "azure.app.services"; public static final String INTERNAL_EXIT_ON_FAILURE = "trace.internal.exit.on.failure"; diff --git a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/AdditionalTagsMetricsBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/AdditionalTagsMetricsBenchmark.java new file mode 100644 index 00000000000..ccf03e0aa7e --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/AdditionalTagsMetricsBenchmark.java @@ -0,0 +1,106 @@ +package datadog.trace.common.metrics; + +import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND; +import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND_CLIENT; +import static java.util.concurrent.TimeUnit.SECONDS; + +import datadog.trace.api.WellKnownTags; +import datadog.trace.core.CoreSpan; +import de.thetaphi.forbiddenapis.SuppressForbidden; +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.concurrent.ThreadLocalRandom; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** + * Regression benchmark for the additional-tags hot path; {@code limitsEnabled=true} is slower here + * because it records more (sentinel collapses), not because limiting is expensive. + */ +@State(Scope.Benchmark) +@Warmup(iterations = 2, time = 15, timeUnit = SECONDS) +@Measurement(iterations = 5, time = 15, timeUnit = SECONDS) +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(SECONDS) +@Threads(8) +@Fork(value = 1) +public class AdditionalTagsMetricsBenchmark { + + private ClientStatsAggregator aggregator; + private AdversarialMetricsBenchmark.CountingHealthMetrics health; + + @Param({"false", "true"}) + public boolean limitsEnabled; + + @State(Scope.Thread) + public static class ThreadState { + int cursor; + } + + @Setup + public void setup() { + this.health = new AdversarialMetricsBenchmark.CountingHealthMetrics(); + AdditionalTagsSchema additionalTagsSchema = + AdditionalTagsSchema.from( + new LinkedHashSet<>(Arrays.asList("region", "tenant_id")), + MetricCardinalityLimits.ADDITIONAL_TAG_VALUE, + limitsEnabled, + this.health); + this.aggregator = + new ClientStatsAggregator( + new WellKnownTags("", "", "", "", "", ""), + Collections.emptySet(), + additionalTagsSchema, + new ClientStatsAggregatorBenchmark.FixedAgentFeaturesDiscovery( + Collections.singleton("peer.hostname"), Collections.emptySet()), + this.health, + new ClientStatsAggregatorBenchmark.NullSink(), + 2048, + 2048, + false); + this.aggregator.start(); + } + + @TearDown + @SuppressForbidden + public void tearDown() { + aggregator.close(); + System.err.println("[ADDITIONAL-TAGS] counters (across all threads, single fork):"); + System.err.println(" onStatsInboxFull = " + health.inboxFull.sum()); + System.err.println(" onStatsAggregateDropped = " + health.aggregateDropped.sum()); + } + + @Benchmark + public void publish(ThreadState ts, Blackhole blackhole) { + int idx = ts.cursor++; + ThreadLocalRandom rng = ThreadLocalRandom.current(); + + int scrambled = idx * 0x9E3779B1; + String region = "region-" + ((scrambled >>> 4) & 0xFFFF); + String tenant = "tenant-" + ((scrambled >>> 16) & 0xFFFF); + long durationNanos = 1L + (rng.nextLong() & 0x3FFFFFFFL); + + SimpleSpan span = + new SimpleSpan("svc", "op", "res", "web", true, true, false, 0, durationNanos, 200); + span.setTag(SPAN_KIND, SPAN_KIND_CLIENT); + span.setTag("region", region); + span.setTag("tenant_id", tenant); + + List> trace = Collections.singletonList(span); + blackhole.consume(aggregator.publish(trace)); + } +} diff --git a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/AdversarialMetricsBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/AdversarialMetricsBenchmark.java index 01ba90097a4..65d2ad49e9b 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/AdversarialMetricsBenchmark.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/AdversarialMetricsBenchmark.java @@ -75,6 +75,7 @@ public void setup() { new ClientStatsAggregator( new WellKnownTags("", "", "", "", "", ""), Collections.emptySet(), + AdditionalTagsSchema.EMPTY, new ClientStatsAggregatorBenchmark.FixedAgentFeaturesDiscovery( Collections.singleton("peer.hostname"), Collections.emptySet()), this.health, diff --git a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/ClientStatsAggregatorBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/ClientStatsAggregatorBenchmark.java index b9d72eaf3ab..2c71101956a 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/ClientStatsAggregatorBenchmark.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/ClientStatsAggregatorBenchmark.java @@ -42,6 +42,7 @@ public class ClientStatsAggregatorBenchmark { new ClientStatsAggregator( new WellKnownTags("", "", "", "", "", ""), Collections.emptySet(), + AdditionalTagsSchema.EMPTY, featuresDiscovery, HealthMetrics.NO_OP, new NullSink(), diff --git a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/ClientStatsAggregatorDDSpanBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/ClientStatsAggregatorDDSpanBenchmark.java index 0453b8888db..901d5a96381 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/ClientStatsAggregatorDDSpanBenchmark.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/ClientStatsAggregatorDDSpanBenchmark.java @@ -62,6 +62,7 @@ public class ClientStatsAggregatorDDSpanBenchmark { new ClientStatsAggregator( new WellKnownTags("", "", "", "", "", ""), Collections.emptySet(), + AdditionalTagsSchema.EMPTY, featuresDiscovery, HealthMetrics.NO_OP, new ClientStatsAggregatorBenchmark.NullSink(), diff --git a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/HighCardinalityPeerMetricsBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/HighCardinalityPeerMetricsBenchmark.java index 8fc45b4acd8..839657f16e5 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/HighCardinalityPeerMetricsBenchmark.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/HighCardinalityPeerMetricsBenchmark.java @@ -65,6 +65,7 @@ public void setup() { new ClientStatsAggregator( new WellKnownTags("", "", "", "", "", ""), Collections.emptySet(), + AdditionalTagsSchema.EMPTY, new ClientStatsAggregatorBenchmark.FixedAgentFeaturesDiscovery( Collections.singleton("peer.hostname"), Collections.emptySet()), this.health, diff --git a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/HighCardinalityResourceMetricsBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/HighCardinalityResourceMetricsBenchmark.java index 90c1f088935..f34028f41da 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/HighCardinalityResourceMetricsBenchmark.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/HighCardinalityResourceMetricsBenchmark.java @@ -61,6 +61,7 @@ public void setup() { new ClientStatsAggregator( new WellKnownTags("", "", "", "", "", ""), Collections.emptySet(), + AdditionalTagsSchema.EMPTY, new ClientStatsAggregatorBenchmark.FixedAgentFeaturesDiscovery( Collections.singleton("peer.hostname"), Collections.emptySet()), this.health, diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/AdditionalTagsSchema.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AdditionalTagsSchema.java new file mode 100644 index 00000000000..b6924f1eace --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AdditionalTagsSchema.java @@ -0,0 +1,122 @@ +package datadog.trace.common.metrics; + +import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import datadog.trace.core.monitor.HealthMetrics; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Immutable schema of configured span-derived primary tag keys; built once at aggregator + * construction. + */ +final class AdditionalTagsSchema { + + private static final Logger log = LoggerFactory.getLogger(AdditionalTagsSchema.class); + + // Backend stats pipeline only supports a few primary tag dimensions; drop overflow at startup. + static final int MAX_ADDITIONAL_TAG_KEYS = 4; + + /** Singleton empty schema returned when no additional tags are configured. */ + static final AdditionalTagsSchema EMPTY = + new AdditionalTagsSchema(new String[0], new TagCardinalityHandler[0], HealthMetrics.NO_OP); + + final String[] names; + + /** Per-key handlers providing UTF8 caching and per-cycle cardinality limiting. */ + private final TagCardinalityHandler[] handlers; + + private final HealthMetrics healthMetrics; + + private AdditionalTagsSchema( + String[] names, TagCardinalityHandler[] handlers, HealthMetrics healthMetrics) { + this.names = names; + this.handlers = handlers; + this.healthMetrics = healthMetrics; + } + + /** Test convenience: uses {@link HealthMetrics#NO_OP} and limits enabled. */ + static AdditionalTagsSchema from(Set configured) { + return from( + configured, MetricCardinalityLimits.ADDITIONAL_TAG_VALUE, true, HealthMetrics.NO_OP); + } + + static AdditionalTagsSchema from( + Set configured, int limit, boolean useBlockedSentinel, HealthMetrics healthMetrics) { + if (configured == null || configured.isEmpty()) { + return EMPTY; + } + List valid = new ArrayList<>(); + for (String key : configured) { + if (key == null || key.isEmpty()) { + log.warn("Ignoring empty additional metric tag key"); + continue; + } + if (key.contains(":")) { + log.warn("Ignoring additional metric tag key '{}': keys must not contain ':'", key); + continue; + } + valid.add(key); + } + if (valid.isEmpty()) { + return EMPTY; + } + Collections.sort(valid); + // Dedup (sort brings duplicates adjacent) + List deduped = new ArrayList<>(valid.size()); + String prev = null; + for (String key : valid) { + if (!key.equals(prev)) { + deduped.add(key); + prev = key; + } + } + if (deduped.size() > MAX_ADDITIONAL_TAG_KEYS) { + log.warn( + "Configured additional metric tag keys ({}) exceeds the supported limit of {}; " + + "dropping extra keys: {}", + deduped.size(), + MAX_ADDITIONAL_TAG_KEYS, + deduped.subList(MAX_ADDITIONAL_TAG_KEYS, deduped.size())); + deduped = deduped.subList(0, MAX_ADDITIONAL_TAG_KEYS); + } + String[] namesArr = deduped.toArray(new String[0]); + TagCardinalityHandler[] handlersArr = new TagCardinalityHandler[namesArr.length]; + for (int i = 0; i < namesArr.length; i++) { + handlersArr[i] = + new TagCardinalityHandler( + namesArr[i], + limit, + useBlockedSentinel, + MetricCardinalityLimits.ADDITIONAL_TAG_MAX_VALUE_LENGTH); + } + return new AdditionalTagsSchema(namesArr, handlersArr, healthMetrics); + } + + int size() { + return names.length; + } + + String name(int i) { + return names[i]; + } + + UTF8BytesString register(int i, String value) { + return handlers[i].register(value); + } + + void resetHandlers() { + for (int i = 0; i < handlers.length; i++) { + long blocked = handlers[i].reset(); + if (blocked > 0) { + log.warn( + "Cardinality limit reached for additional metric tag '{}'; further values will be reported as tracer_blocked_value", + names[i]); + healthMetrics.onTagCardinalityBlocked(handlers[i].statsDTag(), blocked); + } + } + } +} diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateEntry.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateEntry.java index dbfd436b60a..4e58b9ec23d 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateEntry.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateEntry.java @@ -1,27 +1,17 @@ package datadog.trace.common.metrics; import datadog.metrics.api.Histogram; -import datadog.trace.api.Config; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.util.Hashtable; import datadog.trace.util.LongHashingUtils; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicLongArray; import javax.annotation.Nullable; -/** - * {@link datadog.trace.util.Hashtable} entry used by the aggregator thread. - * - *

Stores the canonical UTF8 label values that identify one aggregate row, plus the mutable - * counter and histogram state for that row. Labels are canonicalized before hashing, so overflow - * values replaced with the sentinel use the same hash and map to the same row. - * - *

Not thread-safe — all mutation is on the aggregator thread. Tests must call {@link - * #resetCardinalityHandlers()} in setup to avoid cross-test handler pollution (handlers are - * static); tests using {@link PeerTagSchema} must also call {@code PeerTagSchema#resetHandlers} on - * the schema instance. - */ +/** Aggregator hashtable entry: UTF8 label fields + counter/histogram state. */ final class AggregateEntry extends Hashtable.Entry { static final long ERROR_TAG = 0x8000000000000000L; @@ -29,49 +19,6 @@ final class AggregateEntry extends Hashtable.Entry { private static final UTF8BytesString[] EMPTY_TAGS = new UTF8BytesString[0]; - // Configurable per-field cardinality handlers — tunable via env var. - static final PropertyCardinalityHandler RESOURCE_HANDLER = - new PropertyCardinalityHandler( - "resource", - Config.get().getTraceStatsCardinalityLimit("resource", MetricCardinalityLimits.RESOURCE)); - static final PropertyCardinalityHandler HTTP_ENDPOINT_HANDLER = - new PropertyCardinalityHandler( - "http_endpoint", - Config.get() - .getTraceStatsCardinalityLimit( - "http_endpoint", MetricCardinalityLimits.HTTP_ENDPOINT)); - - // Fixed per-field cardinality handlers — hardcoded, not user-configurable. - static final PropertyCardinalityHandler SERVICE_HANDLER = - new PropertyCardinalityHandler("service", MetricCardinalityLimits.SERVICE); - static final PropertyCardinalityHandler OPERATION_HANDLER = - new PropertyCardinalityHandler("operation", MetricCardinalityLimits.OPERATION); - static final PropertyCardinalityHandler SERVICE_SOURCE_HANDLER = - new PropertyCardinalityHandler("service_source", MetricCardinalityLimits.SERVICE_SOURCE); - static final PropertyCardinalityHandler TYPE_HANDLER = - new PropertyCardinalityHandler("type", MetricCardinalityLimits.TYPE); - static final PropertyCardinalityHandler SPAN_KIND_HANDLER = - new PropertyCardinalityHandler("span_kind", MetricCardinalityLimits.SPAN_KIND); - static final PropertyCardinalityHandler HTTP_METHOD_HANDLER = - new PropertyCardinalityHandler("http_method", MetricCardinalityLimits.HTTP_METHOD); - static final PropertyCardinalityHandler GRPC_STATUS_CODE_HANDLER = - new PropertyCardinalityHandler("grpc_status_code", MetricCardinalityLimits.GRPC_STATUS_CODE); - - // Single authoritative list used by resetCardinalityHandlers(). populateFrom() and hashOf() keep - // named access for readability and to avoid per-span iteration overhead; this array ensures the - // reset site stays in sync even if a new field is added without updating the loop by name. - static final PropertyCardinalityHandler[] FIELD_HANDLERS = { - RESOURCE_HANDLER, - SERVICE_HANDLER, - OPERATION_HANDLER, - SERVICE_SOURCE_HANDLER, - TYPE_HANDLER, - SPAN_KIND_HANDLER, - HTTP_METHOD_HANDLER, - HTTP_ENDPOINT_HANDLER, - GRPC_STATUS_CODE_HANDLER, - }; - final UTF8BytesString resource; final UTF8BytesString service; final UTF8BytesString operationName; @@ -89,22 +36,19 @@ final class AggregateEntry extends Hashtable.Entry { final boolean traceRoot; final List peerTags; - // Mutable aggregate state -- single-thread (aggregator) writer. - private final Histogram okLatencies = Histogram.newHistogram(); + // Schema-ordered "key:value" strings; "key:" prefix makes packing unambiguous without null slots. + final UTF8BytesString[] additionalTags; - /** - * Lazily allocated on the first recorded error. Most entries never see an error and keep this - * null for life; {@link SerializingMetricWriter} writes a cached empty-histogram form when null - * to keep the wire payload identical. Once allocated, it survives {@link #clear()} (cleared, not - * nulled) since an entry that errored once tends to error again. - */ + // Recording state. Mutated only on the aggregator thread. Not thread-safe. + private final Histogram okLatencies; + + // Null until first error; SerializingMetricWriter writes empty histogram form when null. @Nullable private Histogram errorLatencies; private int errorCount; private int hitCount; private int topLevelCount; - private long okDuration; - private long errorDuration; + private long duration; /** * Field-bearing constructor. Package-private so {@link AggregateEntryTestUtils} can build @@ -124,7 +68,8 @@ final class AggregateEntry extends Hashtable.Entry { short httpStatusCode, boolean synthetic, boolean traceRoot, - List peerTags) { + List peerTags, + UTF8BytesString[] additionalTags) { super(keyHash); this.resource = resource; this.service = service; @@ -139,103 +84,8 @@ final class AggregateEntry extends Hashtable.Entry { this.synthetic = synthetic; this.traceRoot = traceRoot; this.peerTags = peerTags; - } - - /** - * Records a single hit. {@code tagAndDuration} carries the duration nanos with optional {@link - * #ERROR_TAG} / {@link #TOP_LEVEL_TAG} bits OR-ed in. - */ - AggregateEntry recordOneDuration(long tagAndDuration) { - ++hitCount; - if ((tagAndDuration & TOP_LEVEL_TAG) == TOP_LEVEL_TAG) { - tagAndDuration ^= TOP_LEVEL_TAG; - ++topLevelCount; - } - if ((tagAndDuration & ERROR_TAG) == ERROR_TAG) { - tagAndDuration ^= ERROR_TAG; - errorLatenciesForWrite().accept(tagAndDuration); - errorDuration += tagAndDuration; - ++errorCount; - } else { - okLatencies.accept(tagAndDuration); - okDuration += tagAndDuration; - } - return this; - } - - int getErrorCount() { - return errorCount; - } - - int getHitCount() { - return hitCount; - } - - int getTopLevelCount() { - return topLevelCount; - } - - long getDuration() { - return okDuration + errorDuration; - } - - long getOkDuration() { - return okDuration; - } - - long getErrorDuration() { - return errorDuration; - } - - Histogram getOkLatencies() { - return okLatencies; - } - - /** - * Returns the entry's error-latency histogram, or {@code null} if no error has been recorded. - * Callers serializing this should treat {@code null} as "emit a cached empty histogram"; see - * {@link SerializingMetricWriter}. - */ - @Nullable - Histogram getErrorLatencies() { - return errorLatencies; - } - - /** Lazy-allocates {@link #errorLatencies} on the first error. */ - private Histogram errorLatenciesForWrite() { - Histogram h = errorLatencies; - if (h == null) { - h = Histogram.newHistogram(); - errorLatencies = h; - } - return h; - } - - /** - * Resets the per-cycle counters and histograms. Label fields ({@code resource}, {@code service}, - * ..., {@code peerTags}) are deliberately left intact -- they're the entry's bucket identity and - * must persist so a subsequent snapshot with the same key reuses this entry instead of allocating - * a fresh one. Entries that stay at {@code hitCount == 0} across a cycle are reaped by {@link - * AggregateTable#expungeStaleAggregates}. - */ - void clear() { - this.errorCount = 0; - this.hitCount = 0; - this.topLevelCount = 0; - this.okDuration = 0; - this.errorDuration = 0; - this.okLatencies.clear(); - // errorLatencies stays null on entries that never errored. Only clear if it was allocated. - if (this.errorLatencies != null) { - this.errorLatencies.clear(); - } - } - - /** Resets the static per-field cardinality handlers. Does not cover {@link PeerTagSchema}. */ - static void resetCardinalityHandlers() { - for (PropertyCardinalityHandler handler : FIELD_HANDLERS) { - handler.reset(); - } + this.additionalTags = additionalTags; + this.okLatencies = Histogram.newHistogram(); } /** @@ -263,7 +113,9 @@ static long hashOf( boolean synthetic, boolean traceRoot, UTF8BytesString[] peerTags, - int peerTagCount) { + int peerTagCount, + UTF8BytesString[] additionalTags, + int additionalTagCount) { long h = 0; h = LongHashingUtils.addToHash(h, resource); h = LongHashingUtils.addToHash(h, service); @@ -277,6 +129,11 @@ static long hashOf( for (int i = 0; i < peerTagCount; i++) { h = LongHashingUtils.addToHash(h, peerTags[i]); } + // Additional tags are packed compactly in schema order (alphabetical by key); each carries its + // "key:" prefix so the packed form is unambiguous without positional null slots. + for (int i = 0; i < additionalTagCount; i++) { + h = LongHashingUtils.addToHash(h, additionalTags[i]); + } h = LongHashingUtils.addToHash(h, httpStatusCode); h = LongHashingUtils.addToHash(h, synthetic); h = LongHashingUtils.addToHash(h, traceRoot); @@ -368,9 +225,131 @@ List getPeerTags() { return peerTags; } - // Production AggregateEntry intentionally has no equals/hashCode override -- AggregateTable - // bucketing uses keyHash + Canonical.matches and never invokes Object.equals. For tests that - // need value-equality (Spock argument matchers), use AggregateEntryTestUtils in src/test. + /** + * @return the packed additional-tag values this entry recorded, as canonical {@code "key:value"} + * UTF8BytesStrings in schema (alphabetical-by-key) order. Only tags the span actually set are + * present (no null slots), so the length is the count of present tags -- empty when the span + * set none or no additional tags are configured. + */ + UTF8BytesString[] getAdditionalTags() { + return additionalTags; + } + + // ----- recording state accessors ----- + + int getHitCount() { + return hitCount; + } + + int getErrorCount() { + return errorCount; + } + + int getTopLevelCount() { + return topLevelCount; + } + + long getDuration() { + return duration; + } + + Histogram getOkLatencies() { + return okLatencies; + } + + /** + * Returns the entry's error latency histogram, or {@code null} if no error has been recorded yet. + * Callers should treat null as "serialize as an empty histogram" (see {@link + * SerializingMetricWriter}). + */ + Histogram getErrorLatencies() { + return errorLatencies; + } + + /** Lazy-allocates {@link #errorLatencies} on the first error. */ + private Histogram errorLatenciesForWrite() { + Histogram h = errorLatencies; + if (h == null) { + h = Histogram.newHistogram(); + errorLatencies = h; + } + return h; + } + + /** + * Records a single hit. {@code tagAndDuration} carries the duration nanos with optional {@link + * #ERROR_TAG} / {@link #TOP_LEVEL_TAG} bits OR-ed in. + */ + @SuppressFBWarnings( + value = "AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE", + justification = + "Single-writer by design: recording counters are mutated only on the aggregator thread" + + " (see class javadoc); no cross-thread atomicity guarantee is needed.") + AggregateEntry recordOneDuration(long tagAndDuration) { + ++hitCount; + if ((tagAndDuration & TOP_LEVEL_TAG) == TOP_LEVEL_TAG) { + tagAndDuration ^= TOP_LEVEL_TAG; + ++topLevelCount; + } + if ((tagAndDuration & ERROR_TAG) == ERROR_TAG) { + tagAndDuration ^= ERROR_TAG; + errorLatenciesForWrite().accept(tagAndDuration); + ++errorCount; + } else { + okLatencies.accept(tagAndDuration); + } + duration += tagAndDuration; + return this; + } + + /** + * Records {@code count} durations from {@code durations} (positions 0..count-1). Used by + * integration tests; production code uses {@link #recordOneDuration}. + */ + @SuppressFBWarnings( + value = "AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE", + justification = + "Single-writer by design: recording counters are mutated only on the aggregator thread" + + " (see class javadoc); no cross-thread atomicity guarantee is needed.") + AggregateEntry recordDurations(int count, AtomicLongArray durations) { + this.hitCount += count; + for (int i = 0; i < count && i < durations.length(); ++i) { + long d = durations.getAndSet(i, 0); + if ((d & TOP_LEVEL_TAG) == TOP_LEVEL_TAG) { + d ^= TOP_LEVEL_TAG; + ++topLevelCount; + } + if ((d & ERROR_TAG) == ERROR_TAG) { + d ^= ERROR_TAG; + errorLatenciesForWrite().accept(d); + ++errorCount; + } else { + okLatencies.accept(d); + } + this.duration += d; + } + return this; + } + + /** + * Clears the recording state. The OK histogram is reused; the error histogram (if allocated) is + * reused too, but entries that never saw an error keep their {@code errorLatencies} field null. + */ + @SuppressFBWarnings( + value = {"AT_NONATOMIC_64BIT_PRIMITIVE", "AT_STALE_THREAD_WRITE_OF_PRIMITIVE"}, + justification = + "Single-writer by design: recording counters are reset only on the aggregator thread" + + " (see class javadoc); no cross-thread visibility guarantee is needed.") + void clearAggregate() { + this.errorCount = 0; + this.hitCount = 0; + this.topLevelCount = 0; + this.duration = 0; + this.okLatencies.clear(); + if (this.errorLatencies != null) { + this.errorLatencies.clear(); + } + } /** * Reusable scratch buffer for canonicalizing a {@link SpanSnapshot} into UTF8 fields, computing @@ -404,39 +383,69 @@ static final class Canonical { int peerTagsSize = 0; + /** Schema + per-key blocked sentinels for additional metric tags. Immutable. */ + final AdditionalTagsSchema additionalTagsSchema; + + /** Per-field property cardinality handlers; owned by the enclosing {@link AggregateTable}. */ + final PropertyHandlers handlers; + + /** + * Reusable scratch for canonicalized additional-tag values, sized to the schema. Present values + * are packed at the front in schema order (alphabetical by key); {@link #additionalTagsSize} + * gives the count. Each entry is a {@code "key:value"} UTF8BytesString, so packing loses no + * information -- the key prefix disambiguates which key a value belongs to. Mirrors the {@code + * peerTagsBuffer + peerTagsSize} pattern. {@link #createEntry} copies the populated prefix into + * the new entry. + */ + final UTF8BytesString[] additionalTagsBuffer; + + int additionalTagsSize; + long keyHash; + Canonical(AdditionalTagsSchema additionalTagsSchema, PropertyHandlers handlers) { + this.additionalTagsSchema = additionalTagsSchema; + this.handlers = handlers; + this.additionalTagsBuffer = new UTF8BytesString[additionalTagsSchema.size()]; + } + /** Canonicalize all fields from {@code s} through the handlers into this buffer. */ - void populateFrom(SpanSnapshot s) { - this.resource = RESOURCE_HANDLER.register(s.resourceName); - this.service = SERVICE_HANDLER.register(s.serviceName); - this.operationName = OPERATION_HANDLER.register(s.operationName); - this.serviceSource = SERVICE_SOURCE_HANDLER.register(s.serviceNameSource); - this.type = TYPE_HANDLER.register(s.spanType); - this.spanKind = SPAN_KIND_HANDLER.register(s.spanKind); - this.httpMethod = HTTP_METHOD_HANDLER.register(s.httpMethod); - this.httpEndpoint = HTTP_ENDPOINT_HANDLER.register(s.httpEndpoint); - this.grpcStatusCode = GRPC_STATUS_CODE_HANDLER.register(s.grpcStatusCode); + void populate(SpanSnapshot s) { + this.resource = handlers.resource.register(s.resourceName); + this.service = handlers.service.register(s.serviceName); + this.operationName = handlers.operation.register(s.operationName); + this.serviceSource = handlers.serviceSource.register(s.serviceNameSource); + this.type = handlers.type.register(s.spanType); + this.spanKind = handlers.spanKind.register(s.spanKind); + this.httpMethod = handlers.httpMethod.register(s.httpMethod); + this.httpEndpoint = handlers.httpEndpoint.register(s.httpEndpoint); + this.grpcStatusCode = handlers.grpcStatusCode.register(s.grpcStatusCode); this.httpStatusCode = s.httpStatusCode; this.synthetic = s.synthetic; this.traceRoot = s.traceRoot; populatePeerTags(s.peerTagSchema, s.peerTagValues); - this.keyHash = - hashOf( - resource, - service, - operationName, - serviceSource, - type, - spanKind, - httpMethod, - httpEndpoint, - grpcStatusCode, - httpStatusCode, - synthetic, - traceRoot, - peerTagsBuffer, - peerTagsSize); + populateAdditionalTags(s.additionalTagValues); + this.keyHash = computeKeyHash(); + } + + private long computeKeyHash() { + return hashOf( + resource, + service, + operationName, + serviceSource, + type, + spanKind, + httpMethod, + httpEndpoint, + grpcStatusCode, + httpStatusCode, + synthetic, + traceRoot, + peerTagsBuffer, + peerTagsSize, + additionalTagsBuffer, + additionalTagsSize); } /** @@ -448,7 +457,7 @@ void populateFrom(SpanSnapshot s) { * filtered out anyway. Producer-side {@code capturePeerTagValues} produces sparse-null arrays, * so the skip pays off whenever a span carries only a subset of the configured peer tags. */ - private void populatePeerTags(PeerTagSchema schema, String[] values) { + private void populatePeerTags(@Nullable PeerTagSchema schema, @Nullable String[] values) { peerTagsSize = 0; if (schema == null || values == null) { return; @@ -466,6 +475,27 @@ private void populatePeerTags(PeerTagSchema schema, String[] values) { } } + /** + * Packs canonical {@code "key:value"} UTF8BytesStrings for each present slot of {@code values} + * into the front of {@link #additionalTagsBuffer} (schema order), via {@link + * AdditionalTagsSchema#register}, and sets {@link #additionalTagsSize}. The handler returns the + * per-key blocked sentinel when the per-cycle value budget is exhausted. + */ + private void populateAdditionalTags(@Nullable String[] values) { + additionalTagsSize = 0; + int n = additionalTagsBuffer.length; + if (n == 0 || values == null) { + return; + } + for (int i = 0; i < n; i++) { + String v = values[i]; + if (v == null) { + continue; + } + additionalTagsBuffer[additionalTagsSize++] = additionalTagsSchema.register(i, v); + } + } + /** * Whether this canonicalized snapshot matches the given entry. Compares UTF8 fields via * content-equality (so an entry surviving a handler reset still matches a freshly-canonicalized @@ -489,7 +519,22 @@ boolean matches(AggregateEntry e) { && peerTagsEqual(peerTagsBuffer, peerTagsSize, e.peerTags) && httpStatusCode == e.httpStatusCode && synthetic == e.synthetic - && traceRoot == e.traceRoot; + && traceRoot == e.traceRoot + && additionalTagsEqual(additionalTagsBuffer, additionalTagsSize, e.additionalTags); + } + + /** Compact compare: first {@code aSize} slots of {@code a} against the entry's packed array. */ + private static boolean additionalTagsEqual( + UTF8BytesString[] a, int aSize, UTF8BytesString[] b) { + if (aSize != b.length) { + return false; + } + for (int i = 0; i < aSize; i++) { + if (!a[i].equals(b[i])) { + return false; + } + } + return true; } private static boolean peerTagsEqual(UTF8BytesString[] a, int aSize, List b) { @@ -519,6 +564,10 @@ AggregateEntry createEntry() { } else { snapshottedPeerTags = Arrays.asList(Arrays.copyOf(peerTagsBuffer, n)); } + UTF8BytesString[] snapshottedAdditionalTags = + additionalTagsSize == 0 + ? EMPTY_TAGS + : Arrays.copyOf(additionalTagsBuffer, additionalTagsSize); return new AggregateEntry( keyHash, resource, @@ -533,7 +582,8 @@ AggregateEntry createEntry() { httpStatusCode, synthetic, traceRoot, - snapshottedPeerTags); + snapshottedPeerTags, + snapshottedAdditionalTags); } } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateTable.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateTable.java index 6f2072f6495..34e931651c8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateTable.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateTable.java @@ -1,5 +1,6 @@ package datadog.trace.common.metrics; +import datadog.trace.core.monitor.HealthMetrics; import datadog.trace.util.Hashtable; import datadog.trace.util.Hashtable.MutatingTableIterator; import java.util.function.BiConsumer; @@ -26,7 +27,7 @@ final class AggregateTable { private final Hashtable.Entry[] buckets; private final int maxAggregates; - private final AggregateEntry.Canonical canonical = new AggregateEntry.Canonical(); + private final AggregateEntry.Canonical canonical; private int size; /** @@ -37,8 +38,22 @@ final class AggregateTable { private int evictCursor; AggregateTable(int maxAggregates) { + this(maxAggregates, AdditionalTagsSchema.EMPTY); + } + + AggregateTable(int maxAggregates, AdditionalTagsSchema additionalTagsSchema) { + this(maxAggregates, additionalTagsSchema, new PropertyHandlers()); + } + + AggregateTable( + int maxAggregates, AdditionalTagsSchema additionalTagsSchema, PropertyHandlers handlers) { this.buckets = Hashtable.Support.create(maxAggregates, Hashtable.Support.MAX_RATIO); this.maxAggregates = maxAggregates; + this.canonical = new AggregateEntry.Canonical(additionalTagsSchema, handlers); + } + + void resetHandlers(HealthMetrics healthMetrics) { + canonical.handlers.reset(healthMetrics); } int size() { @@ -55,7 +70,7 @@ boolean isEmpty() { * caller should drop the data point in that case. */ AggregateEntry findOrInsert(SpanSnapshot snapshot) { - canonical.populateFrom(snapshot); + canonical.populate(snapshot); long keyHash = canonical.keyHash; for (AggregateEntry candidate = Hashtable.Support.bucket(buckets, keyHash); candidate != null; @@ -64,6 +79,7 @@ AggregateEntry findOrInsert(SpanSnapshot snapshot) { return candidate; } } + // Miss path. if (size >= maxAggregates && !evictOneStale()) { return null; } @@ -83,11 +99,11 @@ AggregateEntry findOrInsert(SpanSnapshot snapshot) { * {@code onStatsAggregateDropped}) rather than evicting an established one. Cap is sized to the * steady-state working set, so eviction is rare in the common case. * - *

With per-field cardinality limits enabled, over-cap values for a given field collapse into a - * shared {@code tracer_blocked_value} bucket, so the table itself rarely reaches {@code - * maxAggregates}. Without per-field limits, over-cap values flow to distinct buckets and {@code - * maxAggregates} becomes the load-bearing backstop -- the cursor-resumed scan was added - * specifically for this regime. + *

How often this fires depends on {@link MetricCardinalityLimits#ENABLED}. With limits + * enabled, over-cap values for a given field collapse into a shared {@code blocked_by_tracer} + * bucket, so the table itself rarely reaches {@code maxAggregates}. With limits disabled (the + * default), over-cap values flow to distinct buckets and {@code maxAggregates} becomes the + * load-bearing backstop -- the cursor-resumed scan was added specifically for this regime. */ private boolean evictOneStale() { // Two passes -- [cursor, length) then [0, cursor) -- using the half-open-range iterator. The diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/Aggregator.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/Aggregator.java index 8a33d3f1ea7..afdfcd614e5 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/Aggregator.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/Aggregator.java @@ -49,6 +49,7 @@ final class Aggregator implements Runnable { long reportingInterval, TimeUnit reportingIntervalTimeUnit, HealthMetrics healthMetrics, + AdditionalTagsSchema additionalTagsSchema, Runnable onReportCycle) { this( writer, @@ -58,6 +59,7 @@ final class Aggregator implements Runnable { reportingIntervalTimeUnit, DEFAULT_SLEEP_MILLIS, healthMetrics, + additionalTagsSchema, onReportCycle); } @@ -69,16 +71,21 @@ final class Aggregator implements Runnable { TimeUnit reportingIntervalTimeUnit, long sleepMillis, HealthMetrics healthMetrics, + AdditionalTagsSchema additionalTagsSchema, Runnable onReportCycle) { this.writer = writer; this.inbox = inbox; - this.aggregates = new AggregateTable(maxAggregates); + this.aggregates = new AggregateTable(maxAggregates, additionalTagsSchema); this.reportingIntervalNanos = reportingIntervalTimeUnit.toNanos(reportingInterval); this.sleepMillis = sleepMillis; this.healthMetrics = healthMetrics; this.onReportCycle = onReportCycle; } + void resetPropertyHandlers(HealthMetrics healthMetrics) { + aggregates.resetHandlers(healthMetrics); + } + @Override public void run() { Thread currentThread = Thread.currentThread(); @@ -174,7 +181,7 @@ private void report(long when, SignalItem signal) { writer, (w, entry) -> { w.add(entry); - entry.clear(); + entry.clearAggregate(); }); // note that this may do IO and block writer.finishBucket(); 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 3a3ded31ff8..bac7508f335 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 @@ -71,6 +71,7 @@ public final class ClientStatsAggregator implements MetricsAggregator, EventList private final TimeUnit reportingIntervalTimeUnit; private final DDAgentFeaturesDiscovery features; private final HealthMetrics healthMetrics; + private final AdditionalTagsSchema additionalTagsSchema; private final boolean includeEndpointInMetrics; /** @@ -100,6 +101,12 @@ public ClientStatsAggregator( this( config.getWellKnownTags(), config.getMetricsIgnoredResources(), + AdditionalTagsSchema.from( + config.getTraceStatsAdditionalTags(), + config.getTraceStatsCardinalityLimit( + "additional_tags", MetricCardinalityLimits.ADDITIONAL_TAG_VALUE), + true, + healthMetrics), sharedCommunicationObjects.featuresDiscovery(config), healthMetrics, new OkHttpSink( @@ -117,6 +124,7 @@ public ClientStatsAggregator( ClientStatsAggregator( WellKnownTags wellKnownTags, Set ignoredResources, + AdditionalTagsSchema additionalTagsSchema, DDAgentFeaturesDiscovery features, HealthMetrics healthMetric, Sink sink, @@ -126,6 +134,7 @@ public ClientStatsAggregator( this( wellKnownTags, ignoredResources, + additionalTagsSchema, features, healthMetric, sink, @@ -139,6 +148,7 @@ public ClientStatsAggregator( ClientStatsAggregator( WellKnownTags wellKnownTags, Set ignoredResources, + AdditionalTagsSchema additionalTagsSchema, DDAgentFeaturesDiscovery features, HealthMetrics healthMetric, Sink sink, @@ -149,6 +159,7 @@ public ClientStatsAggregator( boolean includeEndpointInMetrics) { this( ignoredResources, + additionalTagsSchema, features, healthMetric, sink, @@ -160,6 +171,7 @@ public ClientStatsAggregator( includeEndpointInMetrics); } + /** Test-only: defaults to no additional tags schema. */ ClientStatsAggregator( Set ignoredResources, DDAgentFeaturesDiscovery features, @@ -171,7 +183,34 @@ public ClientStatsAggregator( long reportingInterval, TimeUnit timeUnit, boolean includeEndpointInMetrics) { + this( + ignoredResources, + AdditionalTagsSchema.EMPTY, + features, + healthMetric, + sink, + metricWriter, + maxAggregates, + queueSize, + reportingInterval, + timeUnit, + includeEndpointInMetrics); + } + + ClientStatsAggregator( + Set ignoredResources, + AdditionalTagsSchema additionalTagsSchema, + DDAgentFeaturesDiscovery features, + HealthMetrics healthMetric, + Sink sink, + MetricWriter metricWriter, + int maxAggregates, + int queueSize, + long reportingInterval, + TimeUnit timeUnit, + boolean includeEndpointInMetrics) { this.ignoredResources = ignoredResources; + this.additionalTagsSchema = additionalTagsSchema; this.includeEndpointInMetrics = includeEndpointInMetrics; this.inbox = Queues.mpscArrayQueue(queueSize); this.features = features; @@ -185,6 +224,7 @@ public ClientStatsAggregator( reportingInterval, timeUnit, healthMetric, + additionalTagsSchema, this::resetCardinalityHandlers); this.thread = newAgentThread(METRICS_AGGREGATOR, aggregator); this.reportingInterval = reportingInterval; @@ -279,8 +319,8 @@ public boolean publish(List> trace) { boolean isTopLevel = span.isTopLevel(); if (shouldComputeMetric(span, isTopLevel)) { final CharSequence resourceName = span.getResourceName(); - if (resourceName != null - && !ignoredResources.isEmpty() + if (!ignoredResources.isEmpty() + && resourceName != null && ignoredResources.contains(resourceName.toString())) { // skip publishing all children break; @@ -343,6 +383,8 @@ private boolean publish(CoreSpan span, boolean isTopLevel, PeerTagSchema peer spanPeerTagSchema = null; } + String[] additionalTagValues = captureAdditionalTagValues(span); + SpanSnapshot snapshot = new SpanSnapshot( span.getResourceName(), @@ -359,6 +401,7 @@ private boolean publish(CoreSpan span, boolean isTopLevel, PeerTagSchema peer httpMethod, httpEndpoint, grpcStatusCode, + additionalTagValues, tagAndDuration); if (!inbox.offer(snapshot)) { healthMetrics.onStatsInboxFull(); @@ -367,6 +410,16 @@ private boolean publish(CoreSpan span, boolean isTopLevel, PeerTagSchema peer return error; } + /** + * Captures the span's additional-metric-tag values into a {@code String[]} parallel to {@code + * additionalTagsSchema.names}. Returns {@code null} when no additional tags are configured or + * none of the configured keys are set on the span. Raw values only -- length cap and + * canonicalization run on the aggregator thread. + */ + private String[] captureAdditionalTagValues(CoreSpan span) { + return captureTagValues(span, additionalTagsSchema.names); + } + /** * One-time producer-side bootstrap of {@link #cachedPeerTagSchema}. Synchronized double-check * guards against two producers racing on the very first publish; after this returns, {@code @@ -404,38 +457,18 @@ private PeerTagSchema buildPeerTagSchema() { /** * Single reset hook invoked on the aggregator thread at the end of each report cycle. Reconciles * the cached peer-tag schema against the latest feature discovery, then resets all cardinality - * state in lockstep: the static property handlers, {@code PeerTagSchema.INTERNAL}, and the cached - * peer-tag schema. New handlers added anywhere in this pipeline should be reset from here. + * state in lockstep: the property handlers, both peer-tag schemas, and the additional tags + * schema. New handlers added anywhere in this pipeline should be reset from here. */ private void resetCardinalityHandlers() { reconcilePeerTagSchema(); - for (PropertyCardinalityHandler handler : AggregateEntry.FIELD_HANDLERS) { - long blocked = handler.reset(); - if (blocked > 0) { - log.warn( - "Cardinality limit reached for stats field '{}'; further values will be reported as tracer_blocked_value", - handler.name); - healthMetrics.onTagCardinalityBlocked(handler.statsDTag(), blocked); - } - } - resetPeerTagSchema(PeerTagSchema.INTERNAL); + aggregator.resetPropertyHandlers(healthMetrics); + PeerTagSchema.INTERNAL.resetHandlers(healthMetrics); PeerTagSchema schema = cachedPeerTagSchema; if (schema != null) { - resetPeerTagSchema(schema); - } - } - - private void resetPeerTagSchema(PeerTagSchema schema) { - for (int i = 0; i < schema.handlers.length; i++) { - long blocked = schema.handlers[i].reset(); - if (blocked > 0) { - log.warn( - "Cardinality limit reached for peer tag '{}'; further values are reported as" - + " 'tracer_blocked_value' until the next reporting cycle", - schema.names[i]); - healthMetrics.onTagCardinalityBlocked(schema.handlers[i].statsDTag(), blocked); - } + schema.resetHandlers(healthMetrics); } + additionalTagsSchema.resetHandlers(); } /** @@ -465,7 +498,7 @@ private void reconcilePeerTagSchema() { } else { // Tags actually changed: flush the outgoing schema's accumulated block telemetry before // discarding it, otherwise the partial-cycle blockedCounts would silently disappear. - resetPeerTagSchema(cached); + cached.resetHandlers(healthMetrics); cachedPeerTagSchema = PeerTagSchema.of(normalized, latestState); } } @@ -492,7 +525,10 @@ private static PeerTagSchema peerTagSchemaFor(String spanKind, PeerTagSchema pee * Returns {@code null} when none of the configured peer tags are set on the span. */ private static String[] capturePeerTagValues(CoreSpan span, PeerTagSchema schema) { - String[] names = schema.names; + return captureTagValues(span, schema.names); + } + + private static String[] captureTagValues(CoreSpan span, String[] names) { int n = names.length; String[] values = null; for (int i = 0; i < n; i++) { diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricCardinalityLimits.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricCardinalityLimits.java index 6a679e98133..d0fd1e19eb1 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricCardinalityLimits.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricCardinalityLimits.java @@ -71,4 +71,18 @@ private MetricCardinalityLimits() {} * peer tag gets its own handler at this limit. */ static final int PEER_TAG_VALUE = 512; + + /** + * Distinct values per additional-tag key (e.g. distinct values of a span-derived primary tag). + * Each configured additional tag gets its own {@link TagCardinalityHandler} at this limit. + */ + static final int ADDITIONAL_TAG_VALUE = 100; + + /** + * Maximum character length for a single additional-tag value. Values longer than this are + * replaced with {@code tracer_blocked_value}; the application could accidentally populate a tag + * with a stack trace, SQL statement, or JSON blob, which would bloat every MetricKey and msgpack + * payload until the bucket flushes. + */ + static final int ADDITIONAL_TAG_MAX_VALUE_LENGTH = 200; } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/PeerTagSchema.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/PeerTagSchema.java index 1e0d51e84c8..b917d2f5f7d 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/PeerTagSchema.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/PeerTagSchema.java @@ -5,7 +5,10 @@ import datadog.communication.ddagent.DDAgentFeaturesDiscovery; import datadog.trace.api.Config; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import datadog.trace.core.monitor.HealthMetrics; import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Parallel arrays of peer-tag names and their {@link TagCardinalityHandler}s, using matching @@ -40,6 +43,8 @@ */ final class PeerTagSchema { + private static final Logger log = LoggerFactory.getLogger(PeerTagSchema.class); + /** * Sentinel {@link #state} for schemas that are never reconciled against feature discovery: the * {@link #INTERNAL} singleton and test-built schemas. A {@code null} state always mismatches a @@ -109,6 +114,24 @@ UTF8BytesString register(int i, String value) { return handlers[i].register(value); } + /** + * Resets every {@link TagCardinalityHandler}'s working set, flushes accumulated per-tag block + * counts to {@link HealthMetrics}, and emits a warn log for each tag that hit its limit this + * cycle. Must be called on the aggregator thread; handlers are not thread-safe. + */ + void resetHandlers(HealthMetrics healthMetrics) { + for (int i = 0; i < handlers.length; i++) { + long blocked = handlers[i].reset(); + if (blocked > 0) { + log.warn( + "Cardinality limit reached for peer tag '{}'; further values are reported as" + + " 'tracer_blocked_value' until the next reporting cycle", + names[i]); + healthMetrics.onTagCardinalityBlocked(handlers[i].statsDTag(), blocked); + } + } + } + int size() { return names.length; } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/PropertyHandlers.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/PropertyHandlers.java new file mode 100644 index 00000000000..a3139e5032d --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/PropertyHandlers.java @@ -0,0 +1,76 @@ +package datadog.trace.common.metrics; + +import datadog.trace.api.Config; +import datadog.trace.core.monitor.HealthMetrics; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Bundles the nine per-field property cardinality handlers; owned by {@link ClientStatsAggregator}. + */ +final class PropertyHandlers { + + private static final Logger log = LoggerFactory.getLogger(PropertyHandlers.class); + + final PropertyCardinalityHandler resource; + final PropertyCardinalityHandler service; + final PropertyCardinalityHandler operation; + final PropertyCardinalityHandler serviceSource; + final PropertyCardinalityHandler type; + final PropertyCardinalityHandler spanKind; + final PropertyCardinalityHandler httpMethod; + final PropertyCardinalityHandler httpEndpoint; + final PropertyCardinalityHandler grpcStatusCode; + + private final PropertyCardinalityHandler[] handlers; + + PropertyHandlers() { + Config config = Config.get(); + // Configurable limits — tunable via env var. + this.resource = + new PropertyCardinalityHandler( + "resource", + config.getTraceStatsCardinalityLimit("resource", MetricCardinalityLimits.RESOURCE)); + this.httpEndpoint = + new PropertyCardinalityHandler( + "http_endpoint", + config.getTraceStatsCardinalityLimit( + "http_endpoint", MetricCardinalityLimits.HTTP_ENDPOINT)); + // Fixed limits — hardcoded, not user-configurable. + this.service = new PropertyCardinalityHandler("service", MetricCardinalityLimits.SERVICE); + this.operation = new PropertyCardinalityHandler("operation", MetricCardinalityLimits.OPERATION); + this.serviceSource = + new PropertyCardinalityHandler("service_source", MetricCardinalityLimits.SERVICE_SOURCE); + this.type = new PropertyCardinalityHandler("type", MetricCardinalityLimits.TYPE); + this.spanKind = new PropertyCardinalityHandler("span_kind", MetricCardinalityLimits.SPAN_KIND); + this.httpMethod = + new PropertyCardinalityHandler("http_method", MetricCardinalityLimits.HTTP_METHOD); + this.grpcStatusCode = + new PropertyCardinalityHandler( + "grpc_status_code", MetricCardinalityLimits.GRPC_STATUS_CODE); + this.handlers = + new PropertyCardinalityHandler[] { + resource, + service, + operation, + serviceSource, + type, + spanKind, + httpMethod, + httpEndpoint, + grpcStatusCode + }; + } + + void reset(HealthMetrics healthMetrics) { + for (PropertyCardinalityHandler h : handlers) { + long blocked = h.reset(); + if (blocked > 0) { + log.warn( + "Cardinality limit reached for stats field '{}'; further values will be reported as tracer_blocked_value", + h.name); + healthMetrics.onTagCardinalityBlocked(h.statsDTag(), blocked); + } + } + } +} diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/SerializingMetricWriter.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/SerializingMetricWriter.java index 622a4a14cb0..fc9822afec2 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/SerializingMetricWriter.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/SerializingMetricWriter.java @@ -43,6 +43,7 @@ public final class SerializingMetricWriter implements MetricWriter { private static final byte[] IS_TRACE_ROOT = "IsTraceRoot".getBytes(ISO_8859_1); private static final byte[] SPAN_KIND = "SpanKind".getBytes(ISO_8859_1); private static final byte[] PEER_TAGS = "PeerTags".getBytes(ISO_8859_1); + private static final byte[] ADDITIONAL_METRIC_TAGS = "AdditionalMetricTags".getBytes(ISO_8859_1); private static final byte[] HTTP_METHOD = "HTTPMethod".getBytes(ISO_8859_1); private static final byte[] HTTP_ENDPOINT = "HTTPEndpoint".getBytes(ISO_8859_1); private static final byte[] GRPC_STATUS_CODE = "GRPCStatusCode".getBytes(ISO_8859_1); @@ -157,12 +158,15 @@ public void add(AggregateEntry entry) { final boolean hasHttpEndpoint = entry.hasHttpEndpoint(); final boolean hasServiceSource = entry.hasServiceSource(); final boolean hasGrpcStatusCode = entry.hasGrpcStatusCode(); + final UTF8BytesString[] additionalTags = entry.getAdditionalTags(); + final boolean hasAdditionalTags = additionalTags.length > 0; final int mapSize = 15 + (hasServiceSource ? 1 : 0) + (hasHttpMethod ? 1 : 0) + (hasHttpEndpoint ? 1 : 0) - + (hasGrpcStatusCode ? 1 : 0); + + (hasGrpcStatusCode ? 1 : 0) + + (hasAdditionalTags ? 1 : 0); writer.startMap(mapSize); @@ -198,6 +202,17 @@ public void add(AggregateEntry entry) { writer.writeUTF8(peerTag); } + // Emit AdditionalMetricTags as a packed array of pre-built "key:value" UTF8BytesStrings, in + // schema (alphabetical-by-key) order. The field is omitted entirely when the entry carries no + // additional tags, so spans that set none pay zero payload overhead. + if (hasAdditionalTags) { + writer.writeUTF8(ADDITIONAL_METRIC_TAGS); + writer.startArray(additionalTags.length); + for (UTF8BytesString slot : additionalTags) { + writer.writeUTF8(slot); + } + } + if (hasServiceSource) { writer.writeUTF8(SERVICE_SOURCE); writer.writeUTF8(entry.getServiceSource()); diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/SpanSnapshot.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/SpanSnapshot.java index 8bbc6a29edb..1fc732b5dca 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/SpanSnapshot.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/SpanSnapshot.java @@ -40,6 +40,14 @@ final class SpanSnapshot implements InboxItem { @Nullable final String httpEndpoint; @Nullable final String grpcStatusCode; + /** + * Additional metric tag values captured from the span, parallel to {@code + * additionalTagsSchema.names}. A {@code null} entry means the span didn't have that tag set. + * {@code null} (the whole array) when no additional tags are configured or none were set on the + * span. Length cap is applied on the aggregator thread; the producer carries raw values only. + */ + final String[] additionalTagValues; + /** Duration in nanoseconds, OR-ed with {@code ERROR_TAG} / {@code TOP_LEVEL_TAG} as needed. */ final long tagAndDuration; @@ -55,9 +63,10 @@ final class SpanSnapshot implements InboxItem { String spanKind, @Nullable PeerTagSchema peerTagSchema, @Nullable String[] peerTagValues, - @Nullable String httpMethod, - @Nullable String httpEndpoint, - @Nullable String grpcStatusCode, + String httpMethod, + String httpEndpoint, + String grpcStatusCode, + String[] additionalTagValues, long tagAndDuration) { this.resourceName = resourceName; this.serviceName = serviceName; @@ -73,6 +82,7 @@ final class SpanSnapshot implements InboxItem { this.httpMethod = httpMethod; this.httpEndpoint = httpEndpoint; this.grpcStatusCode = grpcStatusCode; + this.additionalTagValues = additionalTagValues; this.tagAndDuration = tagAndDuration; } } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/TagCardinalityHandler.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/TagCardinalityHandler.java index 3c3d151142c..de04ec44b64 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/TagCardinalityHandler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/TagCardinalityHandler.java @@ -21,6 +21,7 @@ final class TagCardinalityHandler { private final String tag; private String[] statsDTag = null; private final int cardinalityLimit; + private final int maxValueLength; private final int capacityMask; /** See {@link PropertyCardinalityHandler}'s field of the same name. */ @@ -38,15 +39,20 @@ final class TagCardinalityHandler { private long blockedCount; /** - * Test convenience: limits-enabled mode. Production uses the three-argument constructor with the - * flag from {@code Config}. + * Test convenience: limits-enabled mode, no per-value length cap. Production uses the + * four-argument constructor. */ @VisibleForTesting TagCardinalityHandler(String tag, int cardinalityLimit) { - this(tag, cardinalityLimit, true); + this(tag, cardinalityLimit, true, Integer.MAX_VALUE); } TagCardinalityHandler(String tag, int cardinalityLimit, boolean useBlockedSentinel) { + this(tag, cardinalityLimit, useBlockedSentinel, Integer.MAX_VALUE); + } + + TagCardinalityHandler( + String tag, int cardinalityLimit, boolean useBlockedSentinel, int maxValueLength) { if (cardinalityLimit <= 0) { throw new IllegalArgumentException("cardinalityLimit must be positive: " + cardinalityLimit); } @@ -57,6 +63,7 @@ final class TagCardinalityHandler { this.tag = tag; this.cardinalityLimit = cardinalityLimit; this.useBlockedSentinel = useBlockedSentinel; + this.maxValueLength = maxValueLength; final int capacity = Integer.highestOneBit(cardinalityLimit * 2 - 1) << 1; this.capacityMask = capacity - 1; this.curKeys = new String[capacity]; @@ -82,6 +89,12 @@ UTF8BytesString register(String value) { if (value == null) { return UTF8BytesString.EMPTY; } + // Values exceeding the length cap are blocked regardless of cardinality budget: application + // code can accidentally populate a tag with a stack trace, SQL statement, or JSON blob. + if (value.length() > this.maxValueLength && this.useBlockedSentinel) { + this.blockedCount++; + return this.tracerBlockedValue(); + } // Compute the initial probe slot once. The same start slot is used for the // current-cycle table and, on miss, for the prior-cycle table. int h = value.hashCode(); diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/ClientStatsAggregatorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/ClientStatsAggregatorTest.groovy index 2f409d7baa5..cd37d7487f4 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/ClientStatsAggregatorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/ClientStatsAggregatorTest.groovy @@ -20,10 +20,6 @@ import spock.lang.Shared class ClientStatsAggregatorTest extends DDSpecification { - def setup() { - AggregateEntry.resetCardinalityHandlers() - } - static Set empty = new HashSet<>() static final int HTTP_OK = 200 @@ -42,6 +38,7 @@ class ClientStatsAggregatorTest extends DDSpecification { ClientStatsAggregator aggregator = new ClientStatsAggregator( wellKnownTags, empty, + AdditionalTagsSchema.EMPTY, features, HealthMetrics.NO_OP, sink, @@ -72,6 +69,7 @@ class ClientStatsAggregatorTest extends DDSpecification { ClientStatsAggregator aggregator = new ClientStatsAggregator( wellKnownTags, [ignoredResourceName].toSet(), + AdditionalTagsSchema.EMPTY, features, HealthMetrics.NO_OP, sink, @@ -1701,6 +1699,51 @@ class ClientStatsAggregatorTest extends DDSpecification { aggregator.close() } + def "cardinality limits reset between report cycles"() { + setup: + List cycle1Entries = [] + List cycle2Entries = [] + CountDownLatch latch1 = new CountDownLatch(1) + CountDownLatch latch2 = new CountDownLatch(1) + MetricWriter writer = Mock(MetricWriter) + Sink sink = Stub(Sink) + DDAgentFeaturesDiscovery features = Mock(DDAgentFeaturesDiscovery) + features.supportsMetrics() >> true + features.peerTags() >> [] + ClientStatsAggregator aggregator = new ClientStatsAggregator(empty, + features, HealthMetrics.NO_OP, sink, writer, 256, queueSize, reportingInterval, SECONDS, false) + aggregator.start() + + when: "publish SERVICE+1 distinct services to fill and overflow the cardinality budget" + for (int i = 0; i <= MetricCardinalityLimits.SERVICE; i++) { + aggregator.publish([new SimpleSpan("svc-$i", "op", "resource", "web", false, true, false, 0, 100, HTTP_OK)]) + } + aggregator.report() + latch1.await(2, SECONDS) + + then: "the overflow service maps to the tracer_blocked_value sentinel" + 1 * writer.startBucket(MetricCardinalityLimits.SERVICE + 1, _, _) + (1.._) * writer.add(_) >> { AggregateEntry e -> cycle1Entries << e } + 1 * writer.finishBucket() >> { latch1.countDown() } + cycle1Entries.count { it.getService().toString() == "tracer_blocked_value" } == 1 + + when: "publish the overflow service in the next cycle after the cardinality reset" + aggregator.publish([ + new SimpleSpan("svc-${MetricCardinalityLimits.SERVICE}", "op", "resource", "web", false, true, false, 0, 100, HTTP_OK) + ]) + aggregator.report() + latch2.await(2, SECONDS) + + then: "after reset the overflow service name is accepted as a real entry" + 1 * writer.startBucket(1, _, _) + 1 * writer.add(_) >> { AggregateEntry e -> cycle2Entries << e } + 1 * writer.finishBucket() >> { latch2.countDown() } + cycle2Entries[0].getService().toString() == "svc-${MetricCardinalityLimits.SERVICE}" + + cleanup: + aggregator.close() + } + def reportAndWaitUntilEmpty(ClientStatsAggregator aggregator) { waitUntilEmpty(aggregator) aggregator.report() diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/FootprintForkedTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/FootprintForkedTest.groovy index 86a91c23b3f..fb5bc2ed561 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/FootprintForkedTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/FootprintForkedTest.groovy @@ -40,6 +40,7 @@ class FootprintForkedTest extends DDSpecification { ClientStatsAggregator aggregator = new ClientStatsAggregator( new WellKnownTags("runtimeid","hostname", "env", "service", "version","language"), [].toSet() as Set, + AdditionalTagsSchema.EMPTY, features, HealthMetrics.NO_OP, sink, diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AdditionalTagsSchemaTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AdditionalTagsSchemaTest.java new file mode 100644 index 00000000000..3e75f9acd86 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AdditionalTagsSchemaTest.java @@ -0,0 +1,62 @@ +package datadog.trace.common.metrics; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashSet; +import org.junit.jupiter.api.Test; + +class AdditionalTagsSchemaTest { + + @Test + void emptyConfigReturnsSharedEmptySchema() { + assertSame(AdditionalTagsSchema.EMPTY, AdditionalTagsSchema.from(null)); + assertSame(AdditionalTagsSchema.EMPTY, AdditionalTagsSchema.from(Collections.emptySet())); + } + + @Test + void schemaSortsKeysAlphabetically() { + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(new LinkedHashSet<>(Arrays.asList("region", "tenant_id", "az"))); + assertArrayEquals(new String[] {"az", "region", "tenant_id"}, schema.names); + } + + @Test + void schemaDedupesAndCapsAtMaxTagKeys() { + LinkedHashSet configured = new LinkedHashSet<>(); + // 6 distinct keys, more than MAX_ADDITIONAL_TAG_KEYS (4). Sort alphabetically, drop the last 2. + for (int i = 0; i < 6; i++) { + configured.add(String.format("tag%02d", i)); + } + AdditionalTagsSchema schema = AdditionalTagsSchema.from(configured); + assertEquals(AdditionalTagsSchema.MAX_ADDITIONAL_TAG_KEYS, schema.size()); + assertArrayEquals(new String[] {"tag00", "tag01", "tag02", "tag03"}, schema.names); + } + + @Test + void rejectsEmptyAndColonContainingKeys() { + AdditionalTagsSchema schema = + AdditionalTagsSchema.from( + new LinkedHashSet<>(Arrays.asList("region", "", "bad:key", "tenant_id"))); + // Empty key and "bad:key" are dropped; only the two valid keys remain. + assertArrayEquals(new String[] {"region", "tenant_id"}, schema.names); + } + + @Test + void allInvalidKeysReturnsEmptySchema() { + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(new LinkedHashSet<>(Arrays.asList("", "also:bad"))); + assertSame(AdditionalTagsSchema.EMPTY, schema); + } + + @Test + void emptySchemaHasZeroSize() { + AdditionalTagsSchema schema = AdditionalTagsSchema.EMPTY; + assertEquals(0, schema.size()); + assertTrue(schema.names.length == 0); + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTest.java index dd70083d9d4..f2fc1d5710d 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTest.java @@ -14,16 +14,10 @@ import datadog.metrics.impl.MonitoringImpl; import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class AggregateEntryTest { - @BeforeEach - void resetCardinalityHandlers() { - AggregateEntry.resetCardinalityHandlers(); - } - @BeforeAll static void initAgentMeter() { // recordOneDuration -> Histogram.accept needs AgentMeter to be initialized. @@ -47,7 +41,7 @@ void clearResetsAllCounters() { entry.recordOneDuration(5L); entry.recordOneDuration(ERROR_TAG | 6L); entry.recordOneDuration(TOP_LEVEL_TAG | 7L); - entry.clear(); + entry.clearAggregate(); assertEquals(0, entry.getDuration()); assertEquals(0, entry.getErrorCount()); assertEquals(0, entry.getTopLevelCount()); 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 a8ced924c3e..f4f340aa686 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 @@ -56,6 +56,7 @@ 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]; long keyHash = AggregateEntry.hashOf( resourceUtf, @@ -71,7 +72,9 @@ public static AggregateEntry of( synthetic, traceRoot, peerTagsArr, - peerTagsArr.length); + peerTagsArr.length, + emptyAdditional, + 0); return new AggregateEntry( keyHash, resourceUtf, @@ -86,7 +89,8 @@ public static AggregateEntry of( (short) httpStatusCode, synthetic, traceRoot, - peerTagsList); + peerTagsList, + emptyAdditional); } /** diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateTableAdditionalTagsTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateTableAdditionalTagsTest.java new file mode 100644 index 00000000000..20402a662c5 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateTableAdditionalTagsTest.java @@ -0,0 +1,84 @@ +package datadog.trace.common.metrics; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; + +import datadog.metrics.agent.AgentMeter; +import datadog.metrics.api.statsd.StatsDClient; +import datadog.metrics.impl.DDSketchHistograms; +import datadog.metrics.impl.MonitoringImpl; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +class AggregateTableAdditionalTagsTest { + + @BeforeAll + static void initAgentMeter() { + MonitoringImpl monitoring = new MonitoringImpl(StatsDClient.NO_OP, 1, TimeUnit.SECONDS); + AgentMeter.registerIfAbsent(StatsDClient.NO_OP, monitoring, DDSketchHistograms.FACTORY); + monitoring.newTimer("test.init"); + } + + @Test + void distinctAdditionalTagValuesYieldDistinctEntries() { + AdditionalTagsSchema schema = schemaFor("region"); + AggregateTable table = newTable(schema); + + AggregateEntry usEast = table.findOrInsert(snapshot(schema, "us-east-1")); + AggregateEntry euWest = table.findOrInsert(snapshot(schema, "eu-west-1")); + + assertNotNull(usEast); + assertNotNull(euWest); + assertNotSame(usEast, euWest); + assertEquals(2, table.size()); + } + + @Test + void sameAdditionalTagValuesShareEntry() { + AdditionalTagsSchema schema = schemaFor("region"); + AggregateTable table = newTable(schema); + + AggregateEntry first = table.findOrInsert(snapshot(schema, "us-east-1")); + AggregateEntry second = table.findOrInsert(snapshot(schema, "us-east-1")); + + assertSame(first, second); + assertEquals(1, table.size()); + } + + // ---------- helpers ---------- + + private static AdditionalTagsSchema schemaFor(String... names) { + return AdditionalTagsSchema.from(new LinkedHashSet<>(Arrays.asList(names))); + } + + private static AggregateTable newTable(AdditionalTagsSchema schema) { + return new AggregateTable(256, schema); + } + + private static SpanSnapshot snapshot(AdditionalTagsSchema schema, String regionValue) { + String[] values = new String[schema.size()]; + values[0] = regionValue; + return new SpanSnapshot( + "resource", + "service", + "operation", + null, + "web", + (short) 200, + false, + true, + "client", + null, + null, + null, + null, + null, + values, + 0L); + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateTableTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateTableTest.java index 05acd57985d..dcb0a0414c3 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateTableTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateTableTest.java @@ -8,18 +8,21 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import datadog.metrics.agent.AgentMeter; import datadog.metrics.api.statsd.StatsDClient; import datadog.metrics.impl.DDSketchHistograms; import datadog.metrics.impl.MonitoringImpl; +import datadog.trace.core.monitor.HealthMetrics; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class AggregateTableTest { @@ -32,15 +35,6 @@ static void initAgentMeter() { monitoring.newTimer("test.init"); } - @BeforeEach - void resetCardinalityHandlers() { - // AggregateEntry's property handlers are static and accumulate state across tests. Some tests - // in this class (e.g. backToBackEvictionsAllSucceed) drive 40 distinct services, which exceeds - // MetricCardinalityLimits.SERVICE (32) and leaves later tests seeing a shared "blocked" - // canonical for "a"/"b"/"c"-style services -- collapsing distinct snapshots into one entry. - AggregateEntry.resetCardinalityHandlers(); - } - @Test void insertOnMissReturnsNewAggregate() { AggregateTable table = new AggregateTable(8); @@ -292,6 +286,7 @@ private static SpanSnapshot nullServiceKindSnapshot(String service, String spanK null, null, null, + null, 0L); } @@ -312,9 +307,38 @@ private static SpanSnapshot nullableSnapshot( null, null, null, + null, 0L); } + @Test + void resetHandlersClearsBlockedCountsAndRefreshesCapacity() { + // Use limits-enabled handlers injected via the 3-arg constructor to test resetHandlers() + // without relying on the Config flag being set. + PropertyHandlers handlers = new PropertyHandlers(); + AggregateTable table = new AggregateTable(512, AdditionalTagsSchema.EMPTY, handlers); + + // Fill the service cardinality budget and push one value over the limit. + for (int i = 0; i < MetricCardinalityLimits.SERVICE; i++) { + table.findOrInsert(snapshot("svc-" + i, "op", "client")); + } + AggregateEntry blocked = table.findOrInsert(snapshot("svc-overflow", "op", "client")); + // All overflow services map to the same sentinel bucket. + AggregateEntry blocked2 = table.findOrInsert(snapshot("svc-overflow-2", "op", "client")); + assertSame(blocked, blocked2); + + HealthMetrics metrics = mock(HealthMetrics.class); + table.resetHandlers(metrics); + + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:service"}, 2L); + verifyNoMoreInteractions(metrics); + + // After reset, a new service name should land in a fresh bucket, not the sentinel. + AggregateEntry afterReset = table.findOrInsert(snapshot("svc-new", "op", "client")); + assertNotSame(blocked, afterReset); + assertEquals("svc-new", afterReset.getService().toString()); + } + // ---------- helpers ---------- private static SpanSnapshot snapshot(String service, String operation, String spanKind) { @@ -368,6 +392,7 @@ SpanSnapshot build() { null, null, null, + null, tagAndDuration); } } diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/CardinalityHandlerTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/CardinalityHandlerTest.java index 5a882aba11b..52ef0dc54d1 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/metrics/CardinalityHandlerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/CardinalityHandlerTest.java @@ -250,8 +250,6 @@ void tagOverLimitWithSentinelDisabledReturnsFreshUtf8() { @Test void tagOverLimitWithSentinelDisabledNeverSubstitutesBlockedSentinel() { - // The sentinel should never materialize in disabled mode -- over-cap values carry their real - // "tag:value" content rather than the blocked sentinel. TagCardinalityHandler h = new TagCardinalityHandler("peer.service", 1, false); h.register("svc-1"); UTF8BytesString overCap = h.register("svc-2"); diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/PeerTagSchemaTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/PeerTagSchemaTest.java index 710e78a175b..dd5c58c3650 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/metrics/PeerTagSchemaTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/PeerTagSchemaTest.java @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import datadog.trace.core.monitor.HealthMetrics; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -85,10 +86,10 @@ void hasSameTagsAsHandlesEmpty() { } @Test - void handlerAccumulatesBlockedCountsAcrossRegistrations() { + void resetHandlersReportsBlockedCountToHealthMetrics() { // Build a schema then replace its handler with a sentinel-mode instance at a low limit. // (Production schemas use AggregateEntry.LIMITS_ENABLED which is currently false; this test - // exercises the blocked-count path directly so it stays valid before and after the flag flips.) + // exercises the reportingpath directly so it stays valid before and after the flag flips.) PeerTagSchema schema = new PeerTagSchema(new String[] {"peer.hostname"}, PeerTagSchema.NO_STATE); schema.handlers[0] = new TagCardinalityHandler("peer.hostname", 1, true); @@ -97,9 +98,21 @@ void handlerAccumulatesBlockedCountsAcrossRegistrations() { schema.register(0, "host-b"); // blocked schema.register(0, "host-c"); // blocked - assertEquals(2, schema.handlers[0].reset()); + long[] recorded = {0}; + HealthMetrics hm = + new HealthMetrics() { + @Override + public void onTagCardinalityBlocked(String[] tag, long count) { + recorded[0] += count; + } + }; + + schema.resetHandlers(hm); + assertEquals(2, recorded[0]); // After the reset, no new values were registered so the next reset reports nothing. - assertEquals(0, schema.handlers[0].reset()); + recorded[0] = 0; + schema.resetHandlers(hm); + assertEquals(0, recorded[0]); } } diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/PropertyHandlersTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/PropertyHandlersTest.java new file mode 100644 index 00000000000..190499c6f78 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/PropertyHandlersTest.java @@ -0,0 +1,93 @@ +package datadog.trace.common.metrics; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import datadog.trace.core.monitor.HealthMetrics; +import org.junit.jupiter.api.Test; + +class PropertyHandlersTest { + + @Test + void resetReportsBlockedCountForExhaustedHandler() { + PropertyHandlers handlers = new PropertyHandlers(); + // Exhaust span_kind (limit = 8) and record 2 blocked values. + for (int i = 0; i < MetricCardinalityLimits.SPAN_KIND; i++) { + handlers.spanKind.register("kind-" + i); + } + handlers.spanKind.register("overflow-1"); + handlers.spanKind.register("overflow-2"); + + HealthMetrics metrics = mock(HealthMetrics.class); + handlers.reset(metrics); + + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:span_kind"}, 2L); + verifyNoMoreInteractions(metrics); + } + + @Test + void resetReportsBlockedCountForAllNineHandlers() { + PropertyHandlers handlers = new PropertyHandlers(); + exhaustAndBlock(handlers.resource, MetricCardinalityLimits.RESOURCE); + exhaustAndBlock(handlers.service, MetricCardinalityLimits.SERVICE); + exhaustAndBlock(handlers.operation, MetricCardinalityLimits.OPERATION); + exhaustAndBlock(handlers.serviceSource, MetricCardinalityLimits.SERVICE_SOURCE); + exhaustAndBlock(handlers.type, MetricCardinalityLimits.TYPE); + exhaustAndBlock(handlers.spanKind, MetricCardinalityLimits.SPAN_KIND); + exhaustAndBlock(handlers.httpMethod, MetricCardinalityLimits.HTTP_METHOD); + exhaustAndBlock(handlers.httpEndpoint, MetricCardinalityLimits.HTTP_ENDPOINT); + exhaustAndBlock(handlers.grpcStatusCode, MetricCardinalityLimits.GRPC_STATUS_CODE); + + HealthMetrics metrics = mock(HealthMetrics.class); + handlers.reset(metrics); + + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:resource"}, 1L); + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:service"}, 1L); + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:operation"}, 1L); + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:service_source"}, 1L); + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:type"}, 1L); + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:span_kind"}, 1L); + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:http_method"}, 1L); + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:http_endpoint"}, 1L); + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:grpc_status_code"}, 1L); + verifyNoMoreInteractions(metrics); + } + + @Test + void resetRefreshesCapacityForNextCycle() { + PropertyHandlers handlers = new PropertyHandlers(); + for (int i = 0; i < MetricCardinalityLimits.SPAN_KIND; i++) { + handlers.spanKind.register("kind-" + i); + } + assertEquals("tracer_blocked_value", handlers.spanKind.register("overflow").toString()); + + handlers.reset(HealthMetrics.NO_OP); + + // Overflow value should now be accepted as a real value. + assertNotEquals("tracer_blocked_value", handlers.spanKind.register("overflow").toString()); + assertEquals("overflow", handlers.spanKind.register("overflow").toString()); + } + + @Test + void resetWithNoBlockedValuesDoesNotCallHealthMetrics() { + PropertyHandlers handlers = new PropertyHandlers(); + handlers.resource.register("r1"); + handlers.service.register("svc"); + + HealthMetrics metrics = mock(HealthMetrics.class); + handlers.reset(metrics); + + verifyNoMoreInteractions(metrics); + } + + /** Fills {@code handler} to its cardinality limit then registers one more to block it. */ + private static void exhaustAndBlock(PropertyCardinalityHandler handler, int limit) { + for (int i = 0; i < limit; i++) { + handler.register("value-" + i); + } + handler.register("overflow"); + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/SerializingMetricWriterAdditionalTagsTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/SerializingMetricWriterAdditionalTagsTest.java new file mode 100644 index 00000000000..717ffeb7284 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/SerializingMetricWriterAdditionalTagsTest.java @@ -0,0 +1,220 @@ +package datadog.trace.common.metrics; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import datadog.metrics.agent.AgentMeter; +import datadog.metrics.api.Histograms; +import datadog.metrics.api.statsd.StatsDClient; +import datadog.metrics.impl.DDSketchHistograms; +import datadog.metrics.impl.MonitoringImpl; +import datadog.trace.api.WellKnownTags; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.msgpack.core.MessagePack; +import org.msgpack.core.MessageUnpacker; + +class SerializingMetricWriterAdditionalTagsTest { + + @BeforeAll + static void initAgentMeter() { + MonitoringImpl monitoring = new MonitoringImpl(StatsDClient.NO_OP, 1, TimeUnit.SECONDS); + AgentMeter.registerIfAbsent(StatsDClient.NO_OP, monitoring, DDSketchHistograms.FACTORY); + monitoring.newTimer("test.init"); + Histograms.register(DDSketchHistograms.FACTORY); + } + + @Test + void additionalMetricTagsEmittedWhenSet() throws Exception { + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(new LinkedHashSet<>(Arrays.asList("region", "tenant_id"))); + AggregateTable table = newTable(schema); + + AggregateEntry entry = table.findOrInsert(snapshot(schema, "us-east-1", "acme-corp")); + entry.recordOneDuration(1L); + + List additionalTags = parseAdditionalMetricTags(writeBucket(table)); + assertEquals(2, additionalTags.size()); + // Order matches schema (alphabetical): region first, then tenant_id. + assertEquals("region:us-east-1", additionalTags.get(0)); + assertEquals("tenant_id:acme-corp", additionalTags.get(1)); + } + + @Test + void additionalMetricTagsFieldOmittedWhenNoneSet() throws Exception { + // Schema configured, but the span doesn't set any of the configured tags. + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(new LinkedHashSet<>(Arrays.asList("region"))); + AggregateTable table = newTable(schema); + + AggregateEntry entry = table.findOrInsert(snapshot(schema, new String[] {null})); + entry.recordOneDuration(1L); + + assertFalse( + containsKey(writeBucket(table), "AdditionalMetricTags"), + "AdditionalMetricTags should be omitted when no slots are populated"); + } + + @Test + void additionalMetricTagsSkipsNullSlots() throws Exception { + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(new LinkedHashSet<>(Arrays.asList("region", "tenant_id"))); + AggregateTable table = newTable(schema); + + // Set only tenant_id; leave region null. + AggregateEntry entry = + table.findOrInsert( + snapshot( + schema, + new String[] { + /*region*/ + null, /*tenant_id*/ "acme-corp" + })); + entry.recordOneDuration(1L); + + List additionalTags = parseAdditionalMetricTags(writeBucket(table)); + assertEquals(1, additionalTags.size()); + assertEquals("tenant_id:acme-corp", additionalTags.get(0)); + } + + // ---------- helpers ---------- + + private static AggregateTable newTable(AdditionalTagsSchema schema) { + return new AggregateTable(64, schema); + } + + private static SpanSnapshot snapshot(AdditionalTagsSchema schema, String... values) { + String[] padded = new String[schema.size()]; + if (values != null) { + System.arraycopy(values, 0, padded, 0, Math.min(values.length, padded.length)); + } + return new SpanSnapshot( + "resource", + "service", + "operation", + null, + "web", + (short) 200, + false, + true, + "client", + null, + null, + null, + null, + null, + padded, + 0L); + } + + /** + * Serializes a single-bucket payload via {@link SerializingMetricWriter} into a {@link + * ByteBuffer}. The test's {@link CapturingSink} keeps the produced buffer for unpack. + */ + private static ByteBuffer writeBucket(AggregateTable table) { + CapturingSink sink = new CapturingSink(); + SerializingMetricWriter writer = + new SerializingMetricWriter( + new WellKnownTags("rid", "host", "env", "svc", "ver", "lang"), sink, 64 * 1024); + writer.startBucket(table.size(), 0L, TimeUnit.SECONDS.toNanos(10)); + table.forEach(writer::add); + writer.finishBucket(); + return sink.buffer; + } + + private static List parseAdditionalMetricTags(ByteBuffer payload) throws Exception { + MessageUnpacker unpacker = MessagePack.newDefaultUnpacker(payload); + // Top-level map: skip to the per-stat entry. Structure mirrors SerializingMetricWriterTest. + int topMapSize = unpacker.unpackMapHeader(); + for (int i = 0; i < topMapSize; i++) { + String key = unpacker.unpackString(); + if ("Stats".equals(key)) { + // Stats is a 1-element array of buckets; each bucket has Start/Duration/Stats(=array of + // per-metric maps). + unpacker.unpackArrayHeader(); + int bucketMapSize = unpacker.unpackMapHeader(); + for (int j = 0; j < bucketMapSize; j++) { + String bucketKey = unpacker.unpackString(); + if ("Stats".equals(bucketKey)) { + int statsCount = unpacker.unpackArrayHeader(); + // Take the first stat entry and walk its map looking for AdditionalMetricTags. + for (int k = 0; k < statsCount; k++) { + int entryMapSize = unpacker.unpackMapHeader(); + for (int m = 0; m < entryMapSize; m++) { + String entryKey = unpacker.unpackString(); + if ("AdditionalMetricTags".equals(entryKey)) { + int n = unpacker.unpackArrayHeader(); + List result = new ArrayList<>(n); + for (int p = 0; p < n; p++) { + result.add(unpacker.unpackString()); + } + return result; + } else { + unpacker.skipValue(); + } + } + if (k == 0) break; // only inspecting the first stat entry + } + } else { + unpacker.skipValue(); + } + } + } else { + unpacker.skipValue(); + } + } + return new ArrayList<>(); + } + + private static boolean containsKey(ByteBuffer payload, String soughtKey) throws Exception { + MessageUnpacker unpacker = MessagePack.newDefaultUnpacker(payload); + int topMapSize = unpacker.unpackMapHeader(); + for (int i = 0; i < topMapSize; i++) { + String key = unpacker.unpackString(); + if ("Stats".equals(key)) { + unpacker.unpackArrayHeader(); + int bucketMapSize = unpacker.unpackMapHeader(); + for (int j = 0; j < bucketMapSize; j++) { + String bucketKey = unpacker.unpackString(); + if ("Stats".equals(bucketKey)) { + int statsCount = unpacker.unpackArrayHeader(); + for (int k = 0; k < statsCount; k++) { + int entryMapSize = unpacker.unpackMapHeader(); + for (int m = 0; m < entryMapSize; m++) { + String entryKey = unpacker.unpackString(); + if (soughtKey.equals(entryKey)) { + return true; + } + unpacker.skipValue(); + } + if (k == 0) return false; // checked the only entry + } + } else { + unpacker.skipValue(); + } + } + } else { + unpacker.skipValue(); + } + } + return false; + } + + private static final class CapturingSink implements Sink { + ByteBuffer buffer; + + @Override + public void register(EventListener listener) {} + + @Override + public void accept(int messageCount, ByteBuffer buffer) { + this.buffer = buffer.duplicate(); + } + } +} diff --git a/dd-trace-core/src/traceAgentTest/java/datadog/trace/common/metrics/MetricsIntegrationTest.java b/dd-trace-core/src/traceAgentTest/java/datadog/trace/common/metrics/MetricsIntegrationTest.java index ed28085a39a..6ae46f6eeca 100644 --- a/dd-trace-core/src/traceAgentTest/java/datadog/trace/common/metrics/MetricsIntegrationTest.java +++ b/dd-trace-core/src/traceAgentTest/java/datadog/trace/common/metrics/MetricsIntegrationTest.java @@ -76,7 +76,6 @@ static void cleanupSpec() { @BeforeEach void setup() { - AggregateEntry.resetCardinalityHandlers(); injectSysConfig(TracerConfig.AGENT_HOST, agentHost()); injectSysConfig(TracerConfig.TRACE_AGENT_PORT, agentPort()); } @@ -134,6 +133,7 @@ void sendMetricsToTraceAgentShouldNotifyWithOkEvent() throws InterruptedExceptio null, null, null, + null, 0L); AggregateEntry entry1 = table.findOrInsert(snap1); for (long duration : new long[] {2, 1, 2, 250, 4}) { @@ -156,6 +156,7 @@ void sendMetricsToTraceAgentShouldNotifyWithOkEvent() throws InterruptedExceptio null, null, null, + null, 0L); AggregateEntry entry2 = table.findOrInsert(snap2); for (long duration : new long[] {1, 1, 200, 2, 3, 4, 5, 6, 7, 8}) { diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index c74f276cff7..ba4e7264900 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -426,6 +426,7 @@ import static datadog.trace.api.config.GeneralConfig.TRACE_DEBUG; import static datadog.trace.api.config.GeneralConfig.TRACE_LOG_LEVEL; import static datadog.trace.api.config.GeneralConfig.TRACE_OTEL_SEMANTICS_ENABLED; +import static datadog.trace.api.config.GeneralConfig.TRACE_STATS_ADDITIONAL_TAGS; import static datadog.trace.api.config.GeneralConfig.TRACE_STATS_CARDINALITY_LIMIT; import static datadog.trace.api.config.GeneralConfig.TRACE_STATS_COMPUTATION_ENABLED; import static datadog.trace.api.config.GeneralConfig.TRACE_STATS_COMPUTATION_IGNORE_AGENT_VERSION; @@ -5235,6 +5236,14 @@ public Set getMetricsIgnoredResources() { return tryMakeImmutableSet(configProvider.getList(TRACER_METRICS_IGNORED_RESOURCES)); } + public Set getTraceStatsAdditionalTags() { + if (!experimentalFeaturesEnabled.contains( + propertyNameToEnvironmentVariableName(TRACE_STATS_ADDITIONAL_TAGS))) { + return Collections.emptySet(); + } + return tryMakeImmutableSet(configProvider.getList(TRACE_STATS_ADDITIONAL_TAGS)); + } + public String getEnv() { // intentionally not thread safe if (env == null) { diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index 5335b212339..350c3b8bd29 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -10737,6 +10737,22 @@ "aliases": [] } ], + "DD_TRACE_STATS_ADDITIONAL_TAGS": [ + { + "version": "A", + "type": "array", + "default": null, + "aliases": [] + } + ], + "DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT": [ + { + "version": "A", + "type": "int", + "default": "100", + "aliases": [] + } + ], "DD_TRACE_STATUS404DECORATOR_ENABLED": [ { "version": "A", diff --git a/profiling-backend-virtual-thread-analysis.html b/profiling-backend-virtual-thread-analysis.html new file mode 100644 index 00000000000..990b9c28656 --- /dev/null +++ b/profiling-backend-virtual-thread-analysis.html @@ -0,0 +1,301 @@ + + + + +dd-trace-java: Broken traces after switching OkHttp Dispatcher to virtual threads + + + + +

Broken traces after switching OkHttp Dispatcher to virtual threads

+ +

+ Context: profiling-backend + PR #8520 + swaps the OkHttp Dispatcher executor from + Executors.newCachedThreadPool(...) to + Executors.newThreadPerTaskExecutor(Thread.ofVirtual().name(...).factory()). + After the change, OkHttp client spans are reported to be broken (orphaned or attached to the wrong parent). + This document explains why, from the dd-trace-java instrumentation side. +

+ +

The PR in one line

+ +
// before
+var dispatcherExecutor = Executors.newCachedThreadPool(threadFactory);
+
+// after
+var dispatcherExecutor = Executors.newThreadPerTaskExecutor(
+        Thread.ofVirtual().name("okhttp-" + track + "-", 0).factory());
+ +

Why this is entirely a context-propagation question

+ +

+ The OkHttp instrumentation is intentionally thin. OkHttp3Instrumentation + (dd-java-agent/instrumentation/okhttp/okhttp-3.0/.../OkHttp3Instrumentation.java) + only adds TracingInterceptor to the client builder. The client span itself is + created lazily inside the interceptor: +

+ +
// TracingInterceptor.intercept(...)
+final AgentSpan span = startSpan("okhttp", OKHTTP_REQUEST);
+try (final AgentScope scope = activateSpan(span)) {
+  ...
+}
+ +

+ startSpan takes no explicit parent — it uses whatever scope is active + on the thread that runs the interceptor. So the entire question reduces to: +

+ +
+ Does dd-trace-java propagate the caller's active scope onto the worker thread that runs AsyncCall.run()? +
+ +

Why the old code worked

+ +

+ Executors.newCachedThreadPool(...) returns a + java.util.concurrent.ThreadPoolExecutor, which is covered by + ThreadPoolExecutorInstrumentation + (java-concurrent-1.8/.../executor/ThreadPoolExecutorInstrumentation.java) + — a heavyweight, battle-tested instrumentation that: +

+ +
    +
  1. On execute(Runnable): captures the active continuation into a State + keyed by the AsyncCall itself (or wraps it in Wrapper) via + TPEHelper.capture.
    2. +
  2. On beforeExecute(...): re-activates that continuation as an + AgentScope on the worker thread and stashes it in a ThreadLocal.
    3. +
  3. On afterExecute(...): closes the scope (pulled back out of the + ThreadLocal).
    4. +
  4. Plus queue-timer hooks, remove() cancellation, and a per-pool + Boolean propagate flag to avoid double-wrapping in subclasses that + delegate up to ThreadPoolExecutor.execute.
    5. +
+ +
+ Result: the okhttp client span is created inside + AsyncCall.run() with the parent scope reliably active. Trace stitches together. +
+ +

Why the new code breaks

+ +

+ Executors.newThreadPerTaskExecutor(Thread.ofVirtual().factory()) returns + java.util.concurrent.ThreadPerTaskExecutor, which is not + a ThreadPoolExecutor: +

+ +
    +
  • + ThreadPoolExecutorInstrumentation's + hierarchyMatcher is + extendsClass(named("java.util.concurrent.ThreadPoolExecutor")). + It does not match. Zero advice fires on the executor's + execute(...). +
    • +
  • + JavaExecutorInstrumentation doesn't match either — + ThreadPerTaskExecutor is not in its knownMatchingTypes + list (AbstractExecutorInstrumentation.java:28-35) + and dd.trace.executors.all is off by default. +
    • +
+ +

+ The only things that actually fire are two much newer, narrower instrumentations: +

+ + + + + + + + + + + + + + +
InstrumentationWhat it does
+ TaskRunnerInstrumentation + (java-concurrent-21.0/.../virtualthread/TaskRunnerInstrumentation.java) + JDK 19+ + + Advises the constructor and run() of the package-private inner class + ThreadPerTaskExecutor$TaskRunner. The constructor stashes a continuation + in a State keyed by the TaskRunner, not by the user's + AsyncCall. run() re-activates it. +
+ VirtualThreadInstrumentation + (java-lang-21.0/.../jdk21/VirtualThreadInstrumentation.java) + JDK 21+ + + Captures the whole Context at VirtualThread.<init>, + then swaps it in/out on each mount/unmount via + Context.swap(). +
+ +

+ In principle, the combo should propagate the scope. In practice, this is exactly the + area that's been on fire through 2025–2026. The master log for + VirtualThread* / TaskRunner*: +

+ +
    +
  • c0ce9c5738 — fix(virtual-thread): Fix support for multiple mounts / unmounts (#10931)
    • +
  • a1f4ec5d62 — Improve VirtualThread instrumentation (#11009): fixes + "duplicate instrumentation conflict between VirtualThread and Runnable", "duplicate + instrumentation of afterDone", and replaces the ConcurrentState + activate/close model with Context.swap() because the old one closed + scopes out of order.
    • +
  • 95de525c87 — Preload scope classes to prevent virtual thread deadlock (#11111): + DatadogClassLoader does synchronous I/O on JarFile during the first + Context.swap(), which pins the carrier thread and can deadlock the app.
    • +
+ +

Concrete failure modes that fit the symptoms

+ +
+ 1. Dispatcher recursion contaminates context.
+ OkHttp's Dispatcher.promoteAndExecute() is called from + AsyncCall.run() → finally → dispatcher.finished(this), + i.e. on the dispatcher thread (now a virtual thread) of a finishing call. + The active scope there belongs to the just-finished request, not the original + requester. TaskRunnerInstrumentation.Construct captures that scope, + so the newly-promoted call gets stitched under the wrong (or already-finished) parent. + This latent bug existed before too, but with a cached pool the constructor advice + doesn't fire and ThreadPoolExecutorInstrumentation captures continuations + that may be cancellable/no-op for a finished call. With virtual threads it shows up + far more obviously because every call runs on a fresh thread that inherits whatever + context the dispatcher last had. +
+ +
+ 2. Lost context across mount/unmount.
+ OkHttp callbacks block on socket I/O — that's the entire point of moving to + virtual threads. Every blocking read unmounts the carrier. The + VirtualThread.mount/unmount advice is responsible for swapping the + scope stack back in on the new carrier. The fix log above shows this has had multiple + correctness bugs (multi-mount handling, out-of-order scope close); the agent version + in production may simply pre-date one of those fixes. +
+ +
+ 3. InstrumenterModule.ContextTracking gating.
+ Both TaskRunnerInstrumentation and VirtualThreadInstrumentation + extend InstrumenterModule.ContextTracking, which only activates when + TargetSystem.CONTEXT_TRACKING is enabled. It is in the default set + (AgentInstaller.java:311), but if the profiling agent + runs in a non-default configuration that selects only PROFILING + (no TRACING/CONTEXT_TRACKING), neither instrumentation + fires at all and every OkHttp client span becomes a root span. +
+ +

Quickest things to verify on profiling-backend's side

+ +
    +
  • + Agent version. Which dd-trace-java version is the profiling backend + pinned to? Anything before a1f4ec5d62 (April 2026) is missing the + Context.swap() rewrite of virtual thread propagation. +
    • +
  • + Integration flags. Is dd.integration.java-lang-21.enabled + or dd.integration.virtual-thread.enabled set anywhere? There was an + intermediate "disabled by default until non-flaky" state (commit 164a756639) + that didn't land on master's first-parent, but downstream forks may have + inherited it. +
    • +
  • + A/B with the equivalent factory. Try + Executors.newVirtualThreadPerTaskExecutor() (no + .name(prefix, start) builder in between). It's semantically equivalent + but is exactly the code path the agent's tests + (VirtualThreadPerTaskExecutorTest.groovy) cover. If that also breaks, + it's the propagation infrastructure; if it works, the named-builder customization is + somehow the trigger. +
    • +
+ +

Bottom line

+ +

+ Switching the dispatcher's executor swapped a 10-year-old, exhaustively-instrumented + ThreadPoolExecutor path for one that depends on + TaskRunnerInstrumentation + VirtualThreadInstrumentation, + both of which are far younger and have had a steady stream of correctness fixes + through 2025–2026. The most likely root cause is either an agent version that + pre-dates the recent VirtualThread fixes, or the dispatcher-recursion case where a + finishing call's scope is inadvertently captured for the next promoted call. +

+ + +