Making sure TimedAspect does not disrupt the application flow when th…

…ere is an exception recording a metric
micrometer-metrics · Nov 29, 2018 · 328a8b0 · 328a8b0
1 parent 5494daa
commit 328a8b0
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 11 deletions.
diff --git a/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java b/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java
@@ -69,13 +69,17 @@ public Object timedMethod(ProceedingJoinPoint pjp) throws Throwable {
         try {
             return pjp.proceed();
         } finally {
-            sample.stop(Timer.builder(timed.value())
-                    .description(timed.description().isEmpty() ? null : timed.description())
-                    .tags(timed.extraTags())
-                    .tags(tagsBasedOnJoinpoint.apply(pjp))
-                    .publishPercentileHistogram(timed.histogram())
-                    .publishPercentiles(timed.percentiles().length == 0 ? null : timed.percentiles())
-                    .register(registry));
+            try {
+                sample.stop(Timer.builder(timed.value())
+                        .description(timed.description().isEmpty() ? null : timed.description())
+                        .tags(timed.extraTags())
+                        .tags(tagsBasedOnJoinpoint.apply(pjp))
+                        .publishPercentileHistogram(timed.histogram())
+                        .publishPercentiles(timed.percentiles().length == 0 ? null : timed.percentiles())
+                        .register(registry));
+            } catch (Exception e) {
+                // ignoring on purpose
+            }
         }
     }
 }
diff --git a/micrometer-core/src/test/java/io/micrometer/core/aop/TimedAspectTest.java b/micrometer-core/src/test/java/io/micrometer/core/aop/TimedAspectTest.java
@@ -15,13 +15,20 @@
  */
 package io.micrometer.core.aop;
 
-import io.micrometer.core.annotation.Timed;
-import io.micrometer.core.instrument.MeterRegistry;
-import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+
 import org.junit.jupiter.api.Test;
 import org.springframework.aop.aspectj.annotation.AspectJProxyFactory;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import io.micrometer.core.annotation.Timed;
+import io.micrometer.core.instrument.Meter.Id;
+import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.Timer;
+import io.micrometer.core.instrument.distribution.DistributionStatisticConfig;
+import io.micrometer.core.instrument.distribution.pause.PauseDetector;
+import io.micrometer.core.instrument.search.MeterNotFoundException;
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
 
 class TimedAspectTest {
     @Test
@@ -41,6 +48,39 @@ void timeMethod() {
                 .tag("extra", "tag")
                 .timer().count()).isEqualTo(1);
     }
+
+    @Test
+    void timeMethodFailure() {
+        MeterRegistry failingRegistry = new FailingMeterRegistry();
+
+        AspectJProxyFactory pf = new AspectJProxyFactory(new TimedService());
+        pf.addAspect(new TimedAspect(failingRegistry));
+
+        TimedService service = pf.getProxy();
+
+        service.call();
+
+        assertThatExceptionOfType(MeterNotFoundException.class).isThrownBy(() -> {
+            failingRegistry.get("call")
+                    .tag("class", "io.micrometer.core.aop.TimedAspectTest$TimedService")
+                    .tag("method", "call")
+                    .tag("extra", "tag")
+                    .timer();
+        });
+
+
+    }
+
+    private final class FailingMeterRegistry extends SimpleMeterRegistry {
+        private FailingMeterRegistry() {
+            super();
+        }
+
+        @Override
+        protected Timer newTimer(Id id, DistributionStatisticConfig distributionStatisticConfig, PauseDetector pauseDetector) {
+            throw new RuntimeException();
+        }
+    }
 
     static class TimedService {
         @Timed(value = "call", extraTags = {"extra", "tag"})