Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a skip predicate to the TimedAspect and CountedAspect #2551

Merged
merged 5 commits into from
Apr 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,18 +30,50 @@
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}
* annotation and record a few counter metrics about their execution status.
* <p>
* Aspect responsible for intercepting all methods annotated with the {@link Counted @Counted}
* annotation and record a few counter metrics about their execution status.<br />
* The aspect supports programmatic customizations through constructor-injectable custom logic.
* </p>
* <p>
* You might want to add tags programmatically to the {@link Counter}.<br />
* In this case, the tags provider function (<code>Function&lt;ProceedingJoinPoint, Iterable&lt;Tag&gt;&gt;</code>) can help.
* It receives a {@link ProceedingJoinPoint} and returns the {@link Tag}s that will be attached to the {@link Counter}.
* </p>
* <p>
* You might also want to skip the {@link Counter} creation programmatically.<br />
* 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 (<code>Predicate&lt;ProceedingJoinPoint&gt;</code>)
* 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:
*
* <pre>
* &#064;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);
* }
* </pre>
* </p>
*
* @author Ali Dehghani
* @author Jonatan Ivanov
* @since 1.2.0
* @see Counted
*/
@Aspect
@NonNullApi
public class CountedAspect {
private static final Predicate<ProceedingJoinPoint> 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";
Expand All @@ -66,26 +99,63 @@ public class CountedAspect {
private final Function<ProceedingJoinPoint, Iterable<Tag>> 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<ProceedingJoinPoint> 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<ProceedingJoinPoint, Iterable<Tag>> 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<ProceedingJoinPoint> 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<ProceedingJoinPoint, Iterable<Tag>> tagsBasedOnJoinPoint, Predicate<ProceedingJoinPoint> shouldSkip) {
this.meterRegistry = meterRegistry;
this.tagsBasedOnJoinPoint = tagsBasedOnJoinPoint;
this.shouldSkip = shouldSkip;
}

/**
Expand All @@ -106,6 +176,9 @@ public CountedAspect(MeterRegistry meterRegistry, Function<ProceedingJoinPoint,
*/
@Around("@annotation(counted)")
public Object interceptAndRecord(ProceedingJoinPoint pjp, Counted counted) throws Throwable {
if (shouldSkip.test(pjp)) {
return pjp.proceed();
}

final Method method = ((MethodSignature) pjp.getSignature()).getMethod();
final boolean stopWhenCompleted = CompletionStage.class.isAssignableFrom(method.getReturnType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,52 @@
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}.
* <p>
* AspectJ aspect for intercepting types or methods annotated with {@link Timed @Timed}.<br />
* The aspect supports programmatic customizations through constructor-injectable custom logic.
* </p>
* <p>
* You might want to add tags programmatically to the {@link Timer}.<br />
* In this case, the tags provider function (<code>Function&lt;ProceedingJoinPoint, Iterable&lt;Tag&gt;&gt;</code>) can help.
* It receives a {@link ProceedingJoinPoint} and returns the {@link Tag}s that will be attached to the {@link Timer}.
* </p>
* <p>
* You might also want to skip the {@link Timer} creation programmatically.<br />
* 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 (<code>Predicate&lt;ProceedingJoinPoint&gt;</code>)
* 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:
*
* <pre>
* &#064;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);
* }
* </pre>
* </p>
*
* @author David J. M. Karlsen
* @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<ProceedingJoinPoint> DONT_SKIP_ANYTHING = pjp -> false;
public static final String DEFAULT_METRIC_NAME = "method.timed";
public static final String DEFAULT_EXCEPTION_TAG_VALUE = "none";

Expand All @@ -59,30 +91,70 @@ public class TimedAspect {

private final MeterRegistry registry;
private final Function<ProceedingJoinPoint, Iterable<Tag>> tagsBasedOnJoinPoint;
private final Predicate<ProceedingJoinPoint> shouldSkip;

/**
* Create a {@code TimedAspect} instance with {@link Metrics#globalRegistry}.
* Creates a {@code TimedAspect} instance with {@link Metrics#globalRegistry}.
*
* @since 1.2.0
*/
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<ProceedingJoinPoint, Iterable<Tag>> 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<ProceedingJoinPoint> 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<ProceedingJoinPoint, Iterable<Tag>> tagsBasedOnJoinPoint, Predicate<ProceedingJoinPoint> 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.search.MeterNotFoundException;
import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
import org.aspectj.lang.ProceedingJoinPoint;
import org.junit.jupiter.api.Test;
import org.springframework.aop.aspectj.annotation.AspectJProxyFactory;

import java.util.concurrent.CompletableFuture;
import java.util.function.Predicate;

import static java.util.concurrent.CompletableFuture.supplyAsync;
import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -62,6 +64,18 @@ void countedWithSuccessfulMetrics() {
assertThat(counter.getId().getDescription()).isNull();
}

@Test
void countedWithSkipPredicate() {
CountedService countedService = getAdvisedService(
new CountedService(),
new CountedAspect(meterRegistry, (Predicate<ProceedingJoinPoint>) proceedingJoinPoint -> true)
);

countedService.succeedWithMetrics();

assertThat(meterRegistry.find("metric.success").counter()).isNull();
}

@Test
void countedWithFailure() {
try {
Expand Down Expand Up @@ -204,8 +218,12 @@ void emptyMetricNameWithException() {
}

private <T> T getAdvisedService(T countedService) {
return getAdvisedService(countedService, new CountedAspect(meterRegistry));
}

private <T> T getAdvisedService(T countedService, CountedAspect countedAspect) {
AspectJProxyFactory proxyFactory = new AspectJProxyFactory(countedService);
proxyFactory.addAspect(new CountedAspect(meterRegistry));
proxyFactory.addAspect(countedAspect);
return proxyFactory.getProxy();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand All @@ -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<ProceedingJoinPoint>) pjp -> true));

TimedService service = pf.getProxy();

service.call();

assertThat(registry.find("call").timer()).isNull();
}

@Test
void timeMethodWithLongTaskTimer() {
MeterRegistry registry = new SimpleMeterRegistry();
Expand Down