From 0315c976af05b5b287255d57a6cc86469d9e0f3b Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 16 Apr 2019 12:43:44 -0700 Subject: [PATCH] Fixes for ratpack. --- .../agent/decorator/HttpServerDecorator.java | 11 ++- .../server/NettyHttpServerDecorator.java | 2 +- .../latestDepTest/groovy/RatpackTest.groovy | 83 +++++++++++-------- .../ratpack/RatpackServerDecorator.java | 28 +++++-- .../ratpack/TracingHandler.java | 1 + .../src/test/groovy/RatpackTest.groovy | 83 +++++++++++-------- 6 files changed, 129 insertions(+), 79 deletions(-) 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 fc3c82f316..9e63f2e680 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 @@ -5,10 +5,15 @@ import datadog.trace.api.DDSpanTypes; import io.opentracing.Span; import io.opentracing.tag.Tags; import java.net.URI; +import java.util.regex.Pattern; import lombok.extern.slf4j.Slf4j; @Slf4j public abstract class HttpServerDecorator extends ServerDecorator { + // Source: https://www.regextester.com/22 + private static final Pattern VALID_IPV4_ADDRESS = + Pattern.compile( + "^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$"); protected abstract String method(REQUEST request); @@ -68,10 +73,10 @@ public abstract class HttpServerDecorator extends 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); - } else { + if (VALID_IPV4_ADDRESS.matcher(ip).matches()) { Tags.PEER_HOST_IPV4.set(span, ip); + } else if (ip.contains(":")) { + Tags.PEER_HOST_IPV6.set(span, ip); } } Tags.PEER_PORT.set(span, peerPort(connection)); diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java index 2050d7f142..c5bc2dd859 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java @@ -38,7 +38,7 @@ public class NettyHttpServerDecorator try { URI uri = new URI(request.uri()); if ((uri.getHost() == null || uri.getHost().equals("")) && request.headers().contains(HOST)) { - uri = new URI("http://" + request.headers().get(HOST) + request.getUri()); + uri = new URI("http://" + request.headers().get(HOST) + request.uri()); } return new URI( uri.getScheme(), null, uri.getHost(), uri.getPort(), uri.getPath(), null, null); diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy b/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy index 7e6b1839e2..adf95f13ef 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy @@ -1,3 +1,7 @@ +import static datadog.trace.agent.test.server.http.TestHttpServer.distributedRequestTrace +import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer +import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT + import datadog.trace.agent.test.AgentTestRunner import datadog.trace.agent.test.utils.OkHttpUtils import datadog.trace.api.DDSpanTypes @@ -8,6 +12,8 @@ import io.opentracing.Scope import io.opentracing.Span import io.opentracing.tag.Tags import io.opentracing.util.GlobalTracer +import java.util.concurrent.CountDownLatch +import java.util.regex.Pattern import okhttp3.HttpUrl import okhttp3.OkHttpClient import okhttp3.Request @@ -20,13 +26,6 @@ import ratpack.http.client.HttpClient import ratpack.path.PathBinding import ratpack.test.exec.ExecHarness -import java.util.concurrent.CountDownLatch -import java.util.regex.Pattern - -import static datadog.trace.agent.test.server.http.TestHttpServer.distributedRequestTrace -import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer -import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT - class RatpackTest extends AgentTestRunner { OkHttpClient client = OkHttpUtils.client() @@ -85,7 +84,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer defaultTags() } } @@ -98,7 +99,7 @@ class RatpackTest extends AgentTestRunner { def app = GroovyEmbeddedApp.ratpack { handlers { prefix(":foo/:bar?") { - get("baz") { ctx -> + get("baz") {ctx -> context.render(ctx.get(PathBinding).description) } } @@ -149,7 +150,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/a/b/baz" + "$Tags.HTTP_URL.key" "${app.address}a/b/baz" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer defaultTags() } } @@ -209,7 +212,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer errorTags(HandlerException, Pattern.compile("java.lang.ArithmeticException: Division( is)? undefined")) defaultTags() } @@ -274,7 +279,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer "$Tags.ERROR.key" true defaultTags() } @@ -337,7 +344,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer "$Tags.ERROR.key" true defaultTags() } @@ -365,17 +374,17 @@ class RatpackTest extends AgentTestRunner { def app = GroovyEmbeddedApp.ratpack { handlers { - get { HttpClient httpClient -> + get {HttpClient httpClient -> // 1st internal http client call to nested httpClient.get(HttpUrlBuilder.base(external.address).path("nested").build()) - .map { it.body.text } - .flatMap { t -> - // make a 2nd http request and concatenate the two bodies together - httpClient.get(HttpUrlBuilder.base(external.address).path("nested2").build()) map { t + it.body.text } - } - .then { - context.render(it) - } + .map {it.body.text} + .flatMap {t -> + // make a 2nd http request and concatenate the two bodies together + httpClient.get(HttpUrlBuilder.base(external.address).path("nested2").build()) map {t + it.body.text} + } + .then { + context.render(it) + } } } } @@ -431,7 +440,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer defaultTags() } } @@ -485,12 +496,12 @@ class RatpackTest extends AgentTestRunner { def app = GroovyEmbeddedApp.ratpack { handlers { - get { HttpClient httpClient -> + get {HttpClient httpClient -> httpClient.get(badAddress) - .map { it.body.text } + .map {it.body.text} .then { - context.render(it) - } + context.render(it) + } } } } @@ -539,7 +550,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer "$Tags.ERROR.key" true defaultTags() } @@ -642,7 +655,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer defaultTags() } } @@ -666,11 +681,11 @@ class RatpackTest extends AgentTestRunner { scope.span().setBaggageItem("test-baggage", "foo") ParallelBatch.of(testPromise(), testPromise()) .yield() - .map({ now -> - // close the scope now that we got the baggage inside the promises - scope.close() - return now - }) + .map({now -> + // close the scope now that we got the baggage inside the promises + scope.close() + return now + }) }) then: 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 52ea594c83..872f774d85 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 @@ -1,14 +1,19 @@ package datadog.trace.instrumentation.ratpack; +import com.google.common.net.HostAndPort; import datadog.trace.agent.decorator.HttpServerDecorator; import datadog.trace.api.DDTags; import io.opentracing.Span; +import java.net.URI; +import lombok.extern.slf4j.Slf4j; import ratpack.handling.Context; import ratpack.http.Request; import ratpack.http.Response; import ratpack.http.Status; +import ratpack.server.PublicAddress; -public class RatpackServerDecorator extends HttpServerDecorator { +@Slf4j +public class RatpackServerDecorator extends HttpServerDecorator { public static final RatpackServerDecorator DECORATE = new RatpackServerDecorator(); @Override @@ -27,18 +32,27 @@ public class RatpackServerDecorator extends HttpServerDecorator + get("baz") {ctx -> context.render(ctx.get(PathBinding).description) } } @@ -148,7 +149,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/a/b/baz" + "$Tags.HTTP_URL.key" "${app.address}a/b/baz" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer defaultTags() } } @@ -208,7 +211,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer errorTags(HandlerException, Pattern.compile("java.lang.ArithmeticException: Division( is)? undefined")) defaultTags() } @@ -273,7 +278,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer "$Tags.ERROR.key" true defaultTags() } @@ -336,7 +343,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer "$Tags.ERROR.key" true defaultTags() } @@ -364,17 +373,17 @@ class RatpackTest extends AgentTestRunner { def app = GroovyEmbeddedApp.ratpack { handlers { - get { HttpClient httpClient -> + get {HttpClient httpClient -> // 1st internal http client call to nested httpClient.get(HttpUrlBuilder.base(external.address).path("nested").build()) - .map { it.body.text } - .flatMap { t -> - // make a 2nd http request and concatenate the two bodies together - httpClient.get(HttpUrlBuilder.base(external.address).path("nested2").build()) map { t + it.body.text } - } - .then { - context.render(it) - } + .map {it.body.text} + .flatMap {t -> + // make a 2nd http request and concatenate the two bodies together + httpClient.get(HttpUrlBuilder.base(external.address).path("nested2").build()) map {t + it.body.text} + } + .then { + context.render(it) + } } } } @@ -430,7 +439,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer defaultTags() } } @@ -484,12 +495,12 @@ class RatpackTest extends AgentTestRunner { def app = GroovyEmbeddedApp.ratpack { handlers { - get { HttpClient httpClient -> + get {HttpClient httpClient -> httpClient.get(badAddress) - .map { it.body.text } + .map {it.body.text} .then { - context.render(it) - } + context.render(it) + } } } } @@ -538,7 +549,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer errorTags(ConnectException, String) defaultTags() } @@ -641,7 +654,9 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/" + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_PORT.key" Integer defaultTags() } } @@ -665,11 +680,11 @@ class RatpackTest extends AgentTestRunner { scope.span().setBaggageItem("test-baggage", "foo") ParallelBatch.of(testPromise(), testPromise()) .yield() - .map({ now -> - // close the scope now that we got the baggage inside the promises - scope.close() - return now - }) + .map({now -> + // close the scope now that we got the baggage inside the promises + scope.close() + return now + }) }) then: