diff --git a/dd-java-agent/instrumentation/build.gradle b/dd-java-agent/instrumentation/build.gradle index 5e36217ad42..e3817336bb5 100644 --- a/dd-java-agent/instrumentation/build.gradle +++ b/dd-java-agent/instrumentation/build.gradle @@ -159,7 +159,10 @@ TaskProvider registerIndexTask(String indexTaskName, String indexer, S } instrumentationNaming { - exclusions = ["org-json-20230227"] // org-json does not use semver + exclusions = [ + "org-json-20230227", + // org-json does not use semver + ] suffixes = ["-common", "-stubs", "-iast"] } diff --git a/dd-java-agent/instrumentation/commons-httpclient-2.0/build.gradle b/dd-java-agent/instrumentation/commons-httpclient-2.0/build.gradle index b406619e198..cd4e9d50c11 100644 --- a/dd-java-agent/instrumentation/commons-httpclient-2.0/build.gradle +++ b/dd-java-agent/instrumentation/commons-httpclient-2.0/build.gradle @@ -2,19 +2,14 @@ muzzle { pass { group = "commons-httpclient" module = "commons-httpclient" - versions = "[2.0,]" - skipVersions += "20020423" // ancient pre-release version - skipVersions += '2.0-final' // broken metadata on maven central - assertInverse = true + versions = "[2.0,4.0)" + skipVersions += '3.1-jenkins-1' + skipVersions += '2.0-final' } - pass { - name = 'commons-http-client-x' // for IAST instrumenters valid for 1.x + fail { group = "commons-httpclient" module = "commons-httpclient" - versions = "[2.0,]" - skipVersions += "20020423" // ancient pre-release version - skipVersions += '2.0-final' // broken metadata on maven central - assertInverse = false + versions = "[,2.0)" } } @@ -27,5 +22,9 @@ dependencies { testImplementation group: 'commons-httpclient', name: 'commons-httpclient', version: '2.0' - latestDepTestImplementation group: 'commons-httpclient', name: 'commons-httpclient', version: '(2.0,20000000]' + // latestDepTest tests against the highest 2.x release (module instruments 2.x only) + latestDepTestImplementation group: 'commons-httpclient', name: 'commons-httpclient', version: '2+' + + testRuntimeOnly(project(':dd-java-agent:instrumentation:datadog:asm:iast-instrumenter')) + testRuntimeOnly(project(':dd-java-agent:instrumentation:java:java-net:java-net-1.8')) } diff --git a/dd-java-agent/instrumentation/commons-httpclient-2.0/gradle.lockfile b/dd-java-agent/instrumentation/commons-httpclient-2.0/gradle.lockfile deleted file mode 100644 index f5528118944..00000000000 --- a/dd-java-agent/instrumentation/commons-httpclient-2.0/gradle.lockfile +++ /dev/null @@ -1,129 +0,0 @@ -# This is a Gradle generated file for dependency locking. -# Manual edits can break the build and are not advised. -# This file is expected to be part of source control. -# To regenerate this file, run: ./gradlew :dd-java-agent:instrumentation:commons-httpclient-2.0:dependencies --write-locks -cafe.cryptography:curve25519-elisabeth:0.1.0=latestDepTestRuntimeClasspath,testRuntimeClasspath -cafe.cryptography:ed25519-elisabeth:0.1.0=latestDepTestRuntimeClasspath,testRuntimeClasspath -ch.qos.logback:logback-classic:1.2.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -ch.qos.logback:logback-core:1.2.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -com.blogspot.mydailyjava:weak-lock-free:0.17=buildTimeInstrumentationPlugin,compileClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath -com.datadoghq.okhttp3:okhttp:3.12.15=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -com.datadoghq.okio:okio:1.17.6=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -com.datadoghq:dd-instrument-java:0.0.4=buildTimeInstrumentationPlugin,compileClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,muzzleBootstrap,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath -com.datadoghq:dd-javac-plugin-client:0.2.2=buildTimeInstrumentationPlugin,compileClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,muzzleBootstrap,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath -com.datadoghq:java-dogstatsd-client:4.4.5=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.datadoghq:sketches-java:0.8.3=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.github.javaparser:javaparser-core:3.25.6=codenarc -com.github.jnr:jffi:1.3.15=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.github.jnr:jnr-a64asm:1.0.0=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.github.jnr:jnr-constants:0.10.4=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.github.jnr:jnr-enxio:0.32.20=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.github.jnr:jnr-ffi:2.2.19=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.github.jnr:jnr-posix:3.1.22=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.github.jnr:jnr-unixsocket:0.38.25=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.github.jnr:jnr-x86asm:1.0.2=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.github.spotbugs:spotbugs-annotations:4.9.8=compileClasspath,spotbugs -com.github.spotbugs:spotbugs:4.9.8=spotbugs -com.github.stephenc.jcip:jcip-annotations:1.0-1=spotbugs -com.google.auto.service:auto-service-annotations:1.1.1=annotationProcessor,compileClasspath,latestDepTestAnnotationProcessor,latestDepTestCompileClasspath,testAnnotationProcessor,testCompileClasspath -com.google.auto.service:auto-service:1.1.1=annotationProcessor,latestDepTestAnnotationProcessor,testAnnotationProcessor -com.google.auto:auto-common:1.2.1=annotationProcessor,latestDepTestAnnotationProcessor,testAnnotationProcessor -com.google.code.findbugs:jsr305:3.0.2=annotationProcessor,compileClasspath,latestDepTestAnnotationProcessor,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,spotbugs,testAnnotationProcessor,testCompileClasspath,testRuntimeClasspath -com.google.code.gson:gson:2.13.2=spotbugs -com.google.errorprone:error_prone_annotations:2.18.0=annotationProcessor,latestDepTestAnnotationProcessor,testAnnotationProcessor -com.google.errorprone:error_prone_annotations:2.41.0=spotbugs -com.google.guava:failureaccess:1.0.1=annotationProcessor,latestDepTestAnnotationProcessor,testAnnotationProcessor -com.google.guava:guava:20.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -com.google.guava:guava:32.0.1-jre=annotationProcessor,latestDepTestAnnotationProcessor,testAnnotationProcessor -com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava=annotationProcessor,latestDepTestAnnotationProcessor,testAnnotationProcessor -com.google.j2objc:j2objc-annotations:2.8=annotationProcessor,latestDepTestAnnotationProcessor,testAnnotationProcessor -com.google.re2j:re2j:1.7=latestDepTestRuntimeClasspath,testRuntimeClasspath -com.squareup.moshi:moshi:1.11.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -com.squareup.okhttp3:logging-interceptor:3.12.12=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -com.squareup.okhttp3:okhttp:3.12.12=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -com.squareup.okio:okio:1.17.5=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -com.thoughtworks.qdox:qdox:1.12.1=codenarc -commons-codec:commons-codec:1.2=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath -commons-fileupload:commons-fileupload:1.5=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -commons-httpclient:commons-httpclient:2.0=compileClasspath,testCompileClasspath,testRuntimeClasspath -commons-httpclient:commons-httpclient:3.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath -commons-io:commons-io:2.11.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -commons-io:commons-io:2.20.0=spotbugs -commons-lang:commons-lang:1.0.1=compileClasspath,testCompileClasspath,testRuntimeClasspath -commons-logging:commons-logging:1.0.3=compileClasspath,testCompileClasspath,testRuntimeClasspath -commons-logging:commons-logging:1.0.4=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath -de.thetaphi:forbiddenapis:3.10=compileClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -io.leangen.geantyref:geantyref:1.3.16=latestDepTestRuntimeClasspath,testRuntimeClasspath -io.sqreen:libsqreen:17.3.0=latestDepTestRuntimeClasspath,testRuntimeClasspath -javax.servlet:javax.servlet-api:3.1.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -jaxen:jaxen:2.0.0=spotbugs -junit:junit:4.13.2=latestDepTestRuntimeClasspath,testRuntimeClasspath -net.bytebuddy:byte-buddy-agent:1.18.10=buildTimeInstrumentationPlugin,compileClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath -net.bytebuddy:byte-buddy:1.18.10=buildTimeInstrumentationPlugin,compileClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath -net.java.dev.jna:jna-platform:5.8.0=latestDepTestRuntimeClasspath,testRuntimeClasspath -net.java.dev.jna:jna:5.8.0=latestDepTestRuntimeClasspath,testRuntimeClasspath -net.sf.saxon:Saxon-HE:12.9=spotbugs -org.apache.ant:ant-antlr:1.10.14=codenarc -org.apache.ant:ant-junit:1.10.14=codenarc -org.apache.bcel:bcel:6.11.0=spotbugs -org.apache.commons:commons-lang3:3.19.0=spotbugs -org.apache.commons:commons-text:1.14.0=spotbugs -org.apache.logging.log4j:log4j-api:2.25.2=spotbugs -org.apache.logging.log4j:log4j-core:2.25.2=spotbugs -org.apiguardian:apiguardian-api:1.1.2=latestDepTestCompileClasspath,testCompileClasspath -org.checkerframework:checker-qual:3.33.0=annotationProcessor,latestDepTestAnnotationProcessor,testAnnotationProcessor -org.codehaus.groovy:groovy-ant:3.0.23=codenarc -org.codehaus.groovy:groovy-docgenerator:3.0.23=codenarc -org.codehaus.groovy:groovy-groovydoc:3.0.23=codenarc -org.codehaus.groovy:groovy-json:3.0.23=codenarc -org.codehaus.groovy:groovy-json:3.0.25=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.codehaus.groovy:groovy-templates:3.0.23=codenarc -org.codehaus.groovy:groovy-xml:3.0.23=codenarc -org.codehaus.groovy:groovy:3.0.23=codenarc -org.codehaus.groovy:groovy:3.0.25=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.codenarc:CodeNarc:3.7.0=codenarc -org.dom4j:dom4j:2.2.0=spotbugs -org.gmetrics:GMetrics:2.1.0=codenarc -org.hamcrest:hamcrest-core:1.3=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.hamcrest:hamcrest:3.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.jctools:jctools-core-jdk11:4.0.6=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.jctools:jctools-core:4.0.6=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.junit.jupiter:junit-jupiter-api:5.14.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.junit.jupiter:junit-jupiter-engine:5.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.junit.jupiter:junit-jupiter-params:5.14.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.junit.jupiter:junit-jupiter:5.14.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.junit.platform:junit-platform-commons:1.14.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.junit.platform:junit-platform-engine:1.14.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.junit.platform:junit-platform-launcher:1.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.junit.platform:junit-platform-runner:1.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.junit.platform:junit-platform-suite-api:1.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.junit.platform:junit-platform-suite-commons:1.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.junit:junit-bom:5.14.0=spotbugs -org.junit:junit-bom:5.14.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.mockito:mockito-core:4.4.0=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.objenesis:objenesis:3.3=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.opentest4j:opentest4j:1.3.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.ow2.asm:asm-analysis:9.7.1=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.ow2.asm:asm-analysis:9.9=spotbugs -org.ow2.asm:asm-commons:9.9=spotbugs -org.ow2.asm:asm-commons:9.9.1=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.ow2.asm:asm-tree:9.9=spotbugs -org.ow2.asm:asm-tree:9.9.1=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.ow2.asm:asm-util:9.7.1=latestDepTestRuntimeClasspath,testRuntimeClasspath -org.ow2.asm:asm-util:9.9=spotbugs -org.ow2.asm:asm:9.9=spotbugs -org.ow2.asm:asm:9.9.1=buildTimeInstrumentationPlugin,compileClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath -org.slf4j:jcl-over-slf4j:1.7.30=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.slf4j:jul-to-slf4j:1.7.30=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.slf4j:log4j-over-slf4j:1.7.30=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.slf4j:slf4j-api:1.7.30=buildTimeInstrumentationPlugin,compileClasspath,muzzleBootstrap,muzzleTooling,runtimeClasspath -org.slf4j:slf4j-api:1.7.32=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.slf4j:slf4j-api:2.0.17=spotbugs,spotbugsSlf4j -org.slf4j:slf4j-simple:2.0.17=spotbugsSlf4j -org.snakeyaml:snakeyaml-engine:2.9=buildTimeInstrumentationPlugin,latestDepTestRuntimeClasspath,muzzleTooling,runtimeClasspath,testRuntimeClasspath -org.spockframework:spock-bom:2.4-groovy-3.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.spockframework:spock-core:2.4-groovy-3.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.tabletest:tabletest-junit:1.2.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.tabletest:tabletest-parser:1.2.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.xmlresolver:xmlresolver:5.3.3=spotbugs -empty=spotbugsPlugins diff --git a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientDecorator.java b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientDecorator.java index a7596e2f2ea..edc8a204fdd 100644 --- a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientDecorator.java +++ b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientDecorator.java @@ -7,9 +7,9 @@ import org.apache.commons.httpclient.Header; import org.apache.commons.httpclient.HttpMethod; import org.apache.commons.httpclient.StatusLine; -import org.apache.commons.httpclient.URIException; public class CommonsHttpClientDecorator extends HttpClientDecorator { + public static final CharSequence COMMONS_HTTP_CLIENT = UTF8BytesString.create("commons-http-client"); public static final CommonsHttpClientDecorator DECORATE = new CommonsHttpClientDecorator(); @@ -34,16 +34,14 @@ protected String method(final HttpMethod httpMethod) { @Override protected URI url(final HttpMethod httpMethod) throws URISyntaxException { try { - // org.apache.commons.httpclient.URI -> java.net.URI - return new URI(httpMethod.getURI().toString()); - } catch (final URIException e) { - throw new URISyntaxException("", e.getMessage()); + org.apache.commons.httpclient.URI uri = httpMethod.getURI(); + if (uri != null) { + return new URI(uri.toString()); + } + } catch (Exception e) { + // getURI() can throw URIException; fall through to return null } - } - - @Override - protected HttpMethod sourceUrl(final HttpMethod httpMethod) { - return httpMethod; + return null; } @Override @@ -53,18 +51,18 @@ protected int status(final HttpMethod httpMethod) { } @Override - protected String getRequestHeader(HttpMethod request, String headerName) { - Header header = request.getRequestHeader(headerName); - if (null != header) { + protected String getRequestHeader(HttpMethod httpMethod, String headerName) { + Header header = httpMethod.getRequestHeader(headerName); + if (header != null) { return header.getValue(); } return null; } @Override - protected String getResponseHeader(HttpMethod response, String headerName) { - Header header = response.getResponseHeader(headerName); - if (null != header) { + protected String getResponseHeader(HttpMethod httpMethod, String headerName) { + Header header = httpMethod.getResponseHeader(headerName); + if (header != null) { return header.getValue(); } return null; diff --git a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientInstrumentation.java b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientInstrumentation.java index a63ee7c9a82..d2849752f41 100644 --- a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientInstrumentation.java @@ -1,27 +1,15 @@ package datadog.trace.instrumentation.commonshttpclient; -import static datadog.trace.agent.tooling.InstrumenterModule.TargetSystem.CONTEXT_TRACKING; import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; -import static datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge.getCurrentContext; -import static datadog.trace.instrumentation.commonshttpclient.CommonsHttpClientDecorator.DECORATE; -import static datadog.trace.instrumentation.commonshttpclient.CommonsHttpClientDecorator.HTTP_REQUEST; -import static datadog.trace.instrumentation.commonshttpclient.HttpHeadersInjectAdapter.SETTER; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; -import datadog.appsec.api.blocking.BlockingException; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.agent.tooling.annotation.AppliesOn; -import datadog.trace.bootstrap.CallDepthThreadLocalMap; import datadog.trace.bootstrap.instrumentation.api.AgentScope; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import net.bytebuddy.asm.Advice; -import org.apache.commons.httpclient.HttpClient; import org.apache.commons.httpclient.HttpMethod; @AutoService(InstrumenterModule.class) @@ -40,72 +28,40 @@ public String instrumentedType() { @Override public String[] helperClassNames() { return new String[] { - packageName + ".CommonsHttpClientDecorator", packageName + ".HttpHeadersInjectAdapter", + packageName + ".CommonsHttpClientDecorator", + packageName + ".HttpHeadersInjectAdapter", + packageName + ".HelperMethods", }; } @Override public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvices( + // All executeMethod overloads delegate to the 3-arg version: + // executeMethod(HostConfiguration, HttpMethod, HttpState) + // Instrumenting only the delegate avoids duplicate spans. + transformer.applyAdvice( isMethod() .and(named("executeMethod")) .and(takesArguments(3)) - .and(takesArgument(1, named("org.apache.commons.httpclient.HttpMethod"))), - CommonsHttpClientInstrumentation.class.getName() + "$ExecAdvice", - CommonsHttpClientInstrumentation.class.getName() + "$ContextPropagationAdvice"); + .and(takesArgument(0, named("org.apache.commons.httpclient.HostConfiguration"))) + .and(takesArgument(1, named("org.apache.commons.httpclient.HttpMethod"))) + .and(takesArgument(2, named("org.apache.commons.httpclient.HttpState"))), + CommonsHttpClientInstrumentation.class.getName() + "$ExecuteMethodAdvice"); } - public static class ExecAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static AgentScope methodEnter(@Advice.Argument(1) final HttpMethod httpMethod) { - - try { - final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpClient.class); - if (callDepth > 0) { - return null; - } - - final AgentSpan span = startSpan("commons-http-client", HTTP_REQUEST); - final AgentScope scope = activateSpan(span); + public static class ExecuteMethodAdvice { - DECORATE.afterStart(span); - DECORATE.onRequest(span, httpMethod); - - return scope; - } catch (BlockingException e) { - CallDepthThreadLocalMap.reset(HttpClient.class); - // re-throw blocking exceptions - throw e; - } + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope methodEnter(@Advice.Argument(1) final HttpMethod method) { + return HelperMethods.doMethodEnter(method); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Enter final AgentScope scope, - @Advice.Argument(1) final HttpMethod httpMethod, + @Advice.Argument(1) final HttpMethod method, @Advice.Thrown final Throwable throwable) { - - if (scope == null) { - return; - } - final AgentSpan span = scope.span(); - try { - DECORATE.onResponse(span, httpMethod); - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - } finally { - scope.close(); - span.finish(); - CallDepthThreadLocalMap.reset(HttpClient.class); - } - } - } - - @AppliesOn(CONTEXT_TRACKING) - public static class ContextPropagationAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter(@Advice.Argument(1) final HttpMethod httpMethod) { - DECORATE.injectContext(getCurrentContext(), httpMethod, SETTER); + HelperMethods.doMethodExit(scope, method, throwable); } } } diff --git a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/HelperMethods.java b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/HelperMethods.java new file mode 100644 index 00000000000..2d1a9080c15 --- /dev/null +++ b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/HelperMethods.java @@ -0,0 +1,55 @@ +package datadog.trace.instrumentation.commonshttpclient; + +import static datadog.context.Context.current; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.commonshttpclient.CommonsHttpClientDecorator.DECORATE; +import static datadog.trace.instrumentation.commonshttpclient.CommonsHttpClientDecorator.HTTP_REQUEST; +import static datadog.trace.instrumentation.commonshttpclient.HttpHeadersInjectAdapter.SETTER; + +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import org.apache.commons.httpclient.HttpClient; +import org.apache.commons.httpclient.HttpMethod; + +public class HelperMethods { + + public static AgentScope doMethodEnter(final HttpMethod method) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpClient.class); + if (callDepth > 0) { + return null; + } + + final AgentSpan span = + startSpan(CommonsHttpClientDecorator.COMMONS_HTTP_CLIENT.toString(), HTTP_REQUEST); + DECORATE.afterStart(span); + DECORATE.onRequest(span, method); + + final AgentScope scope = activateSpan(span); + + DECORATE.injectContext(current(), method, SETTER); + + return scope; + } + + public static void doMethodExit( + final AgentScope scope, final HttpMethod method, final Throwable throwable) { + if (scope == null) { + return; + } + final AgentSpan span = scope.span(); + try { + if (throwable != null) { + DECORATE.onError(span, throwable); + } else { + DECORATE.onResponse(span, method); + } + DECORATE.beforeFinish(span); + } finally { + scope.close(); + span.finish(); + CallDepthThreadLocalMap.reset(HttpClient.class); + } + } +} diff --git a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/HttpHeadersInjectAdapter.java b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/HttpHeadersInjectAdapter.java index f35bb0eb8af..88297d95d3c 100644 --- a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/HttpHeadersInjectAdapter.java +++ b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/HttpHeadersInjectAdapter.java @@ -2,7 +2,6 @@ import datadog.context.propagation.CarrierSetter; import javax.annotation.ParametersAreNonnullByDefault; -import org.apache.commons.httpclient.Header; import org.apache.commons.httpclient.HttpMethod; @ParametersAreNonnullByDefault @@ -12,6 +11,6 @@ public class HttpHeadersInjectAdapter implements CarrierSetter { @Override public void set(final HttpMethod carrier, final String key, final String value) { - carrier.setRequestHeader(new Header(key, value)); + carrier.setRequestHeader(key, value); } } diff --git a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/IastHttpMethodBaseInstrumentation.java b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/IastHttpMethodBaseInstrumentation.java deleted file mode 100644 index fc6f09dfb11..00000000000 --- a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/main/java/datadog/trace/instrumentation/commonshttpclient/IastHttpMethodBaseInstrumentation.java +++ /dev/null @@ -1,61 +0,0 @@ -package datadog.trace.instrumentation.commonshttpclient; - -import static net.bytebuddy.matcher.ElementMatchers.isConstructor; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.agent.tooling.bytebuddy.iast.TaintableVisitor; -import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.Propagation; -import datadog.trace.api.iast.propagation.PropagationModule; -import net.bytebuddy.asm.Advice; - -@AutoService(InstrumenterModule.class) -public class IastHttpMethodBaseInstrumentation extends InstrumenterModule.Iast - implements Instrumenter.ForSingleType, - Instrumenter.HasTypeAdvice, - Instrumenter.HasMethodAdvice { - - private final String className = IastHttpMethodBaseInstrumentation.class.getName(); - - public IastHttpMethodBaseInstrumentation() { - super("commons-http-client"); - } - - @Override - public String instrumentedType() { - return "org.apache.commons.httpclient.HttpMethodBase"; - } - - @Override - public String muzzleDirective() { - return "commons-http-client-x"; - } - - @Override - public void typeAdvice(TypeTransformer transformer) { - transformer.applyAdvice(new TaintableVisitor(instrumentedType())); - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvice( - isConstructor().and(takesArguments(1)).and(takesArgument(0, String.class)), - className + "$CtorAdvice"); - } - - public static class CtorAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterCtor( - @Advice.This final Object self, @Advice.Argument(0) final Object argument) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - module.taintObjectIfTainted(self, argument); - } - } - } -} diff --git a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/test/groovy/CommonsHttpClientTest.groovy b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/test/groovy/CommonsHttpClientTest.groovy deleted file mode 100644 index 23ded9aa8b0..00000000000 --- a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/test/groovy/CommonsHttpClientTest.groovy +++ /dev/null @@ -1,83 +0,0 @@ -import datadog.trace.agent.test.base.HttpClientTest -import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions -import datadog.trace.instrumentation.commonshttpclient.CommonsHttpClientDecorator -import org.apache.commons.httpclient.HttpClient -import org.apache.commons.httpclient.HttpMethod -import org.apache.commons.httpclient.methods.DeleteMethod -import org.apache.commons.httpclient.methods.GetMethod -import org.apache.commons.httpclient.methods.HeadMethod -import org.apache.commons.httpclient.methods.OptionsMethod -import org.apache.commons.httpclient.methods.PostMethod -import org.apache.commons.httpclient.methods.PutMethod -import org.apache.commons.httpclient.methods.TraceMethod -import spock.lang.Shared -import spock.lang.Timeout - -@Timeout(5) -abstract class CommonsHttpClientTest extends HttpClientTest { - @Shared - HttpClient client = new HttpClient() - - def setupSpec() { - client.setConnectionTimeout(CONNECT_TIMEOUT_MS) - client.setTimeout(READ_TIMEOUT_MS) - } - - @Override - int doRequest(String method, URI uri, Map headers, String body, Closure callback) { - HttpMethod httpMethod - - switch (method) { - case "GET": - httpMethod = new GetMethod(uri.toString()) - break - case "PUT": - httpMethod = new PutMethod(uri.toString()) - break - case "POST": - httpMethod = new PostMethod(uri.toString()) - break - case "HEAD": - httpMethod = new HeadMethod(uri.toString()) - break - case "DELETE": - httpMethod = new DeleteMethod(uri.toString()) - break - case "OPTIONS": - httpMethod = new OptionsMethod(uri.toString()) - break - case "TRACE": - httpMethod = new TraceMethod(uri.toString()) - break - default: - throw new RuntimeException("Unsupported method: " + method) - } - - headers.each { httpMethod.setRequestHeader(it.key, it.value) } - - try { - client.executeMethod(httpMethod) - callback?.call() - return httpMethod.getStatusCode() - } finally { - httpMethod.releaseConnection() - } - } - - @Override - CharSequence component() { - return CommonsHttpClientDecorator.DECORATE.component() - } - - @Override - boolean testRedirects() { - // Generates 4 spans - false - } -} - -class CommonsHttpClientV0ForkedTest extends CommonsHttpClientTest implements TestingGenericHttpNamingConventions.ClientV0 { -} - -class CommonsHttpClientV1ForkedTest extends CommonsHttpClientTest implements TestingGenericHttpNamingConventions.ClientV1 { -} diff --git a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/test/groovy/IastCommonsHttpClientInstrumentationTest.groovy b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/test/groovy/IastCommonsHttpClientInstrumentationTest.groovy deleted file mode 100644 index 28c5b56ad46..00000000000 --- a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/test/groovy/IastCommonsHttpClientInstrumentationTest.groovy +++ /dev/null @@ -1,62 +0,0 @@ -import datadog.trace.agent.test.InstrumentationSpecification -import datadog.trace.api.iast.InstrumentationBridge -import datadog.trace.api.iast.propagation.PropagationModule -import datadog.trace.api.iast.sink.SsrfModule -import org.apache.commons.httpclient.HttpClient -import org.apache.commons.httpclient.methods.GetMethod -import spock.lang.AutoCleanup -import spock.lang.Shared - -import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer - -class IastCommonsHttpClientInstrumentationTest extends InstrumentationSpecification { - - @Override - protected void configurePreAgent() { - injectSysConfig('dd.iast.enabled', 'true') - } - - @AutoCleanup - @Shared - def server = httpServer { - handlers { - prefix('/') { - String msg = "Hello." - response.status(200).send(msg) - } - } - } - - @Shared - Map tainteds = new IdentityHashMap<>() - - void setup() { - tainteds.clear() - mockPropagation() - } - - void 'test ssrf'() { - given: - final url = server.address.toString() - tainteds.put(url, null) - final ssrf = Mock(SsrfModule) - InstrumentationBridge.registerIastModule(ssrf) - - when: - new HttpClient().executeMethod(new GetMethod(url)) - - then: - 1 * ssrf.onURLConnection({ value -> tainteds.containsKey(value) }) - } - - private void mockPropagation() { - final propagation = Mock(PropagationModule) { - taintObjectIfTainted(_, _) >> { - if (tainteds.containsKey(it[1])) { - tainteds.put(it[0], null) - } - } - } - InstrumentationBridge.registerIastModule(propagation) - } -} diff --git a/dd-java-agent/instrumentation/commons-httpclient-2.0/src/test/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientTest.java b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/test/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientTest.java new file mode 100644 index 00000000000..ce4b716417b --- /dev/null +++ b/dd-java-agent/instrumentation/commons-httpclient-2.0/src/test/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientTest.java @@ -0,0 +1,615 @@ +package datadog.trace.instrumentation.commonshttpclient; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import com.sun.net.httpserver.HttpServer; +import datadog.trace.agent.test.AbstractInstrumentationTest; +import datadog.trace.core.DDSpan; +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import org.apache.commons.httpclient.HostConfiguration; +import org.apache.commons.httpclient.HttpClient; +import org.apache.commons.httpclient.methods.GetMethod; +import org.apache.commons.httpclient.methods.HeadMethod; +import org.apache.commons.httpclient.methods.PostMethod; +import org.apache.commons.httpclient.methods.PutMethod; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +/** + * Tests for commons-httpclient instrumentation. + * + *

Verifies that {@code HttpClient.executeMethod()} creates spans with correct operation name, + * span kind, HTTP method, URL, and status code tags for various HTTP operations. + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class CommonsHttpClientTest extends AbstractInstrumentationTest { + + private static final int SERVER_PORT = 19876; + private static final String BASE_URL = "http://localhost:" + SERVER_PORT; + + private HttpServer server; + private HttpClient client; + + /** Headers captured from the most recent request to /propagation endpoint. */ + private final Map capturedHeaders = new ConcurrentHashMap<>(); + + @BeforeAll + void setupServer() throws IOException { + server = HttpServer.create(new InetSocketAddress(SERVER_PORT), 0); + + server.createContext( + "/success", + new HttpHandler() { + @Override + public void handle(HttpExchange exchange) throws IOException { + byte[] response = "{\"status\":\"ok\"}".getBytes("UTF-8"); + exchange.getResponseHeaders().set("Content-Type", "application/json"); + exchange.sendResponseHeaders(200, response.length); + OutputStream os = exchange.getResponseBody(); + os.write(response); + os.close(); + } + }); + + server.createContext( + "/echo", + new HttpHandler() { + @Override + public void handle(HttpExchange exchange) throws IOException { + byte[] response = "{\"echo\":\"ok\"}".getBytes("UTF-8"); + exchange.getResponseHeaders().set("Content-Type", "application/json"); + exchange.sendResponseHeaders(200, response.length); + OutputStream os = exchange.getResponseBody(); + os.write(response); + os.close(); + } + }); + + server.createContext( + "/not-found", + new HttpHandler() { + @Override + public void handle(HttpExchange exchange) throws IOException { + byte[] response = "{\"error\":\"not found\"}".getBytes("UTF-8"); + exchange.sendResponseHeaders(404, response.length); + OutputStream os = exchange.getResponseBody(); + os.write(response); + os.close(); + } + }); + + server.createContext( + "/error", + new HttpHandler() { + @Override + public void handle(HttpExchange exchange) throws IOException { + byte[] response = "{\"error\":\"internal server error\"}".getBytes("UTF-8"); + exchange.sendResponseHeaders(500, response.length); + OutputStream os = exchange.getResponseBody(); + os.write(response); + os.close(); + } + }); + + // Endpoint that captures all request headers for context propagation verification + server.createContext( + "/propagation", + new HttpHandler() { + @Override + public void handle(HttpExchange exchange) throws IOException { + capturedHeaders.clear(); + for (Map.Entry> entry : exchange.getRequestHeaders().entrySet()) { + // HttpServer normalizes header names to Title-Case; store lowercase for easy lookup + capturedHeaders.put(entry.getKey().toLowerCase(), entry.getValue().get(0)); + } + byte[] response = "ok".getBytes("UTF-8"); + exchange.sendResponseHeaders(200, response.length); + OutputStream os = exchange.getResponseBody(); + os.write(response); + os.close(); + } + }); + + server.setExecutor(null); + server.start(); + + client = new HttpClient(); + } + + @AfterAll + void tearDownServer() { + if (server != null) { + server.stop(0); + } + } + + @Test + void getRequestCreatesSpanWithCorrectTags() throws Exception { + GetMethod get = new GetMethod(BASE_URL + "/success"); + try { + int statusCode = client.executeMethod(get); + assertEquals(200, statusCode); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + assertHttpClientSpan(span, "GET", "/success", 200, false); + } + + @Test + void postRequestCreatesSpanWithCorrectMethod() throws Exception { + PostMethod post = new PostMethod(BASE_URL + "/echo"); + post.setRequestBody("{\"key\":\"value\"}"); + try { + int statusCode = client.executeMethod(post); + assertEquals(200, statusCode); + } finally { + post.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + assertHttpClientSpan(span, "POST", "/echo", 200, false); + } + + @Test + void putRequestCreatesSpanWithCorrectMethod() throws Exception { + PutMethod put = new PutMethod(BASE_URL + "/echo"); + put.setRequestBody("{\"updated\":true}"); + try { + int statusCode = client.executeMethod(put); + assertEquals(200, statusCode); + } finally { + put.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + assertHttpClientSpan(span, "PUT", "/echo", 200, false); + } + + @Test + void headRequestCreatesSpan() throws Exception { + HeadMethod head = new HeadMethod(BASE_URL + "/success"); + try { + int statusCode = client.executeMethod(head); + assertEquals(200, statusCode); + } finally { + head.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + assertHttpClientSpan(span, "HEAD", "/success", 200, false); + } + + @Test + void requestWith404SetsStatusCodeTag() throws Exception { + GetMethod get = new GetMethod(BASE_URL + "/not-found"); + try { + int statusCode = client.executeMethod(get); + assertEquals(404, statusCode); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + // 4xx statuses are client errors and flagged as span errors by default + assertHttpClientSpan(span, "GET", "/not-found", 404, true); + } + + @Test + void requestWith500SetsErrorStatus() throws Exception { + GetMethod get = new GetMethod(BASE_URL + "/error"); + try { + int statusCode = client.executeMethod(get); + assertEquals(500, statusCode); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + // 5xx statuses are server errors and not flagged as client span errors by default + assertHttpClientSpan(span, "GET", "/error", 500, false); + } + + @Test + void connectionExceptionSetsErrorTags() throws Exception { + GetMethod get = new GetMethod("http://localhost:1/nonexistent"); + try { + assertThrows( + IOException.class, + () -> { + client.executeMethod(get); + }); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span even on connection error"); + assertTrue(span.isError(), "Span should be marked as error on exception"); + assertNotNull(span.getTag("error.type"), "Expected error.type tag"); + assertNotNull(span.getTag("error.message"), "Expected error.message tag"); + } + + @Test + void executeMethodWithHostConfigCreatesSpan() throws Exception { + HostConfiguration hostConfig = new HostConfiguration(); + hostConfig.setHost("localhost", SERVER_PORT, "http"); + GetMethod get = new GetMethod("/success"); + try { + int statusCode = client.executeMethod(hostConfig, get); + assertEquals(200, statusCode); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span with HostConfiguration overload"); + assertHttpClientSpan(span, "GET", "/success", 200, false); + } + + @Test + void multipleRequestsCreateSeparateSpans() throws Exception { + GetMethod get1 = new GetMethod(BASE_URL + "/success"); + GetMethod get2 = new GetMethod(BASE_URL + "/success"); + try { + client.executeMethod(get1); + } finally { + get1.releaseConnection(); + } + try { + client.executeMethod(get2); + } finally { + get2.releaseConnection(); + } + + writer.waitForTraces(2); + List spans = flattenTraces(); + + long httpRequestCount = + spans.stream().filter(s -> "http.request".equals(s.getOperationName().toString())).count(); + assertEquals(2, httpRequestCount, "Expected two separate http.request spans"); + } + + @Test + void contextPropagationInjectsDatadogTraceHeaders() throws Exception { + GetMethod get = new GetMethod(BASE_URL + "/propagation"); + try { + int statusCode = client.executeMethod(get); + assertEquals(200, statusCode); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + + // Verify Datadog propagation headers were injected into the outgoing request + assertNotNull( + capturedHeaders.get("x-datadog-trace-id"), + "Expected x-datadog-trace-id header to be injected"); + assertNotNull( + capturedHeaders.get("x-datadog-parent-id"), + "Expected x-datadog-parent-id header to be injected"); + assertNotNull( + capturedHeaders.get("x-datadog-sampling-priority"), + "Expected x-datadog-sampling-priority header to be injected"); + + // Verify the injected trace ID matches the span's trace ID + long expectedTraceId = span.getTraceId().toLong(); + assertEquals( + String.valueOf(expectedTraceId), + capturedHeaders.get("x-datadog-trace-id"), + "Injected trace ID must match the span's trace ID"); + + // Verify the injected parent ID matches the span's span ID + long expectedSpanId = span.getSpanId(); + assertEquals( + String.valueOf(expectedSpanId), + capturedHeaders.get("x-datadog-parent-id"), + "Injected parent ID must match the span's span ID"); + } + + @Test + void contextPropagationInjectsW3CTraceparentHeader() throws Exception { + GetMethod get = new GetMethod(BASE_URL + "/propagation"); + try { + int statusCode = client.executeMethod(get); + assertEquals(200, statusCode); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + + // Default propagation style includes W3C traceparent + String traceparent = capturedHeaders.get("traceparent"); + assertNotNull(traceparent, "Expected traceparent header to be injected"); + + // traceparent format: {version}-{trace-id}-{parent-id}-{trace-flags} + String[] parts = traceparent.split("-"); + assertEquals(4, parts.length, "traceparent must have 4 dash-separated parts"); + assertEquals("00", parts[0], "traceparent version must be 00"); + // trace-id is 32 hex chars, parent-id is 16 hex chars + assertEquals(32, parts[1].length(), "traceparent trace-id must be 32 hex chars"); + assertEquals(16, parts[2].length(), "traceparent parent-id must be 16 hex chars"); + } + + @Test + void contextPropagationWithPostRequest() throws Exception { + PostMethod post = new PostMethod(BASE_URL + "/propagation"); + post.setRequestBody("{\"data\":\"test\"}"); + try { + int statusCode = client.executeMethod(post); + assertEquals(200, statusCode); + } finally { + post.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + assertEquals("POST", String.valueOf(span.getTag("http.method"))); + assertEquals(200, span.getTag("http.status_code")); + + // Verify trace headers are injected for POST requests too + assertNotNull( + capturedHeaders.get("x-datadog-trace-id"), + "Expected x-datadog-trace-id header on POST request"); + assertNotNull( + capturedHeaders.get("x-datadog-parent-id"), + "Expected x-datadog-parent-id header on POST request"); + + // Verify IDs match the span + assertEquals( + String.valueOf(span.getTraceId().toLong()), + capturedHeaders.get("x-datadog-trace-id"), + "POST request trace ID must match span"); + assertEquals( + String.valueOf(span.getSpanId()), + capturedHeaders.get("x-datadog-parent-id"), + "POST request parent ID must match span"); + } + + @Test + void peerServiceTagsSetFromRequestUrl() throws Exception { + GetMethod get = new GetMethod(BASE_URL + "/success"); + try { + int statusCode = client.executeMethod(get); + assertEquals(200, statusCode); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + + // peer.hostname is the input tag that PeerServiceCalculator uses to compute peer.service + assertEquals( + "localhost", + String.valueOf(span.getTag("peer.hostname")), + "peer.hostname must be set from the request URL host"); + + // peer.port should be set when the URL has a non-default port + assertEquals( + SERVER_PORT, span.getTag("peer.port"), "peer.port must be set from the request URL port"); + } + + @Test + void peerServiceTagsSetOnErrorResponse() throws Exception { + GetMethod get = new GetMethod(BASE_URL + "/error"); + try { + int statusCode = client.executeMethod(get); + assertEquals(500, statusCode); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + + // peer.hostname should still be set even when the server returns an error + assertEquals( + "localhost", + String.valueOf(span.getTag("peer.hostname")), + "peer.hostname must be set even on error responses"); + assertEquals( + SERVER_PORT, span.getTag("peer.port"), "peer.port must be set even on error responses"); + assertEquals(500, span.getTag("http.status_code")); + } + + @Test + void peerServiceTagsSetWithHostConfiguration() throws Exception { + // When using HostConfiguration with a full URL, peer.hostname is extracted from the URI + HostConfiguration hostConfig = new HostConfiguration(); + hostConfig.setHost("localhost", SERVER_PORT, "http"); + GetMethod get = new GetMethod(BASE_URL + "/success"); + try { + int statusCode = client.executeMethod(hostConfig, get); + assertEquals(200, statusCode); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span"); + + // peer.hostname is extracted from the request URI host + assertEquals( + "localhost", + String.valueOf(span.getTag("peer.hostname")), + "peer.hostname must be set when using HostConfiguration with full URL"); + assertEquals( + SERVER_PORT, + span.getTag("peer.port"), + "peer.port must be set when using HostConfiguration with full URL"); + } + + @Test + void peerServiceTagsSetOnConnectionException() throws Exception { + GetMethod get = new GetMethod("http://localhost:1/nonexistent"); + try { + assertThrows( + IOException.class, + () -> { + client.executeMethod(get); + }); + } finally { + get.releaseConnection(); + } + + writer.waitForTraces(1); + List spans = flattenTraces(); + + DDSpan span = findSpanByOperation(spans, "http.request"); + assertNotNull(span, "Expected http.request span even on connection error"); + assertTrue(span.isError(), "Span should be marked as error"); + + // peer.hostname should still be set from the URL even when connection fails + assertEquals( + "localhost", + String.valueOf(span.getTag("peer.hostname")), + "peer.hostname must be set even on connection failure"); + // Port 1 was used in the request URL + assertEquals(1, span.getTag("peer.port"), "peer.port must be set from the request URL"); + } + + // -- helpers -- + + /** + * Validates the complete structure of an HTTP client span, ensuring all required attributes are + * set correctly. This catches regressions in any span attribute rather than relying on individual + * assertions scattered across test methods. + * + * @param span the span to validate + * @param expectedMethod the expected HTTP method (GET, POST, etc.) + * @param expectedPath the expected URL path (e.g. "/success") + * @param expectedStatusCode the expected HTTP status code, or 0 if no status (e.g. connection + * error) + * @param expectedError whether the span should be marked as an error + */ + private void assertHttpClientSpan( + DDSpan span, + String expectedMethod, + String expectedPath, + int expectedStatusCode, + boolean expectedError) { + // Span metadata + assertEquals( + "http.request", + String.valueOf(span.getOperationName()), + "Operation name must be 'http.request'"); + assertEquals("http", String.valueOf(span.getType()), "Span type must be 'http'"); + assertEquals( + expectedMethod + " " + expectedPath, + String.valueOf(span.getResourceName()), + "Resource must follow 'METHOD /path' format"); + assertNotNull(span.getServiceName(), "Service name must be set"); + assertFalse(span.getServiceName().isEmpty(), "Service name must not be empty"); + + // Required tags + assertEquals("client", String.valueOf(span.getTag("span.kind")), "span.kind must be 'client'"); + assertEquals( + expectedMethod, String.valueOf(span.getTag("http.method")), "http.method must be set"); + assertEquals( + "commons-http-client", + String.valueOf(span.getTag("component")), + "component must be 'commons-http-client'"); + + // URL tag + assertNotNull(span.getTag("http.url"), "http.url must be set"); + assertTrue( + String.valueOf(span.getTag("http.url")).contains(expectedPath), + "http.url must contain the request path"); + + // Status code (only set when a response was received) + if (expectedStatusCode > 0) { + assertEquals( + expectedStatusCode, + span.getTag("http.status_code"), + "http.status_code must match response"); + } + + // Error flag + assertEquals(expectedError, span.isError(), "Span error flag must match expected value"); + } + + private List flattenTraces() { + List result = new ArrayList<>(); + for (List trace : writer) { + result.addAll(trace); + } + return result; + } + + private DDSpan findSpanByOperation(List spans, String operationName) { + for (DDSpan span : spans) { + if (operationName.equals(span.getOperationName().toString())) { + return span; + } + } + return null; + } +}