From 47a23591ff8911e1dd5fe96a48c51403ae6dcdeb Mon Sep 17 00:00:00 2001
From: Artem Bilan <artem.bilan@broadcom.com>
Date: Thu, 8 May 2025 15:20:26 -0400
Subject: [PATCH] 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()`
---
 .../retry/support/MetricsRetryListener.java   |  9 +++++---
 .../retry/support/RetryMetricsTests.java      | 23 ++++++++++++++++++-
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/src/main/java/org/springframework/retry/support/MetricsRetryListener.java b/src/main/java/org/springframework/retry/support/MetricsRetryListener.java
index 92b9d0c..bf0a922 100644
--- a/src/main/java/org/springframework/retry/support/MetricsRetryListener.java
+++ b/src/main/java/org/springframework/retry/support/MetricsRetryListener.java
@@ -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))
diff --git a/src/test/java/org/springframework/retry/support/RetryMetricsTests.java b/src/test/java/org/springframework/retry/support/RetryMetricsTests.java
index 605697d..bb98353 100644
--- a/src/test/java/org/springframework/retry/support/RetryMetricsTests.java
+++ b/src/test/java/org/springframework/retry/support/RetryMetricsTests.java
@@ -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 {