MetricsRetryListener problems

[VladimirLyubimov]

1. In javadoc (add docs) registered tag exception described as "the thrown back to the caller (after all the retry attempts) exception class name". But it is not exactly right due to RetryListener calls on any exception (retryable or not) and class-name of this exception will be put into corresponding label. So exception tag with value distincted from "none" can not guarantee that retryable method has failed after all retry attempts
2. Is sample.stop(this.retryMeterProvider.tags(retryTags).register(this.meterRegistry)); in MetricsRetryListener is thread-safe? Suppose threre are two threads T1 and T2, which are executing close on same instance of MetricsRetryListener. If T1 and T2 executes this.retryMeterProvider.tags(retryTags) and only after that register(this.meterRegistry) the results of T1 (who has called retryMeterProvider.tags first) will be overrided by T2

[artembilan]

I agree about second argument.
We just use the same instance of the Timer.Builder. So, yeah, if close() called concurrently we may end up with a race condition.
We have to use a dedicated Timer.Builder instance for each close() call.

I'm not sure though, abut your first argument.
The close() is called from the finally block of the try..catch around retry loop in the RetryTemplate.doExecute().
And the logic there is like this:

try {
...
			while (canRetry(retryPolicy, context) && !context.isExhaustedOnly()) {

				try {
	...
					lastException = null;
					T result = retryCallback.doWithRetry(context);
					doOnSuccessInterceptors(retryCallback, context, result);
					return result;
				}
				catch (Throwable e) {

					lastException = e;
                                 ...
                               }
                       }
       ...
		catch (Throwable e) {
			throw RetryTemplate.<E>wrapIfNecessary(e);
		}
		finally {
			close(retryPolicy, context, state, lastException == null || exhausted);
			doCloseInterceptors(retryCallback, context, lastException);
			RetrySynchronizationManager.clear();
		}

So, if T result = retryCallback.doWithRetry(context); is successful, the lastException is null.
Therefore close() on the listener would set none.

What do I miss?

[VladimirLyubimov]

Thank you for your response and clarification about my first argument (where I was wrong)!