Skip to content

Commit 3520857

Browse files
adinaueradinauer
authored
Merge pull request #5228 from getsentry/feat/cache-tracing-more-review-changes
test(cache): [Cache Tracing 23] Add missing cache key assertions and use exact equals
2 parents d7b3b0b + 638152a commit 3520857

File tree

8 files changed

+44
-18
lines changed

8 files changed

+44
-18
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import io.sentry.SpanDataConvention;
77
import io.sentry.SpanOptions;
88
import io.sentry.SpanStatus;
9-
import java.util.Arrays;
9+
import java.util.Collections;
1010
import java.util.Iterator;
1111
import java.util.List;
1212
import java.util.Map;
@@ -468,7 +468,8 @@ public Iterator<Entry<K, V>> iterator() {
468468
private @Nullable ISpan startSpan(
469469
final @Nullable Object key, final @NotNull String operationName) {
470470
final String keyString = key != null ? String.valueOf(key) : null;
471-
return startSpan(operationName, keyString, keyString != null ? Arrays.asList(keyString) : null);
471+
return startSpan(
472+
operationName, keyString, keyString != null ? Collections.singletonList(keyString) : null);
472473
}
473474

474475
private @Nullable ISpan startSpanForKeys(

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,9 @@ class SentryJCacheWrapperTest {
9696
assertEquals(1, tx.spans.size)
9797
val span = tx.spans.first()
9898
assertEquals("cache.getAll", span.operation)
99-
assertTrue(span.description!!.contains("k1"))
100-
assertTrue(span.description!!.contains("k2"))
99+
assertEquals("k1, k2", span.description)
101100
assertEquals(true, span.getData(SpanDataConvention.CACHE_HIT))
102-
val cacheKeys = span.getData(SpanDataConvention.CACHE_KEY) as List<*>
103-
assertTrue(cacheKeys.containsAll(listOf("k1", "k2")))
101+
assertEquals(listOf("k1", "k2"), span.getData(SpanDataConvention.CACHE_KEY))
104102
assertEquals("getAll", span.getData(SpanDataConvention.CACHE_OPERATION))
105103
}
106104

@@ -166,11 +164,9 @@ class SentryJCacheWrapperTest {
166164
assertEquals(1, tx.spans.size)
167165
val span = tx.spans.first()
168166
assertEquals("cache.putAll", span.operation)
169-
assertTrue(span.description!!.contains("k1"))
170-
assertTrue(span.description!!.contains("k2"))
167+
assertEquals("k1, k2", span.description)
171168
assertEquals(true, span.getData(SpanDataConvention.CACHE_WRITE))
172-
val cacheKeys = span.getData(SpanDataConvention.CACHE_KEY) as List<*>
173-
assertTrue(cacheKeys.containsAll(listOf("k1", "k2")))
169+
assertEquals(listOf("k1", "k2"), span.getData(SpanDataConvention.CACHE_KEY))
174170
assertEquals("putAll", span.getData(SpanDataConvention.CACHE_OPERATION))
175171
}
176172

@@ -320,9 +316,9 @@ class SentryJCacheWrapperTest {
320316
assertEquals(1, tx.spans.size)
321317
val span = tx.spans.first()
322318
assertEquals("cache.removeAll", span.operation)
323-
assertTrue(span.description!!.contains("k1"))
324-
assertTrue(span.description!!.contains("k2"))
319+
assertEquals("k1, k2", span.description)
325320
assertEquals(true, span.getData(SpanDataConvention.CACHE_WRITE))
321+
assertEquals(listOf("k1", "k2"), span.getData(SpanDataConvention.CACHE_KEY))
326322
assertEquals("removeAll", span.getData(SpanDataConvention.CACHE_OPERATION))
327323
}
328324

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import io.sentry.SpanDataConvention;
66
import io.sentry.SpanOptions;
77
import io.sentry.SpanStatus;
8-
import java.util.Arrays;
8+
import java.util.Collections;
99
import java.util.concurrent.Callable;
1010
import java.util.concurrent.CompletableFuture;
1111
import java.util.concurrent.atomic.AtomicBoolean;
@@ -318,7 +318,7 @@ public boolean invalidate() {
318318
return null;
319319
}
320320
if (keyString != null) {
321-
span.setData(SpanDataConvention.CACHE_KEY, Arrays.asList(keyString));
321+
span.setData(SpanDataConvention.CACHE_KEY, Collections.singletonList(keyString));
322322
}
323323
span.setData(SpanDataConvention.CACHE_OPERATION, operationName);
324324
return span;

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ class SentryCacheWrapperTest {
7878
assertNull(result)
7979
assertEquals(1, tx.spans.size)
8080
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
81+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
8182
}
8283

8384
// -- get(Object key, Class<T>) --
@@ -93,6 +94,7 @@ class SentryCacheWrapperTest {
9394
assertEquals("value", result)
9495
assertEquals(1, tx.spans.size)
9596
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
97+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
9698
}
9799

98100
@Test
@@ -106,6 +108,7 @@ class SentryCacheWrapperTest {
106108
assertNull(result)
107109
assertEquals(1, tx.spans.size)
108110
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
111+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
109112
}
110113

111114
@Test
@@ -138,6 +141,7 @@ class SentryCacheWrapperTest {
138141
assertEquals(1, tx.spans.size)
139142
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
140143
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
144+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
141145
}
142146

143147
@Test
@@ -156,6 +160,7 @@ class SentryCacheWrapperTest {
156160
assertEquals(1, tx.spans.size)
157161
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
158162
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
163+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
159164
}
160165

161166
// -- retrieve(Object key) --
@@ -191,6 +196,7 @@ class SentryCacheWrapperTest {
191196
assertNull(result!!.get())
192197
assertEquals(1, tx.spans.size)
193198
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
199+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
194200
assertTrue(tx.spans.first().isFinished)
195201
}
196202

@@ -206,6 +212,7 @@ class SentryCacheWrapperTest {
206212
assertEquals(1, tx.spans.size)
207213
val span = tx.spans.first()
208214
assertEquals(false, span.getData(SpanDataConvention.CACHE_HIT))
215+
assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY))
209216
assertEquals(SpanStatus.OK, span.status)
210217
assertTrue(span.isFinished)
211218
}
@@ -273,6 +280,7 @@ class SentryCacheWrapperTest {
273280
assertEquals(1, tx.spans.size)
274281
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
275282
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
283+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
276284
assertTrue(tx.spans.first().isFinished)
277285
}
278286

@@ -293,6 +301,7 @@ class SentryCacheWrapperTest {
293301
assertEquals(1, tx.spans.size)
294302
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
295303
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
304+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
296305
assertTrue(tx.spans.first().isFinished)
297306
}
298307

@@ -384,6 +393,7 @@ class SentryCacheWrapperTest {
384393
assertEquals(SpanStatus.OK, span.status)
385394
assertEquals(true, span.getData(SpanDataConvention.CACHE_WRITE))
386395
assertEquals("evict", span.getData(SpanDataConvention.CACHE_OPERATION))
396+
assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY))
387397
}
388398

389399
// -- evictIfPresent --
@@ -401,6 +411,7 @@ class SentryCacheWrapperTest {
401411
assertEquals("cache.evictIfPresent", tx.spans.first().operation)
402412
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
403413
assertEquals("evictIfPresent", tx.spans.first().getData(SpanDataConvention.CACHE_OPERATION))
414+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
404415
}
405416

406417
// -- clear --

‎sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/cache/SentryCacheWrapper.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import io.sentry.SpanDataConvention;
66
import io.sentry.SpanOptions;
77
import io.sentry.SpanStatus;
8-
import java.util.Arrays;
8+
import java.util.Collections;
99
import java.util.concurrent.Callable;
1010
import java.util.concurrent.CompletableFuture;
1111
import java.util.concurrent.atomic.AtomicBoolean;
@@ -318,7 +318,7 @@ public boolean invalidate() {
318318
return null;
319319
}
320320
if (keyString != null) {
321-
span.setData(SpanDataConvention.CACHE_KEY, Arrays.asList(keyString));
321+
span.setData(SpanDataConvention.CACHE_KEY, Collections.singletonList(keyString));
322322
}
323323
span.setData(SpanDataConvention.CACHE_OPERATION, operationName);
324324
return span;

‎sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/cache/SentryCacheWrapperTest.kt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ class SentryCacheWrapperTest {
7878
assertNull(result)
7979
assertEquals(1, tx.spans.size)
8080
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
81+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
8182
}
8283

8384
// -- get(Object key, Class<T>) --
@@ -93,6 +94,7 @@ class SentryCacheWrapperTest {
9394
assertEquals("value", result)
9495
assertEquals(1, tx.spans.size)
9596
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
97+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
9698
}
9799

98100
@Test
@@ -106,6 +108,7 @@ class SentryCacheWrapperTest {
106108
assertNull(result)
107109
assertEquals(1, tx.spans.size)
108110
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
111+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
109112
}
110113

111114
@Test
@@ -138,6 +141,7 @@ class SentryCacheWrapperTest {
138141
assertEquals(1, tx.spans.size)
139142
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
140143
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
144+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
141145
}
142146

143147
@Test
@@ -156,6 +160,7 @@ class SentryCacheWrapperTest {
156160
assertEquals(1, tx.spans.size)
157161
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
158162
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
163+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
159164
}
160165

161166
// -- retrieve(Object key) --
@@ -191,6 +196,7 @@ class SentryCacheWrapperTest {
191196
assertNull(result!!.get())
192197
assertEquals(1, tx.spans.size)
193198
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
199+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
194200
assertTrue(tx.spans.first().isFinished)
195201
}
196202

@@ -206,6 +212,7 @@ class SentryCacheWrapperTest {
206212
assertEquals(1, tx.spans.size)
207213
val span = tx.spans.first()
208214
assertEquals(false, span.getData(SpanDataConvention.CACHE_HIT))
215+
assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY))
209216
assertEquals(SpanStatus.OK, span.status)
210217
assertTrue(span.isFinished)
211218
}
@@ -273,6 +280,7 @@ class SentryCacheWrapperTest {
273280
assertEquals(1, tx.spans.size)
274281
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
275282
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
283+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
276284
assertTrue(tx.spans.first().isFinished)
277285
}
278286

@@ -293,6 +301,7 @@ class SentryCacheWrapperTest {
293301
assertEquals(1, tx.spans.size)
294302
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
295303
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
304+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
296305
assertTrue(tx.spans.first().isFinished)
297306
}
298307

@@ -384,6 +393,7 @@ class SentryCacheWrapperTest {
384393
assertEquals(SpanStatus.OK, span.status)
385394
assertEquals(true, span.getData(SpanDataConvention.CACHE_WRITE))
386395
assertEquals("evict", span.getData(SpanDataConvention.CACHE_OPERATION))
396+
assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY))
387397
}
388398

389399
// -- evictIfPresent --
@@ -401,6 +411,7 @@ class SentryCacheWrapperTest {
401411
assertEquals("cache.evictIfPresent", tx.spans.first().operation)
402412
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
403413
assertEquals("evictIfPresent", tx.spans.first().getData(SpanDataConvention.CACHE_OPERATION))
414+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
404415
}
405416

406417
// -- clear --

‎sentry-spring/src/main/java/io/sentry/spring/cache/SentryCacheWrapper.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import io.sentry.SpanDataConvention;
66
import io.sentry.SpanOptions;
77
import io.sentry.SpanStatus;
8-
import java.util.Arrays;
8+
import java.util.Collections;
99
import java.util.concurrent.Callable;
1010
import java.util.concurrent.atomic.AtomicBoolean;
1111
import org.jetbrains.annotations.ApiStatus;
@@ -245,7 +245,7 @@ public boolean invalidate() {
245245
return null;
246246
}
247247
if (keyString != null) {
248-
span.setData(SpanDataConvention.CACHE_KEY, Arrays.asList(keyString));
248+
span.setData(SpanDataConvention.CACHE_KEY, Collections.singletonList(keyString));
249249
}
250250
span.setData(SpanDataConvention.CACHE_OPERATION, operationName);
251251
return span;

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ class SentryCacheWrapperTest {
7676
assertNull(result)
7777
assertEquals(1, tx.spans.size)
7878
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
79+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
7980
}
8081

8182
// -- get(Object key, Class<T>) --
@@ -91,6 +92,7 @@ class SentryCacheWrapperTest {
9192
assertEquals("value", result)
9293
assertEquals(1, tx.spans.size)
9394
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
95+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
9496
}
9597

9698
@Test
@@ -104,6 +106,7 @@ class SentryCacheWrapperTest {
104106
assertNull(result)
105107
assertEquals(1, tx.spans.size)
106108
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
109+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
107110
}
108111

109112
@Test
@@ -136,6 +139,7 @@ class SentryCacheWrapperTest {
136139
assertEquals(1, tx.spans.size)
137140
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
138141
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
142+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
139143
}
140144

141145
@Test
@@ -154,6 +158,7 @@ class SentryCacheWrapperTest {
154158
assertEquals(1, tx.spans.size)
155159
assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT))
156160
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
161+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
157162
}
158163

159164
// -- put --
@@ -212,6 +217,7 @@ class SentryCacheWrapperTest {
212217
assertEquals(SpanStatus.OK, span.status)
213218
assertEquals(true, span.getData(SpanDataConvention.CACHE_WRITE))
214219
assertEquals("evict", span.getData(SpanDataConvention.CACHE_OPERATION))
220+
assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY))
215221
}
216222

217223
// -- evictIfPresent --
@@ -229,6 +235,7 @@ class SentryCacheWrapperTest {
229235
assertEquals("cache.evictIfPresent", tx.spans.first().operation)
230236
assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_WRITE))
231237
assertEquals("evictIfPresent", tx.spans.first().getData(SpanDataConvention.CACHE_OPERATION))
238+
assertEquals(listOf("myKey"), tx.spans.first().getData(SpanDataConvention.CACHE_KEY))
232239
}
233240

234241
// -- clear --

0 commit comments

Comments
 (0)