From cf230cdc7b968cee329f2c038cea9c8a761a4a32 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 25 Mar 2026 06:15:26 +0100 Subject: [PATCH] fix(spring): [Cache Tracing 24] Track invalidate cache.write accurately Set cache.write based on delegate.invalidate() result instead of always true. This keeps span data aligned with Spring's invalidate semantics when no entries were present. Add tests in spring, spring-jakarta, and spring-7 wrappers to cover the false return path and assert cache.write is false. Co-Authored-By: Claude --- .../sentry/spring7/cache/SentryCacheWrapper.java | 2 +- .../spring7/cache/SentryCacheWrapperTest.kt | 16 ++++++++++++++++ .../spring/jakarta/cache/SentryCacheWrapper.java | 2 +- .../jakarta/cache/SentryCacheWrapperTest.kt | 16 ++++++++++++++++ .../sentry/spring/cache/SentryCacheWrapper.java | 2 +- .../spring/cache/SentryCacheWrapperTest.kt | 16 ++++++++++++++++ 6 files changed, 51 insertions(+), 3 deletions(-) diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/cache/SentryCacheWrapper.java b/sentry-spring-7/src/main/java/io/sentry/spring7/cache/SentryCacheWrapper.java index f53f7a3ce7..e5cb9ce87e 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/cache/SentryCacheWrapper.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/cache/SentryCacheWrapper.java @@ -287,7 +287,7 @@ public boolean invalidate() { } try { final boolean result = delegate.invalidate(); - span.setData(SpanDataConvention.CACHE_WRITE, true); + span.setData(SpanDataConvention.CACHE_WRITE, result); span.setStatus(SpanStatus.OK); return result; } catch (Throwable e) { diff --git a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/cache/SentryCacheWrapperTest.kt b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/cache/SentryCacheWrapperTest.kt index ced7f49103..ef056e7fdb 100644 --- a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/cache/SentryCacheWrapperTest.kt +++ b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/cache/SentryCacheWrapperTest.kt @@ -13,6 +13,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith +import kotlin.test.assertFalse import kotlin.test.assertNull import kotlin.test.assertTrue import org.mockito.kotlin.any @@ -450,6 +451,21 @@ class SentryCacheWrapperTest { assertEquals("invalidate", tx.spans.first().getData(SpanDataConvention.CACHE_OPERATION)) } + @Test + fun `invalidate sets cache write false when cache had no mappings`() { + val tx = createTransaction() + val wrapper = SentryCacheWrapper(delegate, scopes) + whenever(delegate.invalidate()).thenReturn(false) + + val result = wrapper.invalidate() + + assertFalse(result) + assertEquals(1, tx.spans.size) + assertEquals("cache.invalidate", tx.spans.first().operation) + assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE)) + assertEquals("invalidate", tx.spans.first().getData(SpanDataConvention.CACHE_OPERATION)) + } + // -- no span when no active transaction -- @Test diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/cache/SentryCacheWrapper.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/cache/SentryCacheWrapper.java index c09661b304..9b7c551a2d 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/cache/SentryCacheWrapper.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/cache/SentryCacheWrapper.java @@ -287,7 +287,7 @@ public boolean invalidate() { } try { final boolean result = delegate.invalidate(); - span.setData(SpanDataConvention.CACHE_WRITE, true); + span.setData(SpanDataConvention.CACHE_WRITE, result); span.setStatus(SpanStatus.OK); return result; } catch (Throwable e) { diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/cache/SentryCacheWrapperTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/cache/SentryCacheWrapperTest.kt index 7aa0791cf0..a04f548d44 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/cache/SentryCacheWrapperTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/cache/SentryCacheWrapperTest.kt @@ -13,6 +13,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith +import kotlin.test.assertFalse import kotlin.test.assertNull import kotlin.test.assertTrue import org.mockito.kotlin.any @@ -450,6 +451,21 @@ class SentryCacheWrapperTest { assertEquals("invalidate", tx.spans.first().getData(SpanDataConvention.CACHE_OPERATION)) } + @Test + fun `invalidate sets cache write false when cache had no mappings`() { + val tx = createTransaction() + val wrapper = SentryCacheWrapper(delegate, scopes) + whenever(delegate.invalidate()).thenReturn(false) + + val result = wrapper.invalidate() + + assertFalse(result) + assertEquals(1, tx.spans.size) + assertEquals("cache.invalidate", tx.spans.first().operation) + assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE)) + assertEquals("invalidate", tx.spans.first().getData(SpanDataConvention.CACHE_OPERATION)) + } + // -- no span when no active transaction -- @Test diff --git a/sentry-spring/src/main/java/io/sentry/spring/cache/SentryCacheWrapper.java b/sentry-spring/src/main/java/io/sentry/spring/cache/SentryCacheWrapper.java index 39608e35b2..0e0ccb7d22 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/cache/SentryCacheWrapper.java +++ b/sentry-spring/src/main/java/io/sentry/spring/cache/SentryCacheWrapper.java @@ -214,7 +214,7 @@ public boolean invalidate() { } try { final boolean result = delegate.invalidate(); - span.setData(SpanDataConvention.CACHE_WRITE, true); + span.setData(SpanDataConvention.CACHE_WRITE, result); span.setStatus(SpanStatus.OK); return result; } catch (Throwable e) { diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/cache/SentryCacheWrapperTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/cache/SentryCacheWrapperTest.kt index 9feb8fd963..ab21ef77b4 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/cache/SentryCacheWrapperTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/cache/SentryCacheWrapperTest.kt @@ -11,6 +11,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith +import kotlin.test.assertFalse import kotlin.test.assertNull import kotlin.test.assertTrue import org.mockito.kotlin.any @@ -274,6 +275,21 @@ class SentryCacheWrapperTest { assertEquals("invalidate", tx.spans.first().getData(SpanDataConvention.CACHE_OPERATION)) } + @Test + fun `invalidate sets cache write false when cache had no mappings`() { + val tx = createTransaction() + val wrapper = SentryCacheWrapper(delegate, scopes) + whenever(delegate.invalidate()).thenReturn(false) + + val result = wrapper.invalidate() + + assertFalse(result) + assertEquals(1, tx.spans.size) + assertEquals("cache.invalidate", tx.spans.first().operation) + assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE)) + assertEquals("invalidate", tx.spans.first().getData(SpanDataConvention.CACHE_OPERATION)) + } + // -- no span when no active transaction -- @Test