From 5503af5f02eaf7f163ac6189eda7df2aa69c5cd8 Mon Sep 17 00:00:00 2001 From: apasternak Date: Wed, 20 Nov 2024 15:53:25 +0100 Subject: [PATCH] Memory leak fix --- .../ObservationRequestEventListener.java | 11 ++-- ...ctObservationRequestEventListenerTest.java | 52 ++++++++++++++++++- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/ObservationRequestEventListener.java b/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/ObservationRequestEventListener.java index 96c463cddc..6fc0f0166e 100644 --- a/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/ObservationRequestEventListener.java +++ b/ext/micrometer/src/main/java/org/glassfish/jersey/micrometer/server/ObservationRequestEventListener.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 @@ -65,7 +65,7 @@ public void onEvent(RequestEvent event) { switch (event.getType()) { case ON_EXCEPTION: - if (!isNotFoundException(event)) { + if (!isClientError(event) || observations.get(containerRequest) != null) { break; } startObservation(event); @@ -102,13 +102,14 @@ private void startObservation(RequestEvent event) { observations.put(event.getContainerRequest(), new ObservationScopeAndContext(scope, jerseyContext)); } - 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 static class ObservationScopeAndContext { diff --git a/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/observation/AbstractObservationRequestEventListenerTest.java b/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/observation/AbstractObservationRequestEventListenerTest.java index 597e6a61a7..5e6a06e888 100644 --- a/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/observation/AbstractObservationRequestEventListenerTest.java +++ b/ext/micrometer/src/test/java/org/glassfish/jersey/micrometer/server/observation/AbstractObservationRequestEventListenerTest.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 java.util.logging.Logger; import javax.ws.rs.core.Application; +import javax.ws.rs.core.MediaType; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; @@ -38,6 +39,7 @@ import io.micrometer.tracing.test.simple.SpansAssert; import org.glassfish.jersey.micrometer.server.ObservationApplicationEventListener; import org.glassfish.jersey.micrometer.server.ObservationRequestEventListener; +import org.glassfish.jersey.micrometer.server.mapper.ResourceGoneExceptionMapper; import org.glassfish.jersey.micrometer.server.resources.TestResource; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -85,6 +87,7 @@ protected Application configure() { final ResourceConfig config = new ResourceConfig(); config.register(listener); config.register(TestResource.class); + config.register(ResourceGoneExceptionMapper.class); return config; } @@ -131,6 +134,53 @@ void resourcesAreTimed() { .hasTag("outcome", "SUCCESS") .hasTag("status", "200") .hasTag("uri", "/sub-resource/sub-hello/{name}"); + assertThat(observationRegistry.getCurrentObservation()).isNull(); + } + + @Test + void errorResourcesAreTimed() { + try { + target("throws-exception").request().get(); + } + catch (Exception ignored) { + } + try { + target("throws-webapplication-exception").request().get(); + } + catch (Exception ignored) { + } + try { + target("throws-mappable-exception").request().get(); + } + 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")) + .timer() + .count()).isEqualTo(1); + + assertThat(registry.get(METRIC_NAME) + .tags(tagsFrom("/throws-webapplication-exception", "401", "CLIENT_ERROR", "NotAuthorizedException")) + .timer() + .count()).isEqualTo(1); + + assertThat(registry.get(METRIC_NAME) + .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); + + assertThat(observationRegistry.getCurrentObservation()).isNull(); } private static Iterable tagsFrom(String uri, String status, String outcome, String exception) {