diff --git a/buildSrc/src/main/groovy/MuzzlePlugin.groovy b/buildSrc/src/main/groovy/MuzzlePlugin.groovy index 7b8643cab1..2f6256ed3f 100644 --- a/buildSrc/src/main/groovy/MuzzlePlugin.groovy +++ b/buildSrc/src/main/groovy/MuzzlePlugin.groovy @@ -103,15 +103,19 @@ class MuzzlePlugin implements Plugin { Task runAfter = project.tasks.muzzle for (MuzzleDirective muzzleDirective : project.muzzle.directives) { - project.getLogger().info("configured ${muzzleDirective.assertPass ? 'pass' : 'fail'} directive: ${muzzleDirective.group}:${muzzleDirective.module}:${muzzleDirective.versions}") + project.getLogger().info("configured $muzzleDirective") - muzzleDirectiveToArtifacts(muzzleDirective, system, session).collect() { Artifact singleVersion -> - runAfter = addMuzzleTask(muzzleDirective, singleVersion, project, runAfter, bootstrapProject, toolingProject) - } - if (muzzleDirective.assertInverse) { - inverseOf(muzzleDirective, system, session).collect() { MuzzleDirective inverseDirective -> - muzzleDirectiveToArtifacts(inverseDirective, system, session).collect() { Artifact singleVersion -> - runAfter = addMuzzleTask(inverseDirective, singleVersion, project, runAfter, bootstrapProject, toolingProject) + if (muzzleDirective.coreJdk) { + runAfter = addMuzzleTask(muzzleDirective, null, project, runAfter, bootstrapProject, toolingProject) + } else { + muzzleDirectiveToArtifacts(muzzleDirective, system, session).collect() { Artifact singleVersion -> + runAfter = addMuzzleTask(muzzleDirective, singleVersion, project, runAfter, bootstrapProject, toolingProject) + } + if (muzzleDirective.assertInverse) { + inverseOf(muzzleDirective, system, session).collect() { MuzzleDirective inverseDirective -> + muzzleDirectiveToArtifacts(inverseDirective, system, session).collect() { Artifact singleVersion -> + runAfter = addMuzzleTask(inverseDirective, singleVersion, project, runAfter, bootstrapProject, toolingProject) + } } } } @@ -258,17 +262,25 @@ class MuzzlePlugin implements Plugin { * @return The created muzzle task. */ private static Task addMuzzleTask(MuzzleDirective muzzleDirective, Artifact versionArtifact, Project instrumentationProject, Task runAfter, Project bootstrapProject, Project toolingProject) { - def taskName = "muzzle-Assert${muzzleDirective.assertPass ? "Pass" : "Fail"}-$versionArtifact.groupId-$versionArtifact.artifactId-$versionArtifact.version${muzzleDirective.name ? "-${muzzleDirective.getNameSlug()}" : ""}" - def config = instrumentationProject.configurations.create(taskName) - def dep = instrumentationProject.dependencies.create("$versionArtifact.groupId:$versionArtifact.artifactId:$versionArtifact.version") { - transitive = true + def taskName + if (muzzleDirective.coreJdk) { + taskName = "muzzle-Assert$muzzleDirective" + } else { + taskName = "muzzle-Assert${muzzleDirective.assertPass ? "Pass" : "Fail"}-$versionArtifact.groupId-$versionArtifact.artifactId-$versionArtifact.version${muzzleDirective.name ? "-${muzzleDirective.getNameSlug()}" : ""}" } - // The following optional transitive dependencies are brought in by some legacy module such as log4j 1.x but are no - // longer bundled with the JVM and have to be excluded for the muzzle tests to be able to run. - dep.exclude group: 'com.sun.jdmk', module: 'jmxtools' - dep.exclude group: 'com.sun.jmx', module: 'jmxri' + def config = instrumentationProject.configurations.create(taskName) - config.dependencies.add(dep) + if (!muzzleDirective.coreJdk) { + def dep = instrumentationProject.dependencies.create("$versionArtifact.groupId:$versionArtifact.artifactId:$versionArtifact.version") { + transitive = true + } + // The following optional transitive dependencies are brought in by some legacy module such as log4j 1.x but are no + // longer bundled with the JVM and have to be excluded for the muzzle tests to be able to run. + dep.exclude group: 'com.sun.jdmk', module: 'jmxtools' + dep.exclude group: 'com.sun.jmx', module: 'jmxri' + + config.dependencies.add(dep) + } for (String additionalDependency : muzzleDirective.additionalDependencies) { config.dependencies.add(instrumentationProject.dependencies.create(additionalDependency) { transitive = true @@ -369,6 +381,11 @@ class MuzzleDirective { List additionalDependencies = new ArrayList<>() boolean assertPass boolean assertInverse = false + boolean coreJdk = false + + void coreJdk() { + coreJdk = true + } /** * Adds extra dependencies to the current muzzle test. @@ -391,6 +408,14 @@ class MuzzleDirective { return name.trim().replaceAll("[^a-zA-Z0-9]+", "-") } + + String toString() { + if (coreJdk) { + return "${assertPass ? 'Pass' : 'Fail'}-core-jdk" + } else { + return "${assertPass ? 'pass' : 'fail'} $group:$module:$versions" + } + } } /** diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index fe179f5423..d056feef02 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -16,6 +16,7 @@ import org.slf4j.LoggerFactory; *

The intention is for this class to be loaded by bootstrap classloader to make sure we have * unimpeded access to the rest of Datadog's agent parts. */ +// We cannot use lombok here because we need to configure logger first public class Agent { private static final String SIMPLE_LOGGER_SHOW_DATE_TIME_PROPERTY = @@ -28,18 +29,18 @@ public class Agent { "datadog.slf4j.simpleLogger.defaultLogLevel"; // We cannot use lombok here because we need to configure logger first - private static final Logger LOGGER; + private static final Logger log; static { // We can configure logger here because datadog.trace.agent.AgentBootstrap doesn't touch it. configureLogger(); - LOGGER = LoggerFactory.getLogger(Agent.class); + log = LoggerFactory.getLogger(Agent.class); } // fields must be managed under class lock private static ClassLoader AGENT_CLASSLOADER = null; - public static void start(final Instrumentation inst, final URL bootstrapURL) throws Exception { + public static void start(final Instrumentation inst, final URL bootstrapURL) { startDatadogAgent(inst, bootstrapURL); final boolean appUsingCustomLogManager = isAppUsingCustomLogManager(); @@ -64,28 +65,30 @@ public class Agent { * logging facility. */ if (isJavaBefore9WithJFR() && appUsingCustomLogManager) { - LOGGER.debug("Custom logger detected. Delaying Datadog Tracer initialization."); - registerLogManagerCallback(new InstallDatadogTracerCallback(inst, bootstrapURL)); + log.debug("Custom logger detected. Delaying Datadog Tracer initialization."); + registerLogManagerCallback(new InstallDatadogTracerCallback(bootstrapURL)); } else { - installDatadogTracer(inst, bootstrapURL); + installDatadogTracer(); } } - private static void registerLogManagerCallback(final Runnable callback) throws Exception { - final Class agentInstallerClass = - AGENT_CLASSLOADER.loadClass("datadog.trace.agent.tooling.AgentInstaller"); - final Method registerCallbackMethod = - agentInstallerClass.getMethod("registerClassLoadCallback", String.class, Runnable.class); - registerCallbackMethod.invoke(null, "java.util.logging.LogManager", callback); + private static void registerLogManagerCallback(final ClassLoadCallBack callback) { + try { + final Class agentInstallerClass = + AGENT_CLASSLOADER.loadClass("datadog.trace.agent.tooling.AgentInstaller"); + final Method registerCallbackMethod = + agentInstallerClass.getMethod("registerClassLoadCallback", String.class, Runnable.class); + registerCallbackMethod.invoke(null, "java.util.logging.LogManager", callback); + } catch (final Exception ex) { + log.error("Error registering callback for " + callback.getName(), ex); + } } protected abstract static class ClassLoadCallBack implements Runnable { - final Instrumentation inst; final URL bootstrapURL; - ClassLoadCallBack(final Instrumentation inst, final URL bootstrapURL) { - this.inst = inst; + ClassLoadCallBack(final URL bootstrapURL) { this.bootstrapURL = bootstrapURL; } @@ -104,7 +107,7 @@ public class Agent { try { execute(); } catch (final Exception e) { - LOGGER.error("Failed to run class loader callback {}", getName(), e); + log.error("Failed to run class loader callback {}", getName(), e); } } }); @@ -115,12 +118,12 @@ public class Agent { public abstract String getName(); - public abstract void execute() throws Exception; + public abstract void execute(); } protected static class InstallDatadogTracerCallback extends ClassLoadCallBack { - InstallDatadogTracerCallback(final Instrumentation inst, final URL bootstrapURL) { - super(inst, bootstrapURL); + InstallDatadogTracerCallback(final URL bootstrapURL) { + super(bootstrapURL); } @Override @@ -129,18 +132,18 @@ public class Agent { } @Override - public void execute() throws Exception { - installDatadogTracer(inst, bootstrapURL); + public void execute() { + installDatadogTracer(); } } private static synchronized void startDatadogAgent( - final Instrumentation inst, final URL bootstrapURL) throws Exception { + final Instrumentation inst, final URL bootstrapURL) { if (AGENT_CLASSLOADER == null) { - final ClassLoader agentClassLoader = - createDatadogClassLoader("agent-tooling-and-instrumentation.isolated", bootstrapURL); final ClassLoader contextLoader = Thread.currentThread().getContextClassLoader(); try { + final ClassLoader agentClassLoader = + createDatadogClassLoader("agent-tooling-and-instrumentation.isolated", bootstrapURL); Thread.currentThread().setContextClassLoader(agentClassLoader); final Class agentInstallerClass = agentClassLoader.loadClass("datadog.trace.agent.tooling.AgentInstaller"); @@ -148,14 +151,15 @@ public class Agent { agentInstallerClass.getMethod("installBytebuddyAgent", Instrumentation.class); agentInstallerMethod.invoke(null, inst); AGENT_CLASSLOADER = agentClassLoader; + } catch (final Throwable ex) { + log.error("Throwable thrown while installing the Datadog Agent", ex); } finally { Thread.currentThread().setContextClassLoader(contextLoader); } } } - private static synchronized void installDatadogTracer( - final Instrumentation inst, final URL bootstrapURL) throws Exception { + private static synchronized void installDatadogTracer() { if (AGENT_CLASSLOADER == null) { throw new IllegalStateException("Datadog agent should have been started already"); } @@ -171,6 +175,8 @@ public class Agent { tracerInstallerMethod.invoke(null); final Method logVersionInfoMethod = tracerInstallerClass.getMethod("logVersionInfo"); logVersionInfoMethod.invoke(null); + } catch (final Throwable ex) { + log.error("Throwable thrown while installing the Datadog Tracer", ex); } finally { Thread.currentThread().setContextClassLoader(contextLoader); } @@ -263,8 +269,8 @@ public class Agent { System.getenv(tracerCustomLogManSysprop.replace('.', '_').toUpperCase()); if (customLogManagerProp != null || customLogManagerEnv != null) { - LOGGER.debug("Prop - customlogmanager: " + customLogManagerProp); - LOGGER.debug("Env - customlogmanager: " + customLogManagerEnv); + log.debug("Prop - customlogmanager: " + customLogManagerProp); + log.debug("Env - customlogmanager: " + customLogManagerEnv); // Allow setting to skip these automatic checks: return Boolean.parseBoolean(customLogManagerProp) || Boolean.parseBoolean(customLogManagerEnv); @@ -272,7 +278,7 @@ public class Agent { final String jbossHome = System.getenv("JBOSS_HOME"); if (jbossHome != null) { - LOGGER.debug("Env - jboss: " + jbossHome); + log.debug("Env - jboss: " + jbossHome); // JBoss/Wildfly is known to set a custom log manager after startup. // Originally we were checking for the presence of a jboss class, // but it seems some non-jboss applications have jboss classes on the classpath. @@ -285,8 +291,8 @@ public class Agent { if (logManagerProp != null) { final boolean onSysClasspath = ClassLoader.getSystemResource(logManagerProp.replaceAll("\\.", "/") + ".class") != null; - LOGGER.debug("Prop - logging.manager: " + logManagerProp); - LOGGER.debug("logging.manager on system classpath: " + onSysClasspath); + log.debug("Prop - logging.manager: " + logManagerProp); + log.debug("logging.manager on system classpath: " + onSysClasspath); // Some applications set java.util.logging.manager but never actually initialize the logger. // Check to see if the configured manager is on the system classpath. // If so, it should be safe to initialize jmxfetch which will setup the log manager. diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/rmi/ThreadLocalContext.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/rmi/ThreadLocalContext.java new file mode 100644 index 0000000000..d4c7e6fc81 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/rmi/ThreadLocalContext.java @@ -0,0 +1,24 @@ +package datadog.trace.bootstrap.instrumentation.rmi; + +import datadog.trace.instrumentation.api.AgentSpan; + +public class ThreadLocalContext { + public static final ThreadLocalContext THREAD_LOCAL_CONTEXT = new ThreadLocalContext(); + private final ThreadLocal local; + + public ThreadLocalContext() { + local = new ThreadLocal<>(); + } + + public void set(final AgentSpan.Context context) { + local.set(context); + } + + public AgentSpan.Context getAndResetContext() { + final AgentSpan.Context context = local.get(); + if (context != null) { + local.remove(); + } + return context; + } +} diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java index a7b425e261..8af04df97e 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java @@ -92,6 +92,7 @@ public class AgentInstaller { not( named("java.net.URL") .or(named("java.net.HttpURLConnection")) + .or(nameStartsWith("java.rmi.")) .or(nameStartsWith("java.util.concurrent.")) .or( nameStartsWith("java.util.logging.") @@ -111,6 +112,8 @@ public class AgentInstaller { .and( not( nameStartsWith("sun.net.www.protocol.") + .or(nameStartsWith("sun.rmi.server")) + .or(nameStartsWith("sun.rmi.transport")) .or(named("sun.net.www.http.HttpClient"))))) .or(nameStartsWith("jdk.")) .or(nameStartsWith("org.aspectj.")) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentTracerImpl.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentTracerImpl.java index 9281bafe8e..e6beb04802 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentTracerImpl.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentTracerImpl.java @@ -294,7 +294,11 @@ public final class AgentTracerImpl implements TracerAPI { private Extractor(final C carrier, final Getter getter) { extracted = new HashMap<>(); for (final String key : getter.keys(carrier)) { - extracted.put(key, getter.get(carrier, key)); + // extracted header value + String s = getter.get(carrier, key); + // in case of multiple values in the header, need to parse + if (s != null) s = s.split(",")[0].trim(); + extracted.put(key, s); } } diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy index 405cdd04f6..bac68a4426 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy @@ -58,6 +58,9 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest resp.withEntity(endpoint.getBody) + case QUERY_PARAM => resp.withEntity(uri.queryString().orNull) case REDIRECT => resp.withHeaders(headers.Location(endpoint.getBody)) case ERROR => resp.withEntity(endpoint.getBody) case EXCEPTION => throw new Exception(endpoint.getBody) diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestSyncWebServer.scala b/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestSyncWebServer.scala index 11db37268e..fb60cf6d97 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestSyncWebServer.scala +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestSyncWebServer.scala @@ -23,6 +23,7 @@ object AkkaHttpTestSyncWebServer { val resp = HttpResponse(status = endpoint.getStatus) endpoint match { case SUCCESS => resp.withEntity(endpoint.getBody) + case QUERY_PARAM => resp.withEntity(uri.queryString().orNull) case REDIRECT => resp.withHeaders(headers.Location(endpoint.getBody)) case ERROR => resp.withEntity(endpoint.getBody) case EXCEPTION => throw new Exception(endpoint.getBody) diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestWebServer.scala b/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestWebServer.scala index caeb2379cc..6afb6fd788 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestWebServer.scala +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestWebServer.scala @@ -23,6 +23,8 @@ object AkkaHttpTestWebServer { val route = { //handleExceptions(exceptionHandler) { path(SUCCESS.rawPath) { complete(HttpResponse(status = SUCCESS.getStatus).withEntity(SUCCESS.getBody)) + } ~ path(QUERY_PARAM.rawPath) { + complete(HttpResponse(status = QUERY_PARAM.getStatus).withEntity(SUCCESS.getBody)) } ~ path(REDIRECT.rawPath) { redirect(Uri(REDIRECT.getBody), StatusCodes.Found) } ~ path(ERROR.rawPath) { diff --git a/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardAsyncTest.groovy b/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardAsyncTest.groovy index 29261d0f62..f2d7e3b192 100644 --- a/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardAsyncTest.groovy +++ b/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardAsyncTest.groovy @@ -4,6 +4,7 @@ import io.dropwizard.setup.Bootstrap import io.dropwizard.setup.Environment import javax.ws.rs.GET import javax.ws.rs.Path +import javax.ws.rs.QueryParam import javax.ws.rs.container.AsyncResponse import javax.ws.rs.container.Suspended import javax.ws.rs.core.Response @@ -12,6 +13,7 @@ import java.util.concurrent.Executors import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -56,6 +58,14 @@ class DropwizardAsyncTest extends DropwizardTest { } } + @GET + @Path("query") + Response query_param(@QueryParam("some") String param) { + controller(QUERY_PARAM) { + Response.status(QUERY_PARAM.status).entity("some=$param".toString()).build() + } + } + @GET @Path("redirect") void redirect(@Suspended final AsyncResponse asyncResponse) { diff --git a/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardTest.groovy b/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardTest.groovy index 08f6548227..ce1f5718d2 100644 --- a/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardTest.groovy +++ b/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardTest.groovy @@ -12,14 +12,15 @@ import io.dropwizard.setup.Bootstrap import io.dropwizard.setup.Environment import io.dropwizard.testing.ConfigOverride import io.dropwizard.testing.DropwizardTestSupport -import org.eclipse.jetty.servlet.ServletHandler - import javax.ws.rs.GET import javax.ws.rs.Path +import javax.ws.rs.QueryParam import javax.ws.rs.core.Response +import org.eclipse.jetty.servlet.ServletHandler import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -121,6 +122,9 @@ class DropwizardTest extends HttpServerTest { "error.type" { it == null || it == Exception.name } "error.stack" { it == null || it instanceof String } } + if (endpoint.query) { + "$DDTags.HTTP_QUERY" endpoint.query + } } } } diff --git a/dd-java-agent/instrumentation/glassfish/src/test/groovy/TestServlets.java b/dd-java-agent/instrumentation/glassfish/src/test/groovy/TestServlets.java index 74cf401108..5b2ef3904d 100644 --- a/dd-java-agent/instrumentation/glassfish/src/test/groovy/TestServlets.java +++ b/dd-java-agent/instrumentation/glassfish/src/test/groovy/TestServlets.java @@ -26,6 +26,25 @@ public class TestServlets { } } + @WebServlet("/query") + public static class Query extends HttpServlet { + @Override + protected void service(final HttpServletRequest req, final HttpServletResponse resp) { + final HttpServerTest.ServerEndpoint endpoint = + HttpServerTest.ServerEndpoint.forPath(req.getServletPath()); + HttpServerTest.controller( + endpoint, + new Closure(null) { + public Object doCall() throws Exception { + resp.setContentType("text/plain"); + resp.setStatus(endpoint.getStatus()); + resp.getWriter().print(req.getQueryString()); + return null; + } + }); + } + } + @WebServlet("/redirect") public static class Redirect extends HttpServlet { @Override diff --git a/dd-java-agent/instrumentation/grizzly-2/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyDecorator.java b/dd-java-agent/instrumentation/grizzly-2/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyDecorator.java index 3c6a601d72..e5f6cca8dd 100644 --- a/dd-java-agent/instrumentation/grizzly-2/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyDecorator.java +++ b/dd-java-agent/instrumentation/grizzly-2/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyDecorator.java @@ -16,7 +16,14 @@ public class GrizzlyDecorator extends HttpServerDecorator { } } + @GET + @Path("query") + Response query_param(@QueryParam("some") String param) { + controller(QUERY_PARAM) { + Response.status(QUERY_PARAM.status).entity("some=$param".toString()).build() + } + } + @GET @Path("redirect") Response redirect() { diff --git a/dd-java-agent/instrumentation/http-url-connection/http-url-connection.gradle b/dd-java-agent/instrumentation/http-url-connection/http-url-connection.gradle index 38851b24a8..075d73deed 100644 --- a/dd-java-agent/instrumentation/http-url-connection/http-url-connection.gradle +++ b/dd-java-agent/instrumentation/http-url-connection/http-url-connection.gradle @@ -1,3 +1,9 @@ +muzzle { + pass { + coreJdk() + } +} + apply from: "${rootDir}/gradle/java.gradle" dependencies { diff --git a/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle b/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle index 4ff2d984ab..77de36fba9 100644 --- a/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle +++ b/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle @@ -1,3 +1,10 @@ +// This won't work until the akka and scala integrations are split into separate projects. +//muzzle { +// pass { +// coreJdk() +// } +//} + apply from: "${rootDir}/gradle/java.gradle" apply from: "${rootDir}/gradle/test-with-scala.gradle" apply from: "${rootDir}/gradle/test-with-kotlin.gradle" diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-client-2.0/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java index 66ecce3fb1..19af4fe24e 100644 --- a/dd-java-agent/instrumentation/jax-rs-client-2.0/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java @@ -8,10 +8,11 @@ import static net.bytebuddy.matcher.ElementMatchers.returns; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import java.util.Map; -import javax.ws.rs.client.ClientBuilder; +import javax.ws.rs.client.Client; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.implementation.bytecode.assign.Assigner; import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) @@ -48,9 +49,13 @@ public final class JaxRsClientInstrumentation extends Instrumenter.Default { public static class ClientBuilderAdvice { - @Advice.OnMethodEnter - public static void registerFeature(@Advice.This final ClientBuilder builder) { - builder.register(ClientTracingFeature.class); + @Advice.OnMethodExit + public static void registerFeature( + @Advice.Return(typing = Assigner.Typing.DYNAMIC) final Client client) { + // Register on the generated client instead of the builder + // The build() can be called multiple times and is not thread safe + // A client is only created once + client.register(ClientTracingFeature.class); } } } diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/src/test/groovy/JaxMultithreadedClientTest.groovy b/dd-java-agent/instrumentation/jax-rs-client-2.0/src/test/groovy/JaxMultithreadedClientTest.groovy new file mode 100644 index 0000000000..e7695a9082 --- /dev/null +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/src/test/groovy/JaxMultithreadedClientTest.groovy @@ -0,0 +1,54 @@ +import datadog.trace.agent.test.AgentTestRunner +import org.glassfish.jersey.client.JerseyClientBuilder +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.util.concurrent.AsyncConditions + +import javax.ws.rs.client.Client + +import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer + +class JaxMultithreadedClientTest extends AgentTestRunner { + + @AutoCleanup + @Shared + def server = httpServer { + handlers { + prefix("success") { + String msg = "Hello." + response.status(200).send(msg) + } + } + } + + def "multiple threads using the same builder works"() { + given: + def conds = new AsyncConditions(10) + def uri = server.address.resolve("/success") + def builder = new JerseyClientBuilder() + + // Start 10 threads and do 50 requests each + when: + (1..10).each { + Thread.start { + boolean hadErrors = (1..50).any { + try { + Client client = builder.build() + client.target(uri).request().get() + } catch (Exception e) { + e.printStackTrace() + return true + } + return false + } + + conds.evaluate { + assert !hadErrors + } + } + } + + then: + conds.await(10) + } +} diff --git a/dd-java-agent/instrumentation/jdbc/jdbc.gradle b/dd-java-agent/instrumentation/jdbc/jdbc.gradle index 66e4080b7b..b775fae9d9 100644 --- a/dd-java-agent/instrumentation/jdbc/jdbc.gradle +++ b/dd-java-agent/instrumentation/jdbc/jdbc.gradle @@ -1,3 +1,9 @@ +muzzle { + pass { + coreJdk() + } +} + apply from: "${rootDir}/gradle/java.gradle" apply plugin: 'org.unbroken-dome.test-sets' diff --git a/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/JettyDecorator.java b/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/JettyDecorator.java index 16101b2fbb..7eaa7bb48a 100644 --- a/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/JettyDecorator.java +++ b/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/JettyDecorator.java @@ -28,7 +28,14 @@ public class JettyDecorator @Override protected URI url(final HttpServletRequest httpServletRequest) throws URISyntaxException { - return new URI(httpServletRequest.getRequestURL().toString()); + return new URI( + httpServletRequest.getScheme(), + null, + httpServletRequest.getServerName(), + httpServletRequest.getServerPort(), + httpServletRequest.getRequestURI(), + httpServletRequest.getQueryString(), + null); } @Override diff --git a/dd-java-agent/instrumentation/jetty-8/src/test/groovy/JettyHandlerTest.groovy b/dd-java-agent/instrumentation/jetty-8/src/test/groovy/JettyHandlerTest.groovy index e998ccfa06..6933cc77f6 100644 --- a/dd-java-agent/instrumentation/jetty-8/src/test/groovy/JettyHandlerTest.groovy +++ b/dd-java-agent/instrumentation/jetty-8/src/test/groovy/JettyHandlerTest.groovy @@ -4,19 +4,19 @@ import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import datadog.trace.instrumentation.api.Tags import datadog.trace.instrumentation.jetty8.JettyDecorator +import javax.servlet.DispatcherType +import javax.servlet.ServletException +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse import org.eclipse.jetty.server.Request import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.AbstractHandler import org.eclipse.jetty.server.handler.ErrorHandler -import javax.servlet.DispatcherType -import javax.servlet.ServletException -import javax.servlet.http.HttpServletRequest -import javax.servlet.http.HttpServletResponse - import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -79,6 +79,10 @@ class JettyHandlerTest extends HttpServerTest { response.status = endpoint.status response.writer.print(endpoint.body) break + case QUERY_PARAM: + response.status = endpoint.status + response.writer.print(request.queryString) + break case REDIRECT: response.sendRedirect(endpoint.body) break @@ -138,6 +142,9 @@ class JettyHandlerTest extends HttpServerTest { "error.type" { it == null || it == Exception.name } "error.stack" { it == null || it instanceof String } } + if (endpoint.query) { + "$DDTags.HTTP_QUERY" endpoint.query + } } } } diff --git a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/TracingIterator.java b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/TracingIterator.java index 87723f863c..ae1dbc6d8f 100644 --- a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/TracingIterator.java +++ b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/TracingIterator.java @@ -59,6 +59,7 @@ public class TracingIterator implements Iterator { decorator.afterStart(span); decorator.onConsume(span, next); currentScope = activateSpan(span, true); + currentScope.setAsyncPropagation(true); } } catch (final Exception e) { log.debug("Error during decoration", e); diff --git a/dd-java-agent/instrumentation/kafka-streams-0.11/src/main/java/datadog/trace/instrumentation/kafka_streams/KafkaStreamsProcessorInstrumentation.java b/dd-java-agent/instrumentation/kafka-streams-0.11/src/main/java/datadog/trace/instrumentation/kafka_streams/KafkaStreamsProcessorInstrumentation.java index 88f0fba8b4..3d156892b9 100644 --- a/dd-java-agent/instrumentation/kafka-streams-0.11/src/main/java/datadog/trace/instrumentation/kafka_streams/KafkaStreamsProcessorInstrumentation.java +++ b/dd-java-agent/instrumentation/kafka-streams-0.11/src/main/java/datadog/trace/instrumentation/kafka_streams/KafkaStreamsProcessorInstrumentation.java @@ -81,7 +81,7 @@ public class KafkaStreamsProcessorInstrumentation { CONSUMER_DECORATE.afterStart(span); CONSUMER_DECORATE.onConsume(span, record); - activateSpan(span, true); + activateSpan(span, true).setAsyncPropagation(true); } } } @@ -119,6 +119,7 @@ public class KafkaStreamsProcessorInstrumentation { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan(@Advice.Thrown final Throwable throwable) { + // This is dangerous... we assume the span/scope is the one we expect, but it may not be. final AgentSpan span = activeSpan(); if (span != null) { CONSUMER_DECORATE.onError(span, throwable); diff --git a/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy b/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy index 601d07580f..bd2b041204 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy +++ b/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy @@ -24,6 +24,7 @@ import io.netty.util.CharsetUtil import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_LENGTH @@ -48,7 +49,8 @@ class Netty40ServerTest extends HttpServerTest if (msg instanceof HttpRequest) { - ServerEndpoint endpoint = ServerEndpoint.forPath((msg as HttpRequest).uri) + def uri = URI.create((msg as HttpRequest).uri) + ServerEndpoint endpoint = ServerEndpoint.forPath(uri.path) ctx.write controller(endpoint) { ByteBuf content = null FullHttpResponse response = null @@ -58,6 +60,10 @@ class Netty40ServerTest extends HttpServerTest if (msg instanceof HttpRequest) { - ServerEndpoint endpoint = ServerEndpoint.forPath((msg as HttpRequest).uri) + def uri = URI.create((msg as HttpRequest).uri) + ServerEndpoint endpoint = ServerEndpoint.forPath(uri.path) ctx.write controller(endpoint) { ByteBuf content = null FullHttpResponse response = null @@ -57,6 +59,10 @@ class Netty41ServerTest extends HttpServerTest { controller(SUCCESS) { Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) } + } as Supplier) + .GET(QUERY_PARAM.getPath()).routeTo({ + controller(QUERY_PARAM) { + Results.status(QUERY_PARAM.getStatus(), QUERY_PARAM.getBody()) + } } as Supplier) .GET(REDIRECT.getPath()).routeTo({ controller(REDIRECT) { @@ -96,6 +102,9 @@ class PlayServerTest extends HttpServerTest { if (endpoint == EXCEPTION) { errorTags(Exception, EXCEPTION.body) } + if (endpoint.query) { + "$DDTags.HTTP_QUERY" endpoint.query + } } } } diff --git a/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy index 1695438450..90f5f83ac3 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy +++ b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy @@ -14,6 +14,7 @@ import java.util.function.Supplier import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -36,6 +37,13 @@ class PlayAsyncServerTest extends PlayServerTest { Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) } }, execContext) + } as Supplier) + .GET(QUERY_PARAM.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(QUERY_PARAM) { + Results.status(QUERY_PARAM.getStatus(), QUERY_PARAM.getBody()) + } + }, execContext) } as Supplier) .GET(REDIRECT.getPath()).routeAsync({ CompletableFuture.supplyAsync({ diff --git a/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy index 22913be9a6..6ae07fe1e6 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy @@ -18,6 +18,7 @@ import java.util.function.Supplier import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -30,6 +31,11 @@ class PlayServerTest extends HttpServerTest { controller(SUCCESS) { Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) } + } as Supplier) + .GET(QUERY_PARAM.getPath()).routeTo({ + controller(QUERY_PARAM) { + Results.status(QUERY_PARAM.getStatus(), QUERY_PARAM.getBody()) + } } as Supplier) .GET(REDIRECT.getPath()).routeTo({ controller(REDIRECT) { @@ -98,6 +104,9 @@ class PlayServerTest extends HttpServerTest { if (endpoint == EXCEPTION) { errorTags(Exception, EXCEPTION.body) } + if (endpoint.query) { + "$DDTags.HTTP_QUERY" endpoint.query + } } } } @@ -124,6 +133,9 @@ class PlayServerTest extends HttpServerTest { "error.type" { it == null || it == Exception.name } "error.stack" { it == null || it instanceof String } } + if (endpoint.query) { + "$DDTags.HTTP_QUERY" endpoint.query + } } } } diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java index 79c824777c..c923dff319 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java @@ -7,6 +7,7 @@ import datadog.trace.instrumentation.api.AgentSpan; import java.net.URI; import lombok.extern.slf4j.Slf4j; import ratpack.handling.Context; +import ratpack.http.HttpUrlBuilder; import ratpack.http.Request; import ratpack.http.Response; import ratpack.http.Status; @@ -37,7 +38,9 @@ public class RatpackServerDecorator extends HttpServerDecorator + controller(endpoint) { + context.response.status(endpoint.status).send(request.query) + } + } + } + } prefix(REDIRECT.rawPath()) { all { Promise.sync { diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/server/RatpackForkedHttpServerTest.groovy b/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/server/RatpackForkedHttpServerTest.groovy index 39b1547535..255a7ab286 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/server/RatpackForkedHttpServerTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/server/RatpackForkedHttpServerTest.groovy @@ -6,6 +6,7 @@ import ratpack.test.embed.EmbeddedApp import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -32,6 +33,17 @@ class RatpackForkedHttpServerTest extends RatpackHttpServerTest { } } } + prefix(QUERY_PARAM.rawPath()) { + all { + Promise.sync { + QUERY_PARAM + }.fork().then { ServerEndpoint endpoint -> + controller(endpoint) { + context.response.status(endpoint.status).send(request.query) + } + } + } + } prefix(REDIRECT.rawPath()) { all { Promise.sync { diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/server/RatpackHttpServerTest.groovy b/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/server/RatpackHttpServerTest.groovy index 0e953487eb..42c4c0a0ab 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/server/RatpackHttpServerTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/server/RatpackHttpServerTest.groovy @@ -15,6 +15,7 @@ import ratpack.test.embed.EmbeddedApp import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -37,6 +38,13 @@ class RatpackHttpServerTest extends HttpServerTest typeMatcher() { + return not(isInterface()).and(safeHasSuperType(named("sun.rmi.server.UnicastRef"))); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + packageName + ".RmiClientDecorator" + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod() + .and(named("invoke")) + .and(takesArgument(0, named("java.rmi.Remote"))) + .and(takesArgument(1, named("java.lang.reflect.Method"))), + getClass().getName() + "$RmiClientAdvice"); + } + + public static class RmiClientAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope onEnter(@Advice.Argument(value = 1) final Method method) { + if (activeSpan() == null) { + return null; + } + final AgentSpan span = + startSpan("rmi.invoke") + .setTag(DDTags.RESOURCE_NAME, DECORATE.spanNameForMethod(method)) + .setTag("span.origin.type", method.getDeclaringClass().getCanonicalName()); + + DECORATE.afterStart(span); + return activateSpan(span, true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { + if (scope == null) { + return; + } + DECORATE.onError(scope, throwable); + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/ContextPayload.java b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/ContextPayload.java new file mode 100644 index 0000000000..a7fe9b1ff5 --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/ContextPayload.java @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.rmi.context; + +import static datadog.trace.instrumentation.api.AgentTracer.propagate; + +import datadog.trace.instrumentation.api.AgentPropagation; +import datadog.trace.instrumentation.api.AgentSpan; +import java.io.IOException; +import java.io.ObjectInput; +import java.io.ObjectOutput; +import java.util.HashMap; +import java.util.Map; +import lombok.Getter; +import lombok.extern.slf4j.Slf4j; + +/** ContextPayload wraps context information shared between client and server */ +@Slf4j +public class ContextPayload { + @Getter private final Map context; + public static final ExtractAdapter GETTER = new ExtractAdapter(); + public static final InjectAdapter SETTER = new InjectAdapter(); + + public ContextPayload() { + context = new HashMap<>(); + } + + public ContextPayload(final Map context) { + this.context = context; + } + + public static ContextPayload from(final AgentSpan span) { + final ContextPayload payload = new ContextPayload(); + propagate().inject(span, payload, SETTER); + return payload; + } + + public static ContextPayload read(final ObjectInput oi) throws IOException { + try { + final Object object = oi.readObject(); + if (object instanceof Map) { + return new ContextPayload((Map) object); + } + } catch (final ClassCastException | ClassNotFoundException ex) { + log.debug("Error reading object", ex); + } + + return null; + } + + public void write(final ObjectOutput out) throws IOException { + out.writeObject(context); + } + + public static class ExtractAdapter implements AgentPropagation.Getter { + @Override + public Iterable keys(final ContextPayload carrier) { + return carrier.getContext().keySet(); + } + + @Override + public String get(final ContextPayload carrier, final String key) { + return carrier.getContext().get(key); + } + } + + public static class InjectAdapter implements AgentPropagation.Setter { + @Override + public void set(final ContextPayload carrier, final String key, final String value) { + carrier.getContext().put(key, value); + } + } +} diff --git a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/ContextPropagator.java b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/ContextPropagator.java new file mode 100644 index 0000000000..2c8a829f7d --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/ContextPropagator.java @@ -0,0 +1,112 @@ +package datadog.trace.instrumentation.rmi.context; + +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.instrumentation.api.AgentSpan; +import java.io.IOException; +import java.io.ObjectOutput; +import java.rmi.NoSuchObjectException; +import java.rmi.server.ObjID; +import lombok.extern.slf4j.Slf4j; +import sun.rmi.transport.Connection; +import sun.rmi.transport.StreamRemoteCall; +import sun.rmi.transport.TransportConstants; + +@Slf4j +public class ContextPropagator { + // Internal RMI object ids that we don't want to trace + private static final ObjID ACTIVATOR_ID = new ObjID(ObjID.ACTIVATOR_ID); + private static final ObjID DGC_ID = new ObjID(ObjID.DGC_ID); + private static final ObjID REGISTRY_ID = new ObjID(ObjID.REGISTRY_ID); + + // RMI object id used to identify DataDog instrumentation + public static final ObjID DD_CONTEXT_CALL_ID = new ObjID("Datadog.v1.context_call".hashCode()); + + // Operation id used for checking context propagation is possible + // RMI expects these operations to have negative identifier, as positive ones mean legacy + // precompiled Stubs would be used instead + private static final int CONTEXT_CHECK_CALL_OPERATION_ID = -1; + // Seconds step of context propagation which contains actual payload + private static final int CONTEXT_PAYLOAD_OPERATION_ID = -2; + + public static final ContextPropagator PROPAGATOR = new ContextPropagator(); + + public boolean isRMIInternalObject(final ObjID id) { + return ACTIVATOR_ID.equals(id) || DGC_ID.equals(id) || REGISTRY_ID.equals(id); + } + + public boolean isOperationWithPayload(final int operationId) { + return operationId == CONTEXT_PAYLOAD_OPERATION_ID; + } + + public void attemptToPropagateContext( + final ContextStore knownConnections, + final Connection c, + final AgentSpan span) { + if (checkIfContextCanBePassed(knownConnections, c)) { + if (!syntheticCall(c, ContextPayload.from(span), CONTEXT_PAYLOAD_OPERATION_ID)) { + log.debug("Couldn't send context payload"); + } + } + } + + private boolean checkIfContextCanBePassed( + final ContextStore knownConnections, final Connection c) { + final Boolean storedResult = knownConnections.get(c); + if (storedResult != null) { + return storedResult; + } + + final boolean result = syntheticCall(c, null, CONTEXT_CHECK_CALL_OPERATION_ID); + knownConnections.put(c, result); + return result; + } + + /** @returns true when no error happened during call */ + private boolean syntheticCall( + final Connection c, final ContextPayload payload, final int operationId) { + final StreamRemoteCall shareContextCall = new StreamRemoteCall(c); + try { + c.getOutputStream().write(TransportConstants.Call); + + final ObjectOutput out = shareContextCall.getOutputStream(); + + DD_CONTEXT_CALL_ID.write(out); + + // call header, part 2 (read by Dispatcher) + out.writeInt(operationId); // in normal call this is method number (operation index) + out.writeLong(operationId); // in normal RMI call this holds stub/skeleton hash + + // Payload should be sent only after we make sure we're connected to instrumented server + // + // if method is not found by un-instrumented code then writing payload will cause an exception + // in RMI server - as the payload will be interpreted as another call + // but it will not be parsed correctly - closing connection + if (payload != null) { + payload.write(out); + } + + try { + shareContextCall.executeCall(); + } catch (final Exception e) { + final Exception ex = shareContextCall.getServerException(); + if (ex != null) { + if (ex instanceof NoSuchObjectException) { + return false; + } else { + log.debug("Server error when executing synthetic call", ex); + } + } else { + log.debug("Error executing synthetic call", e); + } + return false; + } finally { + shareContextCall.done(); + } + + } catch (final IOException e) { + log.debug("Communication error executing synthetic call", e); + return false; + } + return true; + } +} diff --git a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/client/RmiClientContextInstrumentation.java b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/client/RmiClientContextInstrumentation.java new file mode 100644 index 0000000000..1e63a346e5 --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/client/RmiClientContextInstrumentation.java @@ -0,0 +1,106 @@ +package datadog.trace.instrumentation.rmi.context.client; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.api.AgentTracer.activeSpan; +import static datadog.trace.instrumentation.rmi.context.ContextPropagator.PROPAGATOR; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.instrumentation.api.AgentSpan; +import java.rmi.server.ObjID; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import sun.rmi.transport.Connection; + +/** + * Main entry point for transferring context between RMI service. + * + *

It injects into StreamRemoteCall constructor used for invoking remote tasks and performs a + * backwards compatible check to ensure if the other side is prepared to receive context propagation + * messages then if successful sends a context propagation message + * + *

Context propagation consist of a Serialized HashMap with all data set by usual context + * injection, which includes things like sampling priority, trace and parent id + * + *

As well as optional baggage items + * + *

On the other side of the communication a special Dispatcher is created when a message with + * DD_CONTEXT_CALL_ID is received. + * + *

If the server is not instrumented first call will gracefully fail just like any other unknown + * call. With small caveat that this first call needs to *not* have any parameters, since those will + * not be read from connection and instead will be interpreted as another remote instruction, but + * that instruction will essentially be garbage data and will cause the parsing loop to throw + * exception and shutdown the connection which we do not want + */ +@AutoService(Instrumenter.class) +public class RmiClientContextInstrumentation extends Instrumenter.Default { + + public RmiClientContextInstrumentation() { + super("rmi", "rmi-context-propagator", "rmi-client-context-propagator"); + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()).and(safeHasSuperType(named("sun.rmi.transport.StreamRemoteCall"))); + } + + @Override + public Map contextStore() { + // caching if a connection can support enhanced format + return singletonMap("sun.rmi.transport.Connection", "java.lang.Boolean"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.instrumentation.rmi.context.ContextPayload$InjectAdapter", + "datadog.trace.instrumentation.rmi.context.ContextPayload$ExtractAdapter", + "datadog.trace.instrumentation.rmi.context.ContextPayload", + "datadog.trace.instrumentation.rmi.context.ContextPropagator" + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isConstructor() + .and(takesArgument(0, named("sun.rmi.transport.Connection"))) + .and(takesArgument(1, named("java.rmi.server.ObjID"))), + getClass().getName() + "$StreamRemoteCallConstructorAdvice"); + } + + public static class StreamRemoteCallConstructorAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.Argument(value = 0) final Connection c, + @Advice.Argument(value = 1) final ObjID id) { + if (!c.isReusable()) { + return; + } + if (PROPAGATOR.isRMIInternalObject(id)) { + return; + } + final AgentSpan activeSpan = activeSpan(); + if (activeSpan == null) { + return; + } + + final ContextStore knownConnections = + InstrumentationContext.get(Connection.class, Boolean.class); + + PROPAGATOR.attemptToPropagateContext(knownConnections, c, activeSpan); + } + } +} diff --git a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/server/ContextDispatcher.java b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/server/ContextDispatcher.java new file mode 100644 index 0000000000..c341331e5a --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/server/ContextDispatcher.java @@ -0,0 +1,61 @@ +package datadog.trace.instrumentation.rmi.context.server; + +import static datadog.trace.bootstrap.instrumentation.rmi.ThreadLocalContext.THREAD_LOCAL_CONTEXT; +import static datadog.trace.instrumentation.api.AgentTracer.propagate; +import static datadog.trace.instrumentation.rmi.context.ContextPayload.GETTER; +import static datadog.trace.instrumentation.rmi.context.ContextPropagator.DD_CONTEXT_CALL_ID; +import static datadog.trace.instrumentation.rmi.context.ContextPropagator.PROPAGATOR; + +import datadog.trace.instrumentation.api.AgentSpan; +import datadog.trace.instrumentation.rmi.context.ContextPayload; +import java.io.IOException; +import java.io.ObjectInput; +import java.rmi.Remote; +import java.rmi.server.RemoteCall; +import sun.rmi.server.Dispatcher; +import sun.rmi.transport.Target; + +/** + * ContextDispatcher is responsible for handling both initial context propagation check call and + * following call which carries payload + * + *

Context propagation check is only expected not to throw any exception, hinting to the client + * that its communicating with an instrumented server. Non instrumented server would've thrown + * UnknownObjectException + * + *

Because caching of the result after first call on a connection, only payload calls are + * expected + */ +public class ContextDispatcher implements Dispatcher { + private static final ContextDispatcher CONTEXT_DISPATCHER = new ContextDispatcher(); + private static final NoopRemote NOOP_REMOTE = new NoopRemote(); + + public static Target newDispatcherTarget() { + return new Target(NOOP_REMOTE, CONTEXT_DISPATCHER, NOOP_REMOTE, DD_CONTEXT_CALL_ID, false); + } + + @Override + public void dispatch(final Remote obj, final RemoteCall call) throws IOException { + final ObjectInput in = call.getInputStream(); + final int operationId = in.readInt(); + in.readLong(); // skip 8 bytes + + if (PROPAGATOR.isOperationWithPayload(operationId)) { + final ContextPayload payload = ContextPayload.read(in); + if (payload != null) { + final AgentSpan.Context context = propagate().extract(payload, GETTER); + THREAD_LOCAL_CONTEXT.set(context); + } + } + + // send result stream the client is expecting + call.getResultStream(true); + + // release held streams to allow next call to continue + call.releaseInputStream(); + call.releaseOutputStream(); + call.done(); + } + + public static class NoopRemote implements Remote {} +} diff --git a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/server/RmiServerContextInstrumentation.java b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/server/RmiServerContextInstrumentation.java new file mode 100644 index 0000000000..1ce41f9310 --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/context/server/RmiServerContextInstrumentation.java @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.rmi.context.server; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.rmi.context.ContextPropagator.DD_CONTEXT_CALL_ID; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isStatic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import sun.rmi.transport.Target; + +@AutoService(Instrumenter.class) +public class RmiServerContextInstrumentation extends Instrumenter.Default { + + public RmiServerContextInstrumentation() { + super("rmi", "rmi-context-propagator", "rmi-server-context-propagator"); + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()).and(safeHasSuperType(named("sun.rmi.transport.ObjectTable"))); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.instrumentation.rmi.context.ContextPayload$InjectAdapter", + "datadog.trace.instrumentation.rmi.context.ContextPayload$ExtractAdapter", + "datadog.trace.instrumentation.rmi.context.ContextPayload", + "datadog.trace.instrumentation.rmi.context.ContextPropagator", + packageName + ".ContextDispatcher", + packageName + ".ContextDispatcher$NoopRemote" + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod() + .and(isStatic()) + .and(named("getTarget")) + .and((takesArgument(0, named("sun.rmi.transport.ObjectEndpoint")))), + getClass().getName() + "$ObjectTableAdvice"); + } + + public static class ObjectTableAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void methodExit( + @Advice.Argument(0) final Object oe, @Advice.Return(readOnly = false) Target result) { + // comparing toString() output allows us to avoid using reflection to be able to compare + // ObjID and ObjectEndpoint objects + // ObjectEndpoint#toString() only returns this.objId.toString() value which is exactly + // what we're interested in here. + if (!DD_CONTEXT_CALL_ID.toString().equals(oe.toString())) { + return; + } + result = ContextDispatcher.newDispatcherTarget(); + } + } +} diff --git a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/server/RmiServerDecorator.java b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/server/RmiServerDecorator.java new file mode 100644 index 0000000000..155d64c7b2 --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/server/RmiServerDecorator.java @@ -0,0 +1,23 @@ +package datadog.trace.instrumentation.rmi.server; + +import datadog.trace.agent.decorator.ServerDecorator; +import datadog.trace.api.DDSpanTypes; + +public class RmiServerDecorator extends ServerDecorator { + public static final RmiServerDecorator DECORATE = new RmiServerDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"rmi"}; + } + + @Override + protected String spanType() { + return DDSpanTypes.RPC; + } + + @Override + protected String component() { + return "rmi-server"; + } +} diff --git a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/server/RmiServerInstrumentation.java b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/server/RmiServerInstrumentation.java new file mode 100644 index 0000000000..5a96051c24 --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/server/RmiServerInstrumentation.java @@ -0,0 +1,87 @@ +package datadog.trace.instrumentation.rmi.server; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.bootstrap.instrumentation.rmi.ThreadLocalContext.THREAD_LOCAL_CONTEXT; +import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.rmi.server.RmiServerDecorator.DECORATE; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.isStatic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.DDTags; +import datadog.trace.instrumentation.api.AgentScope; +import datadog.trace.instrumentation.api.AgentSpan; +import java.lang.reflect.Method; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class RmiServerInstrumentation extends Instrumenter.Default { + + public RmiServerInstrumentation() { + super("rmi", "rmi-server"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.BaseDecorator", + packageName + ".RmiServerDecorator" + }; + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()).and(safeHasSuperType(named("java.rmi.server.RemoteServer"))); + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod().and(isPublic()).and(not(isStatic())), getClass().getName() + "$ServerAdvice"); + } + + public static class ServerAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class, inline = true) + public static AgentScope onEnter( + @Advice.This final Object thiz, @Advice.Origin final Method method) { + final AgentSpan.Context context = THREAD_LOCAL_CONTEXT.getAndResetContext(); + + final AgentSpan span; + if (context == null) { + span = startSpan("rmi.request"); + } else { + span = startSpan("rmi.request", context); + } + + span.setTag(DDTags.RESOURCE_NAME, DECORATE.spanNameForMethod(method)) + .setTag("span.origin.type", thiz.getClass().getCanonicalName()); + + DECORATE.afterStart(span); + return activateSpan(span, true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { + if (scope == null) { + return; + } + + DECORATE.onError(scope, throwable); + + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/rmi/src/test/groovy/RmiTest.groovy b/dd-java-agent/instrumentation/rmi/src/test/groovy/RmiTest.groovy new file mode 100644 index 0000000000..6b988e08a0 --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/test/groovy/RmiTest.groovy @@ -0,0 +1,202 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.PortUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags +import datadog.trace.instrumentation.api.Tags +import rmi.app.Greeter +import rmi.app.Server +import rmi.app.ServerLegacy + +import java.rmi.registry.LocateRegistry +import java.rmi.server.UnicastRemoteObject + +import static datadog.trace.agent.test.asserts.ListWriterAssert.assertTraces +import static datadog.trace.agent.test.utils.TraceUtils.basicSpan +import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace + +class RmiTest extends AgentTestRunner { + def registryPort = PortUtils.randomOpenPort() + def serverRegistry = LocateRegistry.createRegistry(registryPort) + def clientRegistry = LocateRegistry.getRegistry("localhost", registryPort) + + def cleanup() { + UnicastRemoteObject.unexportObject(serverRegistry, true) + } + + def "Client call creates spans"() { + setup: + def server = new Server() + serverRegistry.rebind(Server.RMI_ID, server) + + when: + def response = runUnderTrace("parent") { + def client = (Greeter) clientRegistry.lookup(Server.RMI_ID) + return client.hello("you") + } + + then: + response.contains("Hello you") + assertTraces(TEST_WRITER, 2) { + trace(1, 2) { + basicSpan(it, 0, "parent") + span(1) { + operationName "rmi.invoke" + childOf span(0) + tags { + "$DDTags.SERVICE_NAME" "rmi" + "$DDTags.RESOURCE_NAME" "Greeter.hello" + "$DDTags.SPAN_TYPE" DDSpanTypes.RPC + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.COMPONENT" "rmi-client" + "span.origin.type" Greeter.canonicalName + } + } + } + trace(0, 2) { + span(0) { + operationName "rmi.request" + tags { + "$DDTags.RESOURCE_NAME" "Server.hello" + "$DDTags.SPAN_TYPE" DDSpanTypes.RPC + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + "$Tags.COMPONENT" "rmi-server" + "span.origin.type" server.class.canonicalName + } + } + span(1) { + operationName "rmi.request" + tags { + "$DDTags.RESOURCE_NAME" "Server.someMethod" + "$DDTags.SPAN_TYPE" DDSpanTypes.RPC + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + "$Tags.COMPONENT" "rmi-server" + "span.origin.type" server.class.canonicalName + } + } + } + } + + cleanup: + serverRegistry.unbind("Server") + } + + def "Calling server builtin methods doesn't create server spans"() { + setup: + def server = new Server() + serverRegistry.rebind(Server.RMI_ID, server) + + when: + server.equals(new Server()) + server.getRef() + server.hashCode() + server.toString() + server.getClass() + + then: + assertTraces(TEST_WRITER, 0) {} + + cleanup: + serverRegistry.unbind("Server") + } + + def "Service throws exception and its propagated to spans"() { + setup: + def server = new Server() + serverRegistry.rebind(Server.RMI_ID, server) + + when: + runUnderTrace("parent") { + def client = (Greeter) clientRegistry.lookup(Server.RMI_ID) + client.exceptional() + } + + then: + def thrownException = thrown(RuntimeException) + assertTraces(TEST_WRITER, 2) { + trace(1, 2) { + basicSpan(it, 0, "parent", null, null, thrownException) + span(1) { + operationName "rmi.invoke" + childOf span(0) + errored true + tags { + "$DDTags.SERVICE_NAME" "rmi" + "$DDTags.RESOURCE_NAME" "Greeter.exceptional" + "$DDTags.SPAN_TYPE" DDSpanTypes.RPC + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.COMPONENT" "rmi-client" + "span.origin.type" Greeter.canonicalName + errorTags(RuntimeException, String) + } + } + } + trace(0, 1) { + span(0) { + operationName "rmi.request" + errored true + tags { + "$DDTags.RESOURCE_NAME" "Server.exceptional" + "$DDTags.SPAN_TYPE" DDSpanTypes.RPC + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + "$Tags.COMPONENT" "rmi-server" + "span.origin.type" server.class.canonicalName + errorTags(RuntimeException, String) + } + } + } + } + + cleanup: + serverRegistry.unbind("Server") + } + + def "Client call using ServerLegacy_stub creates spans"() { + setup: + def server = new ServerLegacy() + serverRegistry.rebind(ServerLegacy.RMI_ID, server) + + when: + def response = runUnderTrace("parent") { + def client = (Greeter) clientRegistry.lookup(ServerLegacy.RMI_ID) + return client.hello("you") + } + + then: + response.contains("Hello you") + assertTraces(TEST_WRITER, 2) { + def parentSpan = TEST_WRITER[1][1] + trace(1, 2) { + basicSpan(it, 0, "parent") + span(1) { + operationName "rmi.invoke" + childOf span(0) + tags { + "$DDTags.SERVICE_NAME" "rmi" + "$DDTags.RESOURCE_NAME" "Greeter.hello" + "$DDTags.SPAN_TYPE" DDSpanTypes.RPC + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.COMPONENT" "rmi-client" + "span.origin.type" Greeter.canonicalName + } + } + } + + trace(0, 1) { + span(0) { + childOf parentSpan + operationName "rmi.request" + tags { + "$DDTags.RESOURCE_NAME" "ServerLegacy.hello" + "$DDTags.SPAN_TYPE" DDSpanTypes.RPC + "$Tags.COMPONENT" "rmi-server" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + "span.origin.type" server.class.canonicalName + } + } + } + } + + cleanup: + serverRegistry.unbind(ServerLegacy.RMI_ID) + } +} diff --git a/dd-java-agent/instrumentation/rmi/src/test/java/rmi/app/Greeter.java b/dd-java-agent/instrumentation/rmi/src/test/java/rmi/app/Greeter.java new file mode 100644 index 0000000000..0e1d1d3fa8 --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/test/java/rmi/app/Greeter.java @@ -0,0 +1,10 @@ +package rmi.app; + +import java.rmi.Remote; +import java.rmi.RemoteException; + +public interface Greeter extends Remote { + String hello(String name) throws RemoteException; + + void exceptional() throws RemoteException, RuntimeException; +} diff --git a/dd-java-agent/instrumentation/rmi/src/test/java/rmi/app/Server.java b/dd-java-agent/instrumentation/rmi/src/test/java/rmi/app/Server.java new file mode 100644 index 0000000000..892a85f38a --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/test/java/rmi/app/Server.java @@ -0,0 +1,28 @@ +package rmi.app; + +import java.rmi.RemoteException; +import java.rmi.server.UnicastRemoteObject; + +public class Server extends UnicastRemoteObject implements Greeter { + public static String RMI_ID = Server.class.getSimpleName(); + + private static final long serialVersionUID = 1L; + + public Server() throws RemoteException { + super(); + } + + @Override + public String hello(final String name) { + return someMethod(name); + } + + public String someMethod(final String name) { + return "Hello " + name; + } + + @Override + public void exceptional() throws RuntimeException { + throw new RuntimeException("expected"); + } +} diff --git a/dd-java-agent/instrumentation/rmi/src/test/java/rmi/app/ServerLegacy.java b/dd-java-agent/instrumentation/rmi/src/test/java/rmi/app/ServerLegacy.java new file mode 100644 index 0000000000..7635ccd5b6 --- /dev/null +++ b/dd-java-agent/instrumentation/rmi/src/test/java/rmi/app/ServerLegacy.java @@ -0,0 +1,24 @@ +package rmi.app; + +import java.rmi.RemoteException; +import java.rmi.server.UnicastRemoteObject; + +public class ServerLegacy extends UnicastRemoteObject implements Greeter { + static String RMI_ID = ServerLegacy.class.getSimpleName(); + + private static final long serialVersionUID = 1L; + + public ServerLegacy() throws RemoteException { + super(); + } + + @Override + public String hello(final String name) { + return "Hello " + name; + } + + @Override + public void exceptional() throws RuntimeException { + throw new RuntimeException("expected"); + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/test/groovy/JettyServlet2Test.groovy b/dd-java-agent/instrumentation/servlet/request-2/src/test/groovy/JettyServlet2Test.groovy index 6ce3a5e46b..9e526fa0eb 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/test/groovy/JettyServlet2Test.groovy +++ b/dd-java-agent/instrumentation/servlet/request-2/src/test/groovy/JettyServlet2Test.groovy @@ -4,15 +4,15 @@ import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import datadog.trace.instrumentation.api.Tags import datadog.trace.instrumentation.servlet2.Servlet2Decorator +import javax.servlet.http.HttpServletRequest import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.ErrorHandler import org.eclipse.jetty.servlet.ServletContextHandler -import javax.servlet.http.HttpServletRequest - import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -37,9 +37,10 @@ class JettyServlet2Test extends HttpServerTest { // servletContext.setSecurityHandler(security) servletContext.addServlet(TestServlet2.Sync, SUCCESS.path) + servletContext.addServlet(TestServlet2.Sync, QUERY_PARAM.path) + servletContext.addServlet(TestServlet2.Sync, REDIRECT.path) servletContext.addServlet(TestServlet2.Sync, ERROR.path) servletContext.addServlet(TestServlet2.Sync, EXCEPTION.path) - servletContext.addServlet(TestServlet2.Sync, REDIRECT.path) servletContext.addServlet(TestServlet2.Sync, AUTH_REQUIRED.path) jettyServer.setHandler(servletContext) @@ -103,6 +104,9 @@ class JettyServlet2Test extends HttpServerTest { "error.type" { it == null || it == Exception.name } "error.stack" { it == null || it instanceof String } } + if (endpoint.query) { + "$DDTags.HTTP_QUERY" endpoint.query + } } } } diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/test/groovy/TestServlet2.groovy b/dd-java-agent/instrumentation/servlet/request-2/src/test/groovy/TestServlet2.groovy index ce0ec922f6..124c711cd8 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/test/groovy/TestServlet2.groovy +++ b/dd-java-agent/instrumentation/servlet/request-2/src/test/groovy/TestServlet2.groovy @@ -1,11 +1,11 @@ import datadog.trace.agent.test.base.HttpServerTest import groovy.servlet.AbstractHttpServlet - import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -23,6 +23,10 @@ class TestServlet2 { resp.status = endpoint.status resp.writer.print(endpoint.body) break + case QUERY_PARAM: + resp.status = endpoint.status + resp.writer.print(req.queryString) + break case REDIRECT: resp.sendRedirect(endpoint.body) break diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/AbstractServlet3Test.groovy b/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/AbstractServlet3Test.groovy index 8fcef9fadf..0778dd9ab8 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/AbstractServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/AbstractServlet3Test.groovy @@ -4,14 +4,14 @@ import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import datadog.trace.instrumentation.api.Tags import datadog.trace.instrumentation.servlet3.Servlet3Decorator +import javax.servlet.Servlet import okhttp3.Request import org.apache.catalina.core.ApplicationFilterChain -import javax.servlet.Servlet - import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -49,6 +49,7 @@ abstract class AbstractServlet3Test extends HttpServerTest extends HttpServerTest servlet() { - TestServlet3.Sync // dispatch to sync servlet - } +// FIXME: not working right now... +//class JettyServlet3TestForward extends JettyDispatchTest { +// @Override +// Class servlet() { +// TestServlet3.Sync // dispatch to sync servlet +// } +// +// @Override +// boolean testNotFound() { +// false +// } +// +// @Override +// protected void setupServlets(ServletContextHandler context) { +// super.setupServlets(context) +// +// addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward) +// addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Forward) +// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward) +// addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward) +// addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward) +// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward) +// } +//} - @Override - boolean testNotFound() { - false - } - - @Override - protected void setupServlets(ServletContextHandler context) { - super.setupServlets(context) - - addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward) - addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward) - addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward) - addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward) - addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward) - } -} - -class JettyServlet3TestInclude extends JettyServlet3Test { - @Override - Class servlet() { - TestServlet3.Sync // dispatch to sync servlet - } - - @Override - boolean testNotFound() { - false - } - - @Override - protected void setupServlets(ServletContextHandler context) { - super.setupServlets(context) - - addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) - addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) - addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include) - addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include) - addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include) - } -} +// FIXME: not working right now... +//class JettyServlet3TestInclude extends JettyDispatchTest { +// @Override +// Class servlet() { +// TestServlet3.Sync // dispatch to sync servlet +// } +// +// @Override +// boolean testNotFound() { +// false +// } +// +// @Override +// protected void setupServlets(ServletContextHandler context) { +// super.setupServlets(context) +// +// addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) +// addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Include) +// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include) +// addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) +// addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include) +// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include) +// } +//} class JettyServlet3TestDispatchImmediate extends JettyDispatchTest { @@ -175,6 +179,7 @@ class JettyServlet3TestDispatchImmediate extends JettyDispatchTest { super.setupServlets(context) addServlet(context, "/dispatch" + SUCCESS.path, TestServlet3.DispatchImmediate) + addServlet(context, "/dispatch" + QUERY_PARAM.path, TestServlet3.DispatchImmediate) addServlet(context, "/dispatch" + ERROR.path, TestServlet3.DispatchImmediate) addServlet(context, "/dispatch" + EXCEPTION.path, TestServlet3.DispatchImmediate) addServlet(context, "/dispatch" + REDIRECT.path, TestServlet3.DispatchImmediate) @@ -194,6 +199,7 @@ class JettyServlet3TestDispatchAsync extends JettyDispatchTest { super.setupServlets(context) addServlet(context, "/dispatch" + SUCCESS.path, TestServlet3.DispatchAsync) + addServlet(context, "/dispatch" + QUERY_PARAM.path, TestServlet3.DispatchAsync) addServlet(context, "/dispatch" + ERROR.path, TestServlet3.DispatchAsync) addServlet(context, "/dispatch" + EXCEPTION.path, TestServlet3.DispatchAsync) addServlet(context, "/dispatch" + REDIRECT.path, TestServlet3.DispatchAsync) @@ -262,6 +268,9 @@ abstract class JettyDispatchTest extends JettyServlet3Test { "error.type" { it == null || it == Exception.name } "error.stack" { it == null || it instanceof String } } + if (endpoint.query) { + "$DDTags.HTTP_QUERY" endpoint.query + } } } } diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/TestServlet3.groovy b/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/TestServlet3.groovy index d5178a47f8..24844321de 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/TestServlet3.groovy +++ b/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/TestServlet3.groovy @@ -1,13 +1,14 @@ import datadog.trace.agent.test.base.HttpServerTest import groovy.servlet.AbstractHttpServlet - import javax.servlet.annotation.WebServlet import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse + import java.util.concurrent.Phaser import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -25,6 +26,10 @@ class TestServlet3 { resp.status = endpoint.status resp.writer.print(endpoint.body) break + case QUERY_PARAM: + resp.status = endpoint.status + resp.writer.print(req.queryString) + break case REDIRECT: resp.sendRedirect(endpoint.body) break @@ -52,15 +57,25 @@ class TestServlet3 { resp.contentType = "text/plain" switch (endpoint) { case SUCCESS: - case ERROR: resp.status = endpoint.status resp.writer.print(endpoint.body) context.complete() break + case QUERY_PARAM: + resp.status = endpoint.status + resp.writer.print(req.queryString) + context.complete() + break case REDIRECT: resp.sendRedirect(endpoint.body) context.complete() break + case ERROR: + resp.status = endpoint.status + resp.writer.print(endpoint.body) +// resp.sendError(endpoint.status, endpoint.body) + context.complete() + break case EXCEPTION: resp.status = endpoint.status resp.writer.print(endpoint.body) @@ -88,13 +103,19 @@ class TestServlet3 { resp.contentType = "text/plain" switch (endpoint) { case SUCCESS: - case ERROR: resp.status = endpoint.status resp.writer.print(endpoint.body) break + case QUERY_PARAM: + resp.status = endpoint.status + resp.writer.print(req.queryString) + break case REDIRECT: resp.sendRedirect(endpoint.body) break + case ERROR: + resp.sendError(endpoint.status, endpoint.body) + break case EXCEPTION: throw new Exception(endpoint.body) } diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/TomcatServlet3Test.groovy b/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/TomcatServlet3Test.groovy index 68a1edd0f9..5eb1f6c768 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/TomcatServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/TomcatServlet3Test.groovy @@ -6,6 +6,7 @@ import datadog.trace.api.DDTags import datadog.trace.instrumentation.api.Tags import groovy.transform.stc.ClosureParams import groovy.transform.stc.SimpleType +import javax.servlet.Servlet import org.apache.catalina.Context import org.apache.catalina.connector.Request import org.apache.catalina.connector.Response @@ -16,13 +17,12 @@ import org.apache.catalina.valves.ErrorReportValve import org.apache.tomcat.JarScanFilter import org.apache.tomcat.JarScanType -import javax.servlet.Servlet - import static datadog.trace.agent.test.asserts.TraceAssert.assertTrace import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS import static datadog.trace.agent.test.utils.TraceUtils.basicSpan @@ -154,51 +154,55 @@ class TomcatServlet3TestFakeAsync extends TomcatServlet3Test { } } -class TomcatServlet3TestForward extends TomcatServlet3Test { - @Override - Class servlet() { - TestServlet3.Sync // dispatch to sync servlet - } +// FIXME: not working right now... +//class TomcatServlet3TestForward extends TomcatDispatchTest { +// @Override +// Class servlet() { +// TestServlet3.Sync // dispatch to sync servlet +// } +// +// @Override +// boolean testNotFound() { +// false +// } +// +// @Override +// protected void setupServlets(Context context) { +// super.setupServlets(context) +// +// addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward) +// addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Forward) +// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward) +// addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward) +// addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward) +// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward) +// } +//} - @Override - boolean testNotFound() { - false - } - - @Override - protected void setupServlets(Context context) { - super.setupServlets(context) - - addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward) - addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward) - addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward) - addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward) - addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward) - } -} - -class TomcatServlet3TestInclude extends TomcatServlet3Test { - @Override - Class servlet() { - TestServlet3.Sync // dispatch to sync servlet - } - - @Override - boolean testNotFound() { - false - } - - @Override - protected void setupServlets(Context context) { - super.setupServlets(context) - - addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) - addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) - addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include) - addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include) - addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include) - } -} +// FIXME: not working right now... +//class TomcatServlet3TestInclude extends TomcatDispatchTest { +// @Override +// Class servlet() { +// TestServlet3.Sync // dispatch to sync servlet +// } +// +// @Override +// boolean testNotFound() { +// false +// } +// +// @Override +// protected void setupServlets(Context context) { +// super.setupServlets(context) +// +// addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) +// addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Include) +// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include) +// addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) +// addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include) +// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include) +// } +//} class TomcatServlet3TestDispatchImmediate extends TomcatDispatchTest { @Override @@ -216,6 +220,7 @@ class TomcatServlet3TestDispatchImmediate extends TomcatDispatchTest { super.setupServlets(context) addServlet(context, "/dispatch" + SUCCESS.path, TestServlet3.DispatchImmediate) + addServlet(context, "/dispatch" + QUERY_PARAM.path, TestServlet3.DispatchImmediate) addServlet(context, "/dispatch" + ERROR.path, TestServlet3.DispatchImmediate) addServlet(context, "/dispatch" + EXCEPTION.path, TestServlet3.DispatchImmediate) addServlet(context, "/dispatch" + REDIRECT.path, TestServlet3.DispatchImmediate) @@ -235,6 +240,7 @@ class TomcatServlet3TestDispatchAsync extends TomcatDispatchTest { super.setupServlets(context) addServlet(context, "/dispatch" + SUCCESS.path, TestServlet3.DispatchAsync) + addServlet(context, "/dispatch" + QUERY_PARAM.path, TestServlet3.DispatchAsync) addServlet(context, "/dispatch" + ERROR.path, TestServlet3.DispatchAsync) addServlet(context, "/dispatch" + EXCEPTION.path, TestServlet3.DispatchAsync) addServlet(context, "/dispatch" + REDIRECT.path, TestServlet3.DispatchAsync) @@ -296,7 +302,7 @@ abstract class TomcatDispatchTest extends TomcatServlet3Test { "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER "$Tags.COMPONENT" serverDecorator.component() "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "$Tags.PEER_HOSTNAME" "localhost" + "$Tags.PEER_HOSTNAME" { it == "localhost" || it == "127.0.0.1" } "$Tags.PEER_HOST_IPV4" { it == null || it == "127.0.0.1" } // Optional "$Tags.PEER_PORT" Integer "$Tags.HTTP_URL" "${endpoint.resolve(address)}" @@ -313,6 +319,9 @@ abstract class TomcatDispatchTest extends TomcatServlet3Test { "error.type" { it == null || it == Exception.name } "error.stack" { it == null || it instanceof String } } + if (endpoint.query) { + "$DDTags.HTTP_QUERY" endpoint.query + } } } } diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java index 13818879bd..f709e1503c 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java @@ -45,7 +45,14 @@ public class SpringWebHttpServerDecorator @Override protected URI url(final HttpServletRequest httpServletRequest) throws URISyntaxException { - return new URI(httpServletRequest.getRequestURL().toString()); + return new URI( + httpServletRequest.getScheme(), + null, + httpServletRequest.getServerName(), + httpServletRequest.getServerPort(), + httpServletRequest.getRequestURI(), + httpServletRequest.getQueryString(), + null); } @Override diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/SpringBootBasedTest.groovy index 750c0a6c41..65d4eb05ac 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/SpringBootBasedTest.groovy @@ -136,6 +136,9 @@ class SpringBootBasedTest extends HttpServerTest + controller(QUERY_PARAM) { + ctx.response().setStatusCode(QUERY_PARAM.status).end(ctx.request().query()) + } + } router.route(REDIRECT.path).handler { ctx -> controller(REDIRECT) { ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end() diff --git a/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy index d9c38f6140..5db78a337d 100644 --- a/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy +++ b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy @@ -8,6 +8,7 @@ import io.vertx.reactivex.ext.web.Router import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -45,6 +46,19 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxHttpServerTest { } }) } + router.route(QUERY_PARAM.path).handler { ctx -> + breaker.executeCommand({ future -> + future.complete(QUERY_PARAM) + }, { it -> + if (it.failed()) { + throw it.cause() + } + HttpServerTest.ServerEndpoint endpoint = it.result() + controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).end(ctx.request().query()) + } + }) + } router.route(REDIRECT.path).handler { ctx -> breaker.executeCommand({ future -> future.complete(REDIRECT) diff --git a/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxHttpServerTest.groovy b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxHttpServerTest.groovy index e2e7637f28..7516173c3a 100644 --- a/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxHttpServerTest.groovy +++ b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxHttpServerTest.groovy @@ -7,6 +7,7 @@ import io.vertx.reactivex.ext.web.Router import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -29,6 +30,11 @@ class VertxRxHttpServerTest extends VertxHttpServerTest { ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) } } + router.route(QUERY_PARAM.path).handler { ctx -> + controller(QUERY_PARAM) { + ctx.response().setStatusCode(QUERY_PARAM.status).end(ctx.request().query()) + } + } router.route(REDIRECT.path).handler { ctx -> controller(REDIRECT) { ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end() diff --git a/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java b/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java index ed19b12962..9d12306fa5 100644 --- a/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java +++ b/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java @@ -41,19 +41,23 @@ import java.util.regex.Pattern; */ public class AgentBootstrap { - public static void premain(final String agentArgs, final Instrumentation inst) throws Exception { + public static void premain(final String agentArgs, final Instrumentation inst) { agentmain(agentArgs, inst); } - public static void agentmain(final String agentArgs, final Instrumentation inst) - throws Exception { + public static void agentmain(final String agentArgs, final Instrumentation inst) { + try { - final URL bootstrapURL = installBootstrapJar(inst); + final URL bootstrapURL = installBootstrapJar(inst); - final Class agentClass = - ClassLoader.getSystemClassLoader().loadClass("datadog.trace.bootstrap.Agent"); - final Method startMethod = agentClass.getMethod("start", Instrumentation.class, URL.class); - startMethod.invoke(null, inst, bootstrapURL); + final Class agentClass = + ClassLoader.getSystemClassLoader().loadClass("datadog.trace.bootstrap.Agent"); + final Method startMethod = agentClass.getMethod("start", Instrumentation.class, URL.class); + startMethod.invoke(null, inst, bootstrapURL); + } catch (final Throwable ex) { + // Don't rethrow. We don't have a log manager here, so just print. + ex.printStackTrace(); + } } private static synchronized URL installBootstrapJar(final Instrumentation inst) @@ -106,8 +110,12 @@ public class AgentBootstrap { throw new RuntimeException("Unable to parse javaagent parameter: " + agentArgument); } - bootstrapURL = new URL("file:" + matcher.group(1)); - inst.appendToBootstrapClassLoaderSearch(new JarFile(new File(bootstrapURL.toURI()))); + final File javaagentFile = new File(matcher.group(1)); + if (!(javaagentFile.exists() || javaagentFile.isFile())) { + throw new RuntimeException("Unable to find javaagent file: " + javaagentFile); + } + bootstrapURL = javaagentFile.toURI().toURL(); + inst.appendToBootstrapClassLoaderSearch(new JarFile(javaagentFile)); return bootstrapURL; } diff --git a/dd-java-agent/src/test/groovy/datadog/trace/agent/AgentLoadedIntoBootstrapTest.groovy b/dd-java-agent/src/test/groovy/datadog/trace/agent/AgentLoadedIntoBootstrapTest.groovy index 5afe435c67..a3d8d48181 100644 --- a/dd-java-agent/src/test/groovy/datadog/trace/agent/AgentLoadedIntoBootstrapTest.groovy +++ b/dd-java-agent/src/test/groovy/datadog/trace/agent/AgentLoadedIntoBootstrapTest.groovy @@ -5,7 +5,7 @@ import jvmbootstraptest.AgentLoadedChecker import spock.lang.Specification class AgentLoadedIntoBootstrapTest extends Specification { - + def "Agent loads in when separate jvm is launched"() { expect: IntegrationTestUtils.runOnSeparateJvm(AgentLoadedChecker.getName() diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index 5ebf44cb38..caa972f008 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -25,8 +25,10 @@ import static datadog.trace.agent.test.asserts.TraceAssert.assertTrace import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS +import static datadog.trace.agent.test.utils.ConfigUtils.withConfigOverride import static datadog.trace.agent.test.utils.TraceUtils.basicSpan import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace import static datadog.trace.instrumentation.api.AgentTracer.activeScope @@ -108,16 +110,25 @@ abstract class HttpServerTest ext NOT_FOUND("notFound", 404, "not found"), // TODO: add tests for the following cases: + QUERY_PARAM("query?some=query", 200, "some=query"), + // OkHttp never sends the fragment in the request, so these cases don't work. +// FRAGMENT_PARAM("fragment#some-fragment", 200, "some-fragment"), +// QUERY_FRAGMENT_PARAM("query/fragment?some=query#some-fragment", 200, "some=query#some-fragment"), PATH_PARAM("path/123/param", 200, "123"), AUTH_REQUIRED("authRequired", 200, null), private final String path + final String query + final String fragment final int status final String body final Boolean errored - ServerEndpoint(String path, int status, String body) { - this.path = path + ServerEndpoint(String uri, int status, String body) { + def uriObj = URI.create(uri) + this.path = uriObj.path + this.query = uriObj.query + this.fragment = uriObj.fragment this.status = status this.body = body this.errored = status >= 500 @@ -143,8 +154,12 @@ abstract class HttpServerTest ext } Request.Builder request(ServerEndpoint uri, String method, String body) { + def url = HttpUrl.get(uri.resolve(address)).newBuilder() + .query(uri.query) + .fragment(uri.fragment) + .build() return new Request.Builder() - .url(HttpUrl.get(uri.resolve(address))) + .url(url) .method(method, body) } @@ -229,6 +244,75 @@ abstract class HttpServerTest ext body = null } + def "test tag query string for #endpoint"() { + setup: + def request = request(endpoint, method, body).build() + Response response = withConfigOverride("http.server.tag.query-string", "true") { + client.newCall(request).execute() + } + + expect: + response.code() == endpoint.status + response.body().string() == endpoint.body + + and: + cleanAndAssertTraces(1) { + if (hasHandlerSpan()) { + trace(0, 3) { + serverSpan(it, 0, null, null, method, endpoint) + handlerSpan(it, 1, span(0), method, endpoint) + controllerSpan(it, 2, span(1)) + } + } else { + trace(0, 2) { + serverSpan(it, 0, null, null, method, endpoint) + controllerSpan(it, 1, span(0)) + } + } + } + + where: + method = "GET" + body = null + endpoint << [SUCCESS, QUERY_PARAM] + } + + def "test success with multiple header attached parent"() { + setup: + def traceId = 123G + def parentId = 456G + def request = request(SUCCESS, method, body) + .header("x-datadog-trace-id", traceId.toString() + ", " + traceId.toString()) + .header("x-datadog-parent-id", parentId.toString() + ", " + parentId.toString()) + .header("x-datadog-sampling-priority", "1, 1") + .build() + def response = client.newCall(request).execute() + + expect: + response.code() == SUCCESS.status + response.body().string() == SUCCESS.body + + and: + cleanAndAssertTraces(1) { + if (hasHandlerSpan()) { + trace(0, 3) { + serverSpan(it, 0, traceId, parentId) + handlerSpan(it, 1, span(0)) + controllerSpan(it, 2, span(1)) + } + } else { + trace(0, 2) { + serverSpan(it, 0, traceId, parentId) + controllerSpan(it, 1, span(0)) + } + } + } + + where: + method = "GET" + body = null + } + def "test redirect"() { setup: def request = request(REDIRECT, method, body).build() @@ -439,9 +523,12 @@ abstract class HttpServerTest ext "$Tags.HTTP_URL" "${endpoint.resolve(address)}" "$Tags.HTTP_METHOD" method "$Tags.HTTP_STATUS" endpoint.status -// if (tagQueryString) { -// "$DDTags.HTTP_QUERY" uri.query -// "$DDTags.HTTP_FRAGMENT" { it == null || it == uri.fragment } // Optional + if (endpoint.query) { + "$DDTags.HTTP_QUERY" endpoint.query + } + // OkHttp never sends the fragment in the request. +// if (endpoint.fragment) { +// "$DDTags.HTTP_FRAGMENT" endpoint.fragment // } } } diff --git a/dd-trace-java.gradle b/dd-trace-java.gradle index 30c96b6b37..9bd8059084 100644 --- a/dd-trace-java.gradle +++ b/dd-trace-java.gradle @@ -16,7 +16,7 @@ def isCI = System.getenv("CI") != null allprojects { group = 'com.datadoghq' - version = '0.40.0-SNAPSHOT' + version = '0.41.0-SNAPSHOT' if (isCI) { buildDir = "${rootDir}/workspace/${projectDir.path.replace(rootDir.path, '')}/build/" diff --git a/settings.gradle b/settings.gradle index 0447507d7d..f2a9a14617 100644 --- a/settings.gradle +++ b/settings.gradle @@ -109,8 +109,9 @@ include ':dd-java-agent:instrumentation:play-ws-2' include ':dd-java-agent:instrumentation:play-ws-2.1' include ':dd-java-agent:instrumentation:rabbitmq-amqp-2.7' include ':dd-java-agent:instrumentation:ratpack-1.4' -include ':dd-java-agent:instrumentation:rxjava-1' include ':dd-java-agent:instrumentation:reactor-core-3.1' +include ':dd-java-agent:instrumentation:rmi' +include ':dd-java-agent:instrumentation:rxjava-1' include ':dd-java-agent:instrumentation:servlet' include ':dd-java-agent:instrumentation:servlet:request-2' include ':dd-java-agent:instrumentation:servlet:request-3'