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

Return transaction for Sentry.getSpan when global hub mode is enabled #2855

Merged
merged 14 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Breaking changes:
- Reduce timeout of AsyncHttpTransport to avoid ANR ([#2879](https://github.com/getsentry/sentry-java/pull/2879))
- Add deadline timeout for automatic transactions ([#2865](https://github.com/getsentry/sentry-java/pull/2865))
- This affects all automatically generated transactions on Android (UI, clicks), the default timeout is 30s
- If global hub mode is enabled (default on Android), Sentry.getSpan() returns the root span instead of the latest span ([#2855](https://github.com/getsentry/sentry-java/pull/2855))

### Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
val method: String = request.method

// We start the call span that will contain all the others
callRootSpan = hub.span?.startChild("http.client", "$method $url")
callRootSpan = hub.transaction?.startChild("http.client", "$method $url")
callRootSpan?.spanContext?.origin = TRACE_ORIGIN
urlDetails.applyToSpan(callRootSpan)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class SentryOkHttpInterceptor(
isFromEventListener = true
} else {
// read the span from the bound scope
span = hub.span?.startChild("http.client", "$method $url")
span = hub.transaction?.startChild("http.client", "$method $url")
isFromEventListener = false
}

Expand Down
4 changes: 4 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ public final class io/sentry/Hub : io/sentry/IHub {
public fun getOptions ()Lio/sentry/SentryOptions;
public fun getSpan ()Lio/sentry/ISpan;
public fun getTraceparent ()Lio/sentry/SentryTraceHeader;
public fun getTransaction ()Lio/sentry/ITransaction;
public fun isCrashedLastRun ()Ljava/lang/Boolean;
public fun isEnabled ()Z
public fun popScope ()V
Expand Down Expand Up @@ -411,6 +412,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub {
public fun getOptions ()Lio/sentry/SentryOptions;
public fun getSpan ()Lio/sentry/ISpan;
public fun getTraceparent ()Lio/sentry/SentryTraceHeader;
public fun getTransaction ()Lio/sentry/ITransaction;
public fun isCrashedLastRun ()Ljava/lang/Boolean;
public fun isEnabled ()Z
public fun popScope ()V
Expand Down Expand Up @@ -483,6 +485,7 @@ public abstract interface class io/sentry/IHub {
public abstract fun getOptions ()Lio/sentry/SentryOptions;
public abstract fun getSpan ()Lio/sentry/ISpan;
public abstract fun getTraceparent ()Lio/sentry/SentryTraceHeader;
public abstract fun getTransaction ()Lio/sentry/ITransaction;
public abstract fun isCrashedLastRun ()Ljava/lang/Boolean;
public abstract fun isEnabled ()Z
public abstract fun popScope ()V
Expand Down Expand Up @@ -869,6 +872,7 @@ public final class io/sentry/NoOpHub : io/sentry/IHub {
public fun getOptions ()Lio/sentry/SentryOptions;
public fun getSpan ()Lio/sentry/ISpan;
public fun getTraceparent ()Lio/sentry/SentryTraceHeader;
public fun getTransaction ()Lio/sentry/ITransaction;
public fun isCrashedLastRun ()Ljava/lang/Boolean;
public fun isEnabled ()Z
public fun popScope ()V
Expand Down
16 changes: 16 additions & 0 deletions sentry/src/main/java/io/sentry/Hub.java
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,22 @@ public void flush(long timeoutMillis) {
return span;
}

@Override
@ApiStatus.Internal
public @Nullable ITransaction getTransaction() {
ITransaction span = null;
if (!isEnabled()) {
options
.getLogger()
.log(
SentryLevel.WARNING,
"Instance is disabled and this 'getTransaction' call is a no-op.");
} else {
span = stack.peek().getScope().getTransaction();
}
return span;
}

@Override
@ApiStatus.Internal
public void setSpanContext(
Expand Down
6 changes: 6 additions & 0 deletions sentry/src/main/java/io/sentry/HubAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ public void setSpanContext(
return Sentry.getCurrentHub().getSpan();
}

@Override
@ApiStatus.Internal
public @Nullable ITransaction getTransaction() {
return Sentry.getCurrentHub().getTransaction();
}

@Override
public @NotNull SentryOptions getOptions() {
return Sentry.getCurrentHub().getOptions();
Expand Down
9 changes: 9 additions & 0 deletions sentry/src/main/java/io/sentry/IHub.java
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,15 @@ void setSpanContext(
@Nullable
ISpan getSpan();

/**
* Returns the transaction.
*
* @return the transaction or null when no active transaction is running.
*/
@ApiStatus.Internal
@Nullable
ITransaction getTransaction();

/**
* Gets the {@link SentryOptions} attached to current scope.
*
Expand Down
5 changes: 5 additions & 0 deletions sentry/src/main/java/io/sentry/NoOpHub.java
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@ public void setSpanContext(
return null;
}

@Override
public @Nullable ITransaction getTransaction() {
return null;
}

@Override
public @NotNull SentryOptions getOptions() {
return emptyOptions;
Expand Down
10 changes: 8 additions & 2 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -918,10 +918,16 @@ public static void endSession() {
/**
* Gets the current active transaction or span.
*
* @return the active span or null when no active transaction is running
* @return the active span or null when no active transaction is running. In case of
* globalHubMode=true, always the active transaction is returned, rather than the last active
* span.
*/
public static @Nullable ISpan getSpan() {
return getCurrentHub().getSpan();
if (globalHubMode) {
return getCurrentHub().getTransaction();
} else {
return getCurrentHub().getSpan();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ final class FileIOSpanManager {
private final @NotNull SentryStackTraceFactory stackTraceFactory;

static @Nullable ISpan startSpan(final @NotNull IHub hub, final @NotNull String op) {
final ISpan parent = hub.getSpan();
final ISpan parent = hub.getTransaction();
return parent != null ? parent.startChild(op) : null;
}

Expand Down
5 changes: 5 additions & 0 deletions sentry/src/test/java/io/sentry/HubAdapterTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,11 @@ class HubAdapterTest {
verify(hub).span
}

@Test fun `getTransaction calls Hub`() {
HubAdapter.getInstance().transaction
verify(hub).transaction
}

@Test fun `getOptions calls Hub`() {
HubAdapter.getInstance().options
verify(hub).options
Expand Down
27 changes: 23 additions & 4 deletions sentry/src/test/java/io/sentry/HubTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1607,15 +1607,32 @@ class HubTest {
@Test
fun `when there is no active transaction, getSpan returns null`() {
val hub = generateHub()
assertNull(hub.getSpan())
assertNull(hub.span)
}

@Test
fun `when there is active transaction bound to the scope, getSpan returns active transaction`() {
fun `when there is no active transaction, getTransaction returns null`() {
val hub = generateHub()
assertNull(hub.transaction)
}

@Test
fun `when there is active transaction bound to the scope, getTransaction and getSpan return active transaction`() {
val hub = generateHub()
val tx = hub.startTransaction("aTransaction", "op")
hub.configureScope { it.setTransaction(tx) }
assertEquals(tx, hub.getSpan())
hub.configureScope { it.transaction = tx }

assertEquals(tx, hub.transaction)
assertEquals(tx, hub.span)
}

@Test
fun `when there is a transaction but the hub is closed, getTransaction returns null`() {
val hub = generateHub()
hub.startTransaction("name", "op")
hub.close()

assertNull(hub.transaction)
}

@Test
Expand All @@ -1625,6 +1642,8 @@ class HubTest {
hub.configureScope { it.setTransaction(tx) }
hub.configureScope { it.setTransaction(tx) }
val span = tx.startChild("op")

assertEquals(tx, hub.transaction)
assertEquals(span, hub.span)
}
// endregion
Expand Down
41 changes: 41 additions & 0 deletions sentry/src/test/java/io/sentry/SentryTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,47 @@ class SentryTest {
assertFalse(previousSessionFile.exists())
}

@Test
fun `getSpan calls hub getSpan`() {
val hub = mock<IHub>()
Sentry.init({
it.dsn = dsn
}, false)
Sentry.setCurrentHub(hub)
Sentry.getSpan()
verify(hub).span
}

@Test
fun `getSpan calls returns root span if globalhub mode is enabled`() {
Sentry.init({
it.dsn = dsn
it.enableTracing = true
it.sampleRate = 1.0
}, true)

val transaction = Sentry.startTransaction("name", "op-root", true)
transaction.startChild("op-child")

val span = Sentry.getSpan()!!
assertEquals("op-root", span.operation)
}

@Test
fun `getSpan calls returns child span if globalhub mode is disabled`() {
Sentry.init({
it.dsn = dsn
it.enableTracing = true
it.sampleRate = 1.0
}, false)

val transaction = Sentry.startTransaction("name", "op-root", true)
transaction.startChild("op-child")

val span = Sentry.getSpan()!!
assertEquals("op-child", span.operation)
}

private class InMemoryOptionsObserver : IOptionsObserver {
var release: String? = null
private set
Expand Down