From 5e718aa600f48f356b02eb93db0ff45fd40b38de Mon Sep 17 00:00:00 2001 From: Jonatan Ivanov Date: Wed, 7 Apr 2021 23:01:48 -0700 Subject: [PATCH 1/4] Introducing a skip predicate to the TimedAspect so that users can skip Timer creation based on the JoinPoint, fixes gh-780 --- .../io/micrometer/core/aop/TimedAspect.java | 53 +++++++++++++++++-- .../micrometer/core/aop/TimedAspectTest.java | 16 ++++++ 2 files changed, 64 insertions(+), 5 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 index 3eb7342e47..c0327c5c9c 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java +++ b/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java @@ -33,6 +33,7 @@ import java.util.Optional; import java.util.concurrent.CompletionStage; import java.util.function.Function; +import java.util.function.Predicate; /** * AspectJ aspect for intercepting types or methods annotated with {@link Timed @Timed}. @@ -41,12 +42,14 @@ * @author Jon Schneider * @author Johnny Lim * @author Nejc Korasa + * @author Jonatan Ivanov * @since 1.0.0 */ @Aspect @NonNullApi @Incubating(since = "1.0.0") public class TimedAspect { + private static final Predicate DONT_SKIP_ANYTHING = pjp -> false; public static final String DEFAULT_METRIC_NAME = "method.timed"; public static final String DEFAULT_EXCEPTION_TAG_VALUE = "none"; @@ -59,9 +62,10 @@ public class TimedAspect { private final MeterRegistry registry; private final Function> tagsBasedOnJoinPoint; + private final Predicate shouldSkip; /** - * Create a {@code TimedAspect} instance with {@link Metrics#globalRegistry}. + * Creates a {@code TimedAspect} instance with {@link Metrics#globalRegistry}. * * @since 1.2.0 */ @@ -69,20 +73,59 @@ public TimedAspect() { this(Metrics.globalRegistry); } + /** + * Creates a {@code TimedAspect} instance with the given {@code meterRegistry}. + * + * @param registry Where we're going to register metrics. + */ public TimedAspect(MeterRegistry registry) { - this(registry, pjp -> - Tags.of("class", pjp.getStaticPart().getSignature().getDeclaringTypeName(), - "method", pjp.getStaticPart().getSignature().getName()) - ); + this(registry, DONT_SKIP_ANYTHING); } + /** + * Creates a {@code TimedAspect} instance with the given {@code meterRegistry} and tags provider function. + * + * @param registry Where we're going to register metrics. + * @param tagsBasedOnJoinPoint A function to generate tags given a join point. + */ public TimedAspect(MeterRegistry registry, Function> tagsBasedOnJoinPoint) { + this(registry, tagsBasedOnJoinPoint, DONT_SKIP_ANYTHING); + } + + /** + * Creates a {@code TimedAspect} instance with the given {@code meterRegistry} and skip predicate. + * + * @param registry Where we're going to register metrics. + * @param shouldSkip A predicate to decide if creating the timer should be skipped or not. + */ + public TimedAspect(MeterRegistry registry, Predicate shouldSkip) { + this( + registry, + pjp -> Tags.of("class", pjp.getStaticPart().getSignature().getDeclaringTypeName(), + "method", pjp.getStaticPart().getSignature().getName()), + shouldSkip + ); + } + + /** + * Creates a {@code TimedAspect} instance with the given {@code meterRegistry}, tags provider function and skip predicate. + * + * @param registry Where we're going to register metrics. + * @param tagsBasedOnJoinPoint A function to generate tags given a join point. + * @param shouldSkip A predicate to decide if creating the timer should be skipped or not. + */ + public TimedAspect(MeterRegistry registry, Function> tagsBasedOnJoinPoint, Predicate shouldSkip) { this.registry = registry; this.tagsBasedOnJoinPoint = tagsBasedOnJoinPoint; + this.shouldSkip = shouldSkip; } @Around("execution (@io.micrometer.core.annotation.Timed * *.*(..))") public Object timedMethod(ProceedingJoinPoint pjp) throws Throwable { + if (shouldSkip.test(pjp)) { + return pjp.proceed(); + } + Method method = ((MethodSignature) pjp.getSignature()).getMethod(); Timed timed = method.getAnnotation(Timed.class); if (timed == null) { 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 b0de70c64e..7d49167017 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 @@ -25,12 +25,14 @@ import io.micrometer.core.instrument.search.MeterNotFoundException; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import io.micrometer.core.lang.NonNull; +import org.aspectj.lang.ProceedingJoinPoint; import org.junit.jupiter.api.Test; import org.springframework.aop.aspectj.annotation.AspectJProxyFactory; import javax.annotation.Nonnull; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; +import java.util.function.Predicate; import static java.util.concurrent.CompletableFuture.supplyAsync; import static org.assertj.core.api.Assertions.*; @@ -54,6 +56,20 @@ void timeMethod() { .timer().count()).isEqualTo(1); } + @Test + void timeMethodWithSkipPredicate() { + MeterRegistry registry = new SimpleMeterRegistry(); + + AspectJProxyFactory pf = new AspectJProxyFactory(new TimedService()); + pf.addAspect(new TimedAspect(registry, (Predicate) pjp -> true)); + + TimedService service = pf.getProxy(); + + service.call(); + + assertThat(registry.find("call").timer()).isNull(); + } + @Test void timeMethodWithLongTaskTimer() { MeterRegistry registry = new SimpleMeterRegistry(); From cdb36177b7e2ff2a84df9d87d5ac7909023cb275 Mon Sep 17 00:00:00 2001 From: Jonatan Ivanov Date: Thu, 8 Apr 2021 14:33:08 -0700 Subject: [PATCH 2/4] Adding javadoc for TimedAspect --- .../io/micrometer/core/aop/TimedAspect.java | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) 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 index c0327c5c9c..9f476173e5 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java +++ b/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java @@ -36,7 +36,37 @@ import java.util.function.Predicate; /** - * AspectJ aspect for intercepting types or methods annotated with {@link Timed @Timed}. + *

+ * AspectJ aspect for intercepting types or methods annotated with {@link Timed @Timed}.
+ * The aspect supports programmatic customizations through constructor-injectable custom logic. + *

+ * + *

+ * You might want to add tags programmatically to the {@link Timer}.
+ * In this case, the tags provider function (Function<ProceedingJoinPoint, Iterable<Tag>>) can help. + * It receives a {@link ProceedingJoinPoint} and returns the {@link Tag}s that will be attached to the {@link Timer}. + *

+ *

+ * You might also want to skip the {@link Timer} creation programmatically.
+ * One use-case can be having another component in your application that already processes the {@link Timed @Timed} annotation + * in some cases so that {@code TimedAspect} should not intercept these methods. E.g.: Spring Boot does this for its controllers. + * By using the skip predicate (Predicate<ProceedingJoinPoint>) + * you can tell the {@code TimedAspect} when not to create a {@link Timer}. + * + * Here's an example to disable {@link Timer} creation for Spring controllers: + * + * 

+ * @Bean
+ * public TimedAspect timedAspect(MeterRegistry meterRegistry) {
+ *     return new TimedAspect(meterRegistry, this::skipControllers);
+ * }
+ *
+ * private boolean skipControllers(ProceedingJoinPoint pjp) {
+ *     Class targetClass = pjp.getTarget().getClass();
+ *     return targetClass.isAnnotationPresent(RestController.class) || targetClass.isAnnotationPresent(Controller.class);
+ * }
+ *
+ *

* * @author David J. M. Karlsen * @author Jon Schneider From bb7f6e7a6a4aa5ac1b3d436e3bc48b75dcc98d17 Mon Sep 17 00:00:00 2001 From: Jonatan Ivanov Date: Thu, 8 Apr 2021 15:20:40 -0700 Subject: [PATCH 3/4] Introducing a skip predicate to the CountedAspect so that users can skip Counter creation based on the JoinPoint, see the same thing for TimedAspect: gh-780 --- .../io/micrometer/core/aop/CountedAspect.java | 61 ++++++++++++++++--- .../core/aop/CountedAspectTest.java | 20 +++++- 2 files changed, 71 insertions(+), 10 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 c98ad60d0f..de3694b089 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 @@ -18,6 +18,7 @@ import io.micrometer.core.annotation.Counted; import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tags; import io.micrometer.core.lang.NonNullApi; @@ -29,6 +30,7 @@ import java.lang.reflect.Method; import java.util.concurrent.CompletionStage; import java.util.function.Function; +import java.util.function.Predicate; /** * Aspect responsible for intercepting all methods annotated with the {@link Counted} @@ -41,6 +43,7 @@ @Aspect @NonNullApi public class CountedAspect { + private static final Predicate DONT_SKIP_ANYTHING = pjp -> false; public final String DEFAULT_EXCEPTION_TAG_VALUE = "none"; public final String RESULT_TAG_FAILURE_VALUE = "failure"; public final String RESULT_TAG_SUCCESS_VALUE = "success"; @@ -66,26 +69,63 @@ public class CountedAspect { private final Function> tagsBasedOnJoinPoint; /** - * Construct a new aspect with the given {@code meterRegistry} along with a default - * tags provider. + * A predicate that decides if Timer creation should be skipped for the given join point. + */ + private final Predicate shouldSkip; + + /** + * Creates a {@code CountedAspect} instance with {@link Metrics#globalRegistry}. + * + */ + public CountedAspect() { + this(Metrics.globalRegistry); + } + + /** + * Creates a {@code CountedAspect} instance with the given {@code meterRegistry}. * - * @param meterRegistry Where we're going register metrics. + * @param registry Where we're going to register metrics. */ - public CountedAspect(MeterRegistry meterRegistry) { - this(meterRegistry, pjp -> - Tags.of("class", pjp.getStaticPart().getSignature().getDeclaringTypeName(), - "method", pjp.getStaticPart().getSignature().getName())); + public CountedAspect(MeterRegistry registry) { + this(registry, DONT_SKIP_ANYTHING); } /** - * Constructs a new aspect with the given {@code meterRegistry} and tags provider function. + * Creates a {@code CountedAspect} instance with the given {@code meterRegistry} and tags provider function. * - * @param meterRegistry Where we're going register metrics. + * @param meterRegistry Where we're going to register metrics. * @param tagsBasedOnJoinPoint A function to generate tags given a join point. */ public CountedAspect(MeterRegistry meterRegistry, Function> tagsBasedOnJoinPoint) { + this(meterRegistry, tagsBasedOnJoinPoint, DONT_SKIP_ANYTHING); + } + + /** + * Creates a {@code CountedAspect} instance with the given {@code meterRegistry} and skip predicate. + * + * @param meterRegistry Where we're going to register metrics. + * @param shouldSkip A predicate to decide if creating the timer should be skipped or not. + */ + public CountedAspect(MeterRegistry meterRegistry, Predicate shouldSkip) { + this( + meterRegistry, + pjp -> Tags.of("class", pjp.getStaticPart().getSignature().getDeclaringTypeName(), + "method", pjp.getStaticPart().getSignature().getName()), + shouldSkip + ); + } + + /** + * Creates a {@code CountedAspect} instance with the given {@code meterRegistry}, tags provider function and skip predicate. + * + * @param meterRegistry Where we're going to register metrics. + * @param tagsBasedOnJoinPoint A function to generate tags given a join point. + * @param shouldSkip A predicate to decide if creating the timer should be skipped or not. + */ + public CountedAspect(MeterRegistry meterRegistry, Function> tagsBasedOnJoinPoint, Predicate shouldSkip) { this.meterRegistry = meterRegistry; this.tagsBasedOnJoinPoint = tagsBasedOnJoinPoint; + this.shouldSkip = shouldSkip; } /** @@ -106,6 +146,9 @@ public CountedAspect(MeterRegistry meterRegistry, Function) proceedingJoinPoint -> true) + ); + + countedService.succeedWithMetrics(); + + assertThat(meterRegistry.find("metric.success").counter()).isNull(); + } + @Test void countedWithFailure() { try { @@ -204,8 +218,12 @@ void emptyMetricNameWithException() { } private T getAdvisedService(T countedService) { + return getAdvisedService(countedService, new CountedAspect(meterRegistry)); + } + + private T getAdvisedService(T countedService, CountedAspect countedAspect) { AspectJProxyFactory proxyFactory = new AspectJProxyFactory(countedService); - proxyFactory.addAspect(new CountedAspect(meterRegistry)); + proxyFactory.addAspect(countedAspect); return proxyFactory.getProxy(); } From 9bc284c9e77c277d5ccb3ca0b0bc0ff64ced0844 Mon Sep 17 00:00:00 2001 From: Jonatan Ivanov Date: Thu, 8 Apr 2021 15:21:09 -0700 Subject: [PATCH 4/4] Adding javadoc for CountedAspect --- .../io/micrometer/core/aop/CountedAspect.java | 34 +++++++++++++++++-- .../io/micrometer/core/aop/TimedAspect.java | 1 - 2 files changed, 32 insertions(+), 3 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 de3694b089..429d2b87fd 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 @@ -33,10 +33,40 @@ import java.util.function.Predicate; /** - * Aspect responsible for intercepting all methods annotated with the {@link Counted} - * annotation and record a few counter metrics about their execution status. + *

+ * Aspect responsible for intercepting all methods annotated with the {@link Counted @Counted} + * annotation and record a few counter metrics about their execution status.
+ * The aspect supports programmatic customizations through constructor-injectable custom logic. + *

+ *

+ * You might want to add tags programmatically to the {@link Counter}.
+ * In this case, the tags provider function (Function<ProceedingJoinPoint, Iterable<Tag>>) can help. + * It receives a {@link ProceedingJoinPoint} and returns the {@link Tag}s that will be attached to the {@link Counter}. + *

+ *

+ * You might also want to skip the {@link Counter} creation programmatically.
+ * One use-case can be having another component in your application that already processes the {@link Counted @Counted} annotation + * in some cases so that {@code CountedAspect} should not intercept these methods. + * By using the skip predicate (Predicate<ProceedingJoinPoint>) + * you can tell the {@code CountedAspect} when not to create a {@link Counter}. + * + * Here's a theoretic example to disable {@link Counter} creation for Spring controllers: + * + * 

+ * @Bean
+ * public CountedAspect countedAspect(MeterRegistry meterRegistry) {
+ *     return new CountedAspect(meterRegistry, this::skipControllers);
+ * }
+ *
+ * private boolean skipControllers(ProceedingJoinPoint pjp) {
+ *     Class targetClass = pjp.getTarget().getClass();
+ *     return targetClass.isAnnotationPresent(RestController.class) || targetClass.isAnnotationPresent(Controller.class);
+ * }
+ *
+ *

* * @author Ali Dehghani + * @author Jonatan Ivanov * @since 1.2.0 * @see Counted */ 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 index 9f476173e5..4b98be6748 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java +++ b/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java @@ -40,7 +40,6 @@ * AspectJ aspect for intercepting types or methods annotated with {@link Timed @Timed}.
* The aspect supports programmatic customizations through constructor-injectable custom logic. *

- * *

* You might want to add tags programmatically to the {@link Timer}.
* In this case, the tags provider function (Function<ProceedingJoinPoint, Iterable<Tag>>) can help.