Skip to content

Commit

Permalink
GH-422: Log label in RetryTemplate
Browse files Browse the repository at this point in the history
Fixes: #422

* Push `MethodInvocationRetryCallback.getLabel()` to the `default` method in the `RetryCallback`
* Use `ClassUtils.getQualifiedMethodName(invocation.getMethod())` for default label in the `MethodInvocationRetryCallback`
if not provided from the outside
* Refactor some logic in the `RetryOperationsInterceptor` to cover `context.removeAttribute("__proxy__")`
for use-case when `recoverer` is not provided.
Remove extra label resolution since it is already done in the `MethodInvocationRetryCallback`
* Fix Javadoc typos in the affected classes
  • Loading branch information
artembilan committed Jan 11, 2024
1 parent 6656151 commit e19f5d7
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 63 deletions.
11 changes: 11 additions & 0 deletions src/main/java/org/springframework/retry/RetryCallback.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
* @param <E> the type of exception it declares may be thrown
* @author Rob Harrop
* @author Dave Syer
* @author Artem Bilan
*/
public interface RetryCallback<T, E extends Throwable> {

Expand All @@ -37,4 +38,14 @@ public interface RetryCallback<T, E extends Throwable> {
*/
T doWithRetry(RetryContext context) throws E;

/**
* A logical identifier for this callback to distinguish retries around business
* operations.
* @return the identifier for this callback.
* @since 2.0.6
*/
default String getLabel() {
return null;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2006-2019 the original author or authors.
* Copyright 2006-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,14 +17,17 @@
package org.springframework.retry.interceptor;

import org.aopalliance.intercept.MethodInvocation;

import org.springframework.lang.Nullable;
import org.springframework.retry.RetryCallback;
import org.springframework.retry.RetryOperations;
import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils;

/**
* Callback class for a Spring AOP reflective `MethodInvocation` that can be retried using
* a {@link RetryOperations}.
*
* <p>
* In a concrete {@link org.springframework.retry.RetryListener} implementation, the
* `MethodInvocation` can be analysed for providing insights on the method called as well
* as its parameter values which could then be used for monitoring purposes.
Expand All @@ -35,6 +38,7 @@
* @see RetryOperationsInterceptor
* @see org.springframework.retry.listener.MethodInvocationRetryListenerSupport
* @author Marius Grama
* @author Artem Bilan
* @since 1.3
*/
public abstract class MethodInvocationRetryCallback<T, E extends Throwable> implements RetryCallback<T, E> {
Expand All @@ -48,22 +52,23 @@ public abstract class MethodInvocationRetryCallback<T, E extends Throwable> impl
* @param invocation the method invocation
* @param label a unique label for statistics reporting.
*/
public MethodInvocationRetryCallback(MethodInvocation invocation, String label) {
public MethodInvocationRetryCallback(MethodInvocation invocation, @Nullable String label) {
this.invocation = invocation;
if (StringUtils.hasText(label)) {
this.label = label;
}
else {
this.label = invocation.getMethod().toGenericString();
this.label = ClassUtils.getQualifiedMethodName(invocation.getMethod());
}
}

public MethodInvocation getInvocation() {
return invocation;
return this.invocation;
}

@Override
public String getLabel() {
return label;
return this.label;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2006-2022 the original author or authors.
* Copyright 2006-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,12 +16,11 @@

package org.springframework.retry.interceptor;

import java.util.Arrays;

import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;

import org.springframework.aop.ProxyMethodInvocation;
import org.springframework.lang.Nullable;
import org.springframework.retry.RecoveryCallback;
import org.springframework.retry.RetryCallback;
import org.springframework.retry.RetryContext;
Expand All @@ -35,9 +34,9 @@
/**
* A {@link MethodInterceptor} that can be used to automatically retry calls to a method
* on a service if it fails. The injected {@link RetryOperations} is used to control the
* number of retries. By default it will retry a fixed number of times, according to the
* number of retries. By default, it will retry a fixed number of times, according to the
* defaults in {@link RetryTemplate}.
*
* <p>
* Hint about transaction boundaries. If you want to retry a failed transaction you need
* to make sure that the transaction boundary is inside the retry, otherwise the
* successful attempt will roll back with the whole transaction. If the method being
Expand All @@ -47,11 +46,13 @@
*
* @author Rob Harrop
* @author Dave Syer
* @author Artem Bilan
*/
public class RetryOperationsInterceptor implements MethodInterceptor {

private RetryOperations retryOperations = new RetryTemplate();

@Nullable
private MethodInvocationRecoverer<?> recoverer;

private String label;
Expand All @@ -71,18 +72,7 @@ public void setRecoverer(MethodInvocationRecoverer<?> recoverer) {

@Override
public Object invoke(final MethodInvocation invocation) throws Throwable {

String name;
if (StringUtils.hasText(this.label)) {
name = this.label;
}
else {
name = invocation.getMethod().toGenericString();
}
final String label = name;

RetryCallback<Object, Throwable> retryCallback = new MethodInvocationRetryCallback<Object, Throwable>(
invocation, label) {
RetryCallback<Object, Throwable> retryCallback = new MethodInvocationRetryCallback<>(invocation, this.label) {

@Override
public Object doWithRetry(RetryContext context) throws Exception {
Expand Down Expand Up @@ -117,42 +107,21 @@ public Object doWithRetry(RetryContext context) throws Exception {

};

if (this.recoverer != null) {
ItemRecovererCallback recoveryCallback = new ItemRecovererCallback(invocation.getArguments(),
this.recoverer);
try {
Object recovered = this.retryOperations.execute(retryCallback, recoveryCallback);
return recovered;
}
finally {
RetryContext context = RetrySynchronizationManager.getContext();
if (context != null) {
context.removeAttribute("__proxy__");
}
RecoveryCallback<Object> recoveryCallback = (this.recoverer != null)
? new ItemRecovererCallback(invocation.getArguments(), this.recoverer) : null;
try {
return this.retryOperations.execute(retryCallback, recoveryCallback);
}
finally {
RetryContext context = RetrySynchronizationManager.getContext();
if (context != null) {
context.removeAttribute("__proxy__");
}
}

return this.retryOperations.execute(retryCallback);

}

/**
* @author Dave Syer
*
*/
private static final class ItemRecovererCallback implements RecoveryCallback<Object> {

private final Object[] args;

private final MethodInvocationRecoverer<?> recoverer;

/**
* @param args the item that failed.
*/
private ItemRecovererCallback(Object[] args, MethodInvocationRecoverer<?> recoverer) {
this.args = Arrays.asList(args).toArray();
this.recoverer = recoverer;
}
private record ItemRecovererCallback(Object[] args,
MethodInvocationRecoverer<?> recoverer) implements RecoveryCallback<Object> {

@Override
public Object recover(RetryContext context) {
Expand Down
20 changes: 12 additions & 8 deletions src/main/java/org/springframework/retry/support/RetryTemplate.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2006-2022 the original author or authors.
* Copyright 2006-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -317,6 +317,9 @@ protected <T, E extends Throwable> T doExecute(RetryCallback<T, E> retryCallback
}
}

Object label = retryCallback.getLabel();
String labelMessage = (label != null) ? "; for: '" + label + "'" : "";

/*
* We allow the whole loop to be skipped if the policy or context already
* forbid the first try. This is used in the case of external retry to allow a
Expand All @@ -327,7 +330,7 @@ protected <T, E extends Throwable> T doExecute(RetryCallback<T, E> retryCallback

try {
if (this.logger.isDebugEnabled()) {
this.logger.debug("Retry: count=" + context.getRetryCount());
this.logger.debug("Retry: count=" + context.getRetryCount() + labelMessage);
}
// Reset the last exception, so if we are successful
// the close interceptors will not think we failed...
Expand Down Expand Up @@ -355,22 +358,23 @@ protected <T, E extends Throwable> T doExecute(RetryCallback<T, E> retryCallback
backOffPolicy.backOff(backOffContext);
}
catch (BackOffInterruptedException ex) {
lastException = e;
// back off was prevented by another thread - fail the retry
if (this.logger.isDebugEnabled()) {
this.logger.debug("Abort retry because interrupted: count=" + context.getRetryCount());
this.logger.debug("Abort retry because interrupted: count=" + context.getRetryCount()
+ labelMessage);
}
throw ex;
}
}

if (this.logger.isDebugEnabled()) {
this.logger.debug("Checking for rethrow: count=" + context.getRetryCount());
this.logger.debug("Checking for rethrow: count=" + context.getRetryCount() + labelMessage);
}

if (shouldRethrow(retryPolicy, context, state)) {
if (this.logger.isDebugEnabled()) {
this.logger.debug("Rethrow in retry for policy: count=" + context.getRetryCount());
this.logger
.debug("Rethrow in retry for policy: count=" + context.getRetryCount() + labelMessage);
}
throw RetryTemplate.<E>wrapIfNecessary(e);
}
Expand All @@ -388,7 +392,7 @@ protected <T, E extends Throwable> T doExecute(RetryCallback<T, E> retryCallback
}

if (state == null && this.logger.isDebugEnabled()) {
this.logger.debug("Retry failed last attempt: count=" + context.getRetryCount());
this.logger.debug("Retry failed last attempt: count=" + context.getRetryCount() + labelMessage);
}

exhausted = true;
Expand Down Expand Up @@ -525,7 +529,7 @@ private RetryContext doOpenInternal(RetryPolicy retryPolicy) {
/**
* Actions to take after final attempt has failed. If there is state clean up the
* cache. If there is a recovery callback, execute that and return its result.
* Otherwise throw an exception.
* Otherwise, throw an exception.
* @param recoveryCallback the callback for recovery (might be null)
* @param context the current retry context
* @param state the {@link RetryState}
Expand Down

0 comments on commit e19f5d7

Please sign in to comment.