GH-491: Fix NPE in the MetricsRetryListener when label is null
Fixes: https://github.com/spring-projects/spring-retry/issues/491 The Micrometer tag cannot be with `null` value. When `RetryCallback` does not provide a proper `getLabel()` implementation, the `MetricsRetryListener` fails with a `NullPointerException` * Fix `MetricsRetryListener.close()` to fallback to the `callback.getClass().getName()` if `callback.getLabel() == null` * Cover behavior in the new `RetryMetricsTests.labelFallbackToClassName()`
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2024 the original author or authors.
|
||||
* Copyright 2024-2025 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.
|
||||
@@ -19,6 +19,7 @@ package org.springframework.retry.support;
|
||||
import java.util.Collections;
|
||||
import java.util.IdentityHashMap;
|
||||
import java.util.Map;
|
||||
import java.util.Objects;
|
||||
import java.util.function.Function;
|
||||
|
||||
import io.micrometer.core.instrument.MeterRegistry;
|
||||
@@ -43,7 +44,8 @@ import org.springframework.util.Assert;
|
||||
* <p>
|
||||
* The registered {@value #TIMER_NAME} {@link Timer} has these tags by default:
|
||||
* <ul>
|
||||
* <li>{@code name} - {@link RetryCallback#getLabel()}</li>
|
||||
* <li>{@code name} - {@link RetryCallback#getLabel()} if not null, otherwise
|
||||
* {@code RetryCallback#getClass().getName()}</li>
|
||||
* <li>{@code retry.count} - the number of attempts - 1; essentially the successful first
|
||||
* call means no counts</li>
|
||||
* <li>{@code exception} - the thrown back to the caller (after all the retry attempts)
|
||||
@@ -113,7 +115,8 @@ public class MetricsRetryListener implements RetryListener {
|
||||
Assert.state(sample != null,
|
||||
() -> String.format("No 'Timer.Sample' registered for '%s'. Was the 'open()' called?", context));
|
||||
|
||||
Tags retryTags = Tags.of("name", callback.getLabel())
|
||||
String label = Objects.requireNonNullElse(callback.getLabel(), callback.getClass().getName());
|
||||
Tags retryTags = Tags.of("name", label)
|
||||
.and("retry.count", "" + context.getRetryCount())
|
||||
.and(this.customTags)
|
||||
.and(this.customTagsProvider.apply(context))
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2024 the original author or authors.
|
||||
* Copyright 2024-2025 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.
|
||||
@@ -26,9 +26,12 @@ import org.junit.jupiter.api.Test;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.retry.RetryCallback;
|
||||
import org.springframework.retry.RetryContext;
|
||||
import org.springframework.retry.RetryException;
|
||||
import org.springframework.retry.annotation.EnableRetry;
|
||||
import org.springframework.retry.annotation.Retryable;
|
||||
import org.springframework.retry.policy.SimpleRetryPolicy;
|
||||
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;
|
||||
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
|
||||
|
||||
@@ -49,6 +52,9 @@ public class RetryMetricsTests {
|
||||
@Autowired
|
||||
Service service;
|
||||
|
||||
@Autowired
|
||||
MetricsRetryListener metricsRetryListener;
|
||||
|
||||
@Test
|
||||
void metricsAreCollectedForRetryable() {
|
||||
ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
|
||||
@@ -87,6 +93,21 @@ public class RetryMetricsTests {
|
||||
executor.destroy();
|
||||
}
|
||||
|
||||
@Test
|
||||
void labelFallbackToClassName() {
|
||||
SimpleRetryPolicy simpleRetryPolicy = new SimpleRetryPolicy();
|
||||
RetryContext retryContext = simpleRetryPolicy.open(null);
|
||||
RetryCallback<Object, Throwable> retryCallback = context -> null;
|
||||
this.metricsRetryListener.open(retryContext, retryCallback);
|
||||
this.metricsRetryListener.close(retryContext, retryCallback, null);
|
||||
|
||||
assertThat(this.meterRegistry.get(MetricsRetryListener.TIMER_NAME)
|
||||
.tags(Tags.of("name", retryCallback.getClass().getName(), "retry.count", "0", "exception", "none"))
|
||||
.timer()
|
||||
.count()).isEqualTo(1);
|
||||
|
||||
}
|
||||
|
||||
@Configuration(proxyBeanMethods = false)
|
||||
@EnableRetry
|
||||
public static class TestConfiguration {
|
||||
|
||||
Reference in New Issue
Block a user