From 779360eaf3e2df361d9eef76472aa9545ad22d71 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Fri, 24 May 2024 17:52:08 +0100 Subject: [PATCH 1/2] Align HierarchyMatchers API with OpenTelemetry (#7075) --- .../bytebuddy/matcher/DDElementMatchers.java | 16 +++---- .../bytebuddy/matcher/HierarchyMatchers.java | 46 +++++++++---------- .../bytebuddy/memoize/MemoizedMatchers.java | 16 +++---- .../tooling/bytebuddy/memoize/Memoizer.java | 2 +- 4 files changed, 40 insertions(+), 40 deletions(-) diff --git a/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/DDElementMatchers.java b/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/DDElementMatchers.java index cc24a4bd53e..f1e07970e95 100644 --- a/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/DDElementMatchers.java +++ b/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/DDElementMatchers.java @@ -23,21 +23,21 @@ public static void registerAsSupplier() { @Override @SuppressForbidden public ElementMatcher.Junction declaresAnnotation( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.isAnnotatedWith(matcher); } @Override @SuppressForbidden public ElementMatcher.Junction declaresField( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.declaresField(matcher); } @Override @SuppressForbidden public ElementMatcher.Junction declaresMethod( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.declaresMethod(matcher); } @@ -49,31 +49,31 @@ public ElementMatcher.Junction concreteClass() { @Override public ElementMatcher.Junction extendsClass( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return new SafeHasSuperTypeMatcher<>(matcher, false, true, false); } @Override public ElementMatcher.Junction implementsInterface( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return new SafeHasSuperTypeMatcher<>(matcher, true, true, true); } @Override public ElementMatcher.Junction hasInterface( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return new SafeHasSuperTypeMatcher<>(matcher, true, false, true); } @Override public ElementMatcher.Junction hasSuperType( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return new SafeHasSuperTypeMatcher<>(matcher, false, true, true); } @Override public ElementMatcher.Junction hasSuperMethod( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return new HasSuperMethodMatcher<>(matcher); } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java index 97b699e0803..f874b2d7e93 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java @@ -31,12 +31,12 @@ public static ElementMatcher.Junction declaresAnnotation( } public static ElementMatcher.Junction declaresField( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return SUPPLIER.declaresField(matcher); } public static ElementMatcher.Junction declaresMethod( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return SUPPLIER.declaresMethod(matcher); } @@ -45,12 +45,12 @@ public static ElementMatcher.Junction concreteClass() { } public static ElementMatcher.Junction extendsClass( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return SUPPLIER.extendsClass(matcher); } public static ElementMatcher.Junction implementsInterface( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return SUPPLIER.implementsInterface(matcher); } @@ -60,19 +60,19 @@ public static ElementMatcher.Junction implementsInterface( *

Use this when matching return or parameter types that could be classes or interfaces. */ public static ElementMatcher.Junction hasInterface( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return SUPPLIER.hasInterface(matcher); } /** Considers both interfaces and super-classes when matching the target type's hierarchy. */ public static ElementMatcher.Junction hasSuperType( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return SUPPLIER.hasSuperType(matcher); } /** Targets methods whose declaring class has a super-type that declares a matching method. */ public static ElementMatcher.Junction hasSuperMethod( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return SUPPLIER.hasSuperMethod(matcher); } @@ -106,30 +106,30 @@ public static synchronized void registerIfAbsent(Supplier supplier) { public interface Supplier { ElementMatcher.Junction declaresAnnotation( - ElementMatcher.Junction matcher); + ElementMatcher matcher); ElementMatcher.Junction declaresField( - ElementMatcher.Junction matcher); + ElementMatcher matcher); ElementMatcher.Junction declaresMethod( - ElementMatcher.Junction matcher); + ElementMatcher matcher); ElementMatcher.Junction concreteClass(); ElementMatcher.Junction extendsClass( - ElementMatcher.Junction matcher); + ElementMatcher matcher); ElementMatcher.Junction implementsInterface( - ElementMatcher.Junction matcher); + ElementMatcher matcher); ElementMatcher.Junction hasInterface( - ElementMatcher.Junction matcher); + ElementMatcher matcher); ElementMatcher.Junction hasSuperType( - ElementMatcher.Junction matcher); + ElementMatcher matcher); ElementMatcher.Junction hasSuperMethod( - ElementMatcher.Junction matcher); + ElementMatcher matcher); ElementMatcher.Junction declaresContextField( String keyClassName, String contextClassName); @@ -141,21 +141,21 @@ public static HierarchyMatchers.Supplier simpleChecks() { @Override @SuppressForbidden public ElementMatcher.Junction declaresAnnotation( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.isAnnotatedWith(matcher); } @Override @SuppressForbidden public ElementMatcher.Junction declaresField( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.declaresField(matcher); } @Override @SuppressForbidden public ElementMatcher.Junction declaresMethod( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.declaresMethod(matcher); } @@ -168,35 +168,35 @@ public ElementMatcher.Junction concreteClass() { @Override @SuppressForbidden public ElementMatcher.Junction extendsClass( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.hasSuperClass(matcher); } @Override @SuppressForbidden public ElementMatcher.Junction implementsInterface( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.hasSuperType(matcher); } @Override @SuppressForbidden public ElementMatcher.Junction hasInterface( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.hasSuperType(matcher); } @Override @SuppressForbidden public ElementMatcher.Junction hasSuperType( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.hasSuperType(matcher); } @Override @SuppressForbidden public ElementMatcher.Junction hasSuperMethod( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return ElementMatchers.isDeclaredBy( ElementMatchers.hasSuperType(ElementMatchers.declaresMethod(matcher))); } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/MemoizedMatchers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/MemoizedMatchers.java index 45c9ee9078f..0cdffbcc854 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/MemoizedMatchers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/MemoizedMatchers.java @@ -30,19 +30,19 @@ public static void registerAsSupplier() { @Override public ElementMatcher.Junction declaresAnnotation( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return Memoizer.prepare(ANNOTATION, matcher, false); } @Override public ElementMatcher.Junction declaresField( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return Memoizer.prepare(FIELD, matcher, false); } @Override public ElementMatcher.Junction declaresMethod( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return Memoizer.prepare(METHOD, matcher, false); } @@ -53,31 +53,31 @@ public ElementMatcher.Junction concreteClass() { @Override public ElementMatcher.Junction extendsClass( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return Memoizer.prepare(CLASS, matcher, true); } @Override public ElementMatcher.Junction implementsInterface( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return Memoizer.isClass.and(Memoizer.prepare(INTERFACE, matcher, true)); } @Override public ElementMatcher.Junction hasInterface( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return Memoizer.prepare(INTERFACE, matcher, true); } @Override public ElementMatcher.Junction hasSuperType( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return Memoizer.isClass.and(Memoizer.prepare(TYPE, matcher, true)); } @Override public ElementMatcher.Junction hasSuperMethod( - ElementMatcher.Junction matcher) { + ElementMatcher matcher) { return new HasSuperMethod(Memoizer.prepare(METHOD, matcher, true), matcher); } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/Memoizer.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/Memoizer.java index 631368af24e..604912dabce 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/Memoizer.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/Memoizer.java @@ -101,7 +101,7 @@ static MemoizingMatcher withMatcherId(ElementMatcher matcher) { /** Prepares a matcher for memoization. */ static MemoizingMatcher prepare( - MatcherKind kind, ElementMatcher.Junction matcher, boolean inherited) { + MatcherKind kind, ElementMatcher matcher, boolean inherited) { MemoizingMatcher memoizingMatcher = memoizingMatcherCache.computeIfAbsent(matcher, Memoizer::withMatcherId); From f89dfd62d9b3f4f2f19bfdeef821e6858840cbc2 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Fri, 24 May 2024 17:54:57 +0100 Subject: [PATCH 2/2] Disable URL instrumentation by default (#7073) Avoids creating unnecessary spans when exceptions occur during custom non-HTTP connections, like the custom Jar connection recently added in #7049. Note the respective instrumentation in OpenTelemetry was also removed for the same reason. Exceptions during HTTP connections are already captured by HttpUrlConnectionInstrumentation. The URL instrumentation can be re-enabled with `DD_INTEGRATION_URLCONNECTION_ENABLED=true` --- .../UrlInstrumentation.java | 5 ++ .../src/test/groovy/UrlConnectionTest.groovy | 48 ------------------- 2 files changed, 5 insertions(+), 48 deletions(-) diff --git a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java index 18007fba8f2..9af40e56587 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java +++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java @@ -25,6 +25,11 @@ public UrlInstrumentation() { super("urlconnection", "httpurlconnection"); } + @Override + protected boolean defaultEnabled() { + return false; + } + @Override public String instrumentedType() { return "java.net.URL"; diff --git a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy index dbd521fa80c..8ba15a044dd 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy +++ b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy @@ -2,7 +2,6 @@ import datadog.trace.agent.test.naming.VersionedNamingTestBase import datadog.trace.api.DDSpanTypes import datadog.trace.bootstrap.DatadogClassLoader import datadog.trace.bootstrap.instrumentation.api.Tags -import datadog.trace.bootstrap.instrumentation.decorator.UrlConnectionDecorator import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace @@ -68,53 +67,6 @@ abstract class UrlConnectionTest extends VersionedNamingTestBase { url = new URI("$scheme://localhost:$UNUSABLE_PORT").toURL() } - def "trace request with connection failure to a local file with broken url path"() { - setup: - def url = new URI("file:/some-random-file%abc").toURL() - - when: - injectSysConfig(HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") - runUnderTrace("someTrace") { - url.openConnection() - } - - then: - thrown IllegalArgumentException - - expect: - assertTraces(1) { - trace(2) { - span { - operationName "someTrace" - parent() - errored true - tags { - errorTags IllegalArgumentException, String - defaultTags() - } - } - span { - operationName operation(url.protocol) - resourceName "$url.path" - spanType DDSpanTypes.HTTP_CLIENT - childOf span(0) - errored true - tags { - "$Tags.COMPONENT" UrlConnectionDecorator.COMPONENT - "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT - // FIXME: These tags really make no sense for non-http connections, why do we set them? - "$Tags.HTTP_URL" "$url" - errorTags IllegalArgumentException, String - defaultTagsNoPeerService() - } - } - } - } - - where: - renameService << [false, true] - } - def "DatadogClassloader ClassNotFoundException doesn't create span"() { given: ClassLoader datadogLoader = new DatadogClassLoader()