From 75a9755a062da0765a216ed3d188570eba3cc012 Mon Sep 17 00:00:00 2001 From: apasternak Date: Thu, 4 Apr 2024 11:34:36 +0200 Subject: [PATCH] Add missing metrics for cases of client errors Changes ported from micrometer-metrics: https://github.com/micrometer-metrics/micrometer/pull/4326 --- .../jersey/micrometer/server/JerseyTags.java | 13 ++++++++++--- .../server/MetricsRequestEventListener.java | 11 ++++++----- .../server/DefaultJerseyTagsProviderTest.java | 14 +++++++------- .../server/MetricsRequestEventListenerTest.java | 13 ++++++++++++- .../micrometer/server/resources/TestResource.java | 9 ++++++++- 5 files changed, 43 insertions(+), 17 deletions(-) diff --git a/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/JerseyTags.java b/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/JerseyTags.java index d723c7c1b9..29494f413c 100644 --- a/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/JerseyTags.java +++ b/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/JerseyTags.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -43,6 +43,8 @@ public final class JerseyTags { private static final Tag URI_ROOT = Tag.of("uri", "root"); + private static final Tag URI_UNKNOWN = Tag.of("uri", "UNKNOWN"); + private static final Tag EXCEPTION_NONE = Tag.of("exception", "None"); private static final Tag STATUS_SERVER_ERROR = Tag.of("status", "500"); @@ -95,7 +97,10 @@ public static Tag uri(RequestEvent event) { } } String matchingPattern = getMatchingPattern(event); - if (matchingPattern.equals("/")) { + if (matchingPattern == null) { + return URI_UNKNOWN; + } + else if (matchingPattern.equals("/")) { return URI_ROOT; } return Tag.of("uri", matchingPattern); @@ -108,7 +113,9 @@ static boolean isRedirection(int status) { static String getMatchingPattern(RequestEvent event) { ExtendedUriInfo uriInfo = event.getUriInfo(); List templates = uriInfo.getMatchedTemplates(); - + if (templates.isEmpty()) { + return null; + } StringBuilder sb = new StringBuilder(); sb.append(uriInfo.getBaseUri().getPath()); for (int i = templates.size() - 1; i >= 0; i--) { diff --git a/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/MetricsRequestEventListener.java b/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/MetricsRequestEventListener.java index 9036637bee..23da48cb8a 100644 --- a/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/MetricsRequestEventListener.java +++ b/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/MetricsRequestEventListener.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -79,7 +79,7 @@ public void onEvent(RequestEvent event) { switch (event.getType()) { case ON_EXCEPTION: - if (!isNotFoundException(event)) { + if (!isClientError(event)) { break; } time(event, containerRequest); @@ -122,13 +122,14 @@ private void time(RequestEvent event, ContainerRequest containerRequest) { } } - private boolean isNotFoundException(RequestEvent event) { + private boolean isClientError(RequestEvent event) { Throwable t = event.getException(); if (t == null) { return false; } - String className = t.getClass().getCanonicalName(); - return className.equals("jakarta.ws.rs.NotFoundException") || className.equals("javax.ws.rs.NotFoundException"); + String className = t.getClass().getSuperclass().getCanonicalName(); + return className.equals("jakarta.ws.rs.ClientErrorException") + || className.equals("javax.ws.rs.ClientErrorException"); } private Set shortTimers(Set timed, RequestEvent event) { diff --git a/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/DefaultJerseyTagsProviderTest.java b/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/DefaultJerseyTagsProviderTest.java index 4bf17590e6..b76cdf0413 100644 --- a/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/DefaultJerseyTagsProviderTest.java +++ b/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/DefaultJerseyTagsProviderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -51,7 +51,7 @@ class DefaultJerseyTagsProviderTest { @Test void testRootPath() { - assertThat(tagsProvider.httpRequestTags(event(200, null, "/", (String[]) null))) + assertThat(tagsProvider.httpRequestTags(event(200, null, "/", "/"))) .containsExactlyInAnyOrder(tagsFrom("root", 200, null, "SUCCESS")); } @@ -86,21 +86,21 @@ void redirectsAreShunted() { @Test @SuppressWarnings("serial") void exceptionsAreMappedCorrectly() { - assertThat(tagsProvider.httpRequestTags(event(500, new IllegalArgumentException(), "/app", (String[]) null))) + assertThat(tagsProvider.httpRequestTags(event(500, new IllegalArgumentException(), "/app", "/"))) .containsExactlyInAnyOrder(tagsFrom("/app", 500, "IllegalArgumentException", "SERVER_ERROR")); assertThat(tagsProvider.httpRequestTags( - event(500, new IllegalArgumentException(new NullPointerException()), "/app", (String[]) null))) + event(500, new IllegalArgumentException(new NullPointerException()), "/app", "/"))) .containsExactlyInAnyOrder(tagsFrom("/app", 500, "NullPointerException", "SERVER_ERROR")); - assertThat(tagsProvider.httpRequestTags(event(406, new NotAcceptableException(), "/app", (String[]) null))) + assertThat(tagsProvider.httpRequestTags(event(406, new NotAcceptableException(), "/app", "/"))) .containsExactlyInAnyOrder(tagsFrom("/app", 406, "NotAcceptableException", "CLIENT_ERROR")); assertThat(tagsProvider.httpRequestTags(event(500, new Exception("anonymous") { - }, "/app", (String[]) null))).containsExactlyInAnyOrder(tagsFrom("/app", 500, + }, "/app", "/"))).containsExactlyInAnyOrder(tagsFrom("/app", 500, "org.glassfish.jersey.micrometer.server.DefaultJerseyTagsProviderTest$1", "SERVER_ERROR")); } @Test void longRequestTags() { - assertThat(tagsProvider.httpLongRequestTags(event(0, null, "/app", (String[]) null))) + assertThat(tagsProvider.httpLongRequestTags(event(0, null, "/app", "/"))) .containsExactlyInAnyOrder(Tag.of("method", "GET"), Tag.of("uri", "/app")); } diff --git a/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/MetricsRequestEventListenerTest.java b/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/MetricsRequestEventListenerTest.java index b2edeeab26..8fb806f780 100644 --- a/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/MetricsRequestEventListenerTest.java +++ b/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/MetricsRequestEventListenerTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -20,6 +20,7 @@ import javax.ws.rs.NotFoundException; import javax.ws.rs.core.Application; +import javax.ws.rs.core.MediaType; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; @@ -149,6 +150,11 @@ void exceptionsAreMappedCorrectly() { } catch (Exception ignored) { } + try { + target("produces-text-plain").request(MediaType.APPLICATION_JSON).get(); + } + catch (Exception ignored) { + } assertThat(registry.get(METRIC_NAME) .tags(tagsFrom("/throws-exception", "500", "SERVER_ERROR", "IllegalArgumentException")) @@ -164,6 +170,11 @@ void exceptionsAreMappedCorrectly() { .tags(tagsFrom("/throws-mappable-exception", "410", "CLIENT_ERROR", "ResourceGoneException")) .timer() .count()).isEqualTo(1); + + assertThat(registry.get(METRIC_NAME) + .tags(tagsFrom("UNKNOWN", "406", "CLIENT_ERROR", "NotAcceptableException")) + .timer() + .count()).isEqualTo(1); } private static Iterable tagsFrom(String uri, String status, String outcome, String exception) { diff --git a/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/resources/TestResource.java b/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/resources/TestResource.java index 661e1fa4e4..054d145bdc 100644 --- a/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/resources/TestResource.java +++ b/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/resources/TestResource.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -89,6 +89,13 @@ public String throwsMappableException() { throw new ResourceGoneException("Resource has been permanently removed."); } + @GET + @Path("produces-text-plain") + @Produces(MediaType.TEXT_PLAIN) + public String producesTextPlain() { + return "hello"; + } + @GET @Path("redirect/{status}") public Response redirect(@PathParam("status") int status) {