From 8e891cb7f053f1d3190df242f6f09ffdfc895177 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Wed, 12 Apr 2023 12:04:50 -0700 Subject: [PATCH 01/11] Fix nested http.route --- .../webflux/v5_0/server/HandlerAdapterInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java index 4acd899bbac7..7b8df533cba4 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java @@ -66,7 +66,7 @@ public static void methodEnter( Context parentContext = Context.current(); HttpRouteHolder.updateHttpRoute( - parentContext, HttpRouteSource.CONTROLLER, httpRouteGetter(), exchange); + parentContext, HttpRouteSource.NESTED_CONTROLLER, httpRouteGetter(), exchange); if (handler == null) { return; From 6f7d7506b8d8b25fa670f9140413abd5cb55f562 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Fri, 14 Apr 2023 09:02:33 -0700 Subject: [PATCH 02/11] Add test --- .../base/HandlerSpringWebFluxServerTest.java | 1 + .../java/server/base/ServerTestController.java | 12 ++++++++++++ .../java/server/base/ServerTestRouteFactory.java | 14 +++++++++++++- .../java/server/base/SpringWebFluxServerTest.java | 2 ++ .../testing/junit/http/AbstractHttpServerTest.java | 13 +++++++++++++ .../testing/junit/http/HttpServerTestOptions.java | 7 +++++++ .../testing/junit/http/ServerEndpoint.java | 1 + 7 files changed, 49 insertions(+), 1 deletion(-) diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java index 4ad6fcff270d..cf6617a0f1ac 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java @@ -76,5 +76,6 @@ protected void configure(HttpServerTestOptions options) { // Mono that the controller returns completes) should end before the server span (which needs // the result of the Mono) options.setVerifyServerSpanEndTime(false); + options.setTestNestedPath(true); } } diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestController.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestController.java index 48af18d0896c..c185dcf260cf 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestController.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestController.java @@ -116,6 +116,18 @@ public Mono capture_headers(ServerHttpRequest request, ServerHttpRespons }); } + @GetMapping("/nestedPath") + public Mono nested_path(ServerHttpRequest request, ServerHttpResponse response) { + ServerEndpoint endpoint = ServerEndpoint.NESTED_PATH; + + return wrapControllerMethod( + endpoint, + () -> { + setStatus(response, endpoint); + return endpoint.getBody(); + }); + } + protected abstract Mono wrapControllerMethod(ServerEndpoint endpoint, Supplier handler); private static void setStatus(ServerHttpResponse response, ServerEndpoint endpoint) { diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java index 5be798362686..5da59d721429 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java @@ -6,6 +6,8 @@ package server.base; import static org.springframework.web.reactive.function.server.RequestPredicates.GET; +import static org.springframework.web.reactive.function.server.RequestPredicates.path; +import static org.springframework.web.reactive.function.server.RouterFunctions.nest; import static org.springframework.web.reactive.function.server.RouterFunctions.route; import io.opentelemetry.api.trace.Span; @@ -98,7 +100,17 @@ public RouterFunction createRoutes() { request.headers().asHttpHeaders().getFirst("X-Test-Request")), null, null); - }); + }) + .andNest( + path("/nestedPath"), + nest( + path("/hello"), + route( + path("/world"), + request -> { + ServerEndpoint endpoint = ServerEndpoint.NESTED_PATH; + return respond(endpoint, null, null, null); + }))); } protected Mono respond( diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java index 6ceb73ae34be..2dc3847bd8ba 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java @@ -52,6 +52,8 @@ public String expectedHttpRoute(ServerEndpoint endpoint) { return getContextPath() + "/path/{id}/param"; case NOT_FOUND: return "/**"; + case NESTED_PATH: + return "/nestedPath/hello/world"; default: return super.expectedHttpRoute(endpoint); } diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java index ef45b10999b1..3bebca764a48 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java @@ -299,6 +299,19 @@ void captureRequestParameters() { assertTheTraces(1, null, null, spanId, "POST", CAPTURE_PARAMETERS, response); } + @Test + void nestedPath() { + assumeTrue(options.testNestedPath); + String method = "GET"; + AggregatedHttpRequest request = request(NESTED_PATH, method); + AggregatedHttpResponse response = client.execute(request).aggregate().join(); + assertThat(response.status().code()).isEqualTo(NESTED_PATH.getStatus()); + assertThat(response.contentUtf8()).isEqualTo(NESTED_PATH.getBody()); + assertResponseHasCustomizedHeaders(response, NESTED_PATH, null); + + assertTheTraces(1, null, null, null, method, NESTED_PATH, response); + } + /** * This test fires a bunch of parallel request to the fixed backend endpoint. That endpoint is * supposed to create a new child span in the context of the SERVER span. That child span is diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpServerTestOptions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpServerTestOptions.java index ffa51924d9f6..26d634acf1ed 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpServerTestOptions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpServerTestOptions.java @@ -51,6 +51,7 @@ public final class HttpServerTestOptions { boolean testNotFound = true; boolean testPathParam = false; boolean testCaptureHttpHeaders = true; + boolean testNestedPath = false; boolean testCaptureRequestParameters = false; boolean verifyServerSpanEndTime = true; @@ -176,6 +177,12 @@ public HttpServerTestOptions setTestCaptureHttpHeaders(boolean testCaptureHttpHe return this; } + @CanIgnoreReturnValue + public HttpServerTestOptions setTestNestedPath(boolean testNestedPath) { + this.testNestedPath = testNestedPath; + return this; + } + @CanIgnoreReturnValue public HttpServerTestOptions setTestCaptureRequestParameters( boolean testCaptureRequestParameters) { diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/ServerEndpoint.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/ServerEndpoint.java index bccb9b2e323f..1de06503d0bf 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/ServerEndpoint.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/ServerEndpoint.java @@ -19,6 +19,7 @@ public enum ServerEndpoint { EXCEPTION("exception", 500, "controller exception"), NOT_FOUND("notFound", 404, "not found"), CAPTURE_HEADERS("captureHeaders", 200, "headers captured"), + NESTED_PATH("nestedPath/hello/world", 200, "nested path"), CAPTURE_PARAMETERS("captureParameters", 200, "parameters captured"), // TODO: add tests for the following cases: From 5380f5eafa6291db06ace72c60375b9ffbf9b30d Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Fri, 14 Apr 2023 09:13:08 -0700 Subject: [PATCH 03/11] Fix missing import --- .../testing/junit/http/AbstractHttpServerTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java index 3bebca764a48..be7e561d9fe1 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java @@ -10,6 +10,7 @@ import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.ERROR; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.EXCEPTION; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.INDEXED_CHILD; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NESTED_PATH; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NOT_FOUND; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.PATH_PARAM; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.QUERY_PARAM; From 87ad46cde516157d1b1e43e85b444956fc9453c3 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Fri, 14 Apr 2023 12:49:32 -0700 Subject: [PATCH 04/11] Fix null BEST_MATCHING_PATTERN_ATTRIBUTE --- .../spring-webflux-5.0/javaagent/build.gradle.kts | 6 +++--- .../src/test/java/server/SpringWebFluxTestApplication.java | 1 + .../java/server/base/HandlerSpringWebFluxServerTest.java | 2 +- .../src/test/java/server/base/ServerTestRouteFactory.java | 1 + 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/build.gradle.kts b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/build.gradle.kts index 224cc605884e..29bc1be10d05 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/build.gradle.kts +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/build.gradle.kts @@ -53,9 +53,9 @@ dependencies { testImplementation(project(":instrumentation:spring:spring-webflux:spring-webflux-5.3:testing")) - testLibrary("org.springframework.boot:spring-boot-starter-webflux:2.0.0.RELEASE") - testLibrary("org.springframework.boot:spring-boot-starter-test:2.0.0.RELEASE") - testLibrary("org.springframework.boot:spring-boot-starter-reactor-netty:2.0.0.RELEASE") + testLibrary("org.springframework.boot:spring-boot-starter-webflux:2.7.5") + testLibrary("org.springframework.boot:spring-boot-starter-test:2.7.5") + testLibrary("org.springframework.boot:spring-boot-starter-reactor-netty:2.7.5") testImplementation("org.spockframework:spock-spring:2.4-M1-groovy-4.0") } diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/SpringWebFluxTestApplication.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/SpringWebFluxTestApplication.java index 251bce200015..937990513b2d 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/SpringWebFluxTestApplication.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/SpringWebFluxTestApplication.java @@ -74,6 +74,7 @@ RouterFunction greetRouterFunction(GreetingHandler greetingHandl .map(SpringWebFluxTestApplication::tracedMethod))); } + @SuppressWarnings("deprecation") // testing instrumentation of deprecated class @Component public static class GreetingHandler { public static final String DEFAULT_RESPONSE = "HELLO"; diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java index cf6617a0f1ac..367e2fccc247 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java @@ -62,7 +62,7 @@ protected SpanDataAssert assertHandlerSpan( equalTo( EXCEPTION_TYPE, "org.springframework.web.server.ResponseStatusException"), - equalTo(EXCEPTION_MESSAGE, "Response status 404"), + equalTo(EXCEPTION_MESSAGE, "404 NOT_FOUND"), satisfies(EXCEPTION_STACKTRACE, val -> val.isInstanceOf(String.class)))); } } diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java index 5da59d721429..07ea0d7aa514 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java @@ -113,6 +113,7 @@ public RouterFunction createRoutes() { }))); } + @SuppressWarnings("deprecation") // testing instrumentation of deprecated class protected Mono respond( ServerEndpoint endpoint, BodyBuilder bodyBuilder, String body, Runnable spanAction) { if (bodyBuilder == null) { From 3959d797caea29b3070c14e7ae9c135e9897c2fe Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Mon, 17 Apr 2023 16:57:20 -0700 Subject: [PATCH 05/11] Fix tests --- .../test/java/server/base/HandlerSpringWebFluxServerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java index 367e2fccc247..cf6617a0f1ac 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java @@ -62,7 +62,7 @@ protected SpanDataAssert assertHandlerSpan( equalTo( EXCEPTION_TYPE, "org.springframework.web.server.ResponseStatusException"), - equalTo(EXCEPTION_MESSAGE, "404 NOT_FOUND"), + equalTo(EXCEPTION_MESSAGE, "Response status 404"), satisfies(EXCEPTION_STACKTRACE, val -> val.isInstanceOf(String.class)))); } } From 8ae9540de9af7b82aa0b79573d7bf17bfc080264 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Tue, 18 Apr 2023 13:02:27 -0700 Subject: [PATCH 06/11] Run latest webflux against nestedPath --- .../spring-webflux-5.0/javaagent/build.gradle.kts | 6 +++--- .../testing/junit/http/AbstractHttpServerTest.java | 1 + .../instrumentation/testing/junit/http/ServerEndpoint.java | 6 +++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/build.gradle.kts b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/build.gradle.kts index 29bc1be10d05..224cc605884e 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/build.gradle.kts +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/build.gradle.kts @@ -53,9 +53,9 @@ dependencies { testImplementation(project(":instrumentation:spring:spring-webflux:spring-webflux-5.3:testing")) - testLibrary("org.springframework.boot:spring-boot-starter-webflux:2.7.5") - testLibrary("org.springframework.boot:spring-boot-starter-test:2.7.5") - testLibrary("org.springframework.boot:spring-boot-starter-reactor-netty:2.7.5") + testLibrary("org.springframework.boot:spring-boot-starter-webflux:2.0.0.RELEASE") + testLibrary("org.springframework.boot:spring-boot-starter-test:2.0.0.RELEASE") + testLibrary("org.springframework.boot:spring-boot-starter-reactor-netty:2.0.0.RELEASE") testImplementation("org.spockframework:spock-spring:2.4-M1-groovy-4.0") } diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java index be7e561d9fe1..6a9bb3981f76 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java @@ -302,6 +302,7 @@ void captureRequestParameters() { @Test void nestedPath() { + assumeTrue(Boolean.getBoolean("testLatestDeps")); assumeTrue(options.testNestedPath); String method = "GET"; AggregatedHttpRequest request = request(NESTED_PATH, method); diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/ServerEndpoint.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/ServerEndpoint.java index 1de06503d0bf..cbe628ce926c 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/ServerEndpoint.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/ServerEndpoint.java @@ -19,7 +19,11 @@ public enum ServerEndpoint { EXCEPTION("exception", 500, "controller exception"), NOT_FOUND("notFound", 404, "not found"), CAPTURE_HEADERS("captureHeaders", 200, "headers captured"), - NESTED_PATH("nestedPath/hello/world", 200, "nested path"), + NESTED_PATH( + "nestedPath/hello/world", + 200, + "nested path"), // TODO (heya) move it to webflux test module after this has been converted to + // a class CAPTURE_PARAMETERS("captureParameters", 200, "parameters captured"), // TODO: add tests for the following cases: From 196298b1f39a0f415259d04c7d3b04eb9fcea084 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Wed, 19 Apr 2023 12:13:36 -0700 Subject: [PATCH 07/11] Move test down to webfluxServerTest and remove nestedPath option --- .../base/HandlerSpringWebFluxServerTest.java | 1 - .../server/base/ServerTestRouteFactory.java | 1 - .../server/base/SpringWebFluxServerTest.java | 20 +++++++++++++++++++ .../junit/http/AbstractHttpServerTest.java | 17 +--------------- .../junit/http/HttpServerTestOptions.java | 7 ------- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java index cf6617a0f1ac..4ad6fcff270d 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/HandlerSpringWebFluxServerTest.java @@ -76,6 +76,5 @@ protected void configure(HttpServerTestOptions options) { // Mono that the controller returns completes) should end before the server span (which needs // the result of the Mono) options.setVerifyServerSpanEndTime(false); - options.setTestNestedPath(true); } } diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java index 07ea0d7aa514..5da59d721429 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestRouteFactory.java @@ -113,7 +113,6 @@ public RouterFunction createRoutes() { }))); } - @SuppressWarnings("deprecation") // testing instrumentation of deprecated class protected Mono respond( ServerEndpoint endpoint, BodyBuilder bodyBuilder, String body, Runnable spanAction) { if (bodyBuilder == null) { diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java index 2dc3847bd8ba..83108b618f59 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java @@ -6,12 +6,18 @@ package server.base; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.EXCEPTION; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NESTED_PATH; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest; import io.opentelemetry.instrumentation.testing.junit.http.HttpServerInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpServerTestOptions; import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint; +import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpRequest; +import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpResponse; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.springframework.boot.SpringApplication; import org.springframework.context.ConfigurableApplicationContext; @@ -65,4 +71,18 @@ protected void configure(HttpServerTestOptions options) { options.setExpectedException(new IllegalStateException(EXCEPTION.getBody())); options.setHasHandlerSpan(unused -> true); } + + @Test + void nestedPath() { + assumeTrue(Boolean.getBoolean("testLatestDeps")); + + String method = "GET"; + AggregatedHttpRequest request = request(NESTED_PATH, method); + AggregatedHttpResponse response = client.execute(request).aggregate().join(); + assertThat(response.status().code()).isEqualTo(NESTED_PATH.getStatus()); + assertThat(response.contentUtf8()).isEqualTo(NESTED_PATH.getBody()); + assertResponseHasCustomizedHeaders(response, NESTED_PATH, null); + + assertTheTraces(1, null, null, null, method, NESTED_PATH, response); + } } diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java index 6a9bb3981f76..fe48cdc99db4 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java @@ -10,7 +10,6 @@ import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.ERROR; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.EXCEPTION; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.INDEXED_CHILD; -import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NESTED_PATH; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NOT_FOUND; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.PATH_PARAM; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.QUERY_PARAM; @@ -95,7 +94,7 @@ public static T controller(ServerEndpoint endpoint, Supplier closure) { return GlobalTraceUtil.runWithSpan("controller", () -> closure.get()); } - private AggregatedHttpRequest request(ServerEndpoint uri, String method) { + protected AggregatedHttpRequest request(ServerEndpoint uri, String method) { return AggregatedHttpRequest.of(HttpMethod.valueOf(method), resolveAddress(uri)); } @@ -300,20 +299,6 @@ void captureRequestParameters() { assertTheTraces(1, null, null, spanId, "POST", CAPTURE_PARAMETERS, response); } - @Test - void nestedPath() { - assumeTrue(Boolean.getBoolean("testLatestDeps")); - assumeTrue(options.testNestedPath); - String method = "GET"; - AggregatedHttpRequest request = request(NESTED_PATH, method); - AggregatedHttpResponse response = client.execute(request).aggregate().join(); - assertThat(response.status().code()).isEqualTo(NESTED_PATH.getStatus()); - assertThat(response.contentUtf8()).isEqualTo(NESTED_PATH.getBody()); - assertResponseHasCustomizedHeaders(response, NESTED_PATH, null); - - assertTheTraces(1, null, null, null, method, NESTED_PATH, response); - } - /** * This test fires a bunch of parallel request to the fixed backend endpoint. That endpoint is * supposed to create a new child span in the context of the SERVER span. That child span is diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpServerTestOptions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpServerTestOptions.java index 26d634acf1ed..ffa51924d9f6 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpServerTestOptions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpServerTestOptions.java @@ -51,7 +51,6 @@ public final class HttpServerTestOptions { boolean testNotFound = true; boolean testPathParam = false; boolean testCaptureHttpHeaders = true; - boolean testNestedPath = false; boolean testCaptureRequestParameters = false; boolean verifyServerSpanEndTime = true; @@ -177,12 +176,6 @@ public HttpServerTestOptions setTestCaptureHttpHeaders(boolean testCaptureHttpHe return this; } - @CanIgnoreReturnValue - public HttpServerTestOptions setTestNestedPath(boolean testNestedPath) { - this.testNestedPath = testNestedPath; - return this; - } - @CanIgnoreReturnValue public HttpServerTestOptions setTestCaptureRequestParameters( boolean testCaptureRequestParameters) { From 6b2328941f2b072b2ff868a54266d9b8b5fd8431 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Wed, 19 Apr 2023 12:16:34 -0700 Subject: [PATCH 08/11] Remove deprecated warning when testing newer version of webflux --- .../src/test/java/server/SpringWebFluxTestApplication.java | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/SpringWebFluxTestApplication.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/SpringWebFluxTestApplication.java index 937990513b2d..251bce200015 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/SpringWebFluxTestApplication.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/SpringWebFluxTestApplication.java @@ -74,7 +74,6 @@ RouterFunction greetRouterFunction(GreetingHandler greetingHandl .map(SpringWebFluxTestApplication::tracedMethod))); } - @SuppressWarnings("deprecation") // testing instrumentation of deprecated class @Component public static class GreetingHandler { public static final String DEFAULT_RESPONSE = "HELLO"; From 7ee535a1e158effc50eecc4bce78164d32b18334 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Wed, 19 Apr 2023 15:30:43 -0700 Subject: [PATCH 09/11] Add a comment --- .../webflux/v5_0/server/HandlerAdapterInstrumentation.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java index 7b8df533cba4..a32be694cc79 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java @@ -65,6 +65,10 @@ public static void methodEnter( Context parentContext = Context.current(); + // HttpRouteSource.CONTROLLER has userFirst true, and it will make http.route updated once + // using the last portion of the nested path. + // HttpRouteSource.NESTED_CONTROLLER has useFirst false, and it will make http.route updated + // twice: 1st using the last portion of the nested path, 2nd time using the full nested path. HttpRouteHolder.updateHttpRoute( parentContext, HttpRouteSource.NESTED_CONTROLLER, httpRouteGetter(), exchange); From b1edd50fd9a665d5c02e0b67ab4bdeec483b8ed6 Mon Sep 17 00:00:00 2001 From: Helen <56097766+heyams@users.noreply.github.com> Date: Thu, 20 Apr 2023 13:02:48 -0700 Subject: [PATCH 10/11] Reword Co-authored-by: Trask Stalnaker --- .../webflux/v5_0/server/HandlerAdapterInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java index a32be694cc79..9660a1c3981f 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java @@ -65,7 +65,7 @@ public static void methodEnter( Context parentContext = Context.current(); - // HttpRouteSource.CONTROLLER has userFirst true, and it will make http.route updated once + // HttpRouteSource.CONTROLLER has useFirst true, and it will update http.route only once // using the last portion of the nested path. // HttpRouteSource.NESTED_CONTROLLER has useFirst false, and it will make http.route updated // twice: 1st using the last portion of the nested path, 2nd time using the full nested path. From c07f522f07f7e012dd7ec55dc6225e2a6aeb2744 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 20 Apr 2023 14:09:21 -0700 Subject: [PATCH 11/11] Move nestedPath to ImmediateHandlerSpringWebFluxServerTest --- ...mediateHandlerSpringWebFluxServerTest.java | 21 +++++++++++++++++++ .../server/base/SpringWebFluxServerTest.java | 20 ------------------ 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ImmediateHandlerSpringWebFluxServerTest.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ImmediateHandlerSpringWebFluxServerTest.java index 1716e7371db5..1b0ed334f1ab 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ImmediateHandlerSpringWebFluxServerTest.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/ImmediateHandlerSpringWebFluxServerTest.java @@ -5,7 +5,14 @@ package server.base; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NESTED_PATH; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint; +import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpRequest; +import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpResponse; +import org.junit.jupiter.api.Test; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory; import org.springframework.context.annotation.Bean; @@ -54,4 +61,18 @@ protected Mono wrapResponse( }); } } + + @Test + void nestedPath() { + assumeTrue(Boolean.getBoolean("testLatestDeps")); + + String method = "GET"; + AggregatedHttpRequest request = request(NESTED_PATH, method); + AggregatedHttpResponse response = client.execute(request).aggregate().join(); + assertThat(response.status().code()).isEqualTo(NESTED_PATH.getStatus()); + assertThat(response.contentUtf8()).isEqualTo(NESTED_PATH.getBody()); + assertResponseHasCustomizedHeaders(response, NESTED_PATH, null); + + assertTheTraces(1, null, null, null, method, NESTED_PATH, response); + } } diff --git a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java index 83108b618f59..2dc3847bd8ba 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/test/java/server/base/SpringWebFluxServerTest.java @@ -6,18 +6,12 @@ package server.base; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.EXCEPTION; -import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NESTED_PATH; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assumptions.assumeTrue; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest; import io.opentelemetry.instrumentation.testing.junit.http.HttpServerInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpServerTestOptions; import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint; -import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpRequest; -import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpResponse; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.springframework.boot.SpringApplication; import org.springframework.context.ConfigurableApplicationContext; @@ -71,18 +65,4 @@ protected void configure(HttpServerTestOptions options) { options.setExpectedException(new IllegalStateException(EXCEPTION.getBody())); options.setHasHandlerSpan(unused -> true); } - - @Test - void nestedPath() { - assumeTrue(Boolean.getBoolean("testLatestDeps")); - - String method = "GET"; - AggregatedHttpRequest request = request(NESTED_PATH, method); - AggregatedHttpResponse response = client.execute(request).aggregate().join(); - assertThat(response.status().code()).isEqualTo(NESTED_PATH.getStatus()); - assertThat(response.contentUtf8()).isEqualTo(NESTED_PATH.getBody()); - assertResponseHasCustomizedHeaders(response, NESTED_PATH, null); - - assertTheTraces(1, null, null, null, method, NESTED_PATH, response); - } }