Skip to content
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.

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

Fix: associate event with transaction when thrown exception is not a direct cause #1463

Merged
merged 6 commits into from
May 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
# Unreleased

* Fix: associate event with transaction when thrown exception is not a direct cause (#1463)

# 5.0.0-beta.2

* Fix: sentry-android-timber package sets sentry.java.android.timber as SDK name (#1456)
* Fix: When AppLifecycleIntegration is closed, it should remove observer using UI thread (#1459)
* Bump: AGP to 4.2.0 (#1460)
* Fix: NPE when adding Context Data with null values for log4j2 (#1465)


Breaking Changes:

* Remove: Settings.Secure.ANDROID_ID in favor of generated installationId (#1455)
Expand Down
5 changes: 5 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -1869,6 +1869,11 @@ public abstract interface class io/sentry/util/CollectionUtils$Predicate {
public abstract fun test (Ljava/lang/Object;)Z
}

public final class io/sentry/util/ExceptionUtils {
public fun <init> ()V
public static fun findRootCause (Ljava/lang/Throwable;)Ljava/lang/Throwable;
}

public final class io/sentry/util/LogUtils {
public fun <init> ()V
public static fun logIfNotFlushable (Lio/sentry/ILogger;Ljava/lang/Object;)V
Expand Down
13 changes: 9 additions & 4 deletions sentry/src/main/java/io/sentry/Hub.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.sentry.protocol.SentryId;
import io.sentry.protocol.SentryTransaction;
import io.sentry.protocol.User;
import io.sentry.util.ExceptionUtils;
import io.sentry.util.Objects;
import io.sentry.util.Pair;
import java.io.Closeable;
Expand Down Expand Up @@ -175,7 +176,8 @@ public SentryId captureEnvelope(

private void assignTraceContext(final @NotNull SentryEvent event) {
if (event.getThrowable() != null) {
final Pair<ISpan, String> pair = throwableToSpan.get(event.getThrowable());
final Pair<ISpan, String> pair =
throwableToSpan.get(ExceptionUtils.findRootCause(event.getThrowable()));
if (pair != null) {
if (event.getContexts().getTrace() == null) {
event.getContexts().setTrace(pair.getFirst().getSpanContext());
Expand Down Expand Up @@ -643,16 +645,19 @@ public void setSpanContext(
Objects.requireNonNull(throwable, "throwable is required");
Objects.requireNonNull(span, "span is required");
Objects.requireNonNull(transactionName, "transactionName is required");
// to match any cause, span context is always attached to the root cause of the exception
final Throwable rootCause = ExceptionUtils.findRootCause(throwable);
// the most inner span should be assigned to a throwable
if (!throwableToSpan.containsKey(throwable)) {
throwableToSpan.put(throwable, new Pair<>(span, transactionName));
if (!throwableToSpan.containsKey(rootCause)) {
throwableToSpan.put(rootCause, new Pair<>(span, transactionName));
}
}

@Nullable
SpanContext getSpanContext(final @NotNull Throwable throwable) {
Objects.requireNonNull(throwable, "throwable is required");
final Pair<ISpan, String> span = this.throwableToSpan.get(throwable);
final Throwable rootCause = ExceptionUtils.findRootCause(throwable);
final Pair<ISpan, String> span = this.throwableToSpan.get(rootCause);
if (span != null) {
return span.getFirst().getSpanContext();
}
Expand Down
23 changes: 23 additions & 0 deletions sentry/src/main/java/io/sentry/util/ExceptionUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package io.sentry.util;

import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;

@ApiStatus.Internal
public final class ExceptionUtils {

/**
* Returns exception root cause or the exception itself if there are no causes
*
* @param throwable - the throwable
* @return the root cause
*/
public static @NotNull Throwable findRootCause(final @NotNull Throwable throwable) {
Objects.requireNonNull(throwable, "throwable cannot be null");
Throwable rootCause = throwable;
while (rootCause.getCause() != null && rootCause.getCause() != rootCause) {
rootCause = rootCause.getCause();
}
return rootCause;
}
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
}
33 changes: 33 additions & 0 deletions sentry/src/test/java/io/sentry/HubTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,39 @@ class HubTest {
verify(mockClient).captureEvent(eq(event), any(), eq(hint))
}

@Test
fun `when captureEvent is called and event has exception which root cause has been previously attached with span context, sets span context to the event`() {
maciejwalkowiak marked this conversation as resolved.
Show resolved Hide resolved
val (sut, mockClient) = getEnabledHub()
val rootCause = RuntimeException()
val span = mock<Span>()
whenever(span.spanContext).thenReturn(SpanContext("op"))
sut.setSpanContext(rootCause, span, "tx-name")

val event = SentryEvent(RuntimeException(rootCause))

val hint = { }
sut.captureEvent(event, hint)
assertEquals(span.spanContext, event.contexts.trace)
verify(mockClient).captureEvent(eq(event), any(), eq(hint))
}

@Test
fun `when captureEvent is called and event has exception which non-root cause has been previously attached with span context, sets span context to the event`() {
val (sut, mockClient) = getEnabledHub()
val rootCause = RuntimeException()
val exceptionAssignedToSpan = RuntimeException(rootCause)
val span = mock<Span>()
whenever(span.spanContext).thenReturn(SpanContext("op"))
sut.setSpanContext(exceptionAssignedToSpan, span, "tx-name")

val event = SentryEvent(RuntimeException(exceptionAssignedToSpan))

val hint = { }
sut.captureEvent(event, hint)
assertEquals(span.spanContext, event.contexts.trace)
verify(mockClient).captureEvent(eq(event), any(), eq(hint))
}

@Test
fun `when captureEvent is called and event has exception which has been previously attached with span context and trace context already set, does not set new span context to the event`() {
val (sut, mockClient) = getEnabledHub()
Expand Down
22 changes: 22 additions & 0 deletions sentry/src/test/java/io/sentry/util/ExceptionUtilsTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package io.sentry.util

import java.lang.RuntimeException
import kotlin.test.Test
import kotlin.test.assertEquals

class ExceptionUtilsTest {

@Test
fun `returns same exception when there is no cause`() {
val ex = RuntimeException()
assertEquals(ex, ExceptionUtils.findRootCause(ex))
}

@Test
fun `returns first cause when there are multiple causes`() {
val rootCause = RuntimeException()
val cause = RuntimeException(rootCause)
val ex = RuntimeException(cause)
assertEquals(rootCause, ExceptionUtils.findRootCause(ex))
}
}