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

Clean up template URI matching; add support for servlets #15047

Merged
merged 1 commit into from
Apr 1, 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
16 changes: 16 additions & 0 deletions extensions/micrometer/deployment/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-reactive-spi-deployment</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-undertow-spi</artifactId>
</dependency>

<!-- test -->

Expand Down Expand Up @@ -77,6 +81,18 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-undertow-deployment</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-vertx-web-deployment</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>io.rest-assured</groupId>
<artifactId>rest-assured</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.util.function.BooleanSupplier;

import javax.inject.Singleton;
import javax.servlet.DispatcherType;

import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
import io.quarkus.arc.deployment.SyntheticBeanBuildItem;
Expand Down Expand Up @@ -33,9 +34,10 @@ public class HttpBinderProcessor {
// Common MeterFilter (uri variation limiter)
static final String HTTP_METER_FILTER_CONFIGURATION = "io.quarkus.micrometer.runtime.binder.HttpMeterFilterProvider";

// avoid imports due to related deps not being there
// JAX-RS, Servlet Filters
static final String RESTEASY_CONTAINER_FILTER_CLASS_NAME = "io.quarkus.micrometer.runtime.binder.vertx.VertxMeterBinderRestEasyContainerFilter";
static final String QUARKUS_REST_CONTAINER_FILTER_CLASS_NAME = "io.quarkus.micrometer.runtime.binder.vertx.VertxMeterBinderQuarkusRestContainerFilter";
static final String RESTEASY_REACTIVE_CONTAINER_FILTER_CLASS_NAME = "io.quarkus.micrometer.runtime.binder.vertx.VertxMeterBinderRestEasyReactiveContainerFilter";
static final String UNDERTOW_SERVLET_FILTER_CLASS_NAME = "io.quarkus.micrometer.runtime.binder.vertx.VertxMeterBinderUndertowServletFilter";

// Rest client listener SPI
private static final String REST_CLIENT_LISTENER_CLASS_NAME = "org.eclipse.microprofile.rest.client.spi.RestClientListener";
Expand Down Expand Up @@ -100,15 +102,31 @@ FilterBuildItem addVertxMeterFilter() {
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(QUARKUS_REST_CONTAINER_FILTER_CLASS_NAME));
createAdditionalBean(additionalBeans, QUARKUS_REST_CONTAINER_FILTER_CLASS_NAME);
.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(
io.quarkus.undertow.deployment.FilterBuildItem.builder("metricsFilter", UNDERTOW_SERVLET_FILTER_CLASS_NAME)
.setAsyncSupported(true)
.addFilterUrlMapping("*", DispatcherType.FORWARD)
.addFilterUrlMapping("*", DispatcherType.INCLUDE)
.addFilterUrlMapping("*", DispatcherType.REQUEST)
.addFilterUrlMapping("*", DispatcherType.ASYNC)
.addFilterUrlMapping("*", DispatcherType.ERROR)
.build());
createAdditionalBean(additionalBeans, UNDERTOW_SERVLET_FILTER_CLASS_NAME);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package io.quarkus.micrometer.deployment.binder;

import static io.restassured.RestAssured.when;

import javax.inject.Inject;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.micrometer.core.instrument.MeterRegistry;
import io.quarkus.micrometer.test.PingPongResource;
import io.quarkus.micrometer.test.ServletEndpoint;
import io.quarkus.micrometer.test.Util;
import io.quarkus.micrometer.test.VertxWebEndpoint;
import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;

public class UriTagTest {
@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withConfigurationResource("test-logging.properties")
.overrideConfigKey("quarkus.micrometer.binder-enabled-default", "false")
.overrideConfigKey("quarkus.micrometer.binder.http-client.enabled", "true")
.overrideConfigKey("quarkus.micrometer.binder.http-server.enabled", "true")
.overrideConfigKey("quarkus.micrometer.binder.http-server.match-patterns", "/one=/two")
.overrideConfigKey("quarkus.micrometer.binder.http-server.ignore-patterns", "/two")
.overrideConfigKey("quarkus.micrometer.binder.vertx.enabled", "true")
.overrideConfigKey("pingpong/mp-rest/url", "${test.url}")
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(Util.class,
PingPongResource.class,
PingPongResource.PingPongRestClient.class,
ServletEndpoint.class,
VertxWebEndpoint.class));

@Inject
MeterRegistry registry;

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

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

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);

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

// /one should map to /two, which is ignored. Neither should exist w/ timers
Assertions.assertEquals(0, registry.find("http.server.requests").tag("uri", "/one").timers().size(),
"/one is mapped to /two, which should be ignored. Found:\n"
+ Util.listMeters(registry.find("http.server.requests").meters(), "uri"));
Assertions.assertEquals(0, registry.find("http.server.requests").tag("uri", "/two").timers().size(),
"/two should be ignored. Found:\n"
+ Util.listMeters(registry.find("http.server.requests").meters(), "uri"));

// 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(),
"/ping/{message} should be returned by JAX-RS. Found:\n"
+ Util.listMeters(registry.find("http.server.requests").meters(), "uri"));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/pong/{message}").timers().size(),
"/pong/{message} should be returned by JAX-RS. Found:\n"
+ Util.listMeters(registry.find("http.server.requests").meters(), "uri"));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/vertx/item/{id}").timers().size(),
"Vert.x Web template path (/vertx/item/:id) should be detected/translated to /vertx/item/{id}. Found:\n"
+ Util.listMeters(registry.find("http.server.requests").meters(), "uri"));
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/vertx/item/{id}/{sub}").timers().size(),
"Vert.x Web template path (/vertx/item/:id/:sub) should be detected/translated to /vertx/item/{id}/{sub}. Found:\n"
+ Util.listMeters(registry.find("http.server.requests").meters(), "uri"));

Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/servlet").timers().size(),
"Servlet path (/servlet) should be used for servlet. Found:\n"
+ Util.listMeters(registry.find("http.server.requests").meters(), "uri"));

// TODO: #15231
// URIs For client: /pong/{message}
// Assertions.assertEquals(1, registry.find("http.client.requests").tag("uri", "/pong/{message}").timers().size(),
// "/pong/{message} should be returned by Rest client. Found:\n"
// + Util.listMeters(registry.find("http.client.requests").meters(), "uri"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,16 @@ public String pong(@PathParam("message") String message) {
public String ping(@PathParam("message") String message) {
return pingRestClient.pingpong(message);
}

@GET
@Path("one")
public String one() {
return "OK";
}

@GET
@Path("two")
public String two() {
return "OK";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package io.quarkus.micrometer.test;

import java.io.IOException;

import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

@WebServlet(name = "ServletEndpoint", urlPatterns = "/servlet/*")
public class ServletEndpoint extends HttpServlet {
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
resp.setContentType("text/plain");
resp.getWriter().println("OK");
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package io.quarkus.micrometer.test;

import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.logging.LogRecord;
import java.util.stream.Collectors;

import org.junit.jupiter.api.Assertions;

import io.micrometer.core.instrument.Meter;

public class Util {
private Util() {
}
Expand All @@ -26,4 +30,12 @@ static String stackToString(Throwable t) {
Arrays.asList(t.getStackTrace()).forEach(x -> sb.append("\t").append(x.toString()).append("\n"));
return sb.toString();
}

public static String listMeters(Collection<Meter> collection, final String tag) {
return collection.stream()
.map(x -> {
return x.getId().getTag(tag);
})
.collect(Collectors.joining(","));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package io.quarkus.micrometer.test;

import io.quarkus.vertx.web.Param;
import io.quarkus.vertx.web.Route;
import io.quarkus.vertx.web.RouteBase;
import io.vertx.core.http.HttpMethod;

@RouteBase(path = "/vertx")
public class VertxWebEndpoint {
@Route(path = "item/:id", methods = HttpMethod.GET)
public String item(@Param("id") Integer id) {
return "message with id " + id;
}

@Route(path = "item/:id/:sub", methods = HttpMethod.GET)
public String item(@Param("id") Integer id, @Param("sub") Integer sub) {
return "message with id " + id + " and sub " + sub;
}
}
2 changes: 1 addition & 1 deletion extensions/micrometer/runtime/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@

<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-hibernate-orm</artifactId>
<artifactId>quarkus-undertow</artifactId>
<optional>true</optional>
</dependency>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package io.quarkus.micrometer.runtime.binder;

import java.util.regex.Pattern;

import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.binder.http.Outcome;

Expand All @@ -16,9 +14,6 @@ public class HttpCommonTags {

public static final Tag METHOD_UNKNOWN = Tag.of("method", "UNKNOWN");

public static final Pattern TRAILING_SLASH_PATTERN = Pattern.compile("/$");
public static final Pattern MULTIPLE_SLASH_PATTERN = Pattern.compile("//+");

/**
* Creates an {@code method} {@code Tag} derived from the given {@code HTTP method}.
*
Expand Down
Loading