From 03ba21f327507c629967762e4ea7ee1cf5ee1794 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 6 Dec 2022 16:04:04 +0100 Subject: [PATCH 1/2] fix spring webflux path extraction + improve webflux tests --- .../spring-webflux-5/build.gradle | 27 +- .../SingleThreadedSpringWebfluxTest.groovy | 27 - .../groovy/SpringWebfluxTest.groovy | 626 ------------------ .../SingleThreadedSpringWebfluxTest.groovy | 28 - .../springwebflux/server/EchoHandler.groovy | 21 - .../server/EchoHandlerFunction.groovy | 21 - .../springwebflux/server/FooModel.groovy | 16 - .../SpringWebFluxTestApplication.groovy | 120 ---- .../server/TestController.groovy | 63 -- .../server/RedirectComponent.java | 21 - .../SingleThreadedSpringWebfluxTest.groovy | 23 + .../groovy/SpringWebfluxTest.groovy | 17 +- .../springwebflux/server/EchoHandler.groovy | 0 .../server/EchoHandlerFunction.groovy | 0 .../springwebflux/server/FooModel.groovy | 0 .../SpringWebFluxTestApplication.groovy | 0 .../server/TestController.groovy | 0 .../server/RedirectComponent.java | 0 .../server/RouteOnSuccessOrError.java | 25 +- 19 files changed, 59 insertions(+), 976 deletions(-) delete mode 100644 dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/SingleThreadedSpringWebfluxTest.groovy delete mode 100644 dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/SpringWebfluxTest.groovy delete mode 100644 dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/SingleThreadedSpringWebfluxTest.groovy delete mode 100644 dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandler.groovy delete mode 100644 dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandlerFunction.groovy delete mode 100644 dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/FooModel.groovy delete mode 100644 dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/SpringWebFluxTestApplication.groovy delete mode 100644 dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/TestController.groovy delete mode 100644 dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/java/dd/trace/instrumentation/springwebflux/server/RedirectComponent.java create mode 100644 dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/SingleThreadedSpringWebfluxTest.groovy rename dd-java-agent/instrumentation/spring-webflux-5/src/{boot24Test => bootTest}/groovy/SpringWebfluxTest.groovy (99%) rename dd-java-agent/instrumentation/spring-webflux-5/src/{boot20Test => bootTest}/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandler.groovy (100%) rename dd-java-agent/instrumentation/spring-webflux-5/src/{boot20Test => bootTest}/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandlerFunction.groovy (100%) rename dd-java-agent/instrumentation/spring-webflux-5/src/{boot20Test => bootTest}/groovy/dd/trace/instrumentation/springwebflux/server/FooModel.groovy (100%) rename dd-java-agent/instrumentation/spring-webflux-5/src/{boot20Test => bootTest}/groovy/dd/trace/instrumentation/springwebflux/server/SpringWebFluxTestApplication.groovy (100%) rename dd-java-agent/instrumentation/spring-webflux-5/src/{boot20Test => bootTest}/groovy/dd/trace/instrumentation/springwebflux/server/TestController.groovy (100%) rename dd-java-agent/instrumentation/spring-webflux-5/src/{boot20Test => bootTest}/java/dd/trace/instrumentation/springwebflux/server/RedirectComponent.java (100%) diff --git a/dd-java-agent/instrumentation/spring-webflux-5/build.gradle b/dd-java-agent/instrumentation/spring-webflux-5/build.gradle index ae0939aabdd..d7bcb899e5b 100644 --- a/dd-java-agent/instrumentation/spring-webflux-5/build.gradle +++ b/dd-java-agent/instrumentation/spring-webflux-5/build.gradle @@ -1,4 +1,3 @@ - muzzle { pass { name = "webflux_5.0.0+_with_netty_0.8.0" @@ -44,14 +43,16 @@ testSets { dirName = 'test' } - boot20Test {} latestBoot20Test { - dirName = 'boot20Test' + dirName = 'bootTest' } - boot24Test {} latestBoot24Test { - dirName = 'boot24Test' + dirName = 'bootTest' + } + + latestBoot2LatestTest { + dirName = 'bootTest' } } @@ -73,28 +74,22 @@ dependencies { latestDepTestImplementation group: 'io.projectreactor.netty', name: 'reactor-netty', version: '0.+' latestDepTestImplementation group: 'org.springframework', name: 'spring-test', version: '5.+' - boot20TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-webflux', version: '2.0.0.RELEASE' - boot20TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '2.0.0.RELEASE' - boot20TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-reactor-netty', version: '2.0.0.RELEASE' latestBoot20TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-webflux', version: '2.0.+' latestBoot20TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '2.0.+' latestBoot20TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-reactor-netty', version: '2.0.+' - boot24TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-webflux', version: '2.4.0' - boot24TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '2.4.0' - boot24TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-reactor-netty', version: '2.4.0' - latestBoot24TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-webflux', version: '2.4.+' latestBoot24TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '2.4.+' latestBoot24TestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-reactor-netty', version: '2.4.+' -} -tasks.named("test").configure { - dependsOn "boot20Test" - dependsOn "boot24Test" + latestBoot2LatestTestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-webflux', version: '2+' + latestBoot2LatestTestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '2+' + latestBoot2LatestTestImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-reactor-netty', version: '2+' } + tasks.named("latestDepTest").configure { dependsOn "latestBoot20Test" dependsOn "latestBoot24Test" + dependsOn "latestBoot2LatestTest" } diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/SingleThreadedSpringWebfluxTest.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/SingleThreadedSpringWebfluxTest.groovy deleted file mode 100644 index b8bf6f4210b..00000000000 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/SingleThreadedSpringWebfluxTest.groovy +++ /dev/null @@ -1,27 +0,0 @@ -import dd.trace.instrumentation.springwebflux.server.SpringWebFluxTestApplication -import org.springframework.boot.test.context.SpringBootTest -import org.springframework.boot.test.context.TestConfiguration -import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory -import org.springframework.boot.web.embedded.netty.NettyServerCustomizer -import org.springframework.context.annotation.Bean -import reactor.ipc.netty.resources.LoopResources - -/** - * Run all Webflux tests under netty event loop having only 1 thread. - * Some of the bugs are better visible in this setup because same thread is reused - * for different requests. - */ -@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, classes = [SpringWebFluxTestApplication, ForceSingleThreadedNettyAutoConfiguration]) -class SingleThreadedSpringWebfluxTest extends SpringWebfluxTest { - - @TestConfiguration - static class ForceSingleThreadedNettyAutoConfiguration { - @Bean - NettyReactiveWebServerFactory nettyFactory() { - def factory = new NettyReactiveWebServerFactory() - NettyServerCustomizer customizer = { builder -> builder.loopResources(LoopResources.create("my-http", 1, true)) } - factory.addServerCustomizers(customizer) - return factory - } - } -} diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/SpringWebfluxTest.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/SpringWebfluxTest.groovy deleted file mode 100644 index 6e5aa14e5e3..00000000000 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/SpringWebfluxTest.groovy +++ /dev/null @@ -1,626 +0,0 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.agent.test.utils.OkHttpUtils -import datadog.trace.api.DDSpanTypes -import datadog.trace.bootstrap.instrumentation.api.Tags -import dd.trace.instrumentation.springwebflux.server.EchoHandlerFunction -import dd.trace.instrumentation.springwebflux.server.FooModel -import dd.trace.instrumentation.springwebflux.server.SpringWebFluxTestApplication -import dd.trace.instrumentation.springwebflux.server.TestController -import okhttp3.OkHttpClient -import okhttp3.Request -import okhttp3.RequestBody -import org.springframework.boot.test.context.SpringBootTest -import org.springframework.boot.test.context.TestConfiguration -import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory -import org.springframework.boot.web.server.LocalServerPort -import org.springframework.context.annotation.Bean -import org.springframework.web.server.ResponseStatusException - -@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, classes = [SpringWebFluxTestApplication, ForceNettyAutoConfiguration]) -class SpringWebfluxTest extends AgentTestRunner { - - @TestConfiguration - static class ForceNettyAutoConfiguration { - @Bean - NettyReactiveWebServerFactory nettyFactory() { - return new NettyReactiveWebServerFactory() - } - } - - static final okhttp3.MediaType PLAIN_TYPE = okhttp3.MediaType.parse("text/plain; charset=utf-8") - static final String INNER_HANDLER_FUNCTION_CLASS_TAG_PREFIX = SpringWebFluxTestApplication.getName() + "\$" - static final String SPRING_APP_CLASS_ANON_NESTED_CLASS_PREFIX = SpringWebFluxTestApplication.getSimpleName() + "\$" - - @LocalServerPort - private int port - - OkHttpClient client = OkHttpUtils.client(true) - - def "Basic GET test #testName"() { - setup: - String url = "http://localhost:$port$urlPath" - def request = new Request.Builder().url(url).get().build() - when: - def response = client.newCall(request).execute() - - then: - response.code == 200 - response.body().string() == expectedResponseBody - assertTraces(1) { - sortSpansByStart() - trace(2) { - span { - resourceName "GET $urlPathWithVariables" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - parent() - tags { - "$Tags.COMPONENT" "netty" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "$Tags.PEER_HOST_IPV4" "127.0.0.1" - "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" - "$Tags.HTTP_METHOD" "GET" - "$Tags.HTTP_STATUS" 200 - "$Tags.HTTP_USER_AGENT" String - "$Tags.HTTP_CLIENT_IP" "127.0.0.1" - // BUG - if (!testName.startsWith("functional")) { "$Tags.HTTP_ROUTE" "$urlPathWithVariables" } - defaultTags() - } - } - span { - if (annotatedMethod == null) { - // Functional API - resourceNameContains(SPRING_APP_CLASS_ANON_NESTED_CLASS_PREFIX, ".handle") - operationNameContains(SPRING_APP_CLASS_ANON_NESTED_CLASS_PREFIX, ".handle") - } else { - // Annotation API - resourceName TestController.getSimpleName() + "." + annotatedMethod - operationName TestController.getSimpleName() + "." + annotatedMethod - } - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - tags { - "$Tags.COMPONENT" "spring-webflux-controller" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - if (annotatedMethod == null) { - // Functional API - "request.predicate" "(GET && $urlPathWithVariables)" - "handler.type" { String tagVal -> - return tagVal.contains(INNER_HANDLER_FUNCTION_CLASS_TAG_PREFIX) - } - } else { - // Annotation API - "handler.type" TestController.getName() - } - defaultTags() - } - } - } - } - - where: - testName | urlPath | urlPathWithVariables | annotatedMethod | expectedResponseBody - "functional API without parameters" | "/greet" | "/greet" | null | SpringWebFluxTestApplication.GreetingHandler.DEFAULT_RESPONSE - "functional API with one parameter" | "/greet/WORLD" | "/greet/{name}" | null | SpringWebFluxTestApplication.GreetingHandler.DEFAULT_RESPONSE + " WORLD" - "functional API with two parameters" | "/greet/World/Test1" | "/greet/{name}/{word}" | null | SpringWebFluxTestApplication.GreetingHandler.DEFAULT_RESPONSE + " World Test1" - "functional API delayed response" | "/greet-delayed" | "/greet-delayed" | null | SpringWebFluxTestApplication.GreetingHandler.DEFAULT_RESPONSE - - "annotation API without parameters" | "/foo" | "/foo" | "getFooModel" | new FooModel(0L, "DEFAULT").toString() - "annotation API with one parameter" | "/foo/1" | "/foo/{id}" | "getFooModel" | new FooModel(1L, "pass").toString() - "annotation API with two parameters" | "/foo/2/world" | "/foo/{id}/{name}" | "getFooModel" | new FooModel(2L, "world").toString() - "annotation API delayed response" | "/foo-delayed" | "/foo-delayed" | "getFooDelayed" | new FooModel(3L, "delayed").toString() - } - - def "GET test with async response #testName"() { - setup: - String url = "http://localhost:$port$urlPath" - def request = new Request.Builder().url(url).get().build() - when: - def response = client.newCall(request).execute() - - then: - response.code == 200 - response.body().string() == expectedResponseBody - assertTraces(1) { - sortSpansByStart() - trace(3) { - span { - resourceName "GET $urlPathWithVariables" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - parent() - tags { - "$Tags.COMPONENT" "netty" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "$Tags.PEER_HOST_IPV4" "127.0.0.1" - "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" - "$Tags.HTTP_METHOD" "GET" - "$Tags.HTTP_STATUS" 200 - "$Tags.HTTP_USER_AGENT" String - "$Tags.HTTP_CLIENT_IP" "127.0.0.1" - // BUG - if (!testName.startsWith("functional")) { "$Tags.HTTP_ROUTE" "$urlPathWithVariables" } - defaultTags() - } - } - span { - if (annotatedMethod == null) { - // Functional API - resourceNameContains(SPRING_APP_CLASS_ANON_NESTED_CLASS_PREFIX, ".handle") - operationNameContains(SPRING_APP_CLASS_ANON_NESTED_CLASS_PREFIX, ".handle") - } else { - // Annotation API - resourceName TestController.getSimpleName() + "." + annotatedMethod - operationName TestController.getSimpleName() + "." + annotatedMethod - } - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - tags { - "$Tags.COMPONENT" "spring-webflux-controller" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - if (annotatedMethod == null) { - // Functional API - "request.predicate" "(GET && $urlPathWithVariables)" - "handler.type" { String tagVal -> - return tagVal.contains(INNER_HANDLER_FUNCTION_CLASS_TAG_PREFIX) - } - } else { - // Annotation API - "handler.type" TestController.getName() - } - defaultTags() - } - } - span { - if (annotatedMethod == null) { - // Functional API - resourceName "SpringWebFluxTestApplication.tracedMethod" - operationName "trace.annotation" - } else { - // Annotation API - resourceName "TestController.tracedMethod" - operationName "trace.annotation" - } - childOf(span(0)) // FIXME this is wrong - errored false - tags { - "$Tags.COMPONENT" "trace" - defaultTags() - } - } - } - } - - where: - testName | urlPath | urlPathWithVariables | annotatedMethod | expectedResponseBody - "functional API traced method from mono" | "/greet-mono-from-callable/4" | "/greet-mono-from-callable/{id}" | null | SpringWebFluxTestApplication.GreetingHandler.DEFAULT_RESPONSE + " 4" - "functional API traced method with delay" | "/greet-delayed-mono/6" | "/greet-delayed-mono/{id}" | null | SpringWebFluxTestApplication.GreetingHandler.DEFAULT_RESPONSE + " 6" - - "annotation API traced method from mono" | "/foo-mono-from-callable/7" | "/foo-mono-from-callable/{id}" | "getMonoFromCallable" | new FooModel(7L, "tracedMethod").toString() - "annotation API traced method with delay" | "/foo-delayed-mono/9" | "/foo-delayed-mono/{id}" | "getFooDelayedMono" | new FooModel(9L, "tracedMethod").toString() - } - - /* - This test differs from the previous in one important aspect. - The test above calls endpoints which does not create any spans during their invocation. - They merely assemble reactive pipeline where some steps create spans. - Thus all those spans are created when WebFlux span created by DispatcherHandlerInstrumentation - has already finished. Therefore, they have `SERVER` span as their parent. - This test below calls endpoints which do create spans right inside endpoint handler. - Therefore, in theory, those spans should have INTERNAL span created by DispatcherHandlerInstrumentation - as their parent. But there is a difference how Spring WebFlux handles functional endpoints - (created in server.SpringWebFluxTestApplication.greetRouterFunction) and annotated endpoints - (created in server.TestController). - In the former case org.springframework.web.reactive.function.server.support.HandlerFunctionAdapter.handle - calls handler function directly. Thus "tracedMethod" span below has INTERNAL handler span as its parent. - In the latter case org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerAdapter.handle - merely wraps handler call into Mono and thus actual invocation of handler function happens later, - when INTERNAL handler span has already finished. Thus, "tracedMethod" has SERVER Netty span as its parent. - */ - - def "Create span during handler function"() { - setup: - String url = "http://localhost:$port$urlPath" - def request = new Request.Builder().url(url).get().build() - when: - def response = client.newCall(request).execute() - - then: - response.code == 200 - response.body().string() == expectedResponseBody - assertTraces(1) { - sortSpansByStart() - trace(3) { - span { - resourceName "GET $urlPathWithVariables" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - parent() - } - span { - if (annotatedMethod == null) { - // Functional API - operationNameContains(SPRING_APP_CLASS_ANON_NESTED_CLASS_PREFIX, ".handle") - } else { - // Annotation API - operationName TestController.getSimpleName() + "." + annotatedMethod - } - spanType DDSpanTypes.HTTP_SERVER - childOf span(0) - } - span { - operationName "trace.annotation" - childOf span(annotatedMethod ? 0 : 1) - errored false - } - } - } - - where: - testName | urlPath | urlPathWithVariables | annotatedMethod | expectedResponseBody - "functional API traced method" | "/greet-traced-method/5" | "/greet-traced-method/{id}" | null | SpringWebFluxTestApplication.GreetingHandler.DEFAULT_RESPONSE + " 5" - "annotation API traced method" | "/foo-traced-method/8" | "/foo-traced-method/{id}" | "getTracedMethod" | new FooModel(8L, "tracedMethod").toString() - } - - def "404 GET test"() { - setup: - String url = "http://localhost:$port/notfoundgreet" - def request = new Request.Builder().url(url).get().build() - - when: - def response = client.newCall(request).execute() - - then: - response.code == 404 - assertTraces(1) { - sortSpansByStart() - trace(2) { - span { - resourceName "404" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - parent() - tags { - "$Tags.COMPONENT" "netty" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "$Tags.PEER_HOST_IPV4" "127.0.0.1" - "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" - "$Tags.HTTP_METHOD" "GET" - "$Tags.HTTP_STATUS" 404 - "$Tags.HTTP_USER_AGENT" String - "$Tags.HTTP_CLIENT_IP" "127.0.0.1" - defaultTags() - } - } - span { - resourceName "ResourceWebHandler.handle" - operationName "ResourceWebHandler.handle" - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - errored true - tags { - "$Tags.COMPONENT" "spring-webflux-controller" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "handler.type" "org.springframework.web.reactive.resource.ResourceWebHandler" - errorTags(ResponseStatusException, String) - defaultTags() - } - } - } - } - } - - def "Basic POST test"() { - setup: - String echoString = "TEST" - String url = "http://localhost:$port/echo" - RequestBody body = RequestBody.create(PLAIN_TYPE, echoString) - def request = new Request.Builder().url(url).post(body).build() - - when: - def response = client.newCall(request).execute() - - then: - response.code() == 202 - response.body().string() == echoString - assertTraces(1) { - sortSpansByStart() - trace(3) { - span { - resourceName "POST /echo" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - parent() - tags { - "$Tags.COMPONENT" "netty" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "$Tags.PEER_HOST_IPV4" "127.0.0.1" - "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" - "$Tags.HTTP_METHOD" "POST" - "$Tags.HTTP_STATUS" 202 - "$Tags.HTTP_USER_AGENT" String - "$Tags.HTTP_CLIENT_IP" "127.0.0.1" - // BUG "$Tags.HTTP_ROUTE" "/echo" - defaultTags() - } - } - span { - resourceName EchoHandlerFunction.getSimpleName() + ".handle" - operationName EchoHandlerFunction.getSimpleName() + ".handle" - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - tags { - "$Tags.COMPONENT" "spring-webflux-controller" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "request.predicate" "(POST && /echo)" - "handler.type" { String tagVal -> - return tagVal.contains(EchoHandlerFunction.getName()) - } - defaultTags() - } - } - span { - resourceName "echo" - operationName "echo" - childOf(span(1)) - tags { - "$Tags.COMPONENT" "trace" - defaultTags() - } - } - } - } - } - - def "GET to bad endpoint #testName"() { - setup: - String url = "http://localhost:$port$urlPath" - def request = new Request.Builder().url(url).get().build() - - when: - def response = client.newCall(request).execute() - - then: - response.code() == 500 - assertTraces(1) { - sortSpansByStart() - trace(2) { - span { - resourceName "GET $urlPathWithVariables" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - errored true - parent() - tags { - "$Tags.COMPONENT" "netty" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "$Tags.PEER_HOST_IPV4" "127.0.0.1" - "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" - "$Tags.HTTP_METHOD" "GET" - "$Tags.HTTP_STATUS" 500 - "$Tags.HTTP_USER_AGENT" String - "$Tags.HTTP_CLIENT_IP" "127.0.0.1" - // BUG - if (!testName.startsWith("functional")) { "$Tags.HTTP_ROUTE" "$urlPathWithVariables" } - defaultTags() - } - } - span { - if (annotatedMethod == null) { - // Functional API - resourceNameContains(SPRING_APP_CLASS_ANON_NESTED_CLASS_PREFIX, ".handle") - operationNameContains(SPRING_APP_CLASS_ANON_NESTED_CLASS_PREFIX, ".handle") - } else { - // Annotation API - resourceName TestController.getSimpleName() + "." + annotatedMethod - operationName TestController.getSimpleName() + "." + annotatedMethod - } - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - errored true - tags { - "$Tags.COMPONENT" "spring-webflux-controller" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - if (annotatedMethod == null) { - // Functional API - "request.predicate" "(GET && $urlPathWithVariables)" - "handler.type" { String tagVal -> - return tagVal.contains(INNER_HANDLER_FUNCTION_CLASS_TAG_PREFIX) - } - } else { - // Annotation API - "handler.type" TestController.getName() - } - errorTags(RuntimeException, "bad things happen") - defaultTags() - } - } - } - } - - where: - testName | urlPath | urlPathWithVariables | annotatedMethod - "functional API fail fast" | "/greet-failfast/1" | "/greet-failfast/{id}" | null - "functional API fail Mono" | "/greet-failmono/1" | "/greet-failmono/{id}" | null - - "annotation API fail fast" | "/foo-failfast/1" | "/foo-failfast/{id}" | "getFooFailFast" - "annotation API fail Mono" | "/foo-failmono/1" | "/foo-failmono/{id}" | "getFooFailMono" - } - - def "Redirect test"() { - setup: - String url = "http://localhost:$port/double-greet-redirect" - String finalUrl = "http://localhost:$port/double-greet" - def request = new Request.Builder().url(url).get().build() - - when: - def response = client.newCall(request).execute() - - then: - response.code == 200 - assertTraces(2) { - sortSpansByStart() - // TODO: why order of spans is different in these traces? - trace(2) { - span { - resourceName "GET /double-greet-redirect" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - parent() - tags { - "$Tags.COMPONENT" "netty" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "$Tags.PEER_HOST_IPV4" "127.0.0.1" - "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" - "$Tags.HTTP_METHOD" "GET" - "$Tags.HTTP_STATUS" 307 - "$Tags.HTTP_USER_AGENT" String - "$Tags.HTTP_CLIENT_IP" "127.0.0.1" - // BUG "$Tags.HTTP_ROUTE" "/double-greet-redirect" - defaultTags() - } - } - span { - resourceName "RedirectComponent.lambda" - operationName "RedirectComponent.lambda" - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - tags { - "$Tags.COMPONENT" "spring-webflux-controller" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "request.predicate" "(GET && /double-greet-redirect)" - "handler.type" { String tagVal -> - return (tagVal.contains(INNER_HANDLER_FUNCTION_CLASS_TAG_PREFIX) - || tagVal.contains("Lambda")) - } - defaultTags() - } - } - } - trace(2) { - span { - resourceName "GET /double-greet" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - parent() - tags { - "$Tags.COMPONENT" "netty" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "$Tags.PEER_HOST_IPV4" "127.0.0.1" - "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" finalUrl - "$Tags.HTTP_HOSTNAME" "localhost" - "$Tags.HTTP_METHOD" "GET" - "$Tags.HTTP_STATUS" 200 - "$Tags.HTTP_USER_AGENT" String - "$Tags.HTTP_CLIENT_IP" "127.0.0.1" - // BUG "$Tags.HTTP_ROUTE" "/double-greet" - defaultTags() - } - } - span { - resourceNameContains(SpringWebFluxTestApplication.getSimpleName() + "\$", ".handle") - operationNameContains(SpringWebFluxTestApplication.getSimpleName() + "\$", ".handle") - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - tags { - "$Tags.COMPONENT" "spring-webflux-controller" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "request.predicate" "(GET && /double-greet)" - "handler.type" { String tagVal -> - return tagVal.contains(INNER_HANDLER_FUNCTION_CLASS_TAG_PREFIX) - } - defaultTags() - } - } - } - } - } - - def "Multiple GETs to delaying route #testName"() { - setup: - def requestsCount = 50 // Should be more than 2x CPUs to fish out some bugs - String url = "http://localhost:$port$urlPath" - def request = new Request.Builder().url(url).get().build() - when: - def responses = (0..requestsCount - 1).collect { client.newCall(request).execute() } - - then: - responses.every { it.code == 200 } - responses.every { it.body().string() == expectedResponseBody } - assertTraces(responses.size()) { - sortSpansByStart() - responses.eachWithIndex { def response, int i -> - trace(2) { - span { - resourceName "GET $urlPathWithVariables" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - parent() - tags { - "$Tags.COMPONENT" "netty" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "$Tags.PEER_HOST_IPV4" "127.0.0.1" - "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" - "$Tags.HTTP_METHOD" "GET" - "$Tags.HTTP_STATUS" 200 - "$Tags.HTTP_USER_AGENT" String - "$Tags.HTTP_CLIENT_IP" "127.0.0.1" - // BUG - if (!testName.startsWith("functional")) { "$Tags.HTTP_ROUTE" "$urlPathWithVariables" } - defaultTags() - } - } - span { - if (annotatedMethod == null) { - // Functional API - resourceNameContains(SPRING_APP_CLASS_ANON_NESTED_CLASS_PREFIX, ".handle") - operationNameContains(SPRING_APP_CLASS_ANON_NESTED_CLASS_PREFIX, ".handle") - } else { - // Annotation API - resourceName TestController.getSimpleName() + "." + annotatedMethod - operationName TestController.getSimpleName() + "." + annotatedMethod - } - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - tags { - "$Tags.COMPONENT" "spring-webflux-controller" - "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - if (annotatedMethod == null) { - // Functional API - "request.predicate" "(GET && $urlPathWithVariables)" - "handler.type" { String tagVal -> - return tagVal.contains(INNER_HANDLER_FUNCTION_CLASS_TAG_PREFIX) - } - } else { - // Annotation API - "handler.type" TestController.getName() - } - defaultTags() - } - } - } - } - } - - where: - testName | urlPath | urlPathWithVariables | annotatedMethod | expectedResponseBody - "functional API delayed response" | "/greet-delayed" | "/greet-delayed" | null | SpringWebFluxTestApplication.GreetingHandler.DEFAULT_RESPONSE - "annotation API delayed response" | "/foo-delayed" | "/foo-delayed" | "getFooDelayed" | new FooModel(3L, "delayed").toString() - } -} diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/SingleThreadedSpringWebfluxTest.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/SingleThreadedSpringWebfluxTest.groovy deleted file mode 100644 index 8ac38c79eb0..00000000000 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/SingleThreadedSpringWebfluxTest.groovy +++ /dev/null @@ -1,28 +0,0 @@ -import dd.trace.instrumentation.springwebflux.server.SpringWebFluxTestApplication -import org.springframework.boot.test.context.SpringBootTest -import org.springframework.boot.test.context.TestConfiguration -import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory -import org.springframework.boot.web.embedded.netty.NettyServerCustomizer -import org.springframework.context.annotation.Bean -import reactor.netty.http.server.HttpServer -import reactor.netty.resources.LoopResources - -/** - * Run all Webflux tests under netty event loop having only 1 thread. - * Some of the bugs are better visible in this setup because same thread is reused - * for different requests. - */ -@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, classes = [SpringWebFluxTestApplication, ForceSingleThreadedNettyAutoConfiguration]) -class SingleThreadedSpringWebfluxTest extends SpringWebfluxTest { - - @TestConfiguration - static class ForceSingleThreadedNettyAutoConfiguration { - @Bean - NettyReactiveWebServerFactory nettyFactory() { - def factory = new NettyReactiveWebServerFactory() - NettyServerCustomizer customizer = { HttpServer httpServer -> httpServer.runOn(LoopResources.create("my-http", 1, true)) } - factory.addServerCustomizers(customizer) - return factory - } - } -} diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandler.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandler.groovy deleted file mode 100644 index 972b9c9630a..00000000000 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandler.groovy +++ /dev/null @@ -1,21 +0,0 @@ -package dd.trace.instrumentation.springwebflux.server - -import datadog.trace.api.Trace -import org.springframework.http.MediaType -import org.springframework.stereotype.Component -import org.springframework.web.reactive.function.server.ServerRequest -import org.springframework.web.reactive.function.server.ServerResponse -import reactor.core.publisher.Mono - -/** - * Note: this class has to stay outside of 'datadog.*' package because we need - * it transformed by {@code @Trace} annotation. - */ -@Component -class EchoHandler { - @Trace(operationName = "echo", resourceName = "echo") - Mono echo(ServerRequest request) { - return ServerResponse.accepted().contentType(MediaType.TEXT_PLAIN) - .body(request.bodyToMono(String), String) - } -} diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandlerFunction.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandlerFunction.groovy deleted file mode 100644 index ca00a79bb5a..00000000000 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandlerFunction.groovy +++ /dev/null @@ -1,21 +0,0 @@ -package dd.trace.instrumentation.springwebflux.server - - -import org.springframework.web.reactive.function.server.HandlerFunction -import org.springframework.web.reactive.function.server.ServerRequest -import org.springframework.web.reactive.function.server.ServerResponse -import reactor.core.publisher.Mono - -class EchoHandlerFunction implements HandlerFunction { - - EchoHandler echoHandler - - EchoHandlerFunction(EchoHandler echoHandler) { - this.echoHandler = echoHandler - } - - @Override - Mono handle(ServerRequest request) { - return echoHandler.echo(request) - } -} diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/FooModel.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/FooModel.groovy deleted file mode 100644 index b119968487d..00000000000 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/FooModel.groovy +++ /dev/null @@ -1,16 +0,0 @@ -package dd.trace.instrumentation.springwebflux.server - -class FooModel { - public long id - public String name - - FooModel(long id, String name) { - this.id = id - this.name = name - } - - @Override - String toString() { - return "{\"id\":" + id + ",\"name\":\"" + name + "\"}" - } -} diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/SpringWebFluxTestApplication.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/SpringWebFluxTestApplication.groovy deleted file mode 100644 index aa3b3135500..00000000000 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/SpringWebFluxTestApplication.groovy +++ /dev/null @@ -1,120 +0,0 @@ -package dd.trace.instrumentation.springwebflux.server - -import datadog.trace.api.Trace -import org.springframework.boot.autoconfigure.SpringBootApplication -import org.springframework.context.annotation.Bean -import org.springframework.http.MediaType -import org.springframework.stereotype.Component -import org.springframework.web.reactive.function.BodyInserters -import org.springframework.web.reactive.function.server.HandlerFunction -import org.springframework.web.reactive.function.server.RouterFunction -import org.springframework.web.reactive.function.server.ServerRequest -import org.springframework.web.reactive.function.server.ServerResponse -import reactor.core.publisher.Mono - -import java.time.Duration - -import static org.springframework.web.reactive.function.server.RequestPredicates.GET -import static org.springframework.web.reactive.function.server.RequestPredicates.POST -import static org.springframework.web.reactive.function.server.RouterFunctions.route - -@SpringBootApplication -class SpringWebFluxTestApplication { - - @Bean - RouterFunction echoRouterFunction(EchoHandler echoHandler) { - return route(POST("/echo"), new EchoHandlerFunction(echoHandler)) - } - - @Bean - RouterFunction greetRouterFunction(GreetingHandler greetingHandler) { - return route(GET("/greet"), new HandlerFunction() { - @Override - Mono handle(ServerRequest request) { - return greetingHandler.defaultGreet() - } - }).andRoute(GET("/greet/{name}"), new HandlerFunction() { - @Override - Mono handle(ServerRequest request) { - return greetingHandler.customGreet(request) - } - }).andRoute(GET("/greet/{name}/{word}"), new HandlerFunction() { - @Override - Mono handle(ServerRequest request) { - return greetingHandler.customGreetWithWord(request) - } - }).andRoute(GET("/double-greet"), new HandlerFunction() { - @Override - Mono handle(ServerRequest request) { - return greetingHandler.doubleGreet() - } - }).andRoute(GET("/greet-delayed"), new HandlerFunction() { - @Override - Mono handle(ServerRequest request) { - return greetingHandler.defaultGreet().delayElement(Duration.ofMillis(100)) - } - }).andRoute(GET("/greet-failfast/{id}"), new HandlerFunction() { - @Override - Mono handle(ServerRequest request) { - throw new RuntimeException("bad things happen") - } - }).andRoute(GET("/greet-failmono/{id}"), new HandlerFunction() { - @Override - Mono handle(ServerRequest request) { - return Mono.error(new RuntimeException("bad things happen")) - } - }).andRoute(GET("/greet-traced-method/{id}"), new HandlerFunction() { - @Override - Mono handle(ServerRequest request) { - return greetingHandler.intResponse(Mono.just(tracedMethod(request.pathVariable("id").toInteger()))) - } - }).andRoute(GET("/greet-mono-from-callable/{id}"), new HandlerFunction() { - @Override - Mono handle(ServerRequest request) { - return greetingHandler.intResponse(Mono.fromCallable { - return tracedMethod(request.pathVariable("id").toInteger()) - }) - } - }).andRoute(GET("/greet-delayed-mono/{id}"), new HandlerFunction() { - @Override - Mono handle(ServerRequest request) { - return greetingHandler.intResponse(Mono.just(request.pathVariable("id").toInteger()).delayElement(Duration.ofMillis(100)).map { i -> tracedMethod(i) }) - } - }) - } - - @Component - class GreetingHandler { - static final String DEFAULT_RESPONSE = "HELLO" - - Mono defaultGreet() { - return ServerResponse.ok().contentType(MediaType.TEXT_PLAIN) - .body(BodyInserters.fromObject(DEFAULT_RESPONSE)) - } - - Mono doubleGreet() { - return ServerResponse.ok().contentType(MediaType.TEXT_PLAIN) - .body(BodyInserters.fromObject(DEFAULT_RESPONSE + DEFAULT_RESPONSE)) - } - - Mono customGreet(ServerRequest request) { - return ServerResponse.ok().contentType(MediaType.TEXT_PLAIN) - .body(BodyInserters.fromObject(DEFAULT_RESPONSE + " " + request.pathVariable("name"))) - } - - Mono customGreetWithWord(ServerRequest request) { - return ServerResponse.ok().contentType(MediaType.TEXT_PLAIN) - .body(BodyInserters.fromObject(DEFAULT_RESPONSE + " " + request.pathVariable("name") + " " + request.pathVariable("word"))) - } - - Mono intResponse(Mono mono) { - return ServerResponse.ok().contentType(MediaType.TEXT_PLAIN) - .body(BodyInserters.fromPublisher(mono.map { i -> DEFAULT_RESPONSE + " " + i.id }, String)) - } - } - - @Trace() - private static FooModel tracedMethod(long id) { - return new FooModel(id, "tracedMethod") - } -} diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/TestController.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/TestController.groovy deleted file mode 100644 index 4fd639ac363..00000000000 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/dd/trace/instrumentation/springwebflux/server/TestController.groovy +++ /dev/null @@ -1,63 +0,0 @@ -package dd.trace.instrumentation.springwebflux.server - -import datadog.trace.api.Trace -import org.springframework.web.bind.annotation.GetMapping -import org.springframework.web.bind.annotation.PathVariable -import org.springframework.web.bind.annotation.RestController -import reactor.core.publisher.Mono - -import java.time.Duration - -@RestController -class TestController { - - @GetMapping("/foo") - Mono getFooModel() { - return Mono.just(new FooModel(0L, "DEFAULT")) - } - - @GetMapping("/foo/{id}") - Mono getFooModel(@PathVariable("id") long id) { - return Mono.just(new FooModel(id, "pass")) - } - - @GetMapping("/foo/{id}/{name}") - Mono getFooModel(@PathVariable("id") long id, @PathVariable("name") String name) { - return Mono.just(new FooModel(id, name)) - } - - @GetMapping("/foo-delayed") - Mono getFooDelayed() { - return Mono.just(new FooModel(3L, "delayed")).delayElement(Duration.ofMillis(1)) - } - - @GetMapping("/foo-failfast/{id}") - Mono getFooFailFast(@PathVariable("id") long id) { - throw new RuntimeException("bad things happen") - } - - @GetMapping("/foo-failmono/{id}") - Mono getFooFailMono(@PathVariable("id") long id) { - return Mono.error(new RuntimeException("bad things happen")) - } - - @GetMapping("/foo-traced-method/{id}") - Mono getTracedMethod(@PathVariable("id") long id) { - return Mono.just(tracedMethod(id)) - } - - @GetMapping("/foo-mono-from-callable/{id}") - Mono getMonoFromCallable(@PathVariable("id") long id) { - return Mono.fromCallable { return tracedMethod(id) } - } - - @GetMapping("/foo-delayed-mono/{id}") - Mono getFooDelayedMono(@PathVariable("id") long id) { - return Mono.just(id).delayElement(Duration.ofMillis(1)).map { i -> tracedMethod(i) } - } - - @Trace() - private FooModel tracedMethod(long id) { - return new FooModel(id, "tracedMethod") - } -} diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/java/dd/trace/instrumentation/springwebflux/server/RedirectComponent.java b/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/java/dd/trace/instrumentation/springwebflux/server/RedirectComponent.java deleted file mode 100644 index 85917014396..00000000000 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/java/dd/trace/instrumentation/springwebflux/server/RedirectComponent.java +++ /dev/null @@ -1,21 +0,0 @@ -package dd.trace.instrumentation.springwebflux.server; - -import static org.springframework.web.reactive.function.server.RequestPredicates.GET; -import static org.springframework.web.reactive.function.server.RouterFunctions.route; - -import java.net.URI; -import org.springframework.context.annotation.Bean; -import org.springframework.stereotype.Component; -import org.springframework.web.reactive.function.server.RouterFunction; -import org.springframework.web.reactive.function.server.ServerResponse; - -// Need to keep this in Java because groovy creates crazy proxies around lambdas -@Component -public class RedirectComponent { - @Bean - public RouterFunction redirectRouterFunction() { - return route( - GET("/double-greet-redirect"), - req -> ServerResponse.temporaryRedirect(URI.create("/double-greet")).build()); - } -} diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/SingleThreadedSpringWebfluxTest.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/SingleThreadedSpringWebfluxTest.groovy new file mode 100644 index 00000000000..23bddcfcc9a --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/SingleThreadedSpringWebfluxTest.groovy @@ -0,0 +1,23 @@ +import dd.trace.instrumentation.springwebflux.server.SpringWebFluxTestApplication +import org.junit.AfterClass +import org.junit.BeforeClass +import org.springframework.boot.test.context.SpringBootTest + +/** + * Run all Webflux tests under netty event loop having only 1 thread. + * Some of the bugs are better visible in this setup because same thread is reused + * for different requests. + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, classes = [SpringWebFluxTestApplication, ForceNettyAutoConfiguration]) +class SingleThreadedSpringWebfluxTest extends SpringWebfluxTest { + + @BeforeClass + static void init() { + System.setProperty("reactor.netty.ioWorkerCount", "1") + } + + @AfterClass + static void teardown() { + System.clearProperty("reactor.netty.ioWorkerCount") + } +} diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/SpringWebfluxTest.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/SpringWebfluxTest.groovy similarity index 99% rename from dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/SpringWebfluxTest.groovy rename to dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/SpringWebfluxTest.groovy index ca786b08601..6e0a5bb83d9 100644 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/boot24Test/groovy/SpringWebfluxTest.groovy +++ b/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/SpringWebfluxTest.groovy @@ -63,9 +63,9 @@ class SpringWebfluxTest extends AgentTestRunner { "$Tags.HTTP_HOSTNAME" "localhost" "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 200 - "$Tags.HTTP_ROUTE" "$urlPathWithVariables" "$Tags.HTTP_USER_AGENT" String "$Tags.HTTP_CLIENT_IP" "127.0.0.1" + "$Tags.HTTP_ROUTE" "$urlPathWithVariables" defaultTags() } } @@ -140,9 +140,9 @@ class SpringWebfluxTest extends AgentTestRunner { "$Tags.HTTP_HOSTNAME" "localhost" "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 200 - "$Tags.HTTP_ROUTE" "$urlPathWithVariables" "$Tags.HTTP_USER_AGENT" String "$Tags.HTTP_CLIENT_IP" "127.0.0.1" + "$Tags.HTTP_ROUTE" "$urlPathWithVariables" defaultTags() } } @@ -345,9 +345,9 @@ class SpringWebfluxTest extends AgentTestRunner { "$Tags.HTTP_HOSTNAME" "localhost" "$Tags.HTTP_METHOD" "POST" "$Tags.HTTP_STATUS" 202 - "$Tags.HTTP_ROUTE" "/echo" "$Tags.HTTP_USER_AGENT" String "$Tags.HTTP_CLIENT_IP" "127.0.0.1" + "$Tags.HTTP_ROUTE" "/echo" defaultTags() } } @@ -407,9 +407,9 @@ class SpringWebfluxTest extends AgentTestRunner { "$Tags.HTTP_HOSTNAME" "localhost" "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 500 - "$Tags.HTTP_ROUTE" "$urlPathWithVariables" "$Tags.HTTP_USER_AGENT" String "$Tags.HTTP_CLIENT_IP" "127.0.0.1" + "$Tags.HTTP_ROUTE" "$urlPathWithVariables" defaultTags() } } @@ -484,9 +484,9 @@ class SpringWebfluxTest extends AgentTestRunner { "$Tags.HTTP_HOSTNAME" "localhost" "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 307 - "$Tags.HTTP_ROUTE" "/double-greet-redirect" "$Tags.HTTP_USER_AGENT" String "$Tags.HTTP_CLIENT_IP" "127.0.0.1" + "$Tags.HTTP_ROUTE" "/double-greet-redirect" defaultTags() } } @@ -522,9 +522,9 @@ class SpringWebfluxTest extends AgentTestRunner { "$Tags.HTTP_HOSTNAME" "localhost" "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 200 - "$Tags.HTTP_ROUTE" "/double-greet" "$Tags.HTTP_USER_AGENT" String "$Tags.HTTP_CLIENT_IP" "127.0.0.1" + "$Tags.HTTP_ROUTE" "/double-greet" defaultTags() } } @@ -553,10 +553,9 @@ class SpringWebfluxTest extends AgentTestRunner { String url = "http://localhost:$port$urlPath" def request = new Request.Builder().url(url).get().build() when: - def responses = (1..requestsCount).collect { client.newCall(request).execute() } + def responses = (0..requestsCount - 1).collect { client.newCall(request).execute() } then: - responses.size() == requestsCount responses.every { it.code == 200 } responses.every { it.body().string() == expectedResponseBody } assertTraces(responses.size()) { @@ -577,9 +576,9 @@ class SpringWebfluxTest extends AgentTestRunner { "$Tags.HTTP_HOSTNAME" "localhost" "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 200 - "$Tags.HTTP_ROUTE" String "$Tags.HTTP_USER_AGENT" String "$Tags.HTTP_CLIENT_IP" "127.0.0.1" + "$Tags.HTTP_ROUTE" "$urlPathWithVariables" defaultTags() } } diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandler.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandler.groovy similarity index 100% rename from dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandler.groovy rename to dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandler.groovy diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandlerFunction.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandlerFunction.groovy similarity index 100% rename from dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandlerFunction.groovy rename to dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/dd/trace/instrumentation/springwebflux/server/EchoHandlerFunction.groovy diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/dd/trace/instrumentation/springwebflux/server/FooModel.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/dd/trace/instrumentation/springwebflux/server/FooModel.groovy similarity index 100% rename from dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/dd/trace/instrumentation/springwebflux/server/FooModel.groovy rename to dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/dd/trace/instrumentation/springwebflux/server/FooModel.groovy diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/dd/trace/instrumentation/springwebflux/server/SpringWebFluxTestApplication.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/dd/trace/instrumentation/springwebflux/server/SpringWebFluxTestApplication.groovy similarity index 100% rename from dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/dd/trace/instrumentation/springwebflux/server/SpringWebFluxTestApplication.groovy rename to dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/dd/trace/instrumentation/springwebflux/server/SpringWebFluxTestApplication.groovy diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/dd/trace/instrumentation/springwebflux/server/TestController.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/dd/trace/instrumentation/springwebflux/server/TestController.groovy similarity index 100% rename from dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/groovy/dd/trace/instrumentation/springwebflux/server/TestController.groovy rename to dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/groovy/dd/trace/instrumentation/springwebflux/server/TestController.groovy diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/java/dd/trace/instrumentation/springwebflux/server/RedirectComponent.java b/dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/java/dd/trace/instrumentation/springwebflux/server/RedirectComponent.java similarity index 100% rename from dd-java-agent/instrumentation/spring-webflux-5/src/boot20Test/java/dd/trace/instrumentation/springwebflux/server/RedirectComponent.java rename to dd-java-agent/instrumentation/spring-webflux-5/src/bootTest/java/dd/trace/instrumentation/springwebflux/server/RedirectComponent.java diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/main/java/datadog/trace/instrumentation/springwebflux/server/RouteOnSuccessOrError.java b/dd-java-agent/instrumentation/spring-webflux-5/src/main/java/datadog/trace/instrumentation/springwebflux/server/RouteOnSuccessOrError.java index dae08b4abc6..e27c7e86917 100644 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/main/java/datadog/trace/instrumentation/springwebflux/server/RouteOnSuccessOrError.java +++ b/dd-java-agent/instrumentation/spring-webflux-5/src/main/java/datadog/trace/instrumentation/springwebflux/server/RouteOnSuccessOrError.java @@ -1,9 +1,11 @@ package datadog.trace.instrumentation.springwebflux.server; +import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR; + import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import java.util.function.BiConsumer; import java.util.regex.Pattern; +import javax.annotation.Nonnull; import org.springframework.web.reactive.function.server.HandlerFunction; import org.springframework.web.reactive.function.server.RouterFunction; import org.springframework.web.reactive.function.server.ServerRequest; @@ -14,6 +16,9 @@ public class RouteOnSuccessOrError implements BiConsumer, Thr private static final Pattern SPACES_REGEX = Pattern.compile("[ \\t]+"); private static final Pattern ROUTER_FUNCION_REGEX = Pattern.compile("\\s*->.*$"); + private static final Pattern METHOD_REGEX = + Pattern.compile("^(GET|HEAD|POST|PUT|DELETE|CONNECT|OPTIONS|TRACE|PATCH) "); + private final RouterFunction routerFunction; private final ServerRequest serverRequest; @@ -36,8 +41,8 @@ public void accept(final HandlerFunction handler, final Throwable throwable) final AgentSpan parentSpan = (AgentSpan) serverRequest.attributes().get(AdviceUtils.PARENT_SPAN_ATTRIBUTE); if (parentSpan != null) { - parentSpan.setResourceName( - parseResourceName(predicateString), ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE); + HTTP_RESOURCE_DECORATOR.withRoute( + parentSpan, serverRequest.methodName(), parseRoute(predicateString)); } } } @@ -55,10 +60,14 @@ private String parsePredicateString() { } } - private String parseResourceName(final String routerString) { - return SPACES_REGEX - .matcher(SPECIAL_CHARACTERS_REGEX.matcher(routerString).replaceAll("")) - .replaceAll(" ") - .trim(); + @Nonnull + private String parseRoute(@Nonnull String routerString) { + return METHOD_REGEX + .matcher( + SPACES_REGEX + .matcher(SPECIAL_CHARACTERS_REGEX.matcher(routerString).replaceAll("")) + .replaceAll(" ") + .trim()) + .replaceAll(""); } } From e6b875623e679f6b7564fc3ad7954fc6b7d7fd3a Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Mon, 19 Dec 2022 12:05:17 +0100 Subject: [PATCH 2/2] cache parsed routes --- .../server/RouteOnSuccessOrError.java | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/main/java/datadog/trace/instrumentation/springwebflux/server/RouteOnSuccessOrError.java b/dd-java-agent/instrumentation/spring-webflux-5/src/main/java/datadog/trace/instrumentation/springwebflux/server/RouteOnSuccessOrError.java index e27c7e86917..cedee2e5e09 100644 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/main/java/datadog/trace/instrumentation/springwebflux/server/RouteOnSuccessOrError.java +++ b/dd-java-agent/instrumentation/spring-webflux-5/src/main/java/datadog/trace/instrumentation/springwebflux/server/RouteOnSuccessOrError.java @@ -2,8 +2,11 @@ import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR; +import datadog.trace.api.cache.DDCache; +import datadog.trace.api.cache.DDCaches; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import java.util.function.BiConsumer; +import java.util.function.Function; import java.util.regex.Pattern; import javax.annotation.Nonnull; import org.springframework.web.reactive.function.server.HandlerFunction; @@ -14,14 +17,24 @@ public class RouteOnSuccessOrError implements BiConsumer, Thr private static final Pattern SPECIAL_CHARACTERS_REGEX = Pattern.compile("[\\(\\)&|]"); private static final Pattern SPACES_REGEX = Pattern.compile("[ \\t]+"); - private static final Pattern ROUTER_FUNCION_REGEX = Pattern.compile("\\s*->.*$"); - + private static final Pattern ROUTER_FUNCTION_REGEX = Pattern.compile("\\s*->.*$"); private static final Pattern METHOD_REGEX = Pattern.compile("^(GET|HEAD|POST|PUT|DELETE|CONNECT|OPTIONS|TRACE|PATCH) "); + private static final Function PATH_EXTRACTOR = + arg -> + METHOD_REGEX + .matcher( + SPACES_REGEX + .matcher(SPECIAL_CHARACTERS_REGEX.matcher(arg).replaceAll("")) + .replaceAll(" ") + .trim()) + .replaceAll(""); private final RouterFunction routerFunction; private final ServerRequest serverRequest; + private final DDCache parsedRouteCache = DDCaches.newFixedSizeCache(16); + public RouteOnSuccessOrError( final RouterFunction routerFunction, final ServerRequest serverRequest) { this.routerFunction = routerFunction; @@ -56,18 +69,12 @@ private String parsePredicateString() { "org.springframework.web.reactive.function.server.RequestPredicates$$Lambda$")) { return null; } else { - return ROUTER_FUNCION_REGEX.matcher(routerFunctionString).replaceFirst(""); + return ROUTER_FUNCTION_REGEX.matcher(routerFunctionString).replaceFirst(""); } } @Nonnull private String parseRoute(@Nonnull String routerString) { - return METHOD_REGEX - .matcher( - SPACES_REGEX - .matcher(SPECIAL_CHARACTERS_REGEX.matcher(routerString).replaceAll("")) - .replaceAll(" ") - .trim()) - .replaceAll(""); + return parsedRouteCache.computeIfAbsent(routerString, PATH_EXTRACTOR); } }