Skip to content

Commit

Permalink
Do not bind transactions to scope by default. (#1376)
Browse files Browse the repository at this point in the history
Fixes #1355
  • Loading branch information
maciejwalkowiak authored Apr 7, 2021
1 parent 6335c7a commit 142dd44
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 40 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Feat: Sentry closes Android NDK and ShutdownHook integrations (#1358)
* Enhancement: Allow inheritance of SentryHandler class in sentry-jul package(#1367)
* Fix: Accept only non null value maps (#1368)
* Fix: Do not bind transactions to scope by default. (#1376)

# 4.4.0-alpha.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ private void startTracing(final @NonNull Activity activity) {

// we can only bind to the scope if there's no running transaction
final ITransaction transaction =
hub.startTransaction(getActivityName(activity), "navigation", false);
hub.startTransaction(getActivityName(activity), "navigation");

// lets bind to the scope so other integrations can pick it up
hub.configureScope(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class ActivityLifecycleIntegrationTest {
val transaction = SentryTracer(TransactionContext("name", "op"), hub)

fun getSut(): ActivityLifecycleIntegration {
whenever(hub.startTransaction(any<String>(), any(), any<Boolean>())).thenReturn(transaction)
whenever(hub.startTransaction(any<String>(), any())).thenReturn(transaction)
return ActivityLifecycleIntegration(application)
}
}
Expand Down Expand Up @@ -189,7 +189,7 @@ class ActivityLifecycleIntegrationTest {

sut.onActivityPreCreated(fixture.activity, fixture.bundle)

verify(fixture.hub, never()).startTransaction(any<String>(), any(), any<Boolean>())
verify(fixture.hub, never()).startTransaction(any<String>(), any())
}

@Test
Expand All @@ -202,7 +202,7 @@ class ActivityLifecycleIntegrationTest {
sut.onActivityPreCreated(fixture.activity, fixture.bundle)

// call only once
verify(fixture.hub).startTransaction(any<String>(), any(), any<Boolean>())
verify(fixture.hub).startTransaction(any<String>(), any())
}

@Test
Expand All @@ -215,7 +215,7 @@ class ActivityLifecycleIntegrationTest {

verify(fixture.hub).startTransaction(any<String>(), check {
assertEquals("navigation", it)
}, any<Boolean>())
})
}

@Test
Expand All @@ -228,7 +228,7 @@ class ActivityLifecycleIntegrationTest {

verify(fixture.hub).startTransaction(check<String> {
assertEquals("Activity", it)
}, any(), any<Boolean>())
}, any())
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ private ITransaction startTransaction(
final TransactionContext contexts =
TransactionContext.fromSentryTrace(
name, "http.server", new SentryTraceHeader(sentryTraceHeader));
return hub.startTransaction(contexts, customSamplingContext);
return hub.startTransaction(contexts, customSamplingContext, true);
} catch (InvalidSentryTraceHeaderException e) {
hub.getOptions()
.getLogger()
.log(SentryLevel.DEBUG, "Failed to parse Sentry trace header: %s", e.getMessage());
}
}
return hub.startTransaction(name, "http.server", customSamplingContext);
return hub.startTransaction(name, "http.server", customSamplingContext, true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
} else {
operation = "bean";
}
final ITransaction transaction = hub.startTransaction(name, operation);
final ITransaction transaction = hub.startTransaction(name, operation, true);
try {
return invocation.proceed();
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ class SentryTracingFilterTest {
request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}")
if (sentryTraceHeader != null) {
request.addHeader("sentry-trace", sentryTraceHeader)
whenever(hub.startTransaction(any(), any<CustomSamplingContext>())).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) }
whenever(hub.startTransaction(any(), any<CustomSamplingContext>(), eq(true))).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) }
}
response.status = 200
whenever(hub.startTransaction(any(), any(), any<CustomSamplingContext>())).thenAnswer { SentryTracer(TransactionContext(it.arguments[0] as String, it.arguments[1] as String), hub) }
whenever(hub.startTransaction(any(), any(), any(), eq(true))).thenAnswer { SentryTracer(TransactionContext(it.arguments[0] as String, it.arguments[1] as String), hub) }
whenever(hub.isEnabled).thenReturn(isEnabled)
return SentryTracingFilter(hub, sentryRequestResolver, transactionNameProvider)
}
Expand Down Expand Up @@ -85,7 +85,7 @@ class SentryTracingFilterTest {
verify(fixture.hub).startTransaction(eq("POST /product/12"), eq("http.server"), check<CustomSamplingContext> {
assertNotNull(it["request"])
assertTrue(it["request"] is HttpServletRequest)
})
}, eq(true))
verify(fixture.chain).doFilter(fixture.request, fixture.response)
verify(fixture.hub).captureTransaction(check {
assertThat(it.transaction).isEqualTo("POST /product/{id}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package io.sentry.spring.tracing

import com.nhaarman.mockitokotlin2.any
import com.nhaarman.mockitokotlin2.check
import com.nhaarman.mockitokotlin2.eq
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.times
import com.nhaarman.mockitokotlin2.verify
Expand Down Expand Up @@ -41,7 +42,7 @@ class SentryTransactionAdviceTest {

@BeforeTest
fun setup() {
whenever(hub.startTransaction(any<String>(), any())).thenAnswer { io.sentry.SentryTracer(TransactionContext(it.arguments[0] as String, it.arguments[1] as String), hub) }
whenever(hub.startTransaction(any<String>(), any(), eq(true))).thenAnswer { io.sentry.SentryTracer(TransactionContext(it.arguments[0] as String, it.arguments[1] as String), hub) }
}

@Test
Expand Down
32 changes: 15 additions & 17 deletions sentry/src/main/java/io/sentry/IHub.java
Original file line number Diff line number Diff line change
Expand Up @@ -281,17 +281,17 @@ default SentryId captureTransaction(SentryTransaction transaction) {
}

/**
* Creates a Transaction bound to the current hub and returns the instance.
* Creates a Transaction and returns the instance.
*
* @param transactionContexts the transaction contexts
* @return created transaction
*/
default ITransaction startTransaction(TransactionContext transactionContexts) {
return startTransaction(transactionContexts, true);
return startTransaction(transactionContexts, false);
}

/**
* Creates a Transaction bound to the current hub and returns the instance.
* Creates a Transaction and returns the instance.
*
* @param transactionContexts the transaction contexts
* @param bindToScope if transaction should be bound to scope
Expand All @@ -303,8 +303,8 @@ default ITransaction startTransaction(
}

/**
* Creates a Transaction bound to the current hub and returns the instance. Based on the passed
* sampling context the decision if transaction is sampled will be taken by {@link TracesSampler}.
* Creates a Transaction and returns the instance. Based on the passed sampling context the
* decision if transaction is sampled will be taken by {@link TracesSampler}.
*
* @param name the transaction name
* @param operation the operation
Expand All @@ -313,12 +313,12 @@ default ITransaction startTransaction(
*/
default @NotNull ITransaction startTransaction(
String name, String operation, CustomSamplingContext customSamplingContext) {
return startTransaction(name, operation, customSamplingContext, true);
return startTransaction(name, operation, customSamplingContext, false);
}

/**
* Creates a Transaction bound to the current hub and returns the instance. Based on the passed
* sampling context the decision if transaction is sampled will be taken by {@link TracesSampler}.
* Creates a Transaction and returns the instance. Based on the passed sampling context the
* decision if transaction is sampled will be taken by {@link TracesSampler}.
*
* @param name the transaction name
* @param operation the operation
Expand All @@ -336,9 +336,8 @@ default ITransaction startTransaction(
}

/**
* Creates a Transaction bound to the current hub and returns the instance. Based on the passed
* transaction and sampling contexts the decision if transaction is sampled will be taken by
* {@link TracesSampler}.
* Creates a Transaction and returns the instance. Based on the passed transaction and sampling
* contexts the decision if transaction is sampled will be taken by {@link TracesSampler}.
*
* @param transactionContexts the transaction context
* @param customSamplingContext the sampling context
Expand All @@ -347,13 +346,12 @@ default ITransaction startTransaction(
@NotNull
default ITransaction startTransaction(
TransactionContext transactionContexts, CustomSamplingContext customSamplingContext) {
return startTransaction(transactionContexts, customSamplingContext, true);
return startTransaction(transactionContexts, customSamplingContext, false);
}

/**
* Creates a Transaction bound to the current hub and returns the instance. Based on the passed
* transaction and sampling contexts the decision if transaction is sampled will be taken by
* {@link TracesSampler}.
* Creates a Transaction and returns the instance. Based on the passed transaction and sampling
* contexts the decision if transaction is sampled will be taken by {@link TracesSampler}.
*
* @param transactionContexts the transaction context
* @param customSamplingContext the sampling context
Expand All @@ -367,7 +365,7 @@ ITransaction startTransaction(
boolean bindToScope);

/**
* Creates a Transaction bound to the current hub and returns the instance. Based on the {@link
* Creates a Transaction and returns the instance. Based on the {@link
* SentryOptions#getTracesSampleRate()} the decision if transaction is sampled will be taken by
* {@link TracesSampler}.
*
Expand All @@ -381,7 +379,7 @@ ITransaction startTransaction(
}

/**
* Creates a Transaction bound to the current hub and returns the instance. Based on the {@link
* Creates a Transaction and returns the instance. Based on the {@link
* SentryOptions#getTracesSampleRate()} the decision if transaction is sampled will be taken by
* {@link TracesSampler}.
*
Expand Down
10 changes: 3 additions & 7 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ public static void endSession() {
}

/**
* Creates a Transaction and returns the instance. Started transaction is set on the scope.
* Creates a Transaction and returns the instance.
*
* @param name the transaction name
* @param operation the operation
Expand All @@ -535,7 +535,7 @@ public static void endSession() {
final @NotNull String name,
final @NotNull String operation,
final @Nullable String description) {
return startTransaction(name, operation, description, true);
return startTransaction(name, operation, description, false);
}

/**
Expand All @@ -558,7 +558,7 @@ public static void endSession() {
}

/**
* Creates a Transaction and returns the instance. Started transaction is set on the scope.
* Creates a Transaction and returns the instance.
*
* @param transactionContexts the transaction contexts
* @return created transaction
Expand All @@ -584,8 +584,6 @@ public static void endSession() {
* Creates a Transaction and returns the instance. Based on the passed sampling context the
* decision if transaction is sampled will be taken by {@link TracesSampler}.
*
* <p>Started transaction is set on the scope.
*
* @param name the transaction name
* @param operation the operation
* @param customSamplingContext the sampling context
Expand Down Expand Up @@ -620,8 +618,6 @@ public static void endSession() {
* Creates a Transaction and returns the instance. Based on the passed transaction and sampling
* contexts the decision if transaction is sampled will be taken by {@link TracesSampler}.
*
* <p>Started transaction is set on the scope.
*
* @param transactionContexts the transaction context
* @param customSamplingContext the sampling context
* @return created transaction.
Expand Down
6 changes: 3 additions & 3 deletions sentry/src/test/java/io/sentry/HubTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1052,13 +1052,13 @@ class HubTest {
}

@Test
fun `when startTransaction without bindToScope set, transaction is attached to the scope`() {
fun `when startTransaction without bindToScope set, transaction is not attached to the scope`() {
val hub = generateHub()

val transaction = hub.startTransaction("name", "op")
hub.startTransaction("name", "op")

hub.configureScope {
assertEquals(transaction, it.span)
assertNull(it.span)
}
}

Expand Down

0 comments on commit 142dd44

Please sign in to comment.