diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index 40d5753c478..61a7a3af4a2 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -297,7 +297,11 @@ public final class ConfigDefaults { static final Set DEFAULT_TRACE_EXPERIMENTAL_FEATURES_ENABLED = new HashSet<>( - asList("DD_TAGS", "DD_LOGS_INJECTION", "DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED")); + asList( + "DD_TAGS", + "DD_LOGS_INJECTION", + "DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED", + "DD_TRACE_STATS_ADDITIONAL_TAGS")); static final boolean DEFAULT_TRACE_128_BIT_TRACEID_GENERATION_ENABLED = true; static final boolean DEFAULT_TRACE_128_BIT_TRACEID_LOGGING_ENABLED = true; 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..a360df0d7a6 --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/AdditionalTagsMetricsBenchmark.java @@ -0,0 +1,105 @@ +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(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.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..ac677f20a97 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 @@ -57,7 +57,7 @@ @BenchmarkMode(Mode.Throughput) @OutputTimeUnit(SECONDS) @Threads(8) -@Fork(value = 1) +@Fork(1) public class AdversarialMetricsBenchmark { private ClientStatsAggregator aggregator; @@ -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..08449d7a9ac 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 @@ -33,7 +33,7 @@ @Measurement(iterations = 3, time = 30, timeUnit = SECONDS) @BenchmarkMode(Mode.AverageTime) @OutputTimeUnit(MICROSECONDS) -@Fork(value = 1) +@Fork(1) public class ClientStatsAggregatorBenchmark { private final DDAgentFeaturesDiscovery featuresDiscovery = new FixedAgentFeaturesDiscovery( @@ -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..7947ed20a0e 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 @@ -49,7 +49,7 @@ @Measurement(iterations = 3, time = 30, timeUnit = SECONDS) @BenchmarkMode(Mode.AverageTime) @OutputTimeUnit(MICROSECONDS) -@Fork(value = 1) +@Fork(1) public class ClientStatsAggregatorDDSpanBenchmark { private static final CoreTracer TRACER = @@ -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..001a56aabeb 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 @@ -47,7 +47,7 @@ @BenchmarkMode(Mode.Throughput) @OutputTimeUnit(SECONDS) @Threads(8) -@Fork(value = 1) +@Fork(1) public class HighCardinalityPeerMetricsBenchmark { private ClientStatsAggregator aggregator; @@ -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..e9930eb69e7 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 @@ -43,7 +43,7 @@ @BenchmarkMode(Mode.Throughput) @OutputTimeUnit(SECONDS) @Threads(8) -@Fork(value = 1) +@Fork(1) public class HighCardinalityResourceMetricsBenchmark { private ClientStatsAggregator aggregator; @@ -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..a1a95da2f4e --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AdditionalTagsSchema.java @@ -0,0 +1,132 @@ +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; + + // Health-metric statsD tags per the approved Cardinality Limits RFC (section 5): collapses are + // reported under the lowercased protobuf field name additional_metric_tags, with cardinality- + // collapses (collapsed:) and per-value length-collapses (oversized:) tagged distinctly. + private static final String[] COLLAPSED_STATSD_TAG = {"collapsed:additional_metric_tags"}; + private static final String[] OVERSIZED_STATSD_TAG = {"oversized:additional_metric_tags"}; + + /** Singleton empty schema returned when no additional tags are configured. */ + static final AdditionalTagsSchema EMPTY = + new AdditionalTagsSchema(new String[0], new TagCardinalityHandler[0]); + + final String[] names; + + /** Per-key handlers providing UTF8 caching and per-cycle cardinality limiting. */ + private final TagCardinalityHandler[] handlers; + + private AdditionalTagsSchema(String[] names, TagCardinalityHandler[] handlers) { + this.names = names; + this.handlers = handlers; + } + + /** Test convenience: limits enabled. */ + static AdditionalTagsSchema from(Set configured) { + return from( + configured, + MetricCardinalityLimits.ADDITIONAL_TAG_VALUE, + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + } + + static AdditionalTagsSchema from(Set configured, int limit, boolean useBlockedSentinel) { + 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); + // Keys arrive as a Set, so they are already distinct — no dedup needed. + if (valid.size() > MAX_ADDITIONAL_TAG_KEYS) { + log.warn( + "Configured additional metric tag keys ({}) exceeds the supported limit of {}; " + + "dropping extra keys: {}", + valid.size(), + MAX_ADDITIONAL_TAG_KEYS, + valid.subList(MAX_ADDITIONAL_TAG_KEYS, valid.size())); + valid = valid.subList(0, MAX_ADDITIONAL_TAG_KEYS); + } + String[] namesArr = valid.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); + } + + 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(HealthMetrics healthMetrics, CardinalityLimitReporter reporter) { + long totalCollapsed = 0; + long totalOversized = 0; + for (int i = 0; i < handlers.length; i++) { + // oversizedCount() must be read before reset(), which zeroes both per-cycle counts. + long oversized = handlers[i].oversizedCount(); + long collapsed = handlers[i].reset(); + // The human-facing reporter names the specific tag that triggered the block, so it records + // both collapse kinds under the tag's own name. + long blocked = collapsed + oversized; + if (blocked > 0) { + reporter.record(names[i], blocked); + } + totalCollapsed += collapsed; + totalOversized += oversized; + } + // The health metric is reported at the additional_metric_tags field granularity (not per tag + // name), with cardinality-collapses and length-collapses ("oversized") tagged distinctly per + // the approved Cardinality Limits RFC. + if (totalCollapsed > 0) { + healthMetrics.onTagCardinalityBlocked(COLLAPSED_STATSD_TAG, totalCollapsed); + } + if (totalOversized > 0) { + healthMetrics.onTagCardinalityBlocked(OVERSIZED_STATSD_TAG, totalOversized); + } + } +} 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 f2d93ac09d0..b87963b4bd3 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,26 +1,21 @@ 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. + * Aggregator hashtable entry: UTF8 label fields + counter/histogram state. * - *

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. + *

Public so the cross-package OTLP writer ({@code datadog.trace.core.otlp.metrics}) can read the + * per-entry accessors. */ public final class AggregateEntry extends Hashtable.Entry { @@ -29,49 +24,6 @@ public 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,20 +41,23 @@ public 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 (this field through errorDuration below) is thread-confined: the entry is + // mutated only on the aggregator thread, so these fields are intentionally unsynchronized and not + // thread-safe. Producers hand off immutable SpanSnapshots; only the aggregator thread records + // into the entry. + private final Histogram okLatencies; + + // Null until first error; SerializingMetricWriter writes empty histogram form when null. Not + // thread-safe as well. @Nullable private Histogram errorLatencies; private int errorCount; private int hitCount; private int topLevelCount; + // Tracked separately for per-outcome OTLP histogram sums; getDuration() returns the sum. private long okDuration; private long errorDuration; @@ -124,7 +79,8 @@ public 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 +95,8 @@ public 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; - } - - public int getErrorCount() { - return errorCount; - } - - public int getHitCount() { - return hitCount; - } - - public int getTopLevelCount() { - return topLevelCount; - } - - public long getDuration() { - return okDuration + errorDuration; - } - - public long getOkDuration() { - return okDuration; - } - - public long getErrorDuration() { - return errorDuration; - } - - public 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 - public 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 +124,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); @@ -274,16 +137,15 @@ static long hashOf( h = LongHashingUtils.addToHash(h, httpMethod); h = LongHashingUtils.addToHash(h, httpEndpoint); h = LongHashingUtils.addToHash(h, grpcStatusCode); - for (int i = 0; i < peerTagCount; i++) { - h = LongHashingUtils.addToHash(h, peerTags[i]); - } + h = LongHashingUtils.addToHash(h, peerTags, peerTagCount); + h = LongHashingUtils.addToHash(h, additionalTags, additionalTagCount); h = LongHashingUtils.addToHash(h, httpStatusCode); h = LongHashingUtils.addToHash(h, synthetic); h = LongHashingUtils.addToHash(h, traceRoot); return h; } - // Accessors for SerializingMetricWriter. + // Accessors for SerializingMetricWriter and the cross-package OTLP writer. public UTF8BytesString getResource() { return resource; } @@ -368,9 +230,147 @@ public 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. + */ + public UTF8BytesString[] getAdditionalTags() { + return additionalTags; + } + + // ----- recording state accessors ----- + + public int getHitCount() { + return hitCount; + } + + public int getErrorCount() { + return errorCount; + } + + public int getTopLevelCount() { + return topLevelCount; + } + + /** + * Total recorded duration for the native msgpack path -- the sum of {@link #getOkDuration()} and + * {@link #getErrorDuration()}. OK and error durations are tracked separately so the OTLP writer + * can emit per-outcome histogram sums. + */ + public long getDuration() { + return okDuration + errorDuration; + } + + public long getOkDuration() { + return okDuration; + } + + public long getErrorDuration() { + return errorDuration; + } + + public 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}). + */ + public 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); + errorDuration += tagAndDuration; + errorCount++; + } else { + okLatencies.accept(tagAndDuration); + okDuration += 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); + this.errorDuration += d; + errorCount++; + } else { + okLatencies.accept(d); + this.okDuration += 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.okDuration = 0; + this.errorDuration = 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 +404,69 @@ static final class Canonical { int peerTagsSize = 0; + /** Core per-field cardinality handlers; owned by the enclosing {@link AggregateTable}. */ + final CoreHandlers handlers; + + /** Schema + per-key blocked sentinels for additional metric tags. Immutable. */ + final AdditionalTagsSchema additionalTagsSchema; + + /** + * 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(CoreHandlers handlers, AdditionalTagsSchema additionalTagsSchema) { + this.handlers = handlers; + this.additionalTagsSchema = additionalTagsSchema; + 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); + 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 +478,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 +496,29 @@ 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; + // values is captured against the same additionalTagsSchema, so it is either null or exactly + // schema.size() long (== additionalTagsBuffer.length); guard the array we actually index. + if (values == null || values.length == 0) { + return; + } + int n = additionalTagsBuffer.length; + 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 +542,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 +587,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 +605,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..b120cb8b915 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, new CoreHandlers(), additionalTagsSchema); + } + + AggregateTable( + int maxAggregates, CoreHandlers handlers, AdditionalTagsSchema additionalTagsSchema) { this.buckets = Hashtable.Support.create(maxAggregates, Hashtable.Support.MAX_RATIO); this.maxAggregates = maxAggregates; + this.canonical = new AggregateEntry.Canonical(handlers, additionalTagsSchema); + } + + void resetCoreHandlers(HealthMetrics healthMetrics, CardinalityLimitReporter reporter) { + canonical.handlers.reset(healthMetrics, reporter); } int size() { @@ -64,6 +79,7 @@ AggregateEntry findOrInsert(SpanSnapshot snapshot) { return candidate; } } + // Miss path. if (size >= maxAggregates && !evictOneStale()) { return null; } @@ -83,11 +99,12 @@ 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. + *

Cardinality limiting (see {@link MetricCardinalityLimits#USE_BLOCKED_SENTINEL}) reduces how + * often this fires but doesn't eliminate it. Over-cap values for a single field collapse into the + * shared {@code tracer_blocked_value} sentinel, so no one field can fill the table on its own. + * But distinct in-budget combinations across fields (resource x service x operation x ...) can + * still drive the entry count to {@code maxAggregates}, so this cursor-resumed scan remains the + * backstop. */ 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..34d7b2a83fe 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 resetCoreHandlers(HealthMetrics healthMetrics, CardinalityLimitReporter reporter) { + aggregates.resetCoreHandlers(healthMetrics, reporter); + } + @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/CardinalityLimitReporter.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/CardinalityLimitReporter.java new file mode 100644 index 00000000000..215c278bef3 --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/CardinalityLimitReporter.java @@ -0,0 +1,105 @@ +package datadog.trace.common.metrics; + +import static java.util.concurrent.TimeUnit.MINUTES; + +import datadog.logging.RatelimitedLogger; +import datadog.trace.util.Hashtable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Aggregates cardinality-block counts by tag name across reporting cycles and emits a single + * rate-limited summary, so a tag that stays over its limit no longer warns on every reporting cycle + * (default every 10s). The durable per-cycle counts still flow to {@link + * datadog.trace.core.monitor.HealthMetrics#onTagCardinalityBlocked} at each reset site; this + * reporter is only the human-facing log sink for the same counts. + * + *

Counts are keyed by tag name here rather than held on the {@link TagCardinalityHandler}s so a + * peer-tag schema rebuild (a feature-discovery/config update) does not lose them: each reset + * flushes the cycle's delta here by name before the outgoing handler can be discarded, and a + * surviving tag's replacement handler simply keeps adding to the same entry. So the config-update + * transition needs no per-tag transfer or handler reuse -- the count has already left the handler. + * + *

The backing store is a {@link Hashtable.D1} counter table: each tag's count is mutated in + * place with no per-update allocation. (FlatHashtable would be a better fit still, but is not yet + * merged.) + * + *

Only touched from the aggregator thread; no synchronization. + */ +final class CardinalityLimitReporter { + + private static final Logger log = LoggerFactory.getLogger(CardinalityLimitReporter.class); + + // Distinct blocked tag names in a window: 9 property fields + the configured peer tags + up to + // AdditionalTagsSchema.MAX_ADDITIONAL_TAG_KEYS + base.service, with headroom for the brief + // overlap + // of old and new peer names across a schema rebuild. Fixed capacity; the table chains on overflow + // rather than dropping, so an underestimate only adds chain depth on this cold path. + private static final int TAG_CAPACITY = 64; + + // Rough width of one "=, " entry, used to pre-size the summary builder. Cold path, so + // an over-estimate just avoids a resize rather than mattering for footprint. + private static final int APPROX_CHARS_PER_ENTRY = 32; + + private final RatelimitedLogger rlLog; + // Tag name -> blocked count accumulated since the last emitted summary. + private final Hashtable.D1 blockedByTag = new Hashtable.D1<>(TAG_CAPACITY); + + CardinalityLimitReporter() { + this(new RatelimitedLogger(log, 5, MINUTES)); + } + + CardinalityLimitReporter(RatelimitedLogger rlLog) { + this.rlLog = rlLog; + } + + /** Records {@code count} values blocked for {@code tag} in the current reporting cycle. */ + void record(String tag, long count) { + if (count > 0) { + blockedByTag.getOrCreate(tag, TagBlockEntry::new).count += count; + } + } + + /** + * Emits one rate-limited summary of everything blocked since the last emitted line. While the + * rate limiter suppresses, counts keep accumulating; they are cleared only once a line is + * actually logged (keyed off {@link RatelimitedLogger#warn}'s return), so each summary reflects + * the whole period and nothing is dropped in between. + */ + void reportIfDue() { + if (blockedByTag.size() == 0) { + return; + } + if (rlLog.warn( + "Metric tag cardinality limits reached; excess values reported as tracer_blocked_value." + + " Blocked value counts by tag: {}", + summarize())) { + blockedByTag.clear(); + } + } + + private String summarize() { + StringBuilder builder = new StringBuilder(blockedByTag.size() * APPROX_CHARS_PER_ENTRY); + // Non-capturing: the builder is threaded through as forEach's context argument. + blockedByTag.forEach( + builder, + (into, entry) -> { + if (into.length() > 0) { + into.append(", "); + } + into.append(entry.key()).append('=').append(entry.count); + }); + return builder.toString(); + } + + /** + * Single-key counter entry: the tag name (via {@link #key()}) plus its in-place-mutated count. + */ + private static final class TagBlockEntry extends Hashtable.D1.Entry { + long count; + + TagBlockEntry(String tag) { + super(tag); + } + } +} 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 e7f4c5ac160..8a0c69cbed0 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/ClientStatsAggregator.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/ClientStatsAggregator.java @@ -84,6 +84,10 @@ public final class ClientStatsAggregator implements MetricsAggregator, EventList private final TimeUnit reportingIntervalTimeUnit; private final DDAgentFeaturesDiscovery features; private final HealthMetrics healthMetrics; + // Aggregates per-cycle cardinality-block counts by tag name into a single rate-limited warn, + // instead of warning per field on every reporting cycle. Aggregator-thread confined. + private final CardinalityLimitReporter cardinalityLimitReporter = new CardinalityLimitReporter(); + private final AdditionalTagsSchema additionalTagsSchema; private final boolean includeEndpointInMetrics; private final boolean otlpStatsExportEnabled; @@ -105,6 +109,21 @@ public final class ClientStatsAggregator implements MetricsAggregator, EventList */ private volatile PeerTagSchema cachedPeerTagSchema; + /** + * Previous peer-tag schema, kept until the next reporting cycle. + * + *

A producer may read the current schema just before the aggregator replaces it. If that + * producer queues its {@link SpanSnapshot} after the current {@code REPORT} signal, the snapshot + * will be processed during the next cycle and will still update the old schema's blocked-value + * counters. + * + *

Keeping the old schema here allows the next reset to report those late counter updates + * before the schema is discarded. + * + *

This field is accessed only by the aggregator thread, so it does not need to be volatile. + */ + private PeerTagSchema previousPeerTagSchema; + private volatile AgentTaskScheduler.Scheduled cancellation; public ClientStatsAggregator( @@ -114,6 +133,7 @@ public ClientStatsAggregator( this( config.getWellKnownTags(), config.getMetricsIgnoredResources(), + additionalTagsSchemaFrom(config), sharedCommunicationObjects.featuresDiscovery(config), healthMetrics, new OkHttpSink( @@ -141,6 +161,7 @@ public ClientStatsAggregator( OtlpStatsMetricWriter metricWriter) { this( config.getMetricsIgnoredResources(), + additionalTagsSchemaFrom(config), sharedCommunicationObjects.featuresDiscovery(config), healthMetrics, NoOpSink.INSTANCE, @@ -152,9 +173,18 @@ public ClientStatsAggregator( true); } + private static AdditionalTagsSchema additionalTagsSchemaFrom(Config config) { + return AdditionalTagsSchema.from( + config.getTraceStatsAdditionalTags(), + config.getTraceStatsCardinalityLimit( + "additional_tags", MetricCardinalityLimits.ADDITIONAL_TAG_VALUE), + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + } + ClientStatsAggregator( WellKnownTags wellKnownTags, Set ignoredResources, + AdditionalTagsSchema additionalTagsSchema, DDAgentFeaturesDiscovery features, HealthMetrics healthMetric, Sink sink, @@ -164,6 +194,7 @@ public ClientStatsAggregator( this( wellKnownTags, ignoredResources, + additionalTagsSchema, features, healthMetric, sink, @@ -176,10 +207,37 @@ public ClientStatsAggregator( ClientStatsAggregator( WellKnownTags wellKnownTags, + Set ignoredResources, + AdditionalTagsSchema additionalTagsSchema, + DDAgentFeaturesDiscovery features, + HealthMetrics healthMetric, + Sink sink, + int maxAggregates, + int queueSize, + long reportingInterval, + TimeUnit timeUnit, + boolean includeEndpointInMetrics) { + this( + ignoredResources, + additionalTagsSchema, + features, + healthMetric, + sink, + new SerializingMetricWriter(wellKnownTags, sink, additionalTagsSchema.size() > 0), + maxAggregates, + queueSize, + reportingInterval, + timeUnit, + includeEndpointInMetrics); + } + + /** Test-only: defaults to no additional tags schema. */ + ClientStatsAggregator( Set ignoredResources, DDAgentFeaturesDiscovery features, HealthMetrics healthMetric, Sink sink, + MetricWriter metricWriter, int maxAggregates, int queueSize, long reportingInterval, @@ -187,10 +245,11 @@ public ClientStatsAggregator( boolean includeEndpointInMetrics) { this( ignoredResources, + AdditionalTagsSchema.EMPTY, features, healthMetric, sink, - new SerializingMetricWriter(wellKnownTags, sink), + metricWriter, maxAggregates, queueSize, reportingInterval, @@ -200,6 +259,7 @@ public ClientStatsAggregator( ClientStatsAggregator( Set ignoredResources, + AdditionalTagsSchema additionalTagsSchema, DDAgentFeaturesDiscovery features, HealthMetrics healthMetric, Sink sink, @@ -210,6 +270,7 @@ public ClientStatsAggregator( TimeUnit timeUnit, boolean includeEndpointInMetrics) { this.ignoredResources = ignoredResources; + this.additionalTagsSchema = additionalTagsSchema; this.includeEndpointInMetrics = includeEndpointInMetrics; this.otlpStatsExportEnabled = metricWriter instanceof OtlpStatsMetricWriter; this.inbox = Queues.mpscArrayQueue(queueSize); @@ -225,6 +286,7 @@ public ClientStatsAggregator( reportingInterval, timeUnit, healthMetric, + additionalTagsSchema, this::resetCardinalityHandlers); this.thread = newAgentThread(METRICS_AGGREGATOR, aggregator); this.reportingInterval = reportingInterval; @@ -336,15 +398,18 @@ public boolean publish(List> trace) { if (peerTagSchema == null) { peerTagSchema = bootstrapPeerTagSchema(); } + // ignoredResources is fixed for the lifetime of the aggregator and typically empty; hoist the + // check so the common case skips both the lookup and the getResourceName() call per span. + final boolean hasIgnoredResources = !ignoredResources.isEmpty(); for (CoreSpan span : trace) { boolean isTopLevel = span.isTopLevel(); if (shouldComputeMetric(span, isTopLevel)) { - final CharSequence resourceName = span.getResourceName(); - if (resourceName != null - && !ignoredResources.isEmpty() - && ignoredResources.contains(resourceName.toString())) { - // skip publishing all children - break; + if (hasIgnoredResources) { + final CharSequence resourceName = span.getResourceName(); + if (resourceName != null && ignoredResources.contains(resourceName.toString())) { + // skip publishing all children + break; + } } counted++; forceKeep |= publish(span, isTopLevel, peerTagSchema); @@ -413,6 +478,8 @@ private boolean publish(CoreSpan span, boolean isTopLevel, PeerTagSchema peer spanPeerTagSchema = null; } + String[] additionalTagValues = captureAdditionalTagValues(span); + SpanSnapshot snapshot = new SpanSnapshot( span.getResourceName(), @@ -429,6 +496,7 @@ private boolean publish(CoreSpan span, boolean isTopLevel, PeerTagSchema peer httpMethod, httpEndpoint, grpcStatusCode, + additionalTagValues, tagAndDuration); if (!inbox.offer(snapshot)) { healthMetrics.onStatsInboxFull(); @@ -448,6 +516,16 @@ private static String firstTag(CoreSpan span, String[] keys) { return null; } + /** + * 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 @@ -485,38 +563,19 @@ 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 together: 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.resetCoreHandlers(healthMetrics, cardinalityLimitReporter); + PeerTagSchema.INTERNAL.resetHandlers(healthMetrics, cardinalityLimitReporter); 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, cardinalityLimitReporter); } + additionalTagsSchema.resetHandlers(healthMetrics, cardinalityLimitReporter); + cardinalityLimitReporter.reportIfDue(); } /** @@ -530,6 +589,15 @@ private void resetPeerTagSchema(PeerTagSchema schema) { * reference). */ private void reconcilePeerTagSchema() { + // Report and discard the schema retired during the previous cycle. Snapshots queued after the + // previous REPORT may still reference that schema and may have updated its blocked-value + // counters during this cycle. Do this before the swap below, which may retire the current + // schema + // and keep it here until the next cycle. + if (previousPeerTagSchema != null) { + previousPeerTagSchema.resetHandlers(healthMetrics, cardinalityLimitReporter); + previousPeerTagSchema = null; + } PeerTagSchema cached = cachedPeerTagSchema; if (cached == null) { // First reset before the first publish -- producer-side bootstrap hasn't run yet. @@ -545,8 +613,14 @@ private void reconcilePeerTagSchema() { cached.state = latestState; } 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); + // discarding it, otherwise the partial-cycle blockedCounts would silently disappear. Flushing + // into the reporter by tag name is also what carries block counts across the rebuild -- a + // surviving tag's new handler resumes adding to the same reporter entry, so no per-tag + // transfer or handler reuse is needed. + cached.resetHandlers(healthMetrics, cardinalityLimitReporter); + // Retain the outgoing schema for one more reset cycle so snapshots that captured it just + // before this swap still have their post-swap block counts flushed (see the field doc). + previousPeerTagSchema = cached; cachedPeerTagSchema = PeerTagSchema.of(normalized, latestState); } } @@ -573,7 +647,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/CoreHandlers.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/CoreHandlers.java new file mode 100644 index 00000000000..9214e34a89b --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/CoreHandlers.java @@ -0,0 +1,100 @@ +package datadog.trace.common.metrics; + +import datadog.trace.api.Config; +import datadog.trace.core.monitor.HealthMetrics; + +/** + * The core set of cardinality handlers -- the always-present per-field handlers, as opposed to the + * remote-config-driven peer-tag set ({@link PeerTagSchema}) and the local-config additional-tag set + * ({@link AdditionalTagsSchema}). Whether a member is a property or a tag handler is an + * implementation detail of the core set. Owned by {@link ClientStatsAggregator}. + */ +final class CoreHandlers { + + 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; + + CoreHandlers() { + Config config = Config.get(); + // Configurable limits — tunable via env var. + this.resource = + new PropertyCardinalityHandler( + "resource", + config.getTraceStatsCardinalityLimit("resource", MetricCardinalityLimits.RESOURCE), + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + this.httpEndpoint = + new PropertyCardinalityHandler( + "http_endpoint", + config.getTraceStatsCardinalityLimit( + "http_endpoint", MetricCardinalityLimits.HTTP_ENDPOINT), + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + // Fixed limits — hardcoded, not user-configurable. + this.service = + new PropertyCardinalityHandler( + "service", + MetricCardinalityLimits.SERVICE, + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + // operation is carried on the stats wire as the protobuf field "name", so its telemetry tag is + // collapsed:name (RFC section 5) while the human-facing reporter keeps the clearer "operation". + this.operation = + new PropertyCardinalityHandler( + "operation", + "name", + MetricCardinalityLimits.OPERATION, + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + this.serviceSource = + new PropertyCardinalityHandler( + "service_source", + MetricCardinalityLimits.SERVICE_SOURCE, + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + this.type = + new PropertyCardinalityHandler( + "type", MetricCardinalityLimits.TYPE, MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + this.spanKind = + new PropertyCardinalityHandler( + "span_kind", + MetricCardinalityLimits.SPAN_KIND, + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + this.httpMethod = + new PropertyCardinalityHandler( + "http_method", + MetricCardinalityLimits.HTTP_METHOD, + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + this.grpcStatusCode = + new PropertyCardinalityHandler( + "grpc_status_code", + MetricCardinalityLimits.GRPC_STATUS_CODE, + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); + this.handlers = + new PropertyCardinalityHandler[] { + resource, + service, + operation, + serviceSource, + type, + spanKind, + httpMethod, + httpEndpoint, + grpcStatusCode + }; + } + + void reset(HealthMetrics healthMetrics, CardinalityLimitReporter reporter) { + for (PropertyCardinalityHandler h : handlers) { + long numBlocked = h.reset(); + if (numBlocked > 0) { + healthMetrics.onTagCardinalityBlocked(h.statsDTag(), numBlocked); + reporter.record(h.name, numBlocked); + } + } + } +} 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..0b6ef8cdbd8 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 @@ -12,6 +12,15 @@ final class MetricCardinalityLimits { private MetricCardinalityLimits() {} + /** + * Whether over-cap values collapse into the {@code tracer_blocked_value} sentinel (cardinality + * capping). Always {@code true} in shipped builds. Retained as a compile-time flip back to the + * pre-capping behavior -- handlers without a sentinel -- that shipped in an earlier release, in + * case capping needs to be disabled during the internal rollout. This is not a runtime config + * knob; flipping it requires a rebuild. + */ + static final boolean USE_BLOCKED_SENTINEL = true; + /** * Distinct {@code resource.name} values per cycle. Highest-cardinality field by far: DB-query * obfuscations, HTTP route templates, custom resources. Typical service: 30-200 unique; 1024 @@ -71,4 +80,27 @@ 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. + * + *

{@code 100} is the default; the value is configurable via {@code + * DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT} (resolved in {@link ClientStatsAggregator} + * through {@code Config#getTraceStatsCardinalityLimit}), matching the default and env-var name + * the approved Cardinality Limits RFC specifies. The RFC leaves the limiting unit and eviction + * strategy to the SDK ("mechanism in the spec, policy in the SDK"); we apply the limit per key. + */ + 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. + * + *

{@code 200} matches the RFC's "collapsed due to exceeding 200 chars". The cap is measured in + * characters (via {@code String.length()}). + */ + 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..98360d44d47 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,6 +5,7 @@ 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; /** @@ -28,7 +29,9 @@ * * *

Cardinality blocks are counted inside each {@link TagCardinalityHandler} and flushed once per - * cycle (with a warn log) via {@code ClientStatsAggregator#resetCardinalityHandlers}. + * cycle via {@code ClientStatsAggregator#resetCardinalityHandlers} -- to {@link HealthMetrics} as a + * per-cycle count and to the shared {@link CardinalityLimitReporter}, which aggregates by tag name + * and emits a rate-limited warn summary. * *

Each {@link SpanSnapshot} captures its own schema reference so producer and consumer agree on * the indexing even if the current schema is replaced between capture and consumption. @@ -51,6 +54,11 @@ final class PeerTagSchema { /** Singleton schema for internal-kind spans -- only {@code base.service}. */ static final PeerTagSchema INTERNAL = new PeerTagSchema(new String[] {BASE_SERVICE}, NO_STATE); + // Health/telemetry statsD tag per the approved Cardinality Limits RFC (section 5): peer-tag + // collapses are reported under the lowercased protobuf field name peer_tags, aggregated across + // every configured peer tag rather than per individual tag name. + private static final String[] COLLAPSED_STATSD_TAG = {"collapsed:peer_tags"}; + final String[] names; final TagCardinalityHandler[] handlers; @@ -78,7 +86,8 @@ static PeerTagSchema of(Set names, String state) { names[i], Config.get() .getTraceStatsCardinalityLimit( - "peer_tags", MetricCardinalityLimits.PEER_TAG_VALUE)); + "peer_tags", MetricCardinalityLimits.PEER_TAG_VALUE), + MetricCardinalityLimits.USE_BLOCKED_SENTINEL); } } @@ -109,6 +118,34 @@ UTF8BytesString register(int i, String value) { return handlers[i].register(value); } + /** + * Resets cardinality tracking for each peer tag and reports how many values were blocked since + * the previous reset. + * + *

The counts are sent to {@link HealthMetrics} for the current reporting cycle. They are also + * added to {@code reporter}, which groups them by tag name for a rate-limited warning. Grouping + * counts by name ensures they are not lost when a schema rebuild replaces the handlers. + * + *

This method must be called only from the aggregator thread because the handlers are not + * thread-safe. + */ + void resetHandlers(HealthMetrics healthMetrics, CardinalityLimitReporter reporter) { + long totalCollapsed = 0; + for (int i = 0; i < handlers.length; i++) { + long numBlocked = handlers[i].reset(); + if (numBlocked > 0) { + // The human-facing reporter names the specific peer tag that triggered the block. + reporter.record(names[i], numBlocked); + } + totalCollapsed += numBlocked; + } + // The health metric is reported at the peer_tags field granularity (not per tag name) per the + // approved Cardinality Limits RFC. + if (totalCollapsed > 0) { + healthMetrics.onTagCardinalityBlocked(COLLAPSED_STATSD_TAG, totalCollapsed); + } + } + int size() { return names.length; } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/PropertyCardinalityHandler.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/PropertyCardinalityHandler.java index ca86f0be8f6..a813ada53de 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/PropertyCardinalityHandler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/PropertyCardinalityHandler.java @@ -31,6 +31,17 @@ final class PropertyCardinalityHandler { private static final int MAX_CARDINALITY_LIMIT = 1 << 29; final String name; + + /** + * Protobuf field name this handler reports under in the {@code collapsed:} health/telemetry tag, + * per the approved Cardinality Limits RFC (section 5), which keys collapses by the lowercased + * protobuf field name. Usually equal to {@link #name}, but decoupled where the field's + * human-facing name differs from its wire name -- e.g. {@code operation} is carried on the wire + * as {@code name}, so its telemetry tag is {@code collapsed:name} while the human-facing {@link + * CardinalityLimitReporter} still says {@code operation}. + */ + private final String statsDField; + private final int cardinalityLimit; private final int capacityMask; @@ -67,7 +78,13 @@ final class PropertyCardinalityHandler { } PropertyCardinalityHandler(String name, int cardinalityLimit, boolean useBlockedSentinel) { + this(name, name, cardinalityLimit, useBlockedSentinel); + } + + PropertyCardinalityHandler( + String name, String statsDField, int cardinalityLimit, boolean useBlockedSentinel) { this.name = name; + this.statsDField = statsDField; if (cardinalityLimit <= 0) { throw new IllegalArgumentException("cardinalityLimit must be positive: " + cardinalityLimit); } @@ -159,7 +176,7 @@ private UTF8BytesString tracerBlockedValue() { */ String[] statsDTag() { if (statsDTag == null) { - statsDTag = new String[] {"collapsed:" + name}; + statsDTag = new String[] {"collapsed:" + statsDField}; } return statsDTag; } 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..3eacde8bd45 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); @@ -62,6 +63,16 @@ public final class SerializingMetricWriter implements MetricWriter { private final WellKnownTags wellKnownTags; private final WritableFormatter writer; private final Sink sink; + + /** + * Whether the span-derived additional-tags feature is configured (at least one key). When {@code + * true}, the {@code AdditionalMetricTags} field is always emitted -- as an empty array for + * entries that matched no configured key -- so the field is present whenever the feature is on, + * per the Span-Derived Primary Tags RFC. When {@code false} (the common case: feature off) the + * field is omitted entirely, so non-users pay zero payload overhead. + */ + private final boolean additionalTagsConfigured; + private final GrowableBuffer buffer; private final DDCache gitInfoCache = DDCaches.newFixedSizeWeakKeyCache(4); @@ -75,7 +86,12 @@ public final class SerializingMetricWriter implements MetricWriter { private byte[] emptyHistogramBytesCache; public SerializingMetricWriter(WellKnownTags wellKnownTags, Sink sink) { - this(wellKnownTags, sink, 512 * 1024); + this(wellKnownTags, sink, false); + } + + public SerializingMetricWriter( + WellKnownTags wellKnownTags, Sink sink, boolean additionalTagsConfigured) { + this(wellKnownTags, sink, 512 * 1024, GitInfoProvider.INSTANCE, additionalTagsConfigured); } public SerializingMetricWriter(WellKnownTags wellKnownTags, Sink sink, int initialCapacity) { @@ -87,11 +103,21 @@ public SerializingMetricWriter( Sink sink, int initialCapacity, final GitInfoProvider gitInfoProvider) { + this(wellKnownTags, sink, initialCapacity, gitInfoProvider, false); + } + + public SerializingMetricWriter( + WellKnownTags wellKnownTags, + Sink sink, + int initialCapacity, + final GitInfoProvider gitInfoProvider, + boolean additionalTagsConfigured) { this.wellKnownTags = wellKnownTags; this.buffer = new GrowableBuffer(initialCapacity); this.writer = new MsgPackWriter(buffer); this.sink = sink; this.gitInfoProvider = gitInfoProvider; + this.additionalTagsConfigured = additionalTagsConfigured; } @Override @@ -157,12 +183,16 @@ 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(); + // When the feature is configured the field is always emitted (empty array for entries that + // matched no key); when it is off the field is omitted entirely so non-users pay nothing. final int mapSize = 15 + (hasServiceSource ? 1 : 0) + (hasHttpMethod ? 1 : 0) + (hasHttpEndpoint ? 1 : 0) - + (hasGrpcStatusCode ? 1 : 0); + + (hasGrpcStatusCode ? 1 : 0) + + (additionalTagsConfigured ? 1 : 0); writer.startMap(mapSize); @@ -198,6 +228,18 @@ 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. Present whenever the feature is configured -- an empty + // array for entries that matched no key -- and omitted entirely when the feature is off, so + // spans in non-using deployments pay zero payload overhead. + if (additionalTagsConfigured) { + 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..392fb690fe6 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 @@ -27,18 +27,26 @@ final class SpanSnapshot implements InboxItem { * carries the names + {@link TagCardinalityHandler}s in parallel array form; {@code * peerTagValues} holds the per-span tag values at the same indices. */ - @Nullable final PeerTagSchema peerTagSchema; + final @Nullable PeerTagSchema peerTagSchema; /** * Peer tag values captured from the span, parallel to {@code peerTagSchema.names}. A {@code null} * entry means the span didn't have that peer tag set. {@code null} (the whole array) when {@link * #peerTagSchema} is {@code null}. */ - @Nullable final String[] peerTagValues; + final @Nullable String[] peerTagValues; - @Nullable final String httpMethod; - @Nullable final String httpEndpoint; - @Nullable final String grpcStatusCode; + final @Nullable String httpMethod; + final @Nullable String httpEndpoint; + final @Nullable 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 @Nullable String[] additionalTagValues; /** Duration in nanoseconds, OR-ed with {@code ERROR_TAG} / {@code TOP_LEVEL_TAG} as needed. */ final long tagAndDuration; @@ -58,6 +66,7 @@ final class SpanSnapshot implements InboxItem { @Nullable String httpMethod, @Nullable String httpEndpoint, @Nullable String grpcStatusCode, + @Nullable 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..bf116a8fdaa 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 @@ -19,8 +19,8 @@ final class TagCardinalityHandler { private static final int MAX_CARDINALITY_LIMIT = 1 << 29; 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. */ @@ -34,19 +34,36 @@ final class TagCardinalityHandler { private UTF8BytesString cacheBlocked = null; - /** Accumulated block count for the current cycle. Returned and zeroed by {@link #reset()}. */ - private long blockedCount; + /** + * Values collapsed by the per-cycle cardinality (distinct-value) budget in the current cycle. + * Returned and zeroed by {@link #reset()}. Surfaces as the {@code collapsed:} health-metric tag. + */ + private long collapsedCount; + + /** + * Values collapsed by the per-value length cap in the current cycle. Always 0 when {@code + * maxValueLength} is unbounded (e.g. peer tags). Read via {@link #oversizedCount()} before {@link + * #reset()} and surfaces as the {@code oversized:} health-metric tag. Kept separate from {@link + * #collapsedCount} because the approved Cardinality Limits RFC tags length-collapses ({@code + * oversized:}) distinctly from cardinality-collapses ({@code collapsed:}). + */ + private long oversizedCount; /** - * 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 +74,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 +100,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.oversizedCount++; + 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(); @@ -104,7 +128,7 @@ UTF8BytesString register(String value) { // If sentinel mode is enabled and the tag has reached its value budget, // collapse this value to "tag:tracer_blocked_value" and record the block. if (capExhausted && this.useBlockedSentinel) { - this.blockedCount++; + this.collapsedCount++; return this.tracerBlockedValue(); } // Try to find the same raw value in the previous-cycle table so the encoded @@ -142,20 +166,24 @@ private UTF8BytesString tracerBlockedValue() { return cacheBlocked; } - String[] statsDTag() { - if (statsDTag == null) { - statsDTag = new String[] {"collapsed:" + tag}; - } - return statsDTag; + /** + * Number of values collapsed by the per-value length cap in the current cycle. Must be read + * before {@link #reset()}, which zeroes it along with the cardinality count. + */ + long oversizedCount() { + return this.oversizedCount; } /** - * Resets the per-cycle working set and returns the accumulated block count for this cycle. The - * caller is responsible for reporting the count to health metrics if non-zero. + * Resets the per-cycle working set and returns the cardinality-collapse count for this cycle (the + * {@code collapsed:} count). Also zeroes the length-collapse count, so callers that report it + * must read {@link #oversizedCount()} first. The caller reports both counts to health metrics if + * non-zero. */ long reset() { - long count = this.blockedCount; - this.blockedCount = 0; + long count = this.collapsedCount; + this.collapsedCount = 0; + this.oversizedCount = 0; final String[] tmpKeys = this.priorKeys; final UTF8BytesString[] tmpValues = this.priorValues; this.priorKeys = this.curKeys; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java index 5e1deb3989c..f8018ccf765 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriter.java @@ -216,6 +216,13 @@ private void emitDataPointAttributes( if (entry.hasGrpcStatusCode()) { emitStringAttribute(metric, RPC_RESPONSE_STATUS_CODE, entry.getGrpcStatusCode()); } + // Additional metric tags: user-configured span-derived dimensions, carried as packed + // "key:value" UTF8 strings in schema order. Emitted in both modes as plain OTLP string + // attributes keyed by the tag name. NOTE: the attribute-key representation (raw tag name vs a + // datadog.* namespace) is an open cross-team question with the OTLP/agent side -- see the PR. + for (UTF8BytesString additionalTag : entry.getAdditionalTags()) { + emitAdditionalTag(metric, additionalTag); + } // Default (Datadog) mode: emit datadog.* per-point attributes if (!otelSemanticsMode) { emitStringAttribute(metric, DATADOG_OPERATION_NAME, entry.getOperationName()); @@ -227,6 +234,19 @@ private void emitDataPointAttributes( } } + // Splits a packed "key:value" additional-tag string at the first ':' (keys cannot contain ':', + // values may) and emits it as an OTLP string attribute. Skips malformed slots (no ':', empty key + // or empty value) defensively. + private static void emitAdditionalTag(OtlpMetricVisitor metric, UTF8BytesString additionalTag) { + String packed = additionalTag.toString(); + int separator = packed.indexOf(':'); + if (separator <= 0 || separator == packed.length() - 1) { + return; + } + metric.visitAttribute( + STRING_ATTRIBUTE, packed.substring(0, separator), packed.substring(separator + 1)); + } + // accepts both String literals and UTF8BytesString (both CharSequence); skips null values private static void emitStringAttribute( OtlpMetricVisitor metric, String key, @Nullable CharSequence value) { diff --git a/dd-trace-core/src/test/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/groovy/datadog/trace/common/metrics/SerializingMetricWriterTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/SerializingMetricWriterTest.groovy deleted file mode 100644 index 080a77238e4..00000000000 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/SerializingMetricWriterTest.groovy +++ /dev/null @@ -1,385 +0,0 @@ -package datadog.trace.common.metrics - -import static datadog.trace.api.config.GeneralConfig.EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED -import static java.util.concurrent.TimeUnit.MILLISECONDS -import static java.util.concurrent.TimeUnit.SECONDS - -import datadog.metrics.api.Histograms -import datadog.metrics.impl.DDSketchHistograms -import datadog.trace.api.Config -import datadog.trace.api.ProcessTags -import datadog.trace.api.WellKnownTags -import datadog.trace.api.git.CommitInfo -import datadog.trace.api.git.GitInfo -import datadog.trace.api.git.GitInfoProvider -import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString -import datadog.trace.test.util.DDSpecification -import java.nio.ByteBuffer -import org.msgpack.core.MessagePack -import org.msgpack.core.MessageUnpacker - -class SerializingMetricWriterTest extends DDSpecification { - - def setupSpec() { - Histograms.register(DDSketchHistograms.FACTORY) - } - - /** Build an {@link AggregateEntry} with a pre-recorded duration count. */ - private static AggregateEntry entry( - CharSequence resource, - CharSequence service, - CharSequence operationName, - CharSequence serviceSource, - CharSequence type, - int httpStatusCode, - boolean synthetic, - boolean traceRoot, - CharSequence spanKind, - List peerTags, - CharSequence httpMethod, - CharSequence httpEndpoint, - CharSequence grpcStatusCode, - int hitCount) { - AggregateEntry e = AggregateEntryTestUtils.of( - resource, service, operationName, serviceSource, type, - httpStatusCode, synthetic, traceRoot, spanKind, peerTags, - httpMethod, httpEndpoint, grpcStatusCode) - hitCount.times { e.recordOneDuration(1L) } - return e - } - - def "should produce correct message #iterationIndex with process tags enabled #withProcessTags" () { - setup: - if (!withProcessTags) { - injectSysConfig(EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED, "false") - } - ProcessTags.reset() - long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()) - long duration = SECONDS.toNanos(10) - WellKnownTags wellKnownTags = new WellKnownTags("runtimeid", "hostname", "env", "service", "version","language") - ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content) - SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128) - - when: - writer.startBucket(content.size(), startTime, duration) - for (AggregateEntry e : content) { - writer.add(e) - } - writer.finishBucket() - - then: - sink.validatedInput() - - cleanup: - removeSysConfig(EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED) - ProcessTags.reset() - - where: - content << [ - [ - entry( - "resource1", "service1", "operation1", null, "type", 0, - false, false, "client", - [ - UTF8BytesString.create("country:canada"), - UTF8BytesString.create("georegion:amer"), - UTF8BytesString.create("peer.service:remote-service") - ], - null, null, null, - 10), - entry( - "resource2", "service2", "operation2", null, "type2", 200, - true, false, "producer", - [ - UTF8BytesString.create("country:canada"), - UTF8BytesString.create("georegion:amer"), - UTF8BytesString.create("peer.service:remote-service") - ], - null, null, null, - 9), - entry( - "GET /api/users/:id", "web-service", "http.request", null, "web", 200, - false, true, "server", - [], - null, null, null, - 5) - ], - (0..10000).collect({ i -> - entry( - "resource" + i, "service" + i, "operation" + i, null, "type", 0, - false, false, "producer", - [UTF8BytesString.create("messaging.destination:dest" + i)], - null, null, null, - 10) - }) - ] - withProcessTags << [true, false] - } - - def "ServiceSource optional in the payload"() { - setup: - long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()) - long duration = SECONDS.toNanos(10) - WellKnownTags wellKnownTags = new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language") - - def entryNoSource = entry("resource", "service", "operation", null, "type", 200, false, false, "server", [], "GET", "/api/users", null, 1) - def entryWithSource = entry("resource", "service", "operation", "source", "type", 200, false, false, "server", [], "POST", null, null, 1) - - def content = [entryNoSource, entryWithSource] - - ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content) - SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128) - - when: - writer.startBucket(content.size(), startTime, duration) - for (AggregateEntry e : content) { - writer.add(e) - } - writer.finishBucket() - - then: - sink.validatedInput() - } - - def "HTTPMethod and HTTPEndpoint fields are optional in payload"() { - setup: - long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()) - long duration = SECONDS.toNanos(10) - WellKnownTags wellKnownTags = new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language") - - def entryWithBoth = entry("resource", "service", "operation", null, "type", 200, false, false, "server", [], "GET", "/api/users", null, 1) - def entryWithMethodOnly = entry("resource", "service", "operation", null, "type", 200, false, false, "server", [], "POST", null, null, 1) - def entryWithEndpointOnly = entry("resource", "service", "operation", null, "type", 200, false, false, "server", [], null, "/api/orders", null, 1) - def entryWithNeither = entry("resource", "service", "operation", null, "type", 200, false, false, "client", [], null, null, null, 1) - - def content = [entryWithBoth, entryWithMethodOnly, entryWithEndpointOnly, entryWithNeither] - - ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content) - SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128) - - when: - writer.startBucket(content.size(), startTime, duration) - for (AggregateEntry e : content) { - writer.add(e) - } - writer.finishBucket() - - then: - sink.validatedInput() - } - - def "add git sha commit info when sha commit is #shaCommit"() { - setup: - GitInfoProvider gitInfoProvider = Mock(GitInfoProvider) - gitInfoProvider.getGitInfo() >> new GitInfo(null, null, null, new CommitInfo(shaCommit)) - - long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()) - long duration = SECONDS.toNanos(10) - WellKnownTags wellKnownTags = new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language") - - def e = entry("resource", "service", "operation", null, "type", 200, false, false, "server", [], "GET", "/api/users", null, 1) - - def content = [e] - - ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content, shaCommit) - SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128, gitInfoProvider) - - when: - writer.startBucket(content.size(), startTime, duration) - for (AggregateEntry entryItem : content) { - writer.add(entryItem) - } - writer.finishBucket() - - then: - sink.validatedInput() - - where: - shaCommit << [null, "123456"] - } - - def "GRPCStatusCode field is present in payload for rpc-type spans"() { - setup: - long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()) - long duration = SECONDS.toNanos(10) - WellKnownTags wellKnownTags = new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language") - - def entryWithGrpc = entry("grpc.service/Method", "grpc-service", "grpc.server", null, "rpc", 0, false, false, "server", [], null, null, "OK", 1) - def entryWithGrpcError = entry("grpc.service/Method", "grpc-service", "grpc.server", null, "rpc", 0, false, false, "client", [], null, null, "NOT_FOUND", 1) - def entryWithoutGrpc = entry("resource", "service", "operation", null, "web", 200, false, false, "server", [], null, null, null, 1) - - def content = [entryWithGrpc, entryWithGrpcError, entryWithoutGrpc] - - ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content) - SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128) - - when: - writer.startBucket(content.size(), startTime, duration) - for (AggregateEntry e : content) { - writer.add(e) - } - writer.finishBucket() - - then: - sink.validatedInput() - } - - static class ValidatingSink implements Sink { - - private final WellKnownTags wellKnownTags - private final long startTimeNanos - private final long duration - private boolean validated = false - private List content - private final String expectedGitCommitSha - - ValidatingSink(WellKnownTags wellKnownTags, long startTimeNanos, long duration, - List content, String expectedGitCommitSha = null) { - this.wellKnownTags = wellKnownTags - this.startTimeNanos = startTimeNanos - this.duration = duration - this.content = content - this.expectedGitCommitSha = expectedGitCommitSha - } - - @Override - void register(EventListener listener) { - } - - @Override - void accept(int messageCount, ByteBuffer buffer) { - MessageUnpacker unpacker = MessagePack.newDefaultUnpacker(buffer) - int mapSize = unpacker.unpackMapHeader() - String gitCommitSha = expectedGitCommitSha - assert mapSize == (7 + (Config.get().isExperimentalPropagateProcessTagsEnabled() ? 1 : 0) - + (gitCommitSha != null ? 1 : 0)) - assert unpacker.unpackString() == "RuntimeID" - assert unpacker.unpackString() == wellKnownTags.getRuntimeId() as String - assert unpacker.unpackString() == "Sequence" - assert unpacker.unpackLong() == 0L - assert unpacker.unpackString() == "Hostname" - assert unpacker.unpackString() == wellKnownTags.getHostname() as String - assert unpacker.unpackString() == "Service" - assert unpacker.unpackString() == wellKnownTags.getService() as String - assert unpacker.unpackString() == "Env" - assert unpacker.unpackString() == wellKnownTags.getEnv() as String - assert unpacker.unpackString() == "Version" - assert unpacker.unpackString() == wellKnownTags.getVersion() as String - if (Config.get().isExperimentalPropagateProcessTagsEnabled()) { - assert unpacker.unpackString() == "ProcessTags" - assert unpacker.unpackString() == ProcessTags.tagsForSerialization as String - } - if (gitCommitSha != null) { - assert unpacker.unpackString() == "GitCommitSha" - assert unpacker.unpackString() == gitCommitSha - } - assert unpacker.unpackString() == "Stats" - int outerLength = unpacker.unpackArrayHeader() - assert outerLength == 1 - assert unpacker.unpackMapHeader() == 3 - assert unpacker.unpackString() == "Start" - assert unpacker.unpackLong() == startTimeNanos - assert unpacker.unpackString() == "Duration" - assert unpacker.unpackLong() == duration - assert unpacker.unpackString() == "Stats" - int statCount = unpacker.unpackArrayHeader() - assert statCount == content.size() - for (AggregateEntry entry : content) { - // counters now live on AggregateEntry - int metricMapSize = unpacker.unpackMapHeader() - // Calculate expected map size based on optional fields - boolean hasHttpMethod = entry.hasHttpMethod() - boolean hasHttpEndpoint = entry.hasHttpEndpoint() - boolean hasServiceSource = entry.hasServiceSource() - boolean hasGrpcStatusCode = entry.hasGrpcStatusCode() - int expectedMapSize = 15 + (hasServiceSource ? 1 : 0) + (hasHttpMethod ? 1 : 0) + (hasHttpEndpoint ? 1 : 0) + (hasGrpcStatusCode ? 1 : 0) - assert metricMapSize == expectedMapSize - int elementCount = 0 - assert unpacker.unpackString() == "Name" - assert unpacker.unpackString() == entry.getOperationName() as String - ++elementCount - assert unpacker.unpackString() == "Service" - assert unpacker.unpackString() == entry.getService() as String - ++elementCount - assert unpacker.unpackString() == "Resource" - assert unpacker.unpackString() == entry.getResource() as String - ++elementCount - assert unpacker.unpackString() == "Type" - assert unpacker.unpackString() == entry.getType() as String - ++elementCount - assert unpacker.unpackString() == "HTTPStatusCode" - assert unpacker.unpackInt() == entry.getHttpStatusCode() - ++elementCount - assert unpacker.unpackString() == "Synthetics" - assert unpacker.unpackBoolean() == entry.isSynthetics() - ++elementCount - assert unpacker.unpackString() == "IsTraceRoot" - assert unpacker.unpackInt() == (entry.isTraceRoot() ? TriState.TRUE.serialValue : TriState.FALSE.serialValue) - ++elementCount - assert unpacker.unpackString() == "SpanKind" - assert unpacker.unpackString() == entry.getSpanKind() as String - ++elementCount - assert unpacker.unpackString() == "PeerTags" - int peerTagsLength = unpacker.unpackArrayHeader() - assert peerTagsLength == entry.getPeerTags().size() - for (int i = 0; i < peerTagsLength; i++) { - def unpackedPeerTag = unpacker.unpackString() - assert unpackedPeerTag == entry.getPeerTags()[i].toString() - } - ++elementCount - // Service source is only present when the service name has been overridden by the tracer - if (hasServiceSource) { - assert unpacker.unpackString() == "srv_src" - assert unpacker.unpackString() == entry.getServiceSource().toString() - ++elementCount - } - // HTTPMethod and HTTPEndpoint are optional - only present if non-null - if (hasHttpMethod) { - assert unpacker.unpackString() == "HTTPMethod" - assert unpacker.unpackString() == entry.getHttpMethod() as String - ++elementCount - } - if (hasHttpEndpoint) { - assert unpacker.unpackString() == "HTTPEndpoint" - assert unpacker.unpackString() == entry.getHttpEndpoint() as String - ++elementCount - } - if (hasGrpcStatusCode) { - assert unpacker.unpackString() == "GRPCStatusCode" - assert unpacker.unpackString() == entry.getGrpcStatusCode() as String - ++elementCount - } - assert unpacker.unpackString() == "Hits" - assert unpacker.unpackInt() == entry.getHitCount() - ++elementCount - assert unpacker.unpackString() == "Errors" - assert unpacker.unpackInt() == entry.getErrorCount() - ++elementCount - assert unpacker.unpackString() == "TopLevelHits" - assert unpacker.unpackInt() == entry.getTopLevelCount() - ++elementCount - assert unpacker.unpackString() == "Duration" - assert unpacker.unpackLong() == entry.getDuration() - ++elementCount - assert unpacker.unpackString() == "OkSummary" - validateSketch(unpacker) - ++elementCount - assert unpacker.unpackString() == "ErrorSummary" - validateSketch(unpacker) - ++elementCount - assert elementCount == metricMapSize - } - validated = true - } - - private void validateSketch(MessageUnpacker unpacker) { - int length = unpacker.unpackBinaryHeader() - assert length > 0 - unpacker.readPayload(length) - } - - boolean validatedInput() { - return validated - } - } -} 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..475cf86d139 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AdditionalTagsSchemaTest.java @@ -0,0 +1,201 @@ +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 static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import datadog.trace.core.monitor.HealthMetrics; +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 schemaSortsAndCapsAtMaxTagKeys() { + 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); + } + + @Test + void perKeyCardinalityBudgetsAreIndependent() { + // Two configured keys, cardinality limit 1 each. Exhausting one key's budget must neither + // consume nor block the other's -- each key gets its own TagCardinalityHandler. + AdditionalTagsSchema schema = + AdditionalTagsSchema.from( + new LinkedHashSet<>(Arrays.asList("region", "tenant_id")), 1, true); + int region = indexOf(schema, "region"); + int tenant = indexOf(schema, "tenant_id"); + + // region: first value fits, second collapses to the blocked sentinel. + assertEquals("region:us-east-1", schema.register(region, "us-east-1").toString()); + assertEquals("region:tracer_blocked_value", schema.register(region, "eu-west-1").toString()); + + // tenant_id is untouched by region's exhaustion: its first value still flows through, and its + // own budget collapses only its own second value. + assertEquals("tenant_id:acme-corp", schema.register(tenant, "acme-corp").toString()); + assertEquals("tenant_id:tracer_blocked_value", schema.register(tenant, "globex").toString()); + } + + @Test + void resetHandlersReportsCardinalityCollapseUnderFieldName() { + // Cardinality-collapsed values surface as the health-metric tag + // "collapsed:additional_metric_tags" + // -- the lowercased protobuf field name, not the individual tag key -- per the approved + // Cardinality Limits RFC. + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(Collections.singleton("region"), 1, true); + int region = indexOf(schema, "region"); + schema.register(region, "us-east-1"); // within budget + schema.register(region, "eu-west-1"); // collapsed (cardinality) + schema.register(region, "ap-south-1"); // collapsed (cardinality) + + HealthMetrics metrics = mock(HealthMetrics.class); + schema.resetHandlers(metrics, new CardinalityLimitReporter()); + + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:additional_metric_tags"}, 2L); + verifyNoMoreInteractions(metrics); + } + + @Test + void resetHandlersAggregatesCardinalityCollapseAcrossKeysUnderOneFieldTag() { + // Two keys each collapse: the field-level health metric sums both under a single + // "collapsed:additional_metric_tags" tag rather than emitting per key name. + AdditionalTagsSchema schema = + AdditionalTagsSchema.from( + new LinkedHashSet<>(Arrays.asList("region", "tenant_id")), 1, true); + int region = indexOf(schema, "region"); + int tenant = indexOf(schema, "tenant_id"); + schema.register(region, "us-east-1"); // within budget + schema.register(region, "eu-west-1"); // collapsed + schema.register(tenant, "acme-corp"); // within budget + schema.register(tenant, "globex"); // collapsed + + HealthMetrics metrics = mock(HealthMetrics.class); + schema.resetHandlers(metrics, new CardinalityLimitReporter()); + + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:additional_metric_tags"}, 2L); + verifyNoMoreInteractions(metrics); + } + + @Test + void resetHandlersReportsLengthAndCardinalityCollapsesUnderDistinctTags() { + // A single cycle with both a length collapse (oversized) and a cardinality collapse must emit + // two distinct health-metric tags: oversized:additional_metric_tags and + // collapsed:additional_metric_tags. + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(Collections.singleton("region"), 1, true); + int region = indexOf(schema, "region"); + schema.register(region, "us-east-1"); // within budget + schema.register(region, "eu-west-1"); // collapsed (cardinality) + schema.register(region, stringOfLength(201)); // oversized (length) + + HealthMetrics metrics = mock(HealthMetrics.class); + schema.resetHandlers(metrics, new CardinalityLimitReporter()); + + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:additional_metric_tags"}, 1L); + verify(metrics).onTagCardinalityBlocked(new String[] {"oversized:additional_metric_tags"}, 1L); + verifyNoMoreInteractions(metrics); + } + + @Test + void resetHandlersWithNoBlocksDoesNotCallHealthMetrics() { + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(Collections.singleton("region"), 4, true); + schema.register(indexOf(schema, "region"), "us-east-1"); + + HealthMetrics metrics = mock(HealthMetrics.class); + schema.resetHandlers(metrics, new CardinalityLimitReporter()); + + verifyNoMoreInteractions(metrics); + } + + @Test + void additionalTagValueAtLengthCapPassesAndOverCapCollapses() { + // End-to-end through the shipped wiring: AdditionalTagsSchema builds its handlers with + // MetricCardinalityLimits.ADDITIONAL_TAG_MAX_VALUE_LENGTH as the per-value length cap. A value + // of exactly the cap passes verbatim; one character longer collapses to the blocked sentinel. + // The cardinality budget is generous so only the length branch can block. + int cap = MetricCardinalityLimits.ADDITIONAL_TAG_MAX_VALUE_LENGTH; + // Pin the shipped magnitude: RFC and dd-trace-dotnet both use 200. + assertEquals(200, cap); + + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(Collections.singleton("region"), 100, true); + int region = indexOf(schema, "region"); + String atCap = stringOfLength(cap); + String overCap = stringOfLength(cap + 1); + + assertEquals("region:" + atCap, schema.register(region, atCap).toString()); + assertEquals("region:tracer_blocked_value", schema.register(region, overCap).toString()); + + // The over-cap value is a length collapse: it surfaces as oversized:additional_metric_tags, + // not the cardinality collapsed: tag. + HealthMetrics metrics = mock(HealthMetrics.class); + schema.resetHandlers(metrics, new CardinalityLimitReporter()); + verify(metrics).onTagCardinalityBlocked(new String[] {"oversized:additional_metric_tags"}, 1L); + verifyNoMoreInteractions(metrics); + } + + private static String stringOfLength(int length) { + char[] chars = new char[length]; + Arrays.fill(chars, 'x'); + return new String(chars); + } + + private static int indexOf(AdditionalTagsSchema schema, String name) { + for (int i = 0; i < schema.size(); i++) { + if (name.equals(schema.name(i))) { + return i; + } + } + throw new IllegalArgumentException("no such configured key: " + name); + } +} 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 60e0badcb7f..29adc3a6d91 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/AggregateEntryTestUtils.java @@ -45,6 +45,43 @@ public static AggregateEntry of( @Nullable CharSequence httpMethod, @Nullable CharSequence httpEndpoint, @Nullable CharSequence grpcStatusCode) { + return of( + resource, + service, + operationName, + serviceSource, + type, + httpStatusCode, + synthetic, + traceRoot, + spanKind, + peerTags, + httpMethod, + httpEndpoint, + grpcStatusCode, + null); + } + + /** + * Same as {@link #of} but also carries pre-packed {@code "key:value"} additional metric tags (in + * schema order), letting the OTLP/serializing writer tests exercise the additional-tags path + * without driving a full {@code AggregateTable}/{@code AdditionalTagsSchema} canonicalization. + */ + public static AggregateEntry of( + CharSequence resource, + CharSequence service, + CharSequence operationName, + @Nullable CharSequence serviceSource, + CharSequence type, + int httpStatusCode, + boolean synthetic, + boolean traceRoot, + CharSequence spanKind, + @Nullable List peerTags, + @Nullable CharSequence httpMethod, + @Nullable CharSequence httpEndpoint, + @Nullable CharSequence grpcStatusCode, + @Nullable UTF8BytesString[] additionalTags) { UTF8BytesString resourceUtf = AggregateEntry.createUtf8(resource); UTF8BytesString serviceUtf = AggregateEntry.createUtf8(service); UTF8BytesString operationNameUtf = AggregateEntry.createUtf8(operationName); @@ -56,6 +93,8 @@ public static AggregateEntry of( UTF8BytesString grpcUtf = AggregateEntry.createUtf8(grpcStatusCode); List peerTagsList = peerTags == null ? Collections.emptyList() : peerTags; UTF8BytesString[] peerTagsArr = peerTagsList.toArray(new UTF8BytesString[0]); + UTF8BytesString[] additionalTagsArr = + additionalTags == null ? new UTF8BytesString[0] : additionalTags; long keyHash = AggregateEntry.hashOf( resourceUtf, @@ -71,7 +110,9 @@ public static AggregateEntry of( synthetic, traceRoot, peerTagsArr, - peerTagsArr.length); + peerTagsArr.length, + additionalTagsArr, + additionalTagsArr.length); return new AggregateEntry( keyHash, resourceUtf, @@ -86,7 +127,8 @@ public static AggregateEntry of( (short) httpStatusCode, synthetic, traceRoot, - peerTagsList); + peerTagsList, + additionalTagsArr); } /** @@ -110,7 +152,7 @@ public static AggregateEntry recordTopLevel(AggregateEntry e, long durationNanos /** Clears the per-cycle counters and histograms on {@code e}. See {@link #recordOk}. */ public static void clear(AggregateEntry e) { - e.clear(); + e.clearAggregate(); } /** 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..993ef163591 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,37 @@ private static SpanSnapshot nullableSnapshot( null, null, null, + null, 0L); } + @Test + void resetCoreHandlersClearsBlockedCountsAndRefreshesCapacity() { + // Inject the core handlers via the 3-arg constructor to test resetCoreHandlers() directly. + CoreHandlers handlers = new CoreHandlers(); + AggregateTable table = new AggregateTable(512, handlers, AdditionalTagsSchema.EMPTY); + + // 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.resetCoreHandlers(metrics, new CardinalityLimitReporter()); + + 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 +391,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..944558c17b0 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 @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertSame; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import java.util.Arrays; import org.junit.jupiter.api.Test; class CardinalityHandlerTest { @@ -137,6 +138,35 @@ void tagPriorCycleInstancesAreReusedAcrossReset() { assertSame(hostBBefore, h.register("host-b")); } + @Test + void tagValueAtLengthCapPassesAndOverCapCollapses() { + // Length cap is enforced independently of the cardinality budget: give a generous budget (10) + // so only the length branch can block. A value of exactly maxValueLength passes; one character + // longer collapses to the blocked sentinel. The 200-char cap here is the shipped + // MetricCardinalityLimits.ADDITIONAL_TAG_MAX_VALUE_LENGTH. + TagCardinalityHandler h = new TagCardinalityHandler("k", 10, true, 200); + String atCap = stringOfLength(200); + String overCap = stringOfLength(201); + + assertEquals("k:" + atCap, h.register(atCap).toString()); + assertEquals("k:tracer_blocked_value", h.register(overCap).toString()); + // The over-cap value is a length ("oversized") collapse, tracked separately from the + // cardinality-collapse count that reset() returns. + assertEquals(1, h.oversizedCount()); + assertEquals(0, h.reset()); + } + + @Test + void tagValueLengthCapNotEnforcedWhenSentinelDisabled() { + // With the blocked sentinel disabled the length branch is skipped: an oversized value flows + // through verbatim (limits-disabled mode never masks values). + TagCardinalityHandler h = new TagCardinalityHandler("k", 10, false, 200); + String overCap = stringOfLength(201); + assertEquals("k:" + overCap, h.register(overCap).toString()); + assertEquals(0, h.oversizedCount()); + assertEquals(0, h.reset()); + } + @Test void propertyRegisterOfNullReturnsEmpty() { PropertyCardinalityHandler h = new PropertyCardinalityHandler("test", 4); @@ -250,11 +280,15 @@ 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"); assertEquals("peer.service:svc-2", overCap.toString()); } + + private static String stringOfLength(int length) { + char[] chars = new char[length]; + Arrays.fill(chars, 'x'); + return new String(chars); + } } diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/CardinalityLimitReporterTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/CardinalityLimitReporterTest.java new file mode 100644 index 00000000000..8190c07c2c3 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/CardinalityLimitReporterTest.java @@ -0,0 +1,122 @@ +package datadog.trace.common.metrics; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import datadog.logging.RatelimitedLogger; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.slf4j.LoggerFactory; + +class CardinalityLimitReporterTest { + + private Logger logger; + private Level previousLevel; + private ListAppender appender; + + @BeforeEach + void attachAppender() { + logger = (Logger) LoggerFactory.getLogger(CardinalityLimitReporter.class); + previousLevel = logger.getLevel(); + // WARN (not DEBUG) so RatelimitedLogger takes the rate-limited path rather than logging always. + logger.setLevel(Level.WARN); + appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + } + + @AfterEach + void detachAppender() { + logger.detachAppender(appender); + logger.setLevel(previousLevel); + } + + @Test + void reportsNothingWhenNoBlocksRecorded() { + CardinalityLimitReporter reporter = new CardinalityLimitReporter(); + + reporter.reportIfDue(); + + assertEquals(0, appender.list.size()); + } + + @Test + void aggregatesCountsByTagNameIntoOneLine() { + CardinalityLimitReporter reporter = new CardinalityLimitReporter(); + // Same tag recorded twice across cycles must sum; distinct tags each appear once. + reporter.record("resource", 5); + reporter.record("resource", 3); + reporter.record("peer.service", 2); + + reporter.reportIfDue(); + + assertEquals(1, appender.list.size()); + // Iteration order is the Hashtable's bucket order, not sorted -- assert each entry + // independently. + String message = appender.list.get(0).getFormattedMessage(); + assertTrue(message.contains("resource=8"), message); + assertTrue(message.contains("peer.service=2"), message); + assertTrue(message.contains("tracer_blocked_value"), message); + } + + @Test + void rateLimitsRepeatedReportsWithinTheWindow() { + CardinalityLimitReporter reporter = new CardinalityLimitReporter(); + reporter.record("resource", 1); + reporter.reportIfDue(); // first call in the window logs immediately + + // A later cycle within the 5-minute window records more but must not emit a second line. + reporter.record("resource", 4); + reporter.reportIfDue(); + + assertEquals(1, appender.list.size()); + } + + @Test + void retainsCountsUntilAWarningIsActuallyEmitted() { + // Drive the rate limiter directly: suppress the first attempt, permit the second. Counts + // recorded while suppressed must survive and appear -- summed with later counts -- in the line + // that is eventually emitted, and only then be cleared. + RatelimitedLogger rlLog = mock(RatelimitedLogger.class); + when(rlLog.warn(anyString(), any())).thenReturn(false, true); + CardinalityLimitReporter reporter = new CardinalityLimitReporter(rlLog); + + reporter.record("resource", 3); + reporter.reportIfDue(); // suppressed: nothing cleared + reporter.record("resource", 4); + reporter.reportIfDue(); // permitted: emits the retained 3 + new 4 + + ArgumentCaptor summary = ArgumentCaptor.forClass(Object.class); + verify(rlLog, times(2)).warn(anyString(), summary.capture()); + // First (suppressed) attempt still saw only 3; the emitting attempt carries the full 7. + assertEquals("resource=3", summary.getAllValues().get(0)); + assertEquals("resource=7", summary.getAllValues().get(1)); + + // A successful emit cleared the store: with nothing recorded since, the next due-check + // short-circuits and never touches the logger again. + reporter.reportIfDue(); + verify(rlLog, times(2)).warn(anyString(), any()); + } + + @Test + void zeroAndNegativeCountsAreIgnored() { + CardinalityLimitReporter reporter = new CardinalityLimitReporter(); + reporter.record("resource", 0); + + reporter.reportIfDue(); + + assertEquals(0, appender.list.size()); + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/CoreHandlersTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/CoreHandlersTest.java new file mode 100644 index 00000000000..35c2fa8468c --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/CoreHandlersTest.java @@ -0,0 +1,95 @@ +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 CoreHandlersTest { + + @Test + void resetReportsBlockedCountForExhaustedHandler() { + CoreHandlers handlers = new CoreHandlers(); + // 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, new CardinalityLimitReporter()); + + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:span_kind"}, 2L); + verifyNoMoreInteractions(metrics); + } + + @Test + void resetReportsBlockedCountForAllNineHandlers() { + CoreHandlers handlers = new CoreHandlers(); + 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, new CardinalityLimitReporter()); + + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:resource"}, 1L); + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:service"}, 1L); + // operation is carried on the wire as the protobuf field "name", so it reports as + // collapsed:name + verify(metrics).onTagCardinalityBlocked(new String[] {"collapsed:name"}, 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() { + CoreHandlers handlers = new CoreHandlers(); + 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, new CardinalityLimitReporter()); + + // 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() { + CoreHandlers handlers = new CoreHandlers(); + handlers.resource.register("r1"); + handlers.service.register("svc"); + + HealthMetrics metrics = mock(HealthMetrics.class); + handlers.reset(metrics, new CardinalityLimitReporter()); + + 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/PeerTagSchemaTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/PeerTagSchemaTest.java index 710e78a175b..3270917068f 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,10 +5,13 @@ 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.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.List; import java.util.Set; import org.junit.jupiter.api.Test; @@ -85,10 +88,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 +100,55 @@ 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, new CardinalityLimitReporter()); + 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, new CardinalityLimitReporter()); + assertEquals(0, recorded[0]); + } + + @Test + void resetHandlersAggregatesAllPeerTagsUnderSinglePeerTagsField() { + // The Cardinality Limits RFC reports peer-tag collapses under the protobuf field name peer_tags + // aggregated across every configured peer tag, not per individual tag name. + PeerTagSchema schema = + new PeerTagSchema(new String[] {"peer.hostname", "peer.service"}, PeerTagSchema.NO_STATE); + schema.handlers[0] = new TagCardinalityHandler("peer.hostname", 1, true); + schema.handlers[1] = new TagCardinalityHandler("peer.service", 1, true); + + schema.register(0, "host-a"); // within limit + schema.register(0, "host-b"); // blocked + schema.register(1, "svc-a"); // within limit + schema.register(1, "svc-b"); // blocked + schema.register(1, "svc-c"); // blocked + + List tags = new ArrayList<>(); + long[] total = {0}; + HealthMetrics hm = + new HealthMetrics() { + @Override + public void onTagCardinalityBlocked(String[] tag, long count) { + tags.add(tag); + total[0] += count; + } + }; + + schema.resetHandlers(hm, new CardinalityLimitReporter()); + + // One health-metric call, tagged at the field granularity, summing both handlers' blocks. + assertEquals(1, tags.size()); + assertArrayEquals(new String[] {"collapsed:peer_tags"}, tags.get(0)); + assertEquals(3, total[0]); } } diff --git a/dd-trace-core/src/test/java/datadog/trace/common/metrics/SerializingMetricWriterTest.java b/dd-trace-core/src/test/java/datadog/trace/common/metrics/SerializingMetricWriterTest.java new file mode 100644 index 00000000000..bc05393cea3 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/common/metrics/SerializingMetricWriterTest.java @@ -0,0 +1,839 @@ +package datadog.trace.common.metrics; + +import static datadog.trace.api.config.GeneralConfig.EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED; +import static datadog.trace.test.junit.utils.config.WithConfigExtension.injectSysConfig; +import static datadog.trace.test.junit.utils.config.WithConfigExtension.removeSysConfig; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.params.provider.Arguments.arguments; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.metrics.api.Histograms; +import datadog.metrics.impl.DDSketchHistograms; +import datadog.trace.api.Config; +import datadog.trace.api.ProcessTags; +import datadog.trace.api.WellKnownTags; +import datadog.trace.api.git.CommitInfo; +import datadog.trace.api.git.GitInfo; +import datadog.trace.api.git.GitInfoProvider; +import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import datadog.trace.test.util.DDJavaSpecification; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.msgpack.core.MessagePack; +import org.msgpack.core.MessageUnpacker; +import org.tabletest.junit.TableTest; + +class SerializingMetricWriterTest extends DDJavaSpecification { + + @BeforeAll + static void registerHistograms() { + Histograms.register(DDSketchHistograms.FACTORY); + } + + /** Build an {@link AggregateEntry} with a pre-recorded duration count. */ + private static AggregateEntry entry( + CharSequence resource, + CharSequence service, + CharSequence operationName, + CharSequence serviceSource, + CharSequence type, + int httpStatusCode, + boolean synthetic, + boolean traceRoot, + CharSequence spanKind, + List peerTags, + CharSequence httpMethod, + CharSequence httpEndpoint, + CharSequence grpcStatusCode, + int hitCount) { + AggregateEntry aggregateEntry = + AggregateEntryTestUtils.of( + resource, + service, + operationName, + serviceSource, + type, + httpStatusCode, + synthetic, + traceRoot, + spanKind, + peerTags, + httpMethod, + httpEndpoint, + grpcStatusCode); + for (int i = 0; i < hitCount; i++) { + aggregateEntry.recordOneDuration(1L); + } + return aggregateEntry; + } + + static Stream shouldProduceCorrectMessageArguments() { + List smallContent = + Arrays.asList( + entry( + "resource1", + "service1", + "operation1", + null, + "type", + 0, + false, + false, + "client", + Arrays.asList( + UTF8BytesString.create("country:canada"), + UTF8BytesString.create("georegion:amer"), + UTF8BytesString.create("peer.service:remote-service")), + null, + null, + null, + 10), + entry( + "resource2", + "service2", + "operation2", + null, + "type2", + 200, + true, + false, + "producer", + Arrays.asList( + UTF8BytesString.create("country:canada"), + UTF8BytesString.create("georegion:amer"), + UTF8BytesString.create("peer.service:remote-service")), + null, + null, + null, + 9), + entry( + "GET /api/users/:id", + "web-service", + "http.request", + null, + "web", + 200, + false, + true, + "server", + Collections.emptyList(), + null, + null, + null, + 5)); + + List largeContent = new ArrayList<>(); + for (int i = 0; i <= 10000; i++) { + largeContent.add( + entry( + "resource" + i, + "service" + i, + "operation" + i, + null, + "type", + 0, + false, + false, + "producer", + Collections.singletonList(UTF8BytesString.create("messaging.destination:dest" + i)), + null, + null, + null, + 10)); + } + + return Stream.of( + arguments("small content with process tags", smallContent, true), + arguments("large content without process tags", largeContent, false)); + } + + @ParameterizedTest(name = "should produce correct message: {0}") + @MethodSource("shouldProduceCorrectMessageArguments") + void shouldProduceCorrectMessage( + String scenario, List content, boolean withProcessTags) { + // setup + if (!withProcessTags) { + injectSysConfig(EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED, "false"); + } + ProcessTags.reset(Config.get()); + long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()); + long duration = SECONDS.toNanos(10); + WellKnownTags wellKnownTags = + new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language"); + ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content); + SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128); + + try { + // when + writer.startBucket(content.size(), startTime, duration); + for (AggregateEntry aggregateEntry : content) { + writer.add(aggregateEntry); + } + writer.finishBucket(); + + // then + assertTrue(sink.validatedInput()); + } finally { + // cleanup + removeSysConfig(EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED); + ProcessTags.reset(Config.get()); + } + } + + @Test + void serviceSourceOptionalInThePayload() { + // setup + long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()); + long duration = SECONDS.toNanos(10); + WellKnownTags wellKnownTags = + new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language"); + + AggregateEntry entryNoSource = + entry( + "resource", + "service", + "operation", + null, + "type", + 200, + false, + false, + "server", + Collections.emptyList(), + "GET", + "/api/users", + null, + 1); + AggregateEntry entryWithSource = + entry( + "resource", + "service", + "operation", + "source", + "type", + 200, + false, + false, + "server", + Collections.emptyList(), + "POST", + null, + null, + 1); + + List content = Arrays.asList(entryNoSource, entryWithSource); + + ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content); + SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128); + + // when + writer.startBucket(content.size(), startTime, duration); + for (AggregateEntry aggregateEntry : content) { + writer.add(aggregateEntry); + } + writer.finishBucket(); + + // then + assertTrue(sink.validatedInput()); + } + + @Test + void httpMethodAndHttpEndpointFieldsAreOptionalInPayload() { + // setup + long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()); + long duration = SECONDS.toNanos(10); + WellKnownTags wellKnownTags = + new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language"); + + AggregateEntry entryWithBoth = + entry( + "resource", + "service", + "operation", + null, + "type", + 200, + false, + false, + "server", + Collections.emptyList(), + "GET", + "/api/users", + null, + 1); + AggregateEntry entryWithMethodOnly = + entry( + "resource", + "service", + "operation", + null, + "type", + 200, + false, + false, + "server", + Collections.emptyList(), + "POST", + null, + null, + 1); + AggregateEntry entryWithEndpointOnly = + entry( + "resource", + "service", + "operation", + null, + "type", + 200, + false, + false, + "server", + Collections.emptyList(), + null, + "/api/orders", + null, + 1); + AggregateEntry entryWithNeither = + entry( + "resource", + "service", + "operation", + null, + "type", + 200, + false, + false, + "client", + Collections.emptyList(), + null, + null, + null, + 1); + + List content = + Arrays.asList(entryWithBoth, entryWithMethodOnly, entryWithEndpointOnly, entryWithNeither); + + ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content); + SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128); + + // when + writer.startBucket(content.size(), startTime, duration); + for (AggregateEntry aggregateEntry : content) { + writer.add(aggregateEntry); + } + writer.finishBucket(); + + // then + assertTrue(sink.validatedInput()); + } + + @TableTest({ + "scenario | shaCommit", + "no sha commit | ", + "with sha commit | 123456 " + }) + void addGitShaCommitInfo(String shaCommit) { + // setup + GitInfoProvider gitInfoProvider = mock(GitInfoProvider.class); + when(gitInfoProvider.getGitInfo()) + .thenReturn(new GitInfo(null, null, null, new CommitInfo(shaCommit))); + + long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()); + long duration = SECONDS.toNanos(10); + WellKnownTags wellKnownTags = + new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language"); + + AggregateEntry aggregateEntry = + entry( + "resource", + "service", + "operation", + null, + "type", + 200, + false, + false, + "server", + Collections.emptyList(), + "GET", + "/api/users", + null, + 1); + + List content = Collections.singletonList(aggregateEntry); + + ValidatingSink sink = + new ValidatingSink(wellKnownTags, startTime, duration, content, shaCommit); + SerializingMetricWriter writer = + new SerializingMetricWriter(wellKnownTags, sink, 128, gitInfoProvider); + + // when + writer.startBucket(content.size(), startTime, duration); + for (AggregateEntry entryItem : content) { + writer.add(entryItem); + } + writer.finishBucket(); + + // then + assertTrue(sink.validatedInput()); + } + + @Test + void grpcStatusCodeFieldIsPresentInPayloadForRpcTypeSpans() { + // setup + long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()); + long duration = SECONDS.toNanos(10); + WellKnownTags wellKnownTags = + new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language"); + + AggregateEntry entryWithGrpc = + entry( + "grpc.service/Method", + "grpc-service", + "grpc.server", + null, + "rpc", + 0, + false, + false, + "server", + Collections.emptyList(), + null, + null, + "OK", + 1); + AggregateEntry entryWithGrpcError = + entry( + "grpc.service/Method", + "grpc-service", + "grpc.server", + null, + "rpc", + 0, + false, + false, + "client", + Collections.emptyList(), + null, + null, + "NOT_FOUND", + 1); + AggregateEntry entryWithoutGrpc = + entry( + "resource", + "service", + "operation", + null, + "web", + 200, + false, + false, + "server", + Collections.emptyList(), + null, + null, + null, + 1); + + List content = + Arrays.asList(entryWithGrpc, entryWithGrpcError, entryWithoutGrpc); + + ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content); + SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128); + + // when + writer.startBucket(content.size(), startTime, duration); + for (AggregateEntry aggregateEntry : content) { + writer.add(aggregateEntry); + } + writer.finishBucket(); + + // then + assertTrue(sink.validatedInput()); + } + + @Test + void additionalMetricTagsEmittedWhenSet() { + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(new LinkedHashSet<>(Arrays.asList("region", "tenant_id"))); + AggregateTable table = newTable(schema); + + table.findOrInsert(snapshot(schema, "us-east-1", "acme-corp")).recordOneDuration(1L); + + List content = contentOf(table); + assertEquals(1, content.size()); + // Both configured tags are packed in schema (alphabetical) order: region first, then + // tenant_id. + UTF8BytesString[] additionalTags = content.get(0).getAdditionalTags(); + assertEquals(2, additionalTags.length); + assertEquals("region:us-east-1", additionalTags[0].toString()); + assertEquals("tenant_id:acme-corp", additionalTags[1].toString()); + + // ValidatingSink re-checks the serialized AdditionalMetricTags array against the entry. + serializeAndValidate(content); + } + + @Test + void additionalMetricTagsEmittedAsEmptyArrayWhenConfiguredButNoneSet() { + // Schema configured, but the span doesn't set any of the configured tags. Per the Span-Derived + // Primary Tags RFC the field is still present (as an empty array) whenever the feature is on. + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(new LinkedHashSet<>(Arrays.asList("region"))); + AggregateTable table = newTable(schema); + + table.findOrInsert(snapshot(schema, new String[] {null})).recordOneDuration(1L); + + List content = contentOf(table); + assertEquals(1, content.size()); + // No slots populated -> empty packed array, but the field is still emitted since configured. + assertEquals(0, content.get(0).getAdditionalTags().length); + + serializeAndValidate(content); + } + + @Test + void additionalMetricTagsFieldOmittedWhenFeatureOff() { + // Feature off (no schema): the writer omits AdditionalMetricTags entirely so non-users pay no + // payload overhead. + long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()); + long duration = SECONDS.toNanos(10); + WellKnownTags wellKnownTags = + new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language"); + AggregateTable table = newTable(AdditionalTagsSchema.EMPTY); + table.findOrInsert(snapshot(AdditionalTagsSchema.EMPTY)).recordOneDuration(1L); + List content = contentOf(table); + assertEquals(1, content.size()); + assertEquals(0, content.get(0).getAdditionalTags().length); + + // Default (unconfigured) sink + writer: the field must be absent. + ValidatingSink sink = new ValidatingSink(wellKnownTags, startTime, duration, content); + SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, 128); + writer.startBucket(content.size(), startTime, duration); + for (AggregateEntry entry : content) { + writer.add(entry); + } + writer.finishBucket(); + assertTrue(sink.validatedInput()); + } + + @Test + void additionalMetricTagsSkipsNullSlots() { + AdditionalTagsSchema schema = + AdditionalTagsSchema.from(new LinkedHashSet<>(Arrays.asList("region", "tenant_id"))); + AggregateTable table = newTable(schema); + + // Set only tenant_id; leave region null. + table + .findOrInsert( + snapshot( + schema, + new String[] { + /*region*/ + null, /*tenant_id*/ "acme-corp" + })) + .recordOneDuration(1L); + + List content = contentOf(table); + assertEquals(1, content.size()); + // The null region slot is skipped; only tenant_id survives in the packed array. + UTF8BytesString[] additionalTags = content.get(0).getAdditionalTags(); + assertEquals(1, additionalTags.length); + assertEquals("tenant_id:acme-corp", additionalTags[0].toString()); + + serializeAndValidate(content); + } + + // ---------- additional-tags 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); + } + + /** + * Collects the table's entries in iteration order (the same order the writer serializes them). + */ + private static List contentOf(AggregateTable table) { + List content = new ArrayList<>(); + table.forEach(content::add); + return content; + } + + /** + * Serializes {@code content} through the shared {@link ValidatingSink} and asserts it decoded. + */ + private static void serializeAndValidate(List content) { + long startTime = MILLISECONDS.toNanos(System.currentTimeMillis()); + long duration = SECONDS.toNanos(10); + WellKnownTags wellKnownTags = + new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language"); + // These helpers exercise the configured-feature path, so the writer is built configured and the + // sink expects the AdditionalMetricTags field present (possibly empty) on every entry. + ValidatingSink sink = + new ValidatingSink(wellKnownTags, startTime, duration, content, null, true); + SerializingMetricWriter writer = new SerializingMetricWriter(wellKnownTags, sink, true); + + writer.startBucket(content.size(), startTime, duration); + for (AggregateEntry entry : content) { + writer.add(entry); + } + writer.finishBucket(); + + assertTrue(sink.validatedInput()); + } + + static final class ValidatingSink implements Sink { + + private final WellKnownTags wellKnownTags; + private final long startTimeNanos; + private final long duration; + private boolean validated = false; + private final List content; + private final String expectedGitCommitSha; + private final boolean additionalTagsConfigured; + + ValidatingSink( + WellKnownTags wellKnownTags, + long startTimeNanos, + long duration, + List content) { + this(wellKnownTags, startTimeNanos, duration, content, null); + } + + ValidatingSink( + WellKnownTags wellKnownTags, + long startTimeNanos, + long duration, + List content, + String expectedGitCommitSha) { + this(wellKnownTags, startTimeNanos, duration, content, expectedGitCommitSha, false); + } + + ValidatingSink( + WellKnownTags wellKnownTags, + long startTimeNanos, + long duration, + List content, + String expectedGitCommitSha, + boolean additionalTagsConfigured) { + this.wellKnownTags = wellKnownTags; + this.startTimeNanos = startTimeNanos; + this.duration = duration; + this.content = content; + this.expectedGitCommitSha = expectedGitCommitSha; + this.additionalTagsConfigured = additionalTagsConfigured; + } + + @Override + public void register(EventListener listener) {} + + @Override + public void accept(int messageCount, ByteBuffer buffer) { + // Sink.accept can't declare checked exceptions; surface any msgpack decode failure as an + // assertion error so the test reports it. + try { + validate(buffer); + } catch (IOException e) { + throw new AssertionError("Failed to decode metric payload", e); + } + } + + private void validate(ByteBuffer buffer) throws IOException { + MessageUnpacker unpacker = MessagePack.newDefaultUnpacker(buffer); + int mapSize = unpacker.unpackMapHeader(); + String gitCommitSha = expectedGitCommitSha; + assertEquals( + 7 + + (Config.get().isExperimentalPropagateProcessTagsEnabled() ? 1 : 0) + + (gitCommitSha != null ? 1 : 0), + mapSize); + assertEquals("RuntimeID", unpacker.unpackString()); + assertEquals(wellKnownTags.getRuntimeId().toString(), unpacker.unpackString()); + assertEquals("Sequence", unpacker.unpackString()); + assertEquals(0L, unpacker.unpackLong()); + assertEquals("Hostname", unpacker.unpackString()); + assertEquals(wellKnownTags.getHostname().toString(), unpacker.unpackString()); + assertEquals("Service", unpacker.unpackString()); + assertEquals(wellKnownTags.getService().toString(), unpacker.unpackString()); + assertEquals("Env", unpacker.unpackString()); + assertEquals(wellKnownTags.getEnv().toString(), unpacker.unpackString()); + assertEquals("Version", unpacker.unpackString()); + assertEquals(wellKnownTags.getVersion().toString(), unpacker.unpackString()); + if (Config.get().isExperimentalPropagateProcessTagsEnabled()) { + assertEquals("ProcessTags", unpacker.unpackString()); + assertEquals(ProcessTags.getTagsForSerialization().toString(), unpacker.unpackString()); + } + if (gitCommitSha != null) { + assertEquals("GitCommitSha", unpacker.unpackString()); + assertEquals(gitCommitSha, unpacker.unpackString()); + } + assertEquals("Stats", unpacker.unpackString()); + int outerLength = unpacker.unpackArrayHeader(); + assertEquals(1, outerLength); + assertEquals(3, unpacker.unpackMapHeader()); + assertEquals("Start", unpacker.unpackString()); + assertEquals(startTimeNanos, unpacker.unpackLong()); + assertEquals("Duration", unpacker.unpackString()); + assertEquals(duration, unpacker.unpackLong()); + assertEquals("Stats", unpacker.unpackString()); + int statCount = unpacker.unpackArrayHeader(); + assertEquals(content.size(), statCount); + for (AggregateEntry entry : content) { + // counters now live on AggregateEntry + int metricMapSize = unpacker.unpackMapHeader(); + // Calculate expected map size based on optional fields + boolean hasHttpMethod = entry.hasHttpMethod(); + boolean hasHttpEndpoint = entry.hasHttpEndpoint(); + boolean hasServiceSource = entry.hasServiceSource(); + boolean hasGrpcStatusCode = entry.hasGrpcStatusCode(); + UTF8BytesString[] additionalTags = entry.getAdditionalTags(); + int expectedMapSize = + 15 + + (hasServiceSource ? 1 : 0) + + (hasHttpMethod ? 1 : 0) + + (hasHttpEndpoint ? 1 : 0) + + (hasGrpcStatusCode ? 1 : 0) + + (additionalTagsConfigured ? 1 : 0); + assertEquals(expectedMapSize, metricMapSize); + int elementCount = 0; + assertEquals("Name", unpacker.unpackString()); + assertEquals(entry.getOperationName().toString(), unpacker.unpackString()); + ++elementCount; + assertEquals("Service", unpacker.unpackString()); + assertEquals(entry.getService().toString(), unpacker.unpackString()); + ++elementCount; + assertEquals("Resource", unpacker.unpackString()); + assertEquals(entry.getResource().toString(), unpacker.unpackString()); + ++elementCount; + assertEquals("Type", unpacker.unpackString()); + assertEquals(entry.getType().toString(), unpacker.unpackString()); + ++elementCount; + assertEquals("HTTPStatusCode", unpacker.unpackString()); + assertEquals(entry.getHttpStatusCode(), unpacker.unpackInt()); + ++elementCount; + assertEquals("Synthetics", unpacker.unpackString()); + assertEquals(entry.isSynthetics(), unpacker.unpackBoolean()); + ++elementCount; + assertEquals("IsTraceRoot", unpacker.unpackString()); + assertEquals( + entry.isTraceRoot() ? TriState.TRUE.serialValue : TriState.FALSE.serialValue, + unpacker.unpackInt()); + ++elementCount; + assertEquals("SpanKind", unpacker.unpackString()); + assertEquals(entry.getSpanKind().toString(), unpacker.unpackString()); + ++elementCount; + assertEquals("PeerTags", unpacker.unpackString()); + int peerTagsLength = unpacker.unpackArrayHeader(); + assertEquals(entry.getPeerTags().size(), peerTagsLength); + for (int i = 0; i < peerTagsLength; i++) { + String unpackedPeerTag = unpacker.unpackString(); + assertEquals(entry.getPeerTags().get(i).toString(), unpackedPeerTag); + } + ++elementCount; + // AdditionalMetricTags is a packed array of pre-built "key:value" strings in schema + // (alphabetical-by-key) order. Emitted immediately after PeerTags whenever the feature is + // configured -- as an empty array for entries that matched no key -- and omitted entirely + // when the feature is off, mirroring the writer's field handling above. + if (additionalTagsConfigured) { + assertEquals("AdditionalMetricTags", unpacker.unpackString()); + int additionalTagsLength = unpacker.unpackArrayHeader(); + assertEquals(additionalTags.length, additionalTagsLength); + for (int i = 0; i < additionalTagsLength; i++) { + assertEquals(additionalTags[i].toString(), unpacker.unpackString()); + } + ++elementCount; + } + // Service source is only present when the service name has been overridden by the tracer + if (hasServiceSource) { + assertEquals("srv_src", unpacker.unpackString()); + assertEquals(entry.getServiceSource().toString(), unpacker.unpackString()); + ++elementCount; + } + // HTTPMethod and HTTPEndpoint are optional - only present if non-null + if (hasHttpMethod) { + assertEquals("HTTPMethod", unpacker.unpackString()); + assertEquals(entry.getHttpMethod().toString(), unpacker.unpackString()); + ++elementCount; + } + if (hasHttpEndpoint) { + assertEquals("HTTPEndpoint", unpacker.unpackString()); + assertEquals(entry.getHttpEndpoint().toString(), unpacker.unpackString()); + ++elementCount; + } + if (hasGrpcStatusCode) { + assertEquals("GRPCStatusCode", unpacker.unpackString()); + assertEquals(entry.getGrpcStatusCode().toString(), unpacker.unpackString()); + ++elementCount; + } + assertEquals("Hits", unpacker.unpackString()); + assertEquals(entry.getHitCount(), unpacker.unpackInt()); + ++elementCount; + assertEquals("Errors", unpacker.unpackString()); + assertEquals(entry.getErrorCount(), unpacker.unpackInt()); + ++elementCount; + assertEquals("TopLevelHits", unpacker.unpackString()); + assertEquals(entry.getTopLevelCount(), unpacker.unpackInt()); + ++elementCount; + assertEquals("Duration", unpacker.unpackString()); + assertEquals(entry.getDuration(), unpacker.unpackLong()); + ++elementCount; + assertEquals("OkSummary", unpacker.unpackString()); + validateSketch(unpacker); + ++elementCount; + assertEquals("ErrorSummary", unpacker.unpackString()); + validateSketch(unpacker); + ++elementCount; + assertEquals(metricMapSize, elementCount); + } + validated = true; + } + + private void validateSketch(MessageUnpacker unpacker) throws IOException { + int length = unpacker.unpackBinaryHeader(); + assertTrue(length > 0); + unpacker.readPayload(length); + } + + boolean validatedInput() { + return validated; + } + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java b/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java index 670f9098a14..600d712501e 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/otlp/metrics/OtlpStatsMetricWriterTest.java @@ -12,6 +12,7 @@ import com.google.protobuf.WireFormat; import datadog.metrics.api.Histograms; import datadog.metrics.impl.DDSketchHistograms; +import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.common.metrics.AggregateEntry; import datadog.trace.common.metrics.AggregateEntryTestUtils; import datadog.trace.core.otlp.common.OtlpPayload; @@ -424,6 +425,100 @@ void httpAndGrpcAttributesAppearOnlyWhenSet() throws IOException { assertFalse(bareAttrs.containsKey("rpc.response.status_code")); } + @Test + void additionalMetricTagsEmittedAsStringAttributes() throws IOException { + // Additional tags arrive on the entry pre-packed as "key:value" UTF8 strings in schema order; + // the writer splits each at the first ':' and emits it as a plain OTLP string attribute keyed + // by the tag name, in both semantics modes. + AggregateEntry e = + AggregateEntryTestUtils.of( + "GET /users", + "web", + "servlet.request", + null, + "web", + 0, + false, + true, + "server", + null, + null, + null, + null, + new UTF8BytesString[] { + UTF8BytesString.create("region:us-east-1"), + UTF8BytesString.create("tenant_id:acme:corp") + }); + AggregateEntryTestUtils.recordOk(e, SECONDS.toNanos(1)); + + Map attrs = writeAndDecode(false, e).dataPoints.get(0).attributes; + assertEquals("us-east-1", attrs.get("region")); + // value may itself contain ':' — only the first ':' separates key from value + assertEquals("acme:corp", attrs.get("tenant_id")); + } + + @Test + void additionalMetricTagsEmittedInOtelSemanticsMode() throws IOException { + // Unlike datadog.* attributes, additional tags are user-configured dimensions and are emitted + // in otel-semantics mode too. + AggregateEntry e = + AggregateEntryTestUtils.of( + "GET /users", + "web", + "servlet.request", + null, + "web", + 0, + false, + true, + "server", + null, + null, + null, + null, + new UTF8BytesString[] {UTF8BytesString.create("region:us-east-1")}); + AggregateEntryTestUtils.recordOk(e, SECONDS.toNanos(1)); + + Map attrs = writeAndDecode(true, e).dataPoints.get(0).attributes; + assertEquals("us-east-1", attrs.get("region")); + assertFalse( + attrs.containsKey("datadog.operation.name"), "datadog.* still absent in otel-semantics"); + } + + @Test + void malformedAdditionalTagsAreSkipped() throws IOException { + // Defensive: a slot with no ':', an empty key, or an empty value is dropped rather than emitted + // as a malformed attribute. + AggregateEntry e = + AggregateEntryTestUtils.of( + "GET /users", + "web", + "servlet.request", + null, + "web", + 0, + false, + true, + "server", + null, + null, + null, + null, + new UTF8BytesString[] { + UTF8BytesString.create("noseparator"), + UTF8BytesString.create(":emptykey"), + UTF8BytesString.create("emptyvalue:"), + UTF8BytesString.create("region:us-east-1") + }); + AggregateEntryTestUtils.recordOk(e, SECONDS.toNanos(1)); + + Map attrs = writeAndDecode(false, e).dataPoints.get(0).attributes; + assertEquals("us-east-1", attrs.get("region"), "well-formed tag still emitted"); + assertFalse(attrs.containsKey("noseparator"), "no-separator slot skipped"); + assertFalse(attrs.containsKey(""), "empty-key slot skipped"); + assertFalse(attrs.containsKey("emptyvalue"), "empty-value slot skipped"); + } + @Test void serviceNameEmittedOnlyForNonDefaultService() throws IOException { CapturingSender sender = new CapturingSender(); 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 541bf9080fc..607512427ae 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 7d6df22df1c..41023a866a6 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -429,6 +429,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; @@ -3900,9 +3901,23 @@ public int getTraceStatsInterval() { * pattern {@code DD_TRACE_STATS_{tagName}_CARDINALITY_LIMIT} (e.g. {@code * DD_TRACE_STATS_RESOURCE_CARDINALITY_LIMIT}). The caller supplies the default from {@code * MetricCardinalityLimits} so per-field rationale stays co-located with the defaults. + * + *

A non-positive configured value is invalid -- each limit sizes a fixed-capacity handler + * table that requires a positive size -- so it falls back to {@code defaultLimit}, logged at + * debug. */ public int getTraceStatsCardinalityLimit(String tagName, int defaultLimit) { - return configProvider.getInteger("trace.stats." + tagName + ".cardinality.limit", defaultLimit); + int limit = + configProvider.getInteger("trace.stats." + tagName + ".cardinality.limit", defaultLimit); + if (limit <= 0) { + log.debug( + "Invalid trace.stats.{}.cardinality.limit={}; must be positive. Using default {}.", + tagName, + limit, + defaultLimit); + return defaultLimit; + } + return limit; } public boolean isLogsInjectionEnabled() { @@ -5250,6 +5265,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/internal-api/src/main/java/datadog/trace/util/Hashtable.java b/internal-api/src/main/java/datadog/trace/util/Hashtable.java index 84db05b9f01..cc7bc4eddf5 100644 --- a/internal-api/src/main/java/datadog/trace/util/Hashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/Hashtable.java @@ -99,6 +99,11 @@ protected Entry(K key) { this.key = key; } + /** The key this entry was created with. */ + public K key() { + return this.key; + } + public boolean matches(Object key) { return Objects.equals(this.key, key); } diff --git a/internal-api/src/main/java/datadog/trace/util/LongHashingUtils.java b/internal-api/src/main/java/datadog/trace/util/LongHashingUtils.java index 88104baa8d8..67e88d7f3d6 100644 --- a/internal-api/src/main/java/datadog/trace/util/LongHashingUtils.java +++ b/internal-api/src/main/java/datadog/trace/util/LongHashingUtils.java @@ -138,6 +138,17 @@ public static final long addToHash(long hash, double value) { return addToHash(hash, Double.hashCode(value)); } + public static final long addToHash(long hash, Object[] arr, int len) { + for (int i = 0; i < len; i++) { + hash = addToHash(hash, arr[i]); + } + return hash; + } + + public static final long addToHash(long hash, Object[] arr) { + return addToHash(hash, arr, arr.length); + } + public static final long hash(Iterable objs) { long result = 0; for (Object obj : objs) { diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 11f100efc42..47b823e2ec2 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -2253,7 +2253,12 @@ class ConfigTest extends DDSpecification { def config = new Config() then: - config.experimentalFeaturesEnabled == ["DD_TAGS", "DD_LOGS_INJECTION", "DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED"].toSet() + config.experimentalFeaturesEnabled == [ + "DD_TAGS", + "DD_LOGS_INJECTION", + "DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED", + "DD_TRACE_STATS_ADDITIONAL_TAGS" + ].toSet() } def "detect if agent is configured using default values"() { diff --git a/internal-api/src/test/java/datadog/trace/api/TraceStatsCardinalityLimitTest.java b/internal-api/src/test/java/datadog/trace/api/TraceStatsCardinalityLimitTest.java new file mode 100644 index 00000000000..ccffcbececd --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/TraceStatsCardinalityLimitTest.java @@ -0,0 +1,32 @@ +package datadog.trace.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.Properties; +import org.junit.jupiter.api.Test; + +class TraceStatsCardinalityLimitTest { + + private static final int DEFAULT = 1024; + + private static Config configWith(String limit) { + Properties props = new Properties(); + props.setProperty("trace.stats.resource.cardinality.limit", limit); + return Config.get(props); + } + + @Test + void positiveValueIsUsed() { + assertEquals(256, configWith("256").getTraceStatsCardinalityLimit("resource", DEFAULT)); + } + + @Test + void zeroFallsBackToDefault() { + assertEquals(DEFAULT, configWith("0").getTraceStatsCardinalityLimit("resource", DEFAULT)); + } + + @Test + void negativeFallsBackToDefault() { + assertEquals(DEFAULT, configWith("-5").getTraceStatsCardinalityLimit("resource", DEFAULT)); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java b/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java index 11cf93fc1dd..a3cd4c25247 100644 --- a/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java +++ b/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java @@ -30,6 +30,12 @@ void insertedEntryIsRetrievable() { assertSame(e, table.get("foo")); } + @Test + void keyExposesTheConstructionKey() { + StringIntEntry e = new StringIntEntry("foo", 1); + assertEquals("foo", e.key()); + } + @Test void multipleInsertsRetrievableSeparately() { Hashtable.D1 table = new Hashtable.D1<>(16); diff --git a/internal-api/src/test/java/datadog/trace/util/LongHashingUtilsTest.java b/internal-api/src/test/java/datadog/trace/util/LongHashingUtilsTest.java index 795c182df18..2837fa9ff73 100644 --- a/internal-api/src/test/java/datadog/trace/util/LongHashingUtilsTest.java +++ b/internal-api/src/test/java/datadog/trace/util/LongHashingUtilsTest.java @@ -147,6 +147,39 @@ void deprecatedObjectArrayHashMatchesChainedAddToHash() { assertEquals(expected, hash(objs)); } + @Test + void addToHashArrayFoldsFromSeedLikeChainedAddToHash() { + Object[] objs = new Object[] {"alpha", 7, null, true}; + + // Full-array overload folds every element onto the seed, matching an explicit chain. + long fromZero = 0L; + for (Object o : objs) { + fromZero = addToHash(fromZero, o); + } + assertEquals(fromZero, addToHash(0L, objs)); + + // A non-zero seed carries through, so the result differs from the zero-seed fold. + long fromSeed = 100L; + for (Object o : objs) { + fromSeed = addToHash(fromSeed, o); + } + assertEquals(fromSeed, addToHash(100L, objs)); + assertNotEquals(addToHash(0L, objs), addToHash(100L, objs)); + } + + @Test + void addToHashArrayRespectsLen() { + Object[] objs = new Object[] {"alpha", 7, null, true}; + + // The len override folds only the first len elements. + long firstTwo = addToHash(addToHash(0L, objs[0]), objs[1]); + assertEquals(firstTwo, addToHash(0L, objs, 2)); + assertNotEquals(addToHash(0L, objs), addToHash(0L, objs, 2)); + + // len==0 never enters the loop and returns the seed unchanged. + assertEquals(42L, addToHash(42L, objs, 0)); + } + // ----- intHash null behavior is observable via multi-arg overloads ----- @Test diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index d6b02fbfa28..966447b5cd5 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -10753,6 +10753,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",