diff --git a/sentry-jcache/src/main/java/io/sentry/jcache/SentryJCacheWrapper.java b/sentry-jcache/src/main/java/io/sentry/jcache/SentryJCacheWrapper.java index 816e10bfb1..81f84f4d7d 100644 --- a/sentry-jcache/src/main/java/io/sentry/jcache/SentryJCacheWrapper.java +++ b/sentry-jcache/src/main/java/io/sentry/jcache/SentryJCacheWrapper.java @@ -153,29 +153,80 @@ public void putAll(final Map map) { } } - // 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. @Override public boolean putIfAbsent(final K key, final V value) { - return delegate.putIfAbsent(key, value); + final ISpan span = startSpan("cache.put", key, "putIfAbsent"); + if (span == null) { + return delegate.putIfAbsent(key, value); + } + try { + final boolean 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(); + } } - // replace and getAndReplace are not instrumented — like putIfAbsent, they are conditional - // writes (only happen if the key exists / value matches). Emitting a cache.put span for a - // potential no-op would be misleading. @Override public boolean replace(final K key, final V oldValue, final V newValue) { - return delegate.replace(key, oldValue, newValue); + final ISpan span = startSpan("cache.put", key, "replace"); + if (span == null) { + return delegate.replace(key, oldValue, newValue); + } + try { + final boolean result = delegate.replace(key, oldValue, newValue); + span.setStatus(SpanStatus.OK); + return result; + } catch (Throwable e) { + span.setStatus(SpanStatus.INTERNAL_ERROR); + span.setThrowable(e); + throw e; + } finally { + span.finish(); + } } @Override public boolean replace(final K key, final V value) { - return delegate.replace(key, value); + final ISpan span = startSpan("cache.put", key, "replace"); + if (span == null) { + return delegate.replace(key, value); + } + try { + final boolean result = delegate.replace(key, value); + span.setStatus(SpanStatus.OK); + return result; + } catch (Throwable e) { + span.setStatus(SpanStatus.INTERNAL_ERROR); + span.setThrowable(e); + throw e; + } finally { + span.finish(); + } } @Override public V getAndReplace(final K key, final V value) { - return delegate.getAndReplace(key, value); + final ISpan span = startSpan("cache.put", key, "getAndReplace"); + if (span == null) { + return delegate.getAndReplace(key, value); + } + try { + final V result = delegate.getAndReplace(key, value); + span.setStatus(SpanStatus.OK); + return result; + } catch (Throwable e) { + span.setStatus(SpanStatus.INTERNAL_ERROR); + span.setThrowable(e); + throw e; + } finally { + span.finish(); + } } // -- remove operations -- diff --git a/sentry-jcache/src/test/kotlin/io/sentry/jcache/SentryJCacheWrapperTest.kt b/sentry-jcache/src/test/kotlin/io/sentry/jcache/SentryJCacheWrapperTest.kt index b0572878da..a0d6548121 100644 --- a/sentry-jcache/src/test/kotlin/io/sentry/jcache/SentryJCacheWrapperTest.kt +++ b/sentry-jcache/src/test/kotlin/io/sentry/jcache/SentryJCacheWrapperTest.kt @@ -171,7 +171,7 @@ class SentryJCacheWrapperTest { // -- putIfAbsent -- @Test - fun `putIfAbsent delegates without creating span`() { + fun `putIfAbsent creates cache put span`() { val tx = createTransaction() val wrapper = SentryJCacheWrapper(delegate, scopes) whenever(delegate.putIfAbsent("myKey", "myValue")).thenReturn(true) @@ -180,13 +180,18 @@ class SentryJCacheWrapperTest { assertTrue(result) verify(delegate).putIfAbsent("myKey", "myValue") - assertEquals(0, tx.spans.size) + assertEquals(1, tx.spans.size) + val span = tx.spans.first() + assertEquals("cache.put", span.operation) + assertEquals(SpanStatus.OK, span.status) + assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY_KEY)) + assertEquals("putIfAbsent", span.getData("db.operation.name")) } - // -- replace (passthrough, no span — conditional write) -- + // -- replace -- @Test - fun `replace with old value delegates without creating span`() { + fun `replace with old value creates cache put span`() { val tx = createTransaction() val wrapper = SentryJCacheWrapper(delegate, scopes) whenever(delegate.replace("myKey", "old", "new")).thenReturn(true) @@ -195,11 +200,15 @@ class SentryJCacheWrapperTest { assertTrue(result) verify(delegate).replace("myKey", "old", "new") - assertEquals(0, tx.spans.size) + assertEquals(1, tx.spans.size) + val span = tx.spans.first() + assertEquals("cache.put", span.operation) + assertEquals(SpanStatus.OK, span.status) + assertEquals("replace", span.getData("db.operation.name")) } @Test - fun `replace delegates without creating span`() { + fun `replace creates cache put span`() { val tx = createTransaction() val wrapper = SentryJCacheWrapper(delegate, scopes) whenever(delegate.replace("myKey", "value")).thenReturn(true) @@ -208,13 +217,17 @@ class SentryJCacheWrapperTest { assertTrue(result) verify(delegate).replace("myKey", "value") - assertEquals(0, tx.spans.size) + assertEquals(1, tx.spans.size) + val span = tx.spans.first() + assertEquals("cache.put", span.operation) + assertEquals(SpanStatus.OK, span.status) + assertEquals("replace", span.getData("db.operation.name")) } - // -- getAndReplace (passthrough, no span — conditional write) -- + // -- getAndReplace -- @Test - fun `getAndReplace delegates without creating span`() { + fun `getAndReplace creates cache put span`() { val tx = createTransaction() val wrapper = SentryJCacheWrapper(delegate, scopes) whenever(delegate.getAndReplace("myKey", "newValue")).thenReturn("oldValue") @@ -223,7 +236,11 @@ class SentryJCacheWrapperTest { assertEquals("oldValue", result) verify(delegate).getAndReplace("myKey", "newValue") - assertEquals(0, tx.spans.size) + assertEquals(1, tx.spans.size) + val span = tx.spans.first() + assertEquals("cache.put", span.operation) + assertEquals(SpanStatus.OK, span.status) + assertEquals("getAndReplace", span.getData("db.operation.name")) } // -- remove(K) -- 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 1b8b091217..bd04285362 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 @@ -196,14 +196,24 @@ 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) { - return delegate.putIfAbsent(key, value); + final ISpan span = startSpan("cache.put", key, "putIfAbsent"); + 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(); + } } @Override 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 2a90f45516..71ce645b37 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 @@ -328,15 +328,21 @@ class SentryCacheWrapperTest { // -- putIfAbsent -- @Test - fun `putIfAbsent delegates without creating span`() { + fun `putIfAbsent creates cache put span`() { val tx = createTransaction() val wrapper = SentryCacheWrapper(delegate, scopes) whenever(delegate.putIfAbsent("myKey", "myValue")).thenReturn(null) - wrapper.putIfAbsent("myKey", "myValue") + val result = wrapper.putIfAbsent("myKey", "myValue") + assertNull(result) verify(delegate).putIfAbsent("myKey", "myValue") - assertEquals(0, tx.spans.size) + assertEquals(1, tx.spans.size) + val span = tx.spans.first() + assertEquals("cache.put", span.operation) + assertEquals(SpanStatus.OK, span.status) + assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY_KEY)) + assertEquals("putIfAbsent", span.getData("db.operation.name")) } // -- evict -- 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 f4f170cbfd..061669f678 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 @@ -196,14 +196,24 @@ 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) { - return delegate.putIfAbsent(key, value); + final ISpan span = startSpan("cache.put", key, "putIfAbsent"); + 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(); + } } @Override 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 090bae8390..bddde75969 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 @@ -328,15 +328,21 @@ class SentryCacheWrapperTest { // -- putIfAbsent -- @Test - fun `putIfAbsent delegates without creating span`() { + fun `putIfAbsent creates cache put span`() { val tx = createTransaction() val wrapper = SentryCacheWrapper(delegate, scopes) whenever(delegate.putIfAbsent("myKey", "myValue")).thenReturn(null) - wrapper.putIfAbsent("myKey", "myValue") + val result = wrapper.putIfAbsent("myKey", "myValue") + assertNull(result) verify(delegate).putIfAbsent("myKey", "myValue") - assertEquals(0, tx.spans.size) + assertEquals(1, tx.spans.size) + val span = tx.spans.first() + assertEquals("cache.put", span.operation) + assertEquals(SpanStatus.OK, span.status) + assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY_KEY)) + assertEquals("putIfAbsent", span.getData("db.operation.name")) } // -- evict -- 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 c765b1e143..dab20fd4d5 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 @@ -124,14 +124,24 @@ 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) { - return delegate.putIfAbsent(key, value); + final ISpan span = startSpan("cache.put", key, "putIfAbsent"); + 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(); + } } @Override 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 007f537411..163d0935fc 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 @@ -159,15 +159,21 @@ class SentryCacheWrapperTest { // -- putIfAbsent -- @Test - fun `putIfAbsent delegates without creating span`() { + fun `putIfAbsent creates cache put span`() { val tx = createTransaction() val wrapper = SentryCacheWrapper(delegate, scopes) whenever(delegate.putIfAbsent("myKey", "myValue")).thenReturn(null) - wrapper.putIfAbsent("myKey", "myValue") + val result = wrapper.putIfAbsent("myKey", "myValue") + assertNull(result) verify(delegate).putIfAbsent("myKey", "myValue") - assertEquals(0, tx.spans.size) + assertEquals(1, tx.spans.size) + val span = tx.spans.first() + assertEquals("cache.put", span.operation) + assertEquals(SpanStatus.OK, span.status) + assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY_KEY)) + assertEquals("putIfAbsent", span.getData("db.operation.name")) } // -- evict --