From 4e6715961abbe0421a4ada99bcd7d9be1ff372a8 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Fri, 5 May 2023 15:13:15 +0800 Subject: [PATCH] Ignore class level AOP annotation if method level present Do not advise twice --- .../io/micrometer/core/aop/CountedAspect.java | 3 ++- .../io/micrometer/core/aop/TimedAspect.java | 3 ++- .../core/aop/CountedAspectTest.java | 27 +++++++++++++++++-- .../micrometer/core/aop/TimedAspectTest.java | 24 +++++++++++++++++ .../observation/aop/ObservedAspect.java | 3 ++- .../observation/aop/ObservedAspectTests.java | 21 +++++++++++++++ 6 files changed, 76 insertions(+), 5 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/aop/CountedAspect.java b/micrometer-core/src/main/java/io/micrometer/core/aop/CountedAspect.java index 19f82703b9..d0682e98eb 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/aop/CountedAspect.java +++ b/micrometer-core/src/main/java/io/micrometer/core/aop/CountedAspect.java @@ -69,6 +69,7 @@ * @author Ali Dehghani * @author Jonatan Ivanov * @author Johnny Lim + * @author Yanming Zhou * @since 1.2.0 * @see Counted */ @@ -166,7 +167,7 @@ public CountedAspect(MeterRegistry registry, Function meterRegistry.get("class.counted").counter()); + + assertThat(meterRegistry.get("method.counted") + .tag("class", "io.micrometer.core.aop.CountedAspectTest$CountedClassService") + .tag("method", "greet") + .tag("result", "success") + .tag("exception", "none") + .counter() + .count()).isEqualTo(1); + } + @Counted("class.counted") static class CountedClassService { @@ -338,6 +356,11 @@ void fail() { throw new RuntimeException("Oops"); } + @Counted("method.counted") + String greet() { + return "hello"; + } + } } 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 index 3b3ad8b4d4..7dbe570b42 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/aop/TimedAspectTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/aop/TimedAspectTest.java @@ -374,6 +374,26 @@ void timeClassFailure() { }); } + @Test + void ignoreClassLevelAnnotationIfMethodLevelPresent() { + MeterRegistry registry = new SimpleMeterRegistry(); + + AspectJProxyFactory pf = new AspectJProxyFactory(new TimedClass()); + pf.addAspect(new TimedAspect(registry)); + + TimedClass service = pf.getProxy(); + + service.annotatedOnMethod(); + + assertThatExceptionOfType(MeterNotFoundException.class).isThrownBy(() -> registry.get("call").timer()); + assertThat(registry.get("annotatedOnMethod") + .tag("class", "io.micrometer.core.aop.TimedAspectTest$TimedClass") + .tag("method", "annotatedOnMethod") + .tag("extra", "tag2") + .timer() + .count()).isEqualTo(1); + } + static class MeterTagsTests { ValueResolver valueResolver = parameter -> "Value from myCustomTagValueResolver [" + parameter + "]"; @@ -625,6 +645,10 @@ static class TimedClass { void call() { } + @Timed(value = "annotatedOnMethod", extraTags = { "extra", "tag2" }) + void annotatedOnMethod() { + } + } interface TimedInterface { diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java b/micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java index 7595a51621..8976077d67 100644 --- a/micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java +++ b/micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java @@ -68,6 +68,7 @@ * * * @author Jonatan Ivanov + * @author Yanming Zhou * @since 1.10.0 */ @Aspect @@ -104,7 +105,7 @@ public ObservedAspect(ObservationRegistry registry, this.shouldSkip = shouldSkip; } - @Around("@within(io.micrometer.observation.annotation.Observed)") + @Around("@within(io.micrometer.observation.annotation.Observed) and not @annotation(io.micrometer.observation.annotation.Observed)") @Nullable public Object observeClass(ProceedingJoinPoint pjp) throws Throwable { if (shouldSkip.test(pjp)) { diff --git a/micrometer-observation/src/test/java/io/micrometer/observation/aop/ObservedAspectTests.java b/micrometer-observation/src/test/java/io/micrometer/observation/aop/ObservedAspectTests.java index b7eb738e3c..bd66eb19aa 100644 --- a/micrometer-observation/src/test/java/io/micrometer/observation/aop/ObservedAspectTests.java +++ b/micrometer-observation/src/test/java/io/micrometer/observation/aop/ObservedAspectTests.java @@ -335,6 +335,23 @@ void skipPredicateShouldTakeEffectForClass() { TestObservationRegistryAssert.assertThat(registry).doesNotHaveAnyObservation(); } + @Test + void ignoreClassLevelAnnotationIfMethodLevelPresent() { + registry.observationConfig().observationHandler(new ObservationTextPublisher()); + + AspectJProxyFactory pf = new AspectJProxyFactory(new ObservedClassLevelAnnotatedService()); + pf.addAspect(new ObservedAspect(registry)); + + ObservedClassLevelAnnotatedService service = pf.getProxy(); + service.annotatedOnMethod(); + TestObservationRegistryAssert.assertThat(registry) + .doesNotHaveAnyRemainingCurrentObservation() + .hasSingleObservationThat() + .hasBeenStopped() + .hasNameEqualTo("test.class") + .hasContextualNameEqualTo("test.class#annotatedOnMethod"); + } + static class ObservedService { @Observed(name = "test.call", contextualName = "test#call", @@ -387,6 +404,10 @@ CompletableFuture async(FakeAsyncTask fakeAsyncTask) { contextSnapshot.wrapExecutor(Executors.newSingleThreadExecutor())); } + @Observed(name = "test.class", contextualName = "test.class#annotatedOnMethod") + void annotatedOnMethod() { + } + } static class FakeAsyncTask implements Supplier {