diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpServerDecorator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpServerDecorator.java index b815cee8e2..fc3c82f316 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpServerDecorator.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpServerDecorator.java @@ -4,18 +4,21 @@ import datadog.trace.api.Config; import datadog.trace.api.DDSpanTypes; import io.opentracing.Span; import io.opentracing.tag.Tags; +import java.net.URI; +import lombok.extern.slf4j.Slf4j; -public abstract class HttpServerDecorator extends ServerDecorator { +@Slf4j +public abstract class HttpServerDecorator extends ServerDecorator { protected abstract String method(REQUEST request); - protected abstract String url(REQUEST request); + protected abstract URI url(REQUEST request); - protected abstract String peerHostname(REQUEST request); + protected abstract String peerHostname(CONNECTION connection); - protected abstract String peerHostIP(REQUEST request); + protected abstract String peerHostIP(CONNECTION connection); - protected abstract Integer peerPort(REQUEST request); + protected abstract Integer peerPort(CONNECTION connection); protected abstract Integer status(RESPONSE response); @@ -33,9 +36,37 @@ public abstract class HttpServerDecorator extends ServerDecor assert span != null; if (request != null) { Tags.HTTP_METHOD.set(span, method(request)); - Tags.HTTP_URL.set(span, url(request)); - Tags.PEER_HOSTNAME.set(span, peerHostname(request)); - final String ip = peerHostIP(request); + + try { + final URI url = url(request); + final StringBuilder urlNoParams = new StringBuilder(url.getScheme()); + urlNoParams.append("://"); + urlNoParams.append(url.getHost()); + if (url.getPort() > 0 && url.getPort() != 80 && url.getPort() != 443) { + urlNoParams.append(":"); + urlNoParams.append(url.getPort()); + } + final String path = url.getPath(); + if (path.isEmpty()) { + urlNoParams.append("/"); + } else { + urlNoParams.append(path); + } + + Tags.HTTP_URL.set(span, urlNoParams.toString()); + } catch (final Exception e) { + log.debug("Error tagging url", e); + } + // TODO set resource name from URL. + } + return span; + } + + public Span onConnection(final Span span, final CONNECTION connection) { + assert span != null; + if (connection != null) { + Tags.PEER_HOSTNAME.set(span, peerHostname(connection)); + final String ip = peerHostIP(connection); if (ip != null) { if (ip.contains(":")) { Tags.PEER_HOST_IPV6.set(span, ip); @@ -43,8 +74,7 @@ public abstract class HttpServerDecorator extends ServerDecor Tags.PEER_HOST_IPV4.set(span, ip); } } - Tags.PEER_PORT.set(span, peerPort(request)); - // TODO set resource name from URL. + Tags.PEER_PORT.set(span, peerPort(connection)); } return span; } diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpServerDecoratorTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpServerDecoratorTest.groovy index baa3069353..a5cacc3f51 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpServerDecoratorTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpServerDecoratorTest.groovy @@ -1,11 +1,11 @@ package datadog.trace.agent.decorator +import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride + import datadog.trace.api.Config import io.opentracing.Span import io.opentracing.tag.Tags -import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride - class HttpServerDecoratorTest extends ServerDecoratorTest { def span = Mock(Span) @@ -20,7 +20,29 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { then: if (req) { 1 * span.setTag(Tags.HTTP_METHOD.key, "test-method") - 1 * span.setTag(Tags.HTTP_URL.key, "test-url") + 1 * span.setTag(Tags.HTTP_URL.key, url) + } + 0 * _ + + where: + req | url + null | _ + [method: "test-method", url: URI.create("http://test-url?some=query")] | "http://test-url/" + [method: "test-method", url: URI.create("http://a:80/")] | "http://a/" + [method: "test-method", url: URI.create("https://10.0.0.1:443")] | "https://10.0.0.1/" + [method: "test-method", url: URI.create("https://localhost:0/1/")] | "https://localhost/1/" + [method: "test-method", url: URI.create("http://123:8080/some/path")] | "http://123:8080/some/path" + } + + def "test onConnection"() { + setup: + def decorator = newDecorator() + + when: + decorator.onConnection(span, conn) + + then: + if (conn) { 1 * span.setTag(Tags.PEER_HOSTNAME.key, "test-host") 1 * span.setTag(Tags.PEER_PORT.key, 555) if (ipv4) { @@ -32,11 +54,11 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { 0 * _ where: - ipv4 | req + ipv4 | conn null | null - null | [method: "test-method", url: "test-url", host: "test-host", ip: null, port: 555] - true | [method: "test-method", url: "test-url", host: "test-host", ip: "10.0.0.1", port: 555] - false | [method: "test-method", url: "test-url", host: "test-host", ip: "3ffe:1900:4545:3:200:f8ff:fe21:67cf", port: 555] + null | [host: "test-host", ip: null, port: 555] + true | [host: "test-host", ip: "10.0.0.1", port: 555] + false | [host: "test-host", ip: "3ffe:1900:4545:3:200:f8ff:fe21:67cf", port: 555] } def "test onResponse"() { @@ -90,7 +112,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { @Override def newDecorator() { - return new HttpServerDecorator() { + return new HttpServerDecorator() { @Override protected String[] instrumentationNames() { return ["test1", "test2"] @@ -107,7 +129,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { } @Override - protected String url(Map m) { + protected URI url(Map m) { return m.url } diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerDecorator.java b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerDecorator.java index a5c3d63ca8..bbcbb29c20 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerDecorator.java @@ -3,8 +3,10 @@ package datadog.trace.instrumentation.akkahttp; import akka.http.scaladsl.model.HttpRequest; import akka.http.scaladsl.model.HttpResponse; import datadog.trace.agent.decorator.HttpServerDecorator; +import java.net.URI; -public class AkkaHttpServerDecorator extends HttpServerDecorator { +public class AkkaHttpServerDecorator + extends HttpServerDecorator { public static final AkkaHttpServerDecorator DECORATE = new AkkaHttpServerDecorator(); @Override @@ -23,8 +25,8 @@ public class AkkaHttpServerDecorator extends HttpServerDecorator { +public class JettyDecorator + extends HttpServerDecorator { public static final JettyDecorator DECORATE = new JettyDecorator(); @Override @@ -24,8 +26,8 @@ public class JettyDecorator extends HttpServerDecorator { +public class NettyHttpServerDecorator + extends HttpServerDecorator { public static final NettyHttpServerDecorator DECORATE = new NettyHttpServerDecorator(); @Override @@ -29,33 +33,45 @@ public class NettyHttpServerDecorator extends HttpServerDecorator { +public class NettyHttpServerDecorator + extends HttpServerDecorator { public static final NettyHttpServerDecorator DECORATE = new NettyHttpServerDecorator(); @Override @@ -29,33 +33,45 @@ public class NettyHttpServerDecorator extends HttpServerDecorator { +public class PlayHttpServerDecorator extends HttpServerDecorator { public static final PlayHttpServerDecorator DECORATE = new PlayHttpServerDecorator(); @Override @@ -31,18 +31,18 @@ public class PlayHttpServerDecorator extends HttpServerDecorator { +public class Servlet2Decorator + extends HttpServerDecorator { public static final Servlet2Decorator DECORATE = new Servlet2Decorator(); @Override @@ -24,8 +26,8 @@ public class Servlet2Decorator extends HttpServerDecorator { + extends HttpServerDecorator { public static final Servlet3Decorator DECORATE = new Servlet3Decorator(); @Override @@ -25,8 +26,8 @@ public class Servlet3Decorator } @Override - protected String url(final HttpServletRequest httpServletRequest) { - return httpServletRequest.getRequestURL().toString(); + protected URI url(final HttpServletRequest httpServletRequest) { + return URI.create(httpServletRequest.getRequestURL().toString()); } @Override diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java index 1ccd9b125b..2c38bf4398 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java @@ -5,6 +5,7 @@ import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; import io.opentracing.Scope; import io.opentracing.Span; +import java.net.URI; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; @@ -13,7 +14,7 @@ import org.springframework.web.servlet.ModelAndView; @Slf4j public class SpringWebHttpServerDecorator - extends HttpServerDecorator { + extends HttpServerDecorator { public static final SpringWebHttpServerDecorator DECORATE = new SpringWebHttpServerDecorator(); public static final SpringWebHttpServerDecorator DECORATE_RENDER = new SpringWebHttpServerDecorator() { @@ -39,8 +40,8 @@ public class SpringWebHttpServerDecorator } @Override - protected String url(final HttpServletRequest httpServletRequest) { - return httpServletRequest.getRequestURL().toString(); + protected URI url(final HttpServletRequest httpServletRequest) { + return URI.create(httpServletRequest.getRequestURL().toString()); } @Override