Fix nested http.route (#8282)

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
This commit is contained in:
Helen 2023-04-21 00:27:28 -07:00 committed by GitHub
parent 1fdb6ee9d2
commit ffb63d68eb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 59 additions and 3 deletions

View File

@ -65,8 +65,12 @@ public class HandlerAdapterInstrumentation implements TypeInstrumentation {
Context parentContext = Context.current();
// 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.
HttpRouteHolder.updateHttpRoute(
parentContext, HttpRouteSource.CONTROLLER, httpRouteGetter(), exchange);
parentContext, HttpRouteSource.NESTED_CONTROLLER, httpRouteGetter(), exchange);
if (handler == null) {
return;

View File

@ -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 @@ public class ImmediateHandlerSpringWebFluxServerTest extends HandlerSpringWebFlu
});
}
}
@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);
}
}

View File

@ -116,6 +116,18 @@ public abstract class ServerTestController {
});
}
@GetMapping("/nestedPath")
public Mono<String> nested_path(ServerHttpRequest request, ServerHttpResponse response) {
ServerEndpoint endpoint = ServerEndpoint.NESTED_PATH;
return wrapControllerMethod(
endpoint,
() -> {
setStatus(response, endpoint);
return endpoint.getBody();
});
}
protected abstract <T> Mono<T> wrapControllerMethod(ServerEndpoint endpoint, Supplier<T> handler);
private static void setStatus(ServerHttpResponse response, ServerEndpoint endpoint) {

View File

@ -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 abstract class ServerTestRouteFactory {
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<ServerResponse> respond(

View File

@ -52,6 +52,8 @@ public abstract class SpringWebFluxServerTest
return getContextPath() + "/path/{id}/param";
case NOT_FOUND:
return "/**";
case NESTED_PATH:
return "/nestedPath/hello/world";
default:
return super.expectedHttpRoute(endpoint);
}

View File

@ -94,7 +94,7 @@ public abstract class AbstractHttpServerTest<SERVER> extends AbstractHttpServerU
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));
}

View File

@ -19,6 +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"), // 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: