Memory leak fix

jansupol · Dec 2, 2024 · 5503af5 · 5503af5
1 parent 47e37bd
commit 5503af5
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 6 deletions.
diff --git a/...src/main/java/org/glassfish/jersey/micrometer/server/ObservationRequestEventListener.java b/...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/...ish/jersey/micrometer/server/observation/AbstractObservationRequestEventListenerTest.java b/...ish/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<Tag> tagsFrom(String uri, String status, String outcome, String exception) {