fix(cache): Fix span description, putIfAbsent, and Callable hit detection

- Use cache key as span description instead of cache name, matching
  the spec and other SDKs (Python, JavaScript)
- Skip instrumentation for putIfAbsent since we cannot know if a write
  actually occurred; override to bypass default get()+put() delegation
- Wrap valueLoader Callable in get(key, Callable) to detect cache
  hit/miss instead of always reporting hit=true
- Update tests to match new behavior

Co-Authored-By: Claude <noreply@anthropic.com>

Author: adinauer

sentry-spring-7/src/main/java/io/sentry/spring7/cache/SentryCacheWrapper.java

@@ -7,6 +7,7 @@
 import io.sentry.SpanStatus;
 import java.util.Arrays;
 import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicBoolean;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -83,9 +84,15 @@ public SentryCacheWrapper(final @NotNull Cache delegate, final @NotNull IScopes
       return delegate.get(key, valueLoader);
     }
     try {
-      final T result = delegate.get(key, valueLoader);
-      // valueLoader is called on miss, so the method always returns a value
-      span.setData(SpanDataConvention.CACHE_HIT_KEY, true);
+      final AtomicBoolean loaderInvoked = new AtomicBoolean(false);
+      final T result =
+          delegate.get(
+              key,
+              () -> {
+                loaderInvoked.set(true);
+                return valueLoader.call();
+              });
+      span.setData(SpanDataConvention.CACHE_HIT_KEY, !loaderInvoked.get());
       span.setStatus(SpanStatus.OK);
       return result;
     } catch (Throwable e) {
@@ -116,24 +123,14 @@ public void put(final @NotNull Object key, final @Nullable Object value) {
     }
   }
 
+  // putIfAbsent is not instrumented — we cannot know ahead of time whether the put
+  // will actually happen, and emitting a cache.put span for a no-op would be misleading.
+  // This matches sentry-python and sentry-javascript which also skip conditional puts.
+  // We must override to bypass the default implementation which calls this.get() + this.put().
   @Override
   public @Nullable ValueWrapper putIfAbsent(
       final @NotNull Object key, final @Nullable Object value) {
-    final ISpan span = startSpan("cache.put", key);
-    if (span == null) {
-      return delegate.putIfAbsent(key, value);
-    }
-    try {
-      final ValueWrapper result = delegate.putIfAbsent(key, value);
-      span.setStatus(SpanStatus.OK);
-      return result;
-    } catch (Throwable e) {
-      span.setStatus(SpanStatus.INTERNAL_ERROR);
-      span.setThrowable(e);
-      throw e;
-    } finally {
-      span.finish();
-    }
+    return delegate.putIfAbsent(key, value);
   }
 
   @Override
@@ -220,9 +217,10 @@ public boolean invalidate() {
 
     final SpanOptions spanOptions = new SpanOptions();
     spanOptions.setOrigin(TRACE_ORIGIN);
-    final ISpan span = activeSpan.startChild(operation, getName(), spanOptions);
-    if (key != null) {
-      span.setData(SpanDataConvention.CACHE_KEY_KEY, Arrays.asList(String.valueOf(key)));
+    final String keyString = key != null ? String.valueOf(key) : null;
+    final ISpan span = activeSpan.startChild(operation, keyString, spanOptions);
+    if (keyString != null) {
+      span.setData(SpanDataConvention.CACHE_KEY_KEY, Arrays.asList(keyString));
     }
     return span;
   }

sentry-spring-7/src/test/kotlin/io/sentry/spring7/cache/SentryCacheWrapperTest.kt

@@ -13,6 +13,8 @@ import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
+import org.mockito.kotlin.any
+import org.mockito.kotlin.eq
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
@@ -52,7 +54,7 @@ class SentryCacheWrapperTest {
     assertEquals(1, tx.spans.size)
     val span = tx.spans.first()
     assertEquals("cache.get", span.operation)
-    assertEquals("testCache", span.description)
+    assertEquals("myKey", span.description)
     assertEquals(SpanStatus.OK, span.status)
     assertEquals(true, span.getData(SpanDataConvention.CACHE_HIT_KEY))
     assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY_KEY))
@@ -103,19 +105,36 @@ class SentryCacheWrapperTest {
   // -- get(Object key, Callable<T>) --
 
   @Test
-  fun `get with callable creates span with cache hit true`() {
+  fun `get with callable creates span with cache hit true on hit`() {
     val tx = createTransaction()
     val wrapper = SentryCacheWrapper(delegate, scopes)
-    val callable = Callable { "loaded" }
-    whenever(delegate.get("myKey", callable)).thenReturn("loaded")
+    // Simulate cache hit: delegate returns value without invoking the loader
+    whenever(delegate.get(eq("myKey"), any<Callable<String>>())).thenReturn("cached")
 
-    val result = wrapper.get("myKey", callable)
+    val result = wrapper.get("myKey", Callable { "loaded" })
 
-    assertEquals("loaded", result)
+    assertEquals("cached", result)
     assertEquals(1, tx.spans.size)
     assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
   }
 
+  @Test
+  fun `get with callable creates span with cache hit false on miss`() {
+    val tx = createTransaction()
+    val wrapper = SentryCacheWrapper(delegate, scopes)
+    // Simulate cache miss: delegate invokes the loader callable
+    whenever(delegate.get(eq("myKey"), any<Callable<String>>())).thenAnswer { invocation ->
+      val loader = invocation.getArgument<Callable<String>>(1)
+      loader.call()
+    }
+
+    val result = wrapper.get("myKey", Callable { "loaded" })
+
+    assertEquals("loaded", result)
+    assertEquals(1, tx.spans.size)
+    assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
+  }
+
   // -- put --
 
   @Test
@@ -136,15 +155,15 @@ class SentryCacheWrapperTest {
   // -- putIfAbsent --
 
   @Test
-  fun `putIfAbsent creates cache put span`() {
+  fun `putIfAbsent delegates without creating span`() {
     val tx = createTransaction()
     val wrapper = SentryCacheWrapper(delegate, scopes)
     whenever(delegate.putIfAbsent("myKey", "myValue")).thenReturn(null)
 
     wrapper.putIfAbsent("myKey", "myValue")
 
-    assertEquals(1, tx.spans.size)
-    assertEquals("cache.put", tx.spans.first().operation)
+    verify(delegate).putIfAbsent("myKey", "myValue")
+    assertEquals(0, tx.spans.size)
   }
 
   // -- evict --