Skip to content

Commit 93be5c8

Browse files
committed
fix(jcache): Make replace and getAndReplace passthrough (no span)
Like putIfAbsent, these are conditional writes that may be no-ops. Emitting a cache.put span for them would be misleading.
1 parent 4b6759d commit 93be5c8

File tree

2 files changed

+17
-58
lines changed

2 files changed

+17
-58
lines changed

sentry-jcache/src/main/java/io/sentry/jcache/SentryJCacheWrapper.java

Lines changed: 6 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -154,61 +154,22 @@ public boolean putIfAbsent(final K key, final V value) {
154154
return delegate.putIfAbsent(key, value);
155155
}
156156

157+
// replace and getAndReplace are not instrumented — like putIfAbsent, they are conditional
158+
// writes (only happen if the key exists / value matches). Emitting a cache.put span for a
159+
// potential no-op would be misleading.
157160
@Override
158161
public boolean replace(final K key, final V oldValue, final V newValue) {
159-
final ISpan span = startSpan("cache.put", key);
160-
if (span == null) {
161-
return delegate.replace(key, oldValue, newValue);
162-
}
163-
try {
164-
final boolean result = delegate.replace(key, oldValue, newValue);
165-
span.setStatus(SpanStatus.OK);
166-
return result;
167-
} catch (Throwable e) {
168-
span.setStatus(SpanStatus.INTERNAL_ERROR);
169-
span.setThrowable(e);
170-
throw e;
171-
} finally {
172-
span.finish();
173-
}
162+
return delegate.replace(key, oldValue, newValue);
174163
}
175164

176165
@Override
177166
public boolean replace(final K key, final V value) {
178-
final ISpan span = startSpan("cache.put", key);
179-
if (span == null) {
180-
return delegate.replace(key, value);
181-
}
182-
try {
183-
final boolean result = delegate.replace(key, value);
184-
span.setStatus(SpanStatus.OK);
185-
return result;
186-
} catch (Throwable e) {
187-
span.setStatus(SpanStatus.INTERNAL_ERROR);
188-
span.setThrowable(e);
189-
throw e;
190-
} finally {
191-
span.finish();
192-
}
167+
return delegate.replace(key, value);
193168
}
194169

195170
@Override
196171
public V getAndReplace(final K key, final V value) {
197-
final ISpan span = startSpan("cache.put", key);
198-
if (span == null) {
199-
return delegate.getAndReplace(key, value);
200-
}
201-
try {
202-
final V result = delegate.getAndReplace(key, value);
203-
span.setStatus(SpanStatus.OK);
204-
return result;
205-
} catch (Throwable e) {
206-
span.setStatus(SpanStatus.INTERNAL_ERROR);
207-
span.setThrowable(e);
208-
throw e;
209-
} finally {
210-
span.finish();
211-
}
172+
return delegate.getAndReplace(key, value);
212173
}
213174

214175
// -- remove operations --

sentry-jcache/src/test/kotlin/io/sentry/jcache/SentryJCacheWrapperTest.kt

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -178,49 +178,47 @@ class SentryJCacheWrapperTest {
178178
assertEquals(0, tx.spans.size)
179179
}
180180

181-
// -- replace(K, V, V) --
181+
// -- replace (passthrough, no span — conditional write) --
182182

183183
@Test
184-
fun `replace with old value creates cache put span`() {
184+
fun `replace with old value delegates without creating span`() {
185185
val tx = createTransaction()
186186
val wrapper = SentryJCacheWrapper(delegate, scopes)
187187
whenever(delegate.replace("myKey", "old", "new")).thenReturn(true)
188188

189189
val result = wrapper.replace("myKey", "old", "new")
190190

191191
assertTrue(result)
192-
assertEquals(1, tx.spans.size)
193-
assertEquals("cache.put", tx.spans.first().operation)
192+
verify(delegate).replace("myKey", "old", "new")
193+
assertEquals(0, tx.spans.size)
194194
}
195195

196-
// -- replace(K, V) --
197-
198196
@Test
199-
fun `replace creates cache put span`() {
197+
fun `replace delegates without creating span`() {
200198
val tx = createTransaction()
201199
val wrapper = SentryJCacheWrapper(delegate, scopes)
202200
whenever(delegate.replace("myKey", "value")).thenReturn(true)
203201

204202
val result = wrapper.replace("myKey", "value")
205203

206204
assertTrue(result)
207-
assertEquals(1, tx.spans.size)
208-
assertEquals("cache.put", tx.spans.first().operation)
205+
verify(delegate).replace("myKey", "value")
206+
assertEquals(0, tx.spans.size)
209207
}
210208

211-
// -- getAndReplace --
209+
// -- getAndReplace (passthrough, no span — conditional write) --
212210

213211
@Test
214-
fun `getAndReplace creates cache put span`() {
212+
fun `getAndReplace delegates without creating span`() {
215213
val tx = createTransaction()
216214
val wrapper = SentryJCacheWrapper(delegate, scopes)
217215
whenever(delegate.getAndReplace("myKey", "newValue")).thenReturn("oldValue")
218216

219217
val result = wrapper.getAndReplace("myKey", "newValue")
220218

221219
assertEquals("oldValue", result)
222-
assertEquals(1, tx.spans.size)
223-
assertEquals("cache.put", tx.spans.first().operation)
220+
verify(delegate).getAndReplace("myKey", "newValue")
221+
assertEquals(0, tx.spans.size)
224222
}
225223

226224
// -- remove(K) --

0 commit comments

Comments
 (0)