Skip to content

feat(spring): [Cache Tracing 12] Add db.operation.name attribute to cache spans #5201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
adinauer wants to merge 1 commit into
base: fix/cache-tracing-noop-span
Choose a base branch
from
+102 −46
Draft

feat(spring): [Cache Tracing 12] Add db.operation.name attribute to cache spans #5201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 23 additions & 15 deletions sentry-jcache/src/main/java/io/sentry/jcache/SentryJCacheWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
public final class SentryJCacheWrapper<K, V> implements Cache<K, V> {

private static final String TRACE_ORIGIN = "auto.cache.jcache";
private static final String OPERATION_ATTRIBUTE = "db.operation.name";

private final @NotNull Cache<K, V> delegate;
private final @NotNull IScopes scopes;
Expand All @@ -50,7 +51,7 @@ public SentryJCacheWrapper(final @NotNull Cache<K, V> delegate, final @NotNull I

@Override
public V get(final K key) {
final ISpan span = startSpan("cache.get", key);
final ISpan span = startSpan("cache.get", key, "get");
if (span == null) {
return delegate.get(key);
}
Expand All @@ -70,7 +71,7 @@ public V get(final K key) {

@Override
public Map<K, V> getAll(final Set<? extends K> keys) {
final ISpan span = startSpanForKeys("cache.get", keys);
final ISpan span = startSpanForKeys("cache.get", keys, "getAll");
if (span == null) {
return delegate.getAll(keys);
}
Expand All @@ -97,7 +98,7 @@ public boolean containsKey(final K key) {

@Override
public void put(final K key, final V value) {
final ISpan span = startSpan("cache.put", key);
final ISpan span = startSpan("cache.put", key, "put");
if (span == null) {
delegate.put(key, value);
return;
Expand All @@ -116,7 +117,7 @@ public void put(final K key, final V value) {

@Override
public V getAndPut(final K key, final V value) {
final ISpan span = startSpan("cache.put", key);
final ISpan span = startSpan("cache.put", key, "getAndPut");
if (span == null) {
return delegate.getAndPut(key, value);
}
Expand All @@ -135,7 +136,7 @@ public V getAndPut(final K key, final V value) {

@Override
public void putAll(final Map<? extends K, ? extends V> map) {
final ISpan span = startSpanForKeys("cache.put", map.keySet());
final ISpan span = startSpanForKeys("cache.put", map.keySet(), "putAll");
if (span == null) {
delegate.putAll(map);
return;
Expand Down Expand Up @@ -181,7 +182,7 @@ public V getAndReplace(final K key, final V value) {

@Override
public boolean remove(final K key) {
final ISpan span = startSpan("cache.remove", key);
final ISpan span = startSpan("cache.remove", key, "remove");
if (span == null) {
return delegate.remove(key);
}
Expand All @@ -200,7 +201,7 @@ public boolean remove(final K key) {

@Override
public boolean remove(final K key, final V oldValue) {
final ISpan span = startSpan("cache.remove", key);
final ISpan span = startSpan("cache.remove", key, "remove");
if (span == null) {
return delegate.remove(key, oldValue);
}
Expand All @@ -219,7 +220,7 @@ public boolean remove(final K key, final V oldValue) {

@Override
public V getAndRemove(final K key) {
final ISpan span = startSpan("cache.remove", key);
final ISpan span = startSpan("cache.remove", key, "getAndRemove");
if (span == null) {
return delegate.getAndRemove(key);
}
Expand All @@ -238,7 +239,7 @@ public V getAndRemove(final K key) {

@Override
public void removeAll(final Set<? extends K> keys) {
final ISpan span = startSpanForKeys("cache.remove", keys);
final ISpan span = startSpanForKeys("cache.remove", keys, "removeAll");
if (span == null) {
delegate.removeAll(keys);
return;
Expand All @@ -257,7 +258,7 @@ public void removeAll(final Set<? extends K> keys) {

@Override
public void removeAll() {
final ISpan span = startSpan("cache.flush", null);
final ISpan span = startSpan("cache.flush", null, "removeAll");
if (span == null) {
delegate.removeAll();
return;
Expand All @@ -278,7 +279,7 @@ public void removeAll() {

@Override
public void clear() {
final ISpan span = startSpan("cache.flush", null);
final ISpan span = startSpan("cache.flush", null, "clear");
if (span == null) {
delegate.clear();
return;
Expand Down Expand Up @@ -306,7 +307,7 @@ public void close() {
public <T> T invoke(
final K key, final EntryProcessor<K, V, T> entryProcessor, final Object... arguments)
throws EntryProcessorException {
final ISpan span = startSpan("cache.get", key);
final ISpan span = startSpan("cache.get", key, "invoke");
if (span == null) {
return delegate.invoke(key, entryProcessor, arguments);
}
Expand All @@ -328,7 +329,7 @@ public <T> Map<K, EntryProcessorResult<T>> invokeAll(
final Set<? extends K> keys,
final EntryProcessor<K, V, T> entryProcessor,
final Object... arguments) {
final ISpan span = startSpanForKeys("cache.get", keys);
final ISpan span = startSpanForKeys("cache.get", keys, "invokeAll");
if (span == null) {
return delegate.invokeAll(keys, entryProcessor, arguments);
}
Expand Down Expand Up @@ -400,7 +401,10 @@ public Iterator<Entry<K, V>> iterator() {

// -- span helpers --

private @Nullable ISpan startSpan(final @NotNull String operation, final @Nullable Object key) {
private @Nullable ISpan startSpan(
final @NotNull String operation,
final @Nullable Object key,
final @NotNull String operationName) {
if (!scopes.getOptions().isEnableCacheTracing()) {
return null;
}
Expand All @@ -420,11 +424,14 @@ public Iterator<Entry<K, V>> iterator() {
if (keyString != null) {
span.setData(SpanDataConvention.CACHE_KEY_KEY, Arrays.asList(keyString));
}
span.setData(OPERATION_ATTRIBUTE, operationName);
return span;
}

private @Nullable ISpan startSpanForKeys(
final @NotNull String operation, final @NotNull Set<?> keys) {
final @NotNull String operation,
final @NotNull Set<?> keys,
final @NotNull String operationName) {
if (!scopes.getOptions().isEnableCacheTracing()) {
return null;
}
Expand All @@ -443,6 +450,7 @@ public Iterator<Entry<K, V>> iterator() {
span.setData(
SpanDataConvention.CACHE_KEY_KEY,
keys.stream().map(String::valueOf).collect(Collectors.toList()));
span.setData(OPERATION_ATTRIBUTE, operationName);
return span;
}
}
13 changes: 13 additions & 0 deletions sentry-jcache/src/test/kotlin/io/sentry/jcache/SentryJCacheWrapperTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class SentryJCacheWrapperTest {
assertEquals(true, span.getData(SpanDataConvention.CACHE_HIT_KEY))
assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY_KEY))
assertEquals("auto.cache.jcache", span.spanContext.origin)
assertEquals("get", span.getData("db.operation.name"))
}

@Test
Expand Down Expand Up @@ -98,6 +99,7 @@ class SentryJCacheWrapperTest {
assertEquals(true, span.getData(SpanDataConvention.CACHE_HIT_KEY))
val cacheKeys = span.getData(SpanDataConvention.CACHE_KEY_KEY) as List<*>
assertTrue(cacheKeys.containsAll(listOf("k1", "k2")))
assertEquals("getAll", span.getData("db.operation.name"))
}

@Test
Expand Down Expand Up @@ -127,6 +129,7 @@ class SentryJCacheWrapperTest {
assertEquals("cache.put", span.operation)
assertEquals(SpanStatus.OK, span.status)
assertEquals(listOf("myKey"), span.getData(SpanDataConvention.CACHE_KEY_KEY))
assertEquals("put", span.getData("db.operation.name"))
}

// -- getAndPut --
Expand All @@ -142,6 +145,7 @@ class SentryJCacheWrapperTest {
assertEquals("oldValue", result)
assertEquals(1, tx.spans.size)
assertEquals("cache.put", tx.spans.first().operation)
assertEquals("getAndPut", tx.spans.first().getData("db.operation.name"))
}

// -- putAll --
Expand All @@ -161,6 +165,7 @@ class SentryJCacheWrapperTest {
assertEquals("testCache", span.description)
val cacheKeys = span.getData(SpanDataConvention.CACHE_KEY_KEY) as List<*>
assertTrue(cacheKeys.containsAll(listOf("k1", "k2")))
assertEquals("putAll", span.getData("db.operation.name"))
}

// -- putIfAbsent --
Expand Down Expand Up @@ -236,6 +241,7 @@ class SentryJCacheWrapperTest {
val span = tx.spans.first()
assertEquals("cache.remove", span.operation)
assertEquals(SpanStatus.OK, span.status)
assertEquals("remove", span.getData("db.operation.name"))
}

// -- remove(K, V) --
Expand All @@ -251,6 +257,7 @@ class SentryJCacheWrapperTest {
assertTrue(result)
assertEquals(1, tx.spans.size)
assertEquals("cache.remove", tx.spans.first().operation)
assertEquals("remove", tx.spans.first().getData("db.operation.name"))
}

// -- getAndRemove --
Expand All @@ -266,6 +273,7 @@ class SentryJCacheWrapperTest {
assertEquals("value", result)
assertEquals(1, tx.spans.size)
assertEquals("cache.remove", tx.spans.first().operation)
assertEquals("getAndRemove", tx.spans.first().getData("db.operation.name"))
}

// -- removeAll(Set) --
Expand All @@ -283,6 +291,7 @@ class SentryJCacheWrapperTest {
val span = tx.spans.first()
assertEquals("cache.remove", span.operation)
assertEquals("testCache", span.description)
assertEquals("removeAll", span.getData("db.operation.name"))
}

// -- removeAll() --
Expand All @@ -297,6 +306,7 @@ class SentryJCacheWrapperTest {
verify(delegate).removeAll()
assertEquals(1, tx.spans.size)
assertEquals("cache.flush", tx.spans.first().operation)
assertEquals("removeAll", tx.spans.first().getData("db.operation.name"))
}

// -- clear --
Expand All @@ -314,6 +324,7 @@ class SentryJCacheWrapperTest {
assertEquals("cache.flush", span.operation)
assertEquals(SpanStatus.OK, span.status)
assertNull(span.getData(SpanDataConvention.CACHE_KEY_KEY))
assertEquals("clear", span.getData("db.operation.name"))
}

// -- invoke --
Expand All @@ -330,6 +341,7 @@ class SentryJCacheWrapperTest {
assertEquals("result", result)
assertEquals(1, tx.spans.size)
assertEquals("cache.get", tx.spans.first().operation)
assertEquals("invoke", tx.spans.first().getData("db.operation.name"))
}

// -- invokeAll --
Expand All @@ -348,6 +360,7 @@ class SentryJCacheWrapperTest {
assertEquals(resultMap, result)
assertEquals(1, tx.spans.size)
assertEquals("cache.get", tx.spans.first().operation)
assertEquals("invokeAll", tx.spans.first().getData("db.operation.name"))
}

// -- passthrough operations --
Expand Down
27 changes: 16 additions & 11 deletions sentry-spring-7/src/main/java/io/sentry/spring7/cache/SentryCacheWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
public final class SentryCacheWrapper implements Cache {

private static final String TRACE_ORIGIN = "auto.cache.spring";
private static final String OPERATION_ATTRIBUTE = "db.operation.name";

private final @NotNull Cache delegate;
private final @NotNull IScopes scopes;
Expand All @@ -41,7 +42,7 @@ public SentryCacheWrapper(final @NotNull Cache delegate, final @NotNull IScopes

@Override
public @Nullable ValueWrapper get(final @NotNull Object key) {
final ISpan span = startSpan("cache.get", key);
final ISpan span = startSpan("cache.get", key, "get");
if (span == null) {
return delegate.get(key);
}
Expand All @@ -61,7 +62,7 @@ public SentryCacheWrapper(final @NotNull Cache delegate, final @NotNull IScopes

@Override
public @Nullable <T> T get(final @NotNull Object key, final @Nullable Class<T> type) {
final ISpan span = startSpan("cache.get", key);
final ISpan span = startSpan("cache.get", key, "get");
if (span == null) {
return delegate.get(key, type);
}
Expand All @@ -81,7 +82,7 @@ public SentryCacheWrapper(final @NotNull Cache delegate, final @NotNull IScopes

@Override
public @Nullable <T> T get(final @NotNull Object key, final @NotNull Callable<T> valueLoader) {
final ISpan span = startSpan("cache.get", key);
final ISpan span = startSpan("cache.get", key, "get");
if (span == null) {
return delegate.get(key, valueLoader);
}
Expand All @@ -108,7 +109,7 @@ public SentryCacheWrapper(final @NotNull Cache delegate, final @NotNull IScopes

@Override
public @Nullable CompletableFuture<?> retrieve(final @NotNull Object key) {
final ISpan span = startSpan("cache.get", key);
final ISpan span = startSpan("cache.get", key, "retrieve");
if (span == null) {
return delegate.retrieve(key);
}
Expand Down Expand Up @@ -143,7 +144,7 @@ public SentryCacheWrapper(final @NotNull Cache delegate, final @NotNull IScopes
@Override
public <T> CompletableFuture<T> retrieve(
final @NotNull Object key, final @NotNull Supplier<CompletableFuture<T>> valueLoader) {
final ISpan span = startSpan("cache.get", key);
final ISpan span = startSpan("cache.get", key, "retrieve");
if (span == null) {
return delegate.retrieve(key, valueLoader);
}
Expand Down Expand Up @@ -178,7 +179,7 @@ public <T> CompletableFuture<T> retrieve(

@Override
public void put(final @NotNull Object key, final @Nullable Object value) {
final ISpan span = startSpan("cache.put", key);
final ISpan span = startSpan("cache.put", key, "put");
if (span == null) {
delegate.put(key, value);
return;
Expand Down Expand Up @@ -207,7 +208,7 @@ public void put(final @NotNull Object key, final @Nullable Object value) {

@Override
public void evict(final @NotNull Object key) {
final ISpan span = startSpan("cache.remove", key);
final ISpan span = startSpan("cache.remove", key, "evict");
if (span == null) {
delegate.evict(key);
return;
Expand All @@ -226,7 +227,7 @@ public void evict(final @NotNull Object key) {

@Override
public boolean evictIfPresent(final @NotNull Object key) {
final ISpan span = startSpan("cache.remove", key);
final ISpan span = startSpan("cache.remove", key, "evictIfPresent");
if (span == null) {
return delegate.evictIfPresent(key);
}
Expand All @@ -245,7 +246,7 @@ public boolean evictIfPresent(final @NotNull Object key) {

@Override
public void clear() {
final ISpan span = startSpan("cache.flush", null);
final ISpan span = startSpan("cache.flush", null, "clear");
if (span == null) {
delegate.clear();
return;
Expand All @@ -264,7 +265,7 @@ public void clear() {

@Override
public boolean invalidate() {
final ISpan span = startSpan("cache.flush", null);
final ISpan span = startSpan("cache.flush", null, "invalidate");
if (span == null) {
return delegate.invalidate();
}
Expand All @@ -281,7 +282,10 @@ public boolean invalidate() {
}
}

private @Nullable ISpan startSpan(final @NotNull String operation, final @Nullable Object key) {
private @Nullable ISpan startSpan(
final @NotNull String operation,
final @Nullable Object key,
final @NotNull String operationName) {
if (!scopes.getOptions().isEnableCacheTracing()) {
return null;
}
Expand All @@ -301,6 +305,7 @@ public boolean invalidate() {
if (keyString != null) {
span.setData(SpanDataConvention.CACHE_KEY_KEY, Arrays.asList(keyString));
}
span.setData(OPERATION_ATTRIBUTE, operationName);
return span;
}
}
Loading
Loading