Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Common URL Template path handling for REST requests (server and client) #17676

Merged
merged 1 commit into from
Jun 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
import io.quarkus.micrometer.runtime.config.runtime.HttpClientConfig;
import io.quarkus.micrometer.runtime.config.runtime.HttpServerConfig;
import io.quarkus.micrometer.runtime.config.runtime.VertxConfig;
import io.quarkus.resteasy.common.spi.ResteasyJaxrsProviderBuildItem;
import io.quarkus.resteasy.reactive.spi.CustomContainerRequestFilterBuildItem;
import io.quarkus.vertx.http.deployment.FilterBuildItem;

/**
Expand Down Expand Up @@ -94,21 +92,9 @@ SyntheticBeanBuildItem enableHttpBinders(MicrometerRecorder recorder,

@BuildStep(onlyIf = HttpServerBinderEnabled.class)
void enableHttpServerSupport(Capabilities capabilities,
BuildProducer<ResteasyJaxrsProviderBuildItem> resteasyJaxrsProviders,
BuildProducer<CustomContainerRequestFilterBuildItem> customContainerRequestFilter,
BuildProducer<io.quarkus.undertow.deployment.FilterBuildItem> servletFilters,
BuildProducer<AdditionalBeanBuildItem> additionalBeans) {

// Will have one or the other of these (exclusive)
if (capabilities.isPresent(Capability.RESTEASY)) {
resteasyJaxrsProviders.produce(new ResteasyJaxrsProviderBuildItem(RESTEASY_CONTAINER_FILTER_CLASS_NAME));
createAdditionalBean(additionalBeans, RESTEASY_CONTAINER_FILTER_CLASS_NAME);
} else if (capabilities.isPresent(Capability.RESTEASY_REACTIVE)) {
customContainerRequestFilter
.produce(new CustomContainerRequestFilterBuildItem(RESTEASY_REACTIVE_CONTAINER_FILTER_CLASS_NAME));
createAdditionalBean(additionalBeans, RESTEASY_REACTIVE_CONTAINER_FILTER_CLASS_NAME);
}

// But this might be present as well (fallback. Rest URI processing preferred)
if (capabilities.isPresent(Capability.SERVLET)) {
servletFilters.produce(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package io.quarkus.micrometer.deployment.binder;

import static io.restassured.RestAssured.when;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;

import org.hamcrest.Matchers;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
Expand Down Expand Up @@ -30,7 +31,8 @@ public void test() throws Exception {
when().get("/hello/two").then().statusCode(200);
when().get("/hello/three").then().statusCode(200);
when().get("/q/metrics").then().statusCode(200)
.body(Matchers.containsString("/hello/{message}"));
.body(containsString("/hello/{message}"))
.body(not(containsString("/goodbye/{message}")));

test.modifyResourceFile("application.properties",
s -> s.replace("quarkus.micrometer.binder.http-server.ignore-patterns=/http",
Expand All @@ -40,7 +42,7 @@ public void test() throws Exception {
when().get("/hello/two").then().statusCode(200);
when().get("/hello/three").then().statusCode(200);
when().get("/q/metrics").then().statusCode(200)
.body(Matchers.containsString("/goodbye/{message}"));
.body(containsString("/goodbye/{message}"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,34 @@ static void removeRegistry() {
}

@Test
public void testGetRequests() throws Exception {
public void testRequestUris() throws Exception {
RestAssured.basePath = "/";

// If you invoke requests, http server and client meters should be registered

// Server paths (templated)
when().get("/one").then().statusCode(200);
when().get("/two").then().statusCode(200);
when().get("/ping/one").then().statusCode(200);
when().get("/ping/two").then().statusCode(200);
when().get("/ping/three").then().statusCode(200);
when().get("/vertx/item/123").then().statusCode(200);
when().get("/vertx/item/1/123").then().statusCode(200);
when().get("/servlet/12345").then().statusCode(200);

// Server GET vs. HEAD methods -- templated
when().get("/hello/one").then().statusCode(200);
when().get("/hello/two").then().statusCode(200);
when().head("/hello/three").then().statusCode(200);
when().head("/hello/four").then().statusCode(200);
when().get("/vertx/echo/thing1").then().statusCode(200);
when().get("/vertx/echo/thing2").then().statusCode(200);
when().head("/vertx/echo/thing3").then().statusCode(200);
when().head("/vertx/echo/thing4").then().statusCode(200);

// Server -> Rest client -> Server (templated)
when().get("/ping/one").then().statusCode(200);
when().get("/ping/two").then().statusCode(200);
when().get("/ping/three").then().statusCode(200);
when().get("/async-ping/one").then().statusCode(200);
when().get("/async-ping/two").then().statusCode(200);
when().get("/async-ping/three").then().statusCode(200);

System.out.println("Server paths\n" + Util.listMeters(registry, "http.server.requests"));
System.out.println("Client paths\n" + Util.listMeters(registry, "http.client.requests"));

Expand All @@ -76,11 +90,7 @@ public void testGetRequests() throws Exception {
Assertions.assertEquals(0, registry.find("http.server.requests").tag("uri", "/two").timers().size(),
Util.foundServerRequests(registry, "/two should be ignored."));

// URIs for server: /ping/{message}, /pong/{message}, /vertx/item/{id}, /vertx/item/{id}/{sub}, /servlet/
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/ping/{message}").timers().size(),
Util.foundServerRequests(registry, "/ping/{message} should be returned by JAX-RS."));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/pong/{message}").timers().size(),
Util.foundServerRequests(registry, "/pong/{message} should be returned by JAX-RS."));
// URIs for server: /vertx/item/{id}, /vertx/item/{id}/{sub}, /servlet/
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/vertx/item/{id}").timers().size(),
Util.foundServerRequests(registry,
"Vert.x Web template path (/vertx/item/:id) should be detected/translated to /vertx/item/{id}."));
Expand All @@ -90,24 +100,23 @@ public void testGetRequests() throws Exception {
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/servlet").timers().size(),
Util.foundServerRequests(registry, "Servlet path (/servlet) should be used for servlet"));

// TODO: #15231
// URIs For client: /pong/{message}
// Assertions.assertEquals(1, registry.find("http.client.requests").tag("uri", "/pong/{message}").timers().size(),
// Util.foundClientRequests(registry, "/pong/{message} should be returned by Rest client."));

when().get("/hello/one").then().statusCode(200);
when().get("/hello/two").then().statusCode(200);
when().head("/hello/three").then().statusCode(200);
when().head("/hello/four").then().statusCode(200);
when().get("/vertx/echo/thing1").then().statusCode(200);
when().get("/vertx/echo/thing2").then().statusCode(200);
when().head("/vertx/echo/thing3").then().statusCode(200);
when().head("/vertx/echo/thing4").then().statusCode(200);

// GET and HEAD are two different methods, so double these up
// GET and HEAD are two different methods, there should be two timers for each of these URI tag values
Assertions.assertEquals(2, registry.find("http.server.requests").tag("uri", "/hello/{message}").timers().size(),
Util.foundServerRequests(registry, "/hello/{message} should have two timers (GET and HEAD)."));
Assertions.assertEquals(2, registry.find("http.server.requests").tag("uri", "/vertx/echo/{msg}").timers().size(),
Util.foundServerRequests(registry, "/vertx/echo/{msg} should have two timers (GET and HEAD)."));

// URIs to trigger REST request: /ping/{message}, /async-ping/{message},
// URIs for inbound rest client request: /pong/{message}
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/ping/{message}").timers().size(),
Util.foundServerRequests(registry, "/ping/{message} should be returned by JAX-RS."));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/async-ping/{message}").timers().size(),
Util.foundServerRequests(registry, "/async-ping/{message} should be returned by JAX-RS."));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/pong/{message}").timers().size(),
Util.foundServerRequests(registry, "/pong/{message} should be returned by JAX-RS."));

// URI for outbound client request: /pong/{message}
Assertions.assertEquals(1, registry.find("http.client.requests").tag("uri", "/pong/{message}").timers().size(),
Util.foundClientRequests(registry, "/pong/{message} should be returned by Rest client."));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import static io.restassured.RestAssured.when;

import java.util.concurrent.CompletionStage;

import javax.inject.Inject;
import javax.inject.Singleton;
import javax.ws.rs.ApplicationPath;
Expand Down Expand Up @@ -47,31 +49,44 @@ public class UriTagWithHttpApplicationRootTest {
MeterRegistry registry;

@Test
public void test() throws Exception {
public void testRequestUris() throws Exception {
RestAssured.basePath = "/";

// If you invoke requests, http server and client meters should be registered
// Leading context root (/foo) should be stripped from resulting _server_ tag
// Application path (/bar) only impacts REST endpoints

when().get("/foo/vertx/item/123").then().statusCode(200);
when().get("/foo/servlet/12345").then().statusCode(200);

// Server -> Rest client -> Server (templated)
when().get("/foo/bar/ping/one").then().statusCode(200);
when().get("/foo/bar/ping/two").then().statusCode(200);
when().get("/foo/bar/ping/three").then().statusCode(200);
when().get("/foo/vertx/item/123").then().statusCode(200);
when().get("/foo/servlet/12345").then().statusCode(200);
when().get("/foo/bar/async-ping/one").then().statusCode(200);
when().get("/foo/bar/async-ping/two").then().statusCode(200);
when().get("/foo/bar/async-ping/three").then().statusCode(200);

// TODO: These are the current answers, but they aren't quite right
// Application Path does not apply to non-rest endpoints: /vertx/item/{id}, /servlet
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/vertx/item/{id}").timers().size(),
Util.foundServerRequests(registry,
"Vert.x Web template path (/vertx/item/:id) should be detected/translated to /vertx/item/{id}."));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/servlet").timers().size(),
Util.foundServerRequests(registry, "Servlet path (/servlet) should be used for servlet"));

// URIs for server should include Application Path: /bar/ping/{message}, /bar/pong/{message}
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/ping/{message}").timers().size(),
// URIs for server should include Application Path: /bar/ping/{message}, /bar/async-ping/{message}
// URIs for inbound rest client request should include Application Path: /bar/pong/{message}
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/bar/ping/{message}").timers().size(),
Util.foundServerRequests(registry, "/bar/ping/{message} should be returned by JAX-RS"));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/pong/{message}").timers().size(),
Assertions.assertEquals(1,
registry.find("http.server.requests").tag("uri", "/bar/async-ping/{message}").timers().size(),
Util.foundServerRequests(registry, "/bar/async-ping/{message} should be returned by JAX-RS."));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/bar/pong/{message}").timers().size(),
Util.foundServerRequests(registry, "/bar/pong/{message} should be returned by JAX-RS"));

// Application Path does not apply to non-rest endpoints: /vertx/item/{id}
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/vertx/item/{id}").timers().size(),
Util.foundServerRequests(registry,
"Vert.x Web template path (/vertx/item/:id) should be detected/translated to /vertx/item/{id}."));
// URIs For client: /foo/pong/{message}
Assertions.assertEquals(1, registry.find("http.client.requests").tag("uri", "/foo/bar/pong/{message}").timers().size(),
Util.foundClientRequests(registry, "/foo/bar/pong/{message} should be returned by Rest client."));
}

@ApplicationPath("/bar")
Expand All @@ -88,6 +103,10 @@ public interface PingPongRestClient {
@Path("/foo/bar/pong/{message}")
@GET
String pingpong(@PathParam("message") String message);

@GET
@Path("/foo/bar/pong/{message}")
CompletionStage<String> asyncPingPong(@PathParam("message") String message);
}

@Inject
Expand All @@ -105,5 +124,11 @@ public String pong(@PathParam("message") String message) {
public String ping(@PathParam("message") String message) {
return pingRestClient.pingpong(message);
}

@GET
@Path("async-ping/{message}")
public CompletionStage<String> asyncPing(@PathParam("message") String message) {
return pingRestClient.asyncPingPong(message);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import static io.restassured.RestAssured.when;

import java.util.concurrent.CompletionStage;

import javax.inject.Inject;
import javax.inject.Singleton;
import javax.ws.rs.GET;
Expand Down Expand Up @@ -44,26 +46,45 @@ public class UriTagWithHttpRootTest {
MeterRegistry registry;

@Test
public void test() throws Exception {
public void testRequestUris() throws Exception {
RestAssured.basePath = "/";

// If you invoke requests, http server and client meters should be registered
// Leading context root (/foo) should be stripped from resulting _server_ tag

when().get("/foo/vertx/item/123").then().statusCode(200);
when().get("/foo/servlet/12345").then().statusCode(200);

// Server -> Rest client -> Server (templated)
when().get("/foo/ping/one").then().statusCode(200);
when().get("/foo/ping/two").then().statusCode(200);
when().get("/foo/ping/three").then().statusCode(200);
when().get("/foo/vertx/item/123").then().statusCode(200);
when().get("/foo/servlet/12345").then().statusCode(200);
when().get("/foo/async-ping/one").then().statusCode(200);
when().get("/foo/async-ping/two").then().statusCode(200);
when().get("/foo/async-ping/three").then().statusCode(200);

System.out.println("Server paths\n" + Util.listMeters(registry, "http.server.requests"));
System.out.println("Client paths\n" + Util.listMeters(registry, "http.client.requests"));

// URIs for server: /vertx/item/{id}, /servlet
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/vertx/item/{id}").timers().size(),
Util.foundServerRequests(registry,
"Vert.x Web template path (/vertx/item/:id) should be detected/translated to /vertx/item/{id}."));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/servlet").timers().size(),
Util.foundServerRequests(registry, "Servlet path (/servlet) should be used for servlet"));

// URIs for server: /ping/{message}, /pong/{message}, /vertx/item/{id}
// URIs to trigger REST request: /ping/{message}, /async-ping/{message},
// URIs for inbound rest client request: /pong/{message}
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/ping/{message}").timers().size(),
Util.foundServerRequests(registry, "/ping/{message} should be returned by JAX-RS."));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/async-ping/{message}").timers().size(),
Util.foundServerRequests(registry, "/async-ping/{message} should be returned by JAX-RS."));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/pong/{message}").timers().size(),
Util.foundServerRequests(registry, "/pong/{message} should be returned by JAX-RS."));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/vertx/item/{id}").timers().size(),
Util.foundServerRequests(registry,
"Vert.x Web template path (/vertx/item/:id) should be detected/translated to /vertx/item/{id}."));

// URI for outbound client request: /foo/pong/{message}
Assertions.assertEquals(1, registry.find("http.client.requests").tag("uri", "/foo/pong/{message}").timers().size(),
Util.foundClientRequests(registry, "/foo/pong/{message} should be returned by Rest client."));
}

@Path("/")
Expand All @@ -76,6 +97,10 @@ public interface PingPongRestClient {
@Path("/foo/pong/{message}")
@GET
String pingpong(@PathParam("message") String message);

@GET
@Path("/foo/pong/{message}")
CompletionStage<String> asyncPingPong(@PathParam("message") String message);
}

@Inject
Expand All @@ -93,5 +118,11 @@ public String pong(@PathParam("message") String message) {
public String ping(@PathParam("message") String message) {
return pingRestClient.pingpong(message);
}

@GET
@Path("async-ping/{message}")
public CompletionStage<String> asyncPing(@PathParam("message") String message) {
return pingRestClient.asyncPingPong(message);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.quarkus.micrometer.test;

import java.util.concurrent.CompletionStage;

import javax.inject.Inject;
import javax.inject.Singleton;
import javax.ws.rs.GET;
Expand All @@ -14,10 +16,13 @@
public class PingPongResource {
@RegisterRestClient(configKey = "pingpong")
public interface PingPongRestClient {

@Path("pong/{message}")
@GET
@Path("pong/{message}")
String pingpong(@PathParam("message") String message);

@GET
@Path("pong/{message}")
CompletionStage<String> asyncPingPong(@PathParam("message") String message);
}

@Inject
Expand All @@ -36,6 +41,12 @@ public String ping(@PathParam("message") String message) {
return pingRestClient.pingpong(message);
}

@GET
@Path("async-ping/{message}")
public CompletionStage<String> asyncPing(@PathParam("message") String message) {
return pingRestClient.asyncPingPong(message);
}

@GET
@Path("one")
public String one() {
Expand Down
Loading