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 5bc985491de..4d35de40e43 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 @@ -80,6 +80,9 @@ final class AggregateEntry extends Hashtable.Entry { DDCaches.newFixedSizeCache(32); private static final DDCache GRPC_STATUS_CODE_CACHE = DDCaches.newFixedSizeCache(32); + // Origin is a small fixed vocabulary (synthetics, synthetics-browser, rum, ciapp-test, lambda). + private static final DDCache ORIGIN_CACHE = + DDCaches.newFixedSizeCache(8); /** * Outer cache keyed by peer-tag name, with an inner per-name cache keyed by value. The inner @@ -107,9 +110,7 @@ final class AggregateEntry extends Hashtable.Entry { @Nullable private final UTF8BytesString httpEndpoint; @Nullable private final UTF8BytesString grpcStatusCode; private final short httpStatusCode; - - /** Whether the root span carried the {@code synthetics} origin tag (synthetic-monitoring run). */ - private final boolean synthetic; + @Nullable private final UTF8BytesString origin; /** Whether this span is the trace root ({@code parentId == 0}). */ private final boolean traceRoot; @@ -139,7 +140,8 @@ final class AggregateEntry extends Hashtable.Entry { private int errorCount; private int hitCount; private int topLevelCount; - private long duration; + private long okDuration; + private long errorDuration; /** Hot-path constructor for the producer/consumer flow. Builds UTF8 fields via the caches. */ AggregateEntry(SpanSnapshot s, long keyHash) { @@ -154,7 +156,7 @@ final class AggregateEntry extends Hashtable.Entry { this.httpEndpoint = canonicalizeOptional(HTTP_ENDPOINT_CACHE, s.httpEndpoint); this.grpcStatusCode = canonicalizeOptional(GRPC_STATUS_CODE_CACHE, s.grpcStatusCode); this.httpStatusCode = s.httpStatusCode; - this.synthetic = s.synthetic; + this.origin = canonicalizeOptional(ORIGIN_CACHE, s.origin); this.traceRoot = s.traceRoot; this.peerTagNames = s.peerTagSchema == null ? null : s.peerTagSchema.names; this.peerTagValues = s.peerTagValues; @@ -174,11 +176,12 @@ void recordOneDuration(long tagAndDuration) { if ((tagAndDuration & ERROR_TAG) == ERROR_TAG) { tagAndDuration ^= ERROR_TAG; errorLatenciesForWrite().accept(tagAndDuration); + errorDuration += tagAndDuration; ++errorCount; } else { okLatencies.accept(tagAndDuration); + okDuration += tagAndDuration; } - duration += tagAndDuration; } int getErrorCount() { @@ -194,7 +197,15 @@ int getTopLevelCount() { } long getDuration() { - return duration; + return okDuration + errorDuration; + } + + long getOkDuration() { + return okDuration; + } + + long getErrorDuration() { + return errorDuration; } Histogram getOkLatencies() { @@ -232,7 +243,8 @@ void clear() { this.errorCount = 0; this.hitCount = 0; this.topLevelCount = 0; - this.duration = 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) { @@ -243,7 +255,7 @@ void clear() { boolean matches(SpanSnapshot s) { String[] snapshotNames = s.peerTagSchema == null ? null : s.peerTagSchema.names; return httpStatusCode == s.httpStatusCode - && synthetic == s.synthetic + && contentEquals(origin, s.origin) && traceRoot == s.traceRoot && contentEquals(resource, s.resourceName) && contentEquals(service, s.serviceName) @@ -284,7 +296,7 @@ static long hashOf(SpanSnapshot s) { h = LongHashingUtils.addToHash(h, s.serviceNameSource); h = LongHashingUtils.addToHash(h, s.spanType); h = LongHashingUtils.addToHash(h, s.httpStatusCode); - h = LongHashingUtils.addToHash(h, s.synthetic); + h = LongHashingUtils.addToHash(h, s.origin); h = LongHashingUtils.addToHash(h, s.traceRoot); h = LongHashingUtils.addToHash(h, s.spanKind); // Always mix in both the schema's content hash and the values' content hash, unconditionally @@ -352,8 +364,21 @@ int getHttpStatusCode() { return httpStatusCode; } + /** + * The full trace origin, or {@code null} when unset. Used by {@link OtlpStatsMetricWriter} to + * emit {@code datadog.origin}. + */ + @Nullable + UTF8BytesString getOrigin() { + return origin; + } + + /** + * Whether the origin is {@code synthetics}. Derived from {@link #origin} for the native msgpack + * writer, which emits a synthetics boolean rather than the full origin. + */ boolean isSynthetics() { - return synthetic; + return origin != null && "synthetics".contentEquals(origin); } boolean isTraceRoot() { diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java index 895ee434854..594d3e6c4c0 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java @@ -47,8 +47,6 @@ public final class ConflatingMetricsAggregator implements MetricsAggregator, Eve private static final Map DEFAULT_HEADERS = Collections.singletonMap(DDAgentApi.DATADOG_META_TRACER_VERSION, DDTraceCoreInfo.VERSION); - private static final CharSequence SYNTHETICS_ORIGIN = "synthetics"; - private static final SpanKindFilter METRICS_ELIGIBLE_KINDS = SpanKindFilter.builder() .includeServer() @@ -346,7 +344,7 @@ private boolean publish(CoreSpan span, boolean isTopLevel) { span.getServiceNameSource(), spanType, span.getHttpStatusCode(), - isSynthetic(span), + span.getOrigin(), span.getParentId() == 0, spanKind, peerTagSchema, @@ -466,10 +464,6 @@ private static String[] capturePeerTagValues(CoreSpan span, PeerTagSchema sch return values; } - private static boolean isSynthetic(CoreSpan span) { - return span.getOrigin() != null && SYNTHETICS_ORIGIN.equals(span.getOrigin().toString()); - } - public void stop() { if (null != cancellation) { cancellation.cancel(); 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 152ac42bb55..aebb037f977 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 @@ -16,7 +16,8 @@ final class SpanSnapshot implements InboxItem { final CharSequence serviceNameSource; final CharSequence spanType; final short httpStatusCode; - final boolean synthetic; + final CharSequence origin; + final boolean traceRoot; final String spanKind; @@ -48,7 +49,7 @@ final class SpanSnapshot implements InboxItem { CharSequence serviceNameSource, CharSequence spanType, short httpStatusCode, - boolean synthetic, + CharSequence origin, boolean traceRoot, String spanKind, PeerTagSchema peerTagSchema, @@ -63,7 +64,7 @@ final class SpanSnapshot implements InboxItem { this.serviceNameSource = serviceNameSource; this.spanType = spanType; this.httpStatusCode = httpStatusCode; - this.synthetic = synthetic; + this.origin = origin; this.traceRoot = traceRoot; this.spanKind = spanKind; this.peerTagSchema = peerTagSchema; diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/ConflatingMetricAggregatorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/ConflatingMetricAggregatorTest.groovy index 00bd706b8fb..38d44863196 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/ConflatingMetricAggregatorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/ConflatingMetricAggregatorTest.groovy @@ -127,7 +127,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "baz", [], @@ -175,7 +175,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "baz", [], @@ -229,7 +229,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, kind, [], @@ -308,7 +308,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "client", [UTF8BytesString.create("country:france")], @@ -328,7 +328,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "client", [UTF8BytesString.create("country:france"), UTF8BytesString.create("georegion:europe")], @@ -377,7 +377,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, kind, expectedPeerTags, @@ -431,7 +431,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "baz", [], @@ -492,7 +492,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "baz", [], @@ -511,7 +511,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "baz", [], @@ -567,7 +567,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "server", [], @@ -610,7 +610,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "server", [], @@ -629,7 +629,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "server", [], @@ -648,7 +648,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "server", [], @@ -714,7 +714,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", 200, - false, + null, false, "server", [], @@ -733,7 +733,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", 200, - false, + null, false, "server", [], @@ -752,7 +752,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", 404, - false, + null, false, "server", [], @@ -771,7 +771,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", 200, - false, + null, false, "server", [], @@ -826,7 +826,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", 200, - false, + null, false, "server", [], @@ -845,7 +845,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", 200, - false, + null, false, "server", [], @@ -898,7 +898,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { "source", "type", 200, - false, + null, false, "server", [], @@ -917,7 +917,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", 200, - false, + null, false, "server", [], @@ -972,7 +972,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "baz", [], @@ -991,7 +991,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "baz", [], @@ -1121,7 +1121,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "baz", [], @@ -1156,7 +1156,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "baz", [], @@ -1175,7 +1175,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "baz", [], @@ -1225,7 +1225,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "quux", [], @@ -1284,7 +1284,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, true, "garply", [], @@ -1452,7 +1452,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, true, "", [], @@ -1509,7 +1509,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "server", [], @@ -1566,7 +1566,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "server", [], @@ -1586,7 +1586,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "server", [], @@ -1606,7 +1606,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "type", HTTP_OK, - false, + null, false, "server", [], @@ -1660,7 +1660,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "rpc", 0, - false, + null, false, "server", [], @@ -1677,7 +1677,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "rpc", 0, - false, + null, false, "server", [], @@ -1694,7 +1694,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification { null, "web", 200, - false, + null, false, "server", [], 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 index cc0880bc30a..a5f0d28b6d1 100644 --- 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 @@ -32,7 +32,7 @@ class SerializingMetricWriterTest extends DDSpecification { CharSequence serviceSource, CharSequence type, int httpStatusCode, - boolean synthetic, + CharSequence origin, boolean traceRoot, CharSequence spanKind, List peerTags, @@ -42,7 +42,7 @@ class SerializingMetricWriterTest extends DDSpecification { int hitCount) { AggregateEntry e = AggregateEntryTestUtils.of( resource, service, operationName, serviceSource, type, - httpStatusCode, synthetic, traceRoot, spanKind, peerTags, + httpStatusCode, origin, traceRoot, spanKind, peerTags, httpMethod, httpEndpoint, grpcStatusCode) hitCount.times { e.recordOneDuration(1L) } return e @@ -79,7 +79,7 @@ class SerializingMetricWriterTest extends DDSpecification { [ entry( "resource1", "service1", "operation1", null, "type", 0, - false, false, "client", + null, false, "client", [ UTF8BytesString.create("country:canada"), UTF8BytesString.create("georegion:amer"), @@ -89,7 +89,7 @@ class SerializingMetricWriterTest extends DDSpecification { 10), entry( "resource2", "service2", "operation2", null, "type2", 200, - true, false, "producer", + "synthetics", false, "producer", [ UTF8BytesString.create("country:canada"), UTF8BytesString.create("georegion:amer"), @@ -99,7 +99,7 @@ class SerializingMetricWriterTest extends DDSpecification { 9), entry( "GET /api/users/:id", "web-service", "http.request", null, "web", 200, - false, true, "server", + null, true, "server", [], null, null, null, 5) @@ -107,7 +107,7 @@ class SerializingMetricWriterTest extends DDSpecification { (0..10000).collect({ i -> entry( "resource" + i, "service" + i, "operation" + i, null, "type", 0, - false, false, "producer", + null, false, "producer", [UTF8BytesString.create("messaging.destination:dest" + i)], null, null, null, 10) @@ -122,8 +122,8 @@ class SerializingMetricWriterTest extends DDSpecification { 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 entryNoSource = entry("resource", "service", "operation", null, "type", 200, null, false, "server", [], "GET", "/api/users", null, 1) + def entryWithSource = entry("resource", "service", "operation", "source", "type", 200, null, false, "server", [], "POST", null, null, 1) def content = [entryNoSource, entryWithSource] @@ -147,10 +147,10 @@ class SerializingMetricWriterTest extends DDSpecification { 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 entryWithBoth = entry("resource", "service", "operation", null, "type", 200, null, false, "server", [], "GET", "/api/users", null, 1) + def entryWithMethodOnly = entry("resource", "service", "operation", null, "type", 200, null, false, "server", [], "POST", null, null, 1) + def entryWithEndpointOnly = entry("resource", "service", "operation", null, "type", 200, null, false, "server", [], null, "/api/orders", null, 1) + def entryWithNeither = entry("resource", "service", "operation", null, "type", 200, null, false, "client", [], null, null, null, 1) def content = [entryWithBoth, entryWithMethodOnly, entryWithEndpointOnly, entryWithNeither] @@ -177,7 +177,7 @@ class SerializingMetricWriterTest extends DDSpecification { 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 e = entry("resource", "service", "operation", null, "type", 200, null, false, "server", [], "GET", "/api/users", null, 1) def content = [e] @@ -204,9 +204,9 @@ class SerializingMetricWriterTest extends DDSpecification { 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 entryWithGrpc = entry("grpc.service/Method", "grpc-service", "grpc.server", null, "rpc", 0, null, false, "server", [], null, null, "OK", 1) + def entryWithGrpcError = entry("grpc.service/Method", "grpc-service", "grpc.server", null, "rpc", 0, null, false, "client", [], null, null, "NOT_FOUND", 1) + def entryWithoutGrpc = entry("resource", "service", "operation", null, "web", 200, null, false, "server", [], null, null, null, 1) def content = [entryWithGrpc, entryWithGrpcError, entryWithoutGrpc] 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 7fd767533c7..3d109215063 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 @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import datadog.metrics.agent.AgentMeter; @@ -123,26 +124,37 @@ void testUtilsEqualEntriesHaveEqualHashCodes() { assertEquals(AggregateEntryTestUtils.hashCode(a), AggregateEntryTestUtils.hashCode(b)); } - private static SpanSnapshot snapshotWithPeerTags(String[] names, String[] values) { - return new SpanSnapshot( - "resource", - "svc", - "op", - null, - "type", - (short) 200, - false, - true, - "client", - PeerTagSchema.testSchema(names), - values, - null, - null, - null, - 0L); + @Test + void getOriginCarriesFullOriginAndIsSyntheticsIsDerived() { + // The full origin is preserved on the entry; the native msgpack writer reads the derived + // isSynthetics() boolean, which must be true only for the exact "synthetics" origin. + AggregateEntry synthetics = entryWithOrigin("synthetics"); + assertEquals("synthetics", synthetics.getOrigin().toString()); + assertTrue(synthetics.isSynthetics()); + + // A different origin is carried verbatim but is not synthetics. + AggregateEntry rum = entryWithOrigin("rum"); + assertEquals("rum", rum.getOrigin().toString()); + assertFalse(rum.isSynthetics()); + + AggregateEntry ciapp = entryWithOrigin("ciapp-test"); + assertEquals("ciapp-test", ciapp.getOrigin().toString()); + assertFalse(ciapp.isSynthetics()); + + // synthetics-browser is a distinct origin -- a prefix match must not count as synthetics. + AggregateEntry browser = entryWithOrigin("synthetics-browser"); + assertEquals("synthetics-browser", browser.getOrigin().toString()); + assertFalse(browser.isSynthetics()); } - private static AggregateEntry newEntry() { + @Test + void noOriginIsNullAndNotSynthetics() { + AggregateEntry entry = entryWithOrigin(null); + assertNull(entry.getOrigin()); + assertFalse(entry.isSynthetics()); + } + + private static AggregateEntry entryWithOrigin(String origin) { SpanSnapshot snapshot = new SpanSnapshot( "resource", @@ -151,7 +163,7 @@ private static AggregateEntry newEntry() { null, "type", (short) 200, - false, + origin, true, "client", null, @@ -162,4 +174,27 @@ private static AggregateEntry newEntry() { 0L); return AggregateEntryTestUtils.forSnapshot(snapshot); } + + private static SpanSnapshot snapshotWithPeerTags(String[] names, String[] values) { + return new SpanSnapshot( + "resource", + "svc", + "op", + null, + "type", + (short) 200, + null, + true, + "client", + PeerTagSchema.testSchema(names), + values, + null, + null, + null, + 0L); + } + + private static AggregateEntry newEntry() { + return entryWithOrigin(null); + } } 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 ed6fd5a3a7e..bc189f7b60d 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 @@ -46,7 +46,7 @@ public static AggregateEntry of( @Nullable CharSequence serviceSource, CharSequence type, int httpStatusCode, - boolean synthetic, + @Nullable CharSequence origin, boolean traceRoot, CharSequence spanKind, @Nullable List peerTags, @@ -68,7 +68,7 @@ public static AggregateEntry of( } schema = PeerTagSchema.testSchema(names); } - SpanSnapshot syntheticSnapshot = + SpanSnapshot snapshot = new SpanSnapshot( resource, service == null ? null : service.toString(), @@ -76,7 +76,7 @@ public static AggregateEntry of( serviceSource, type, (short) httpStatusCode, - synthetic, + origin, traceRoot, spanKind == null ? null : spanKind.toString(), schema, @@ -85,7 +85,7 @@ public static AggregateEntry of( httpEndpoint == null ? null : httpEndpoint.toString(), grpcStatusCode == null ? null : grpcStatusCode.toString(), 0L); - return forSnapshot(syntheticSnapshot); + return forSnapshot(snapshot); } /** @@ -106,7 +106,7 @@ public static boolean equals(AggregateEntry a, AggregateEntry b) { if (a == b) return true; if (a == null || b == null) return false; return a.getHttpStatusCode() == b.getHttpStatusCode() - && a.isSynthetics() == b.isSynthetics() + && Objects.equals(a.getOrigin(), b.getOrigin()) && a.isTraceRoot() == b.isTraceRoot() && Objects.equals(a.getResource(), b.getResource()) && Objects.equals(a.getService(), b.getService()) 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 618ead2ab43..2118d3f7dfb 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 @@ -86,6 +86,40 @@ void peerTagPairsParticipateInIdentity() { assertEquals(3, table.size()); } + @Test + void distinctOriginsAggregateSeparately() { + // Trace origin is part of the bucket key (AggregateEntry.matches/hashOf), so spans that + // differ only by origin must land on separate entries + AggregateTable table = new AggregateTable(8); + + AggregateEntry none = table.findOrInsert(builder("svc", "op", "server").build()); + AggregateEntry synthetics = + table.findOrInsert(builder("svc", "op", "server").origin("synthetics").build()); + AggregateEntry rum = table.findOrInsert(builder("svc", "op", "server").origin("rum").build()); + AggregateEntry ciapp = + table.findOrInsert(builder("svc", "op", "server").origin("ciapp-test").build()); + + assertNotSame(none, synthetics); + assertNotSame(none, rum); + assertNotSame(synthetics, rum); + assertNotSame(rum, ciapp); + assertEquals(4, table.size()); + } + + @Test + void sameOriginHitsSameEntry() { + // The inverse of distinctOriginsAggregateSeparately: two snapshots with an identical + // non-null origin must conflate onto one entry. + AggregateTable table = new AggregateTable(8); + + AggregateEntry first = table.findOrInsert(builder("svc", "op", "server").origin("rum").build()); + AggregateEntry second = + table.findOrInsert(builder("svc", "op", "server").origin("rum").build()); + + assertSame(first, second); + assertEquals(1, table.size()); + } + @Test void capOverrunEvictsStaleEntry() { AggregateTable table = new AggregateTable(2); @@ -274,7 +308,7 @@ private static SpanSnapshot nullServiceKindSnapshot(String service, String spanK null, "web", (short) 200, - false, + null, true, spanKind, null, @@ -294,7 +328,7 @@ private static SpanSnapshot nullableSnapshot( serviceNameSource, type, (short) 200, - false, + null, true, "client", null, @@ -321,6 +355,7 @@ private static final class SnapshotBuilder { private final String spanKind; private PeerTagSchema peerTagSchema; private String[] peerTagValues; + private String origin; private long tagAndDuration = 0L; SnapshotBuilder(String service, String operation, String spanKind) { @@ -329,6 +364,11 @@ private static final class SnapshotBuilder { this.spanKind = spanKind; } + SnapshotBuilder origin(String origin) { + this.origin = origin; + return this; + } + SnapshotBuilder peerTags(String... namesAndValues) { int pairCount = namesAndValues.length / 2; String[] names = new String[pairCount]; @@ -350,7 +390,7 @@ SpanSnapshot build() { null, "web", (short) 200, - false, + origin, true, spanKind, peerTagSchema, diff --git a/dd-trace-core/src/traceAgentTest/groovy/datadog/trace/common/metrics/MetricsIntegrationTest.groovy b/dd-trace-core/src/traceAgentTest/groovy/datadog/trace/common/metrics/MetricsIntegrationTest.groovy index 401aaef4b7e..de436ca97e3 100644 --- a/dd-trace-core/src/traceAgentTest/groovy/datadog/trace/common/metrics/MetricsIntegrationTest.groovy +++ b/dd-trace-core/src/traceAgentTest/groovy/datadog/trace/common/metrics/MetricsIntegrationTest.groovy @@ -40,13 +40,13 @@ class MetricsIntegrationTest extends AbstractTraceAgentTest { PeerTagSchema schema = PeerTagSchema.testSchema(["grault"] as String[]) SpanSnapshot snap1 = new SpanSnapshot( "resource1", "service1", "operation1", null, "sql", (short) 0, - false, true, "xyzzy", schema, ["quux"] as String[], null, null, null, 0L) + (CharSequence) null, true, "xyzzy", schema, ["quux"] as String[], null, null, null, 0L) def entry1 = new AggregateEntry(snap1, AggregateEntry.hashOf(snap1)) [2, 1, 2, 250, 4].each { entry1.recordOneDuration(it as long) } writer.add(entry1) SpanSnapshot snap2 = new SpanSnapshot( "resource2", "service2", "operation2", null, "web", (short) 200, - false, true, "xyzzy", schema, ["quux"] as String[], null, null, null, 0L) + (CharSequence) null, true, "xyzzy", schema, ["quux"] as String[], null, null, null, 0L) def entry2 = new AggregateEntry(snap2, AggregateEntry.hashOf(snap2)) [1, 1, 200, 2, 3, 4, 5, 6, 7, 8].each { entry2.recordOneDuration(it as long) } writer.add(entry2)