RequestHandlerRetryAdvice fails if provided RetryTemplate is instrumented with MetricsRetryListener

[ikarsokolov]

In what version(s) of Spring Integration are you seeing this issue?
6.4.2

Describe the bug
The RequestHandlerRetryAdvice class in spring-integration uses a RetryTemplate to implement retry logic. This template can be instrumented with a MetricsRetryListener (provided by the spring-retry project) to collect Micrometer metrics. However, when this listener is added, RequestHandlerRetryAdvice fails with a NullPointerException on every received message.

The issue stems from how MetricsRetryListener uses the RetryCallback.getLabel() method to populate the name tag of the timer metric:

https://github.com/spring-projects/spring-retry/blob/544b24e19507571a6ce5e01c41a089ed4f801d9e/src/main/java/org/springframework/retry/support/MetricsRetryListener.java#L116-L120

Unfortunately, the RetryCallback implementation used internally by RequestHandlerRetryAdvice returns null for this method.

To Reproduce

1. Create a RetryTemplate instance.
2. Instrument it with Micrometer using the MetricsRetryListener.
3. Create a RequestHandlerRetryAdvice instance and configure it with the retry template.
4. Attach the advice to an endpoint.

The advice will fail on every message with a NullPointerException.

Expected behavior

The retry advice should function normally, even when MetricsRetryListener is used. Ideally, the name tag in the retry metric should default to the endpoint's id, or alternatively, be configurable via a setter on RequestHandlerRetryAdvice.

As a more advanced solution, RequestHandlerRetryAdvice could attach its own RetryListener to instrument retry operations, consistent with how other observations are collected within the spring-integration project.

[artembilan]

Unfortunately, there is no simple workaround with existing classes unless you can provide your own RetryListener implementing metrics with a custom name or without it at all.

[ikarsokolov]

Unfortunately, there is no simple workaround with existing classes unless you can provide your own RetryListener implementing metrics with a custom name or without it at all.

Yeah, in our project we worked around this problem by providing default retry callback label. The intention was to make FallbackNameTagMetricsRetryListener forward compatible with possible future changes in MetricsRetryListener or internal retry callback inside RequestHandlerRetryAdvice.

public class FallbackNameTagMetricsRetryListener extends MetricsRetryListener {
    private final String defaultNameTagValue;

    public FallbackNameTagMetricsRetryListener(final MeterRegistry meterRegistry,
                                               final String defaultNameTagValue) {
        super(meterRegistry);
        checkArgument(defaultNameTagValue != null && !defaultNameTagValue.isBlank(),
                      "defaultNameTagValue could not be null or blank");
        this.defaultNameTagValue = defaultNameTagValue;
    }

    @Override
    public <T, E extends Throwable> void close(final RetryContext context,
                                               final RetryCallback<T, E> callback,
                                               @Nullable final Throwable throwable) {
        final var callbackWithLabel = new DefaultLabelRetryCallbackDecorator<>(callback, defaultNameTagValue);
        super.close(context, callbackWithLabel, throwable);
    }

    private static class DefaultLabelRetryCallbackDecorator<T, E extends Throwable> implements RetryCallback<T, E> {
        private final RetryCallback<T, E> decoratedRetryCallback;
        private final String defaultLabel;

        public DefaultLabelRetryCallbackDecorator(final RetryCallback<T, E> decoratedRetryCallback, final String defaultLabel) {
            this.decoratedRetryCallback = requireNonNull(decoratedRetryCallback,
                                                         "decoratedRetryCallback could not be null");
            this.defaultLabel = defaultLabel;
        }

        @Override
        public T doWithRetry(final RetryContext context) throws E {
            return decoratedRetryCallback.doWithRetry(context);
        }

        @Override
        public String getLabel() {
            final String originalLabel = decoratedRetryCallback.getLabel();
            return originalLabel != null ? originalLabel : defaultLabel;
        }

        @Override
        public String toString() {
            return "DefaultLabelRetryCallbackDecorator{" +
                   "defaultLabel='" + defaultLabel + '\'' +
                   '}';
        }
    }
}