Skip to content

Commit

Permalink
Fix latest dep tests after the recent Spring release (#9947)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mateusz Rzeszutek authored Nov 24, 2023
1 parent 7c3a412 commit 6346815
Show file tree
Hide file tree
Showing 21 changed files with 156 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ dependencies {
}

tasks {
withType<Test>().configureEach {
systemProperty("testLatestDeps", findProperty("testLatestDeps") as Boolean)
}

val testStableSemconv by registering(Test::class) {
jvmArgs("-Dotel.semconv-stability.opt-in=http")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
super.configure(optionsBuilder);
// apparently apache http client does not report the 302 status code?
optionsBuilder.setResponseCodeOnRedirectError(null);

if (Boolean.getBoolean("testLatestDeps")) {
optionsBuilder.disableTestHttps();
optionsBuilder.disableTestRemoteConnection();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ dependencies {
testImplementation(project(":instrumentation:spring:spring-cloud-gateway:spring-cloud-gateway-common:testing"))

testLibrary("org.springframework.boot:spring-boot-starter-test:2.0.0.RELEASE")

latestDepTestLibrary("org.springframework.cloud:spring-cloud-starter-gateway:2.1.+")
latestDepTestLibrary("org.springframework.boot:spring-boot-starter-test:2.1.+")
}

tasks.withType<Test>().configureEach {
Expand All @@ -33,19 +36,9 @@ tasks.withType<Test>().configureEach {
systemProperty("testLatestDeps", findProperty("testLatestDeps") as Boolean)
}

val latestDepTest = findProperty("testLatestDeps") as Boolean

if (latestDepTest) {
// spring 6 requires java 17
otelJava {
minJavaVersionSupported.set(JavaVersion.VERSION_17)
}
} else {
// spring 5 requires old logback (and therefore also old slf4j)
configurations.testRuntimeClasspath {
resolutionStrategy {
force("ch.qos.logback:logback-classic:1.2.11")
force("org.slf4j:slf4j-api:1.7.36")
}
configurations.testRuntimeClasspath {
resolutionStrategy {
force("ch.qos.logback:logback-classic:1.2.11")
force("org.slf4j:slf4j-api:1.7.36")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ dependencies {

testLibrary("org.springframework.cloud:spring-cloud-starter-gateway:2.2.0.RELEASE")
testLibrary("org.springframework.boot:spring-boot-starter-test:2.2.0.RELEASE")

// spring-cloud-gateway hasn't yet updated to spring 6.2/boot 3.2
latestDepTestLibrary("org.springframework.boot:spring-boot-starter-webflux:3.1.+")
latestDepTestLibrary("org.springframework.boot:spring-boot-starter-test:3.1.+")
}

tasks.withType<Test>().configureEach {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.sdk.testing.assertj.EventDataAssert;
import io.opentelemetry.sdk.testing.assertj.TraceAssert;
import io.opentelemetry.sdk.trace.data.StatusData;
import io.opentelemetry.testing.internal.armeria.client.WebClient;
Expand Down Expand Up @@ -484,28 +485,33 @@ void get404Test() {
.hasKind(SpanKind.INTERNAL)
.hasParent(trace.getSpan(0))
.hasStatus(StatusData.error())
.hasEventsSatisfyingExactly(
event ->
event
.hasName(EXCEPTION_EVENT_NAME)
.hasAttributesSatisfyingExactly(
equalTo(
EXCEPTION_TYPE,
"org.springframework.web.server.ResponseStatusException"),
satisfies(
EXCEPTION_MESSAGE,
val ->
val.containsAnyOf(
"Response status 404", "404 NOT_FOUND")),
satisfies(
EXCEPTION_STACKTRACE,
val -> val.isInstanceOf(String.class))))
.hasEventsSatisfyingExactly(SpringWebfluxTest::resource404Exception)
.hasAttributesSatisfyingExactly(
equalTo(
stringKey("spring-webflux.handler.type"),
"org.springframework.web.reactive.resource.ResourceWebHandler"))));
}

private static void resource404Exception(EventDataAssert event) {
if (Boolean.getBoolean("testLatestDeps")) {
event
.hasName(EXCEPTION_EVENT_NAME)
.hasAttributesSatisfyingExactly(
equalTo(
EXCEPTION_TYPE,
"org.springframework.web.reactive.resource.NoResourceFoundException"),
satisfies(EXCEPTION_MESSAGE, val -> val.isInstanceOf(String.class)),
satisfies(EXCEPTION_STACKTRACE, val -> val.isInstanceOf(String.class)));
} else {
event
.hasName(EXCEPTION_EVENT_NAME)
.hasAttributesSatisfyingExactly(
equalTo(EXCEPTION_TYPE, "org.springframework.web.server.ResponseStatusException"),
equalTo(EXCEPTION_MESSAGE, "Response status 404"),
satisfies(EXCEPTION_STACKTRACE, val -> val.isInstanceOf(String.class)));
}
}

@Test
void basicPostTest() {
String echoString = "TEST";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package server.base;
package io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base;

import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.EXCEPTION;
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NOT_FOUND;
Expand Down Expand Up @@ -52,8 +52,9 @@ protected SpanDataAssert assertHandlerSpan(
.hasAttributesSatisfyingExactly(
equalTo(
EXCEPTION_TYPE,
"org.springframework.web.server.ResponseStatusException"),
equalTo(EXCEPTION_MESSAGE, "404 NOT_FOUND"),
"org.springframework.web.reactive.resource.NoResourceFoundException"),
equalTo(
EXCEPTION_MESSAGE, "404 NOT_FOUND \"No static resource notFound.\""),
satisfies(EXCEPTION_STACKTRACE, val -> val.isInstanceOf(String.class))));
} else {
span.hasEventsSatisfyingExactly(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package server.base;
package io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base;

import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint;
import java.time.Duration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package server.base;
package io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base;

import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint;
import java.time.Duration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package server.base;
package io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base;

import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.EXCEPTION;
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NOT_FOUND;
Expand Down Expand Up @@ -50,8 +50,9 @@ protected SpanDataAssert assertHandlerSpan(
.hasAttributesSatisfyingExactly(
equalTo(
EXCEPTION_TYPE,
"org.springframework.web.server.ResponseStatusException"),
equalTo(EXCEPTION_MESSAGE, "404 NOT_FOUND"),
"org.springframework.web.reactive.resource.NoResourceFoundException"),
equalTo(
EXCEPTION_MESSAGE, "404 NOT_FOUND \"No static resource notFound.\""),
satisfies(EXCEPTION_STACKTRACE, val -> val.isInstanceOf(String.class))));
} else {
span.hasEventsSatisfyingExactly(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package server.base;
package io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base;

import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint;
import java.util.function.Supplier;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package server.base;
package io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

package server.base;
package io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base;

import static server.base.SpringWebFluxServerTest.NESTED_PATH;
import static io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base.SpringWebFluxServerTest.NESTED_PATH;

import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint;
import java.net.URI;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
* SPDX-License-Identifier: Apache-2.0
*/

package server.base;
package io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base;

import static io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base.SpringWebFluxServerTest.NESTED_PATH;
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 static server.base.SpringWebFluxServerTest.NESTED_PATH;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package server.base;
package io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base;

import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.EXCEPTION;
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NOT_FOUND;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
* The classes in this package are specific to tests that extend {@link
* io.opentelemetry.instrumentation.test.base.HttpServerTest}.
*/
package server.base;
package io.opentelemetry.javaagent.instrumentation.spring.webflux.v5_0.server.base;
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.ComponentScan;
import org.springframework.context.annotation.FilterType;
import org.springframework.http.MediaType;
import org.springframework.stereotype.Component;
import org.springframework.web.reactive.function.BodyInserters;
Expand All @@ -27,9 +26,7 @@
import reactor.core.publisher.Mono;

@SpringBootApplication
@ComponentScan(
basePackages = {"server"},
excludeFilters = @ComponentScan.Filter(type = FilterType.REGEX, pattern = "server.base.*"))
@ComponentScan(basePackages = {"server"})
public class SpringWebFluxTestApplication {

private static final Tracer tracer = GlobalOpenTelemetry.getTracer("test");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ Flux<String> success() {

@RequestMapping("/query")
@ResponseBody
String query_param(@RequestParam("some") String param) {
return controller(QUERY_PARAM, () -> "some=" + param);
Mono<String> query_param(@RequestParam("some") String param) {
return Mono.just(controller(QUERY_PARAM, () -> "some=" + param));
}

@RequestMapping("/redirect")
Expand Down Expand Up @@ -102,20 +102,21 @@ Flux<ResponseEntity<String>> exception() throws Exception {
}

@RequestMapping("/captureHeaders")
ResponseEntity<String> capture_headers(
Mono<ResponseEntity<String>> capture_headers(
@RequestHeader("X-Test-Request") String testRequestHeader) {
return controller(
CAPTURE_HEADERS,
() ->
ResponseEntity.ok()
.header("X-Test-Response", testRequestHeader)
.body(CAPTURE_HEADERS.getBody()));
return Mono.just(
controller(
CAPTURE_HEADERS,
() ->
ResponseEntity.ok()
.header("X-Test-Response", testRequestHeader)
.body(CAPTURE_HEADERS.getBody())));
}

@RequestMapping("/path/{id}/param")
@ResponseBody
String path_param(@PathVariable("id") int id) {
return controller(PATH_PARAM, () -> String.valueOf(id));
Mono<String> path_param(@PathVariable("id") int id) {
return Mono.just(controller(PATH_PARAM, () -> String.valueOf(id)));
}

@RequestMapping("/child")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,6 @@ tasks.withType<Test>().configureEach {
// required on jdk17
jvmArgs("--add-opens=java.base/java.lang=ALL-UNNAMED")
jvmArgs("-XX:+IgnoreUnrecognizedVMOptions")

systemProperty("testLatestDeps", findProperty("testLatestDeps") as Boolean)
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@
*/

import filter.AbstractServletFilterTest
import io.opentelemetry.api.trace.SpanKind
import io.opentelemetry.api.trace.StatusCode
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint
import io.opentelemetry.sdk.trace.data.SpanData

import static io.opentelemetry.api.trace.SpanKind.INTERNAL

class ServletFilterTest extends AbstractServletFilterTest {

Expand All @@ -14,4 +21,32 @@ class ServletFilterTest extends AbstractServletFilterTest {
Class<?> filterConfigClass() {
ServletFilterConfig
}

@Override
void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint) {
if (Boolean.getBoolean("testLatestDeps") && endpoint == ServerEndpoint.NOT_FOUND) {
trace.span(index) {
name "ResourceHttpRequestHandler.handleRequest"
kind INTERNAL
childOf((SpanData) parent)
status StatusCode.ERROR
errorEventWithAnyMessage Class.forName("org.springframework.web.servlet.resource.NoResourceFoundException")
}
} else {
super.handlerSpan(trace, index, parent, method, endpoint)
}
}

@Override
void responseSpan(TraceAssert trace, int index, Object parent, String method, ServerEndpoint endpoint) {
if (Boolean.getBoolean("testLatestDeps") && endpoint == ServerEndpoint.NOT_FOUND) {
trace.span(index) {
name ~/\.sendError$/
kind SpanKind.INTERNAL
// not verifying the parent span, in the latest version the responseSpan is the child of the SERVER span, not the handler span
}
} else {
super.responseSpan(trace, index, parent, method, endpoint)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,31 @@
*/

import boot.AbstractSpringBootBasedTest
import io.opentelemetry.api.trace.StatusCode
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint
import io.opentelemetry.sdk.trace.data.SpanData

import static io.opentelemetry.api.trace.SpanKind.INTERNAL

class SpringBootBasedTest extends AbstractSpringBootBasedTest {

Class<?> securityConfigClass() {
SecurityConfig
}

@Override
void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint) {
if (Boolean.getBoolean("testLatestDeps") && endpoint == ServerEndpoint.NOT_FOUND) {
trace.span(index) {
name "ResourceHttpRequestHandler.handleRequest"
kind INTERNAL
childOf((SpanData) parent)
status StatusCode.ERROR
errorEventWithAnyMessage Class.forName("org.springframework.web.servlet.resource.NoResourceFoundException")
}
} else {
super.handlerSpan(trace, index, parent, method, endpoint)
}
}
}
Loading

0 comments on commit 6346815

Please sign in to comment.