diff --git a/sentry-kotlin-extensions/src/main/java/io/sentry/kotlin/SentryContext.kt b/sentry-kotlin-extensions/src/main/java/io/sentry/kotlin/SentryContext.kt index 4c814f2805..a77281a033 100644 --- a/sentry-kotlin-extensions/src/main/java/io/sentry/kotlin/SentryContext.kt +++ b/sentry-kotlin-extensions/src/main/java/io/sentry/kotlin/SentryContext.kt @@ -9,23 +9,19 @@ import kotlin.coroutines.CoroutineContext /** * Sentry context element for [CoroutineContext]. */ -@SuppressWarnings("deprecation") -// TODO fork instead -public class SentryContext(private val scopes: IScopes = Sentry.getCurrentScopes().clone()) : +public class SentryContext(private val scopes: IScopes = Sentry.forkedCurrentScope("coroutine")) : CopyableThreadContextElement, AbstractCoroutineContextElement(Key) { private companion object Key : CoroutineContext.Key @SuppressWarnings("deprecation") override fun copyForChild(): CopyableThreadContextElement { - // TODO fork instead - return SentryContext(scopes.clone()) + return SentryContext(scopes.forkedCurrentScope("coroutine.child")) } @SuppressWarnings("deprecation") override fun mergeForChild(overwritingElement: CoroutineContext.Element): CoroutineContext { - // TODO fork instead? - return overwritingElement[Key] ?: SentryContext(scopes.clone()) + return overwritingElement[Key] ?: SentryContext(scopes.forkedCurrentScope("coroutine.child")) } override fun updateThreadContext(context: CoroutineContext): IScopes { diff --git a/sentry-kotlin-extensions/src/test/java/io/sentry/kotlin/SentryContextTest.kt b/sentry-kotlin-extensions/src/test/java/io/sentry/kotlin/SentryContextTest.kt index 578b610267..bd498846dd 100644 --- a/sentry-kotlin-extensions/src/test/java/io/sentry/kotlin/SentryContextTest.kt +++ b/sentry-kotlin-extensions/src/test/java/io/sentry/kotlin/SentryContextTest.kt @@ -1,5 +1,6 @@ package io.sentry.kotlin +import io.sentry.ScopeType import io.sentry.Sentry import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.joinAll @@ -38,11 +39,40 @@ class SentryContextTest { Sentry.setTag("c2", "c2value") assertEquals("c2value", getTag("c2")) assertEquals("parentValue", getTag("parent")) - assertNull(getTag("c1")) + assertNotNull(getTag("c1")) + } + listOf(c1, c2).joinAll() + assertNotNull(getTag("parent")) + assertNotNull(getTag("c1")) + assertNotNull(getTag("c2")) + return@runBlocking + } + + @Test + fun testContextIsNotPassedByDefaultBetweenCoroutinesCurrentScope() = runBlocking { + Sentry.configureScope(ScopeType.CURRENT) { scope -> + scope.setTag("parent", "parentValue") + } + val c1 = launch(SentryContext()) { + Sentry.configureScope(ScopeType.CURRENT) { scope -> + scope.setTag("c1", "c1value") + } + assertEquals("c1value", getTag("c1", ScopeType.CURRENT)) + assertEquals("parentValue", getTag("parent", ScopeType.CURRENT)) + assertNull(getTag("c2", ScopeType.CURRENT)) + } + val c2 = launch(SentryContext()) { + Sentry.configureScope(ScopeType.CURRENT) { scope -> + scope.setTag("c2", "c2value") + } + assertEquals("c2value", getTag("c2", ScopeType.CURRENT)) + assertEquals("parentValue", getTag("parent", ScopeType.CURRENT)) + assertNull(getTag("c1", ScopeType.CURRENT)) } listOf(c1, c2).joinAll() - assertNull(getTag("c1")) - assertNull(getTag("c2")) + assertNotNull(getTag("parent", ScopeType.CURRENT)) + assertNull(getTag("c1", ScopeType.CURRENT)) + assertNull(getTag("c2", ScopeType.CURRENT)) } @Test @@ -84,7 +114,7 @@ class SentryContextTest { } @Test - fun testContextIsClonedWhenPassedToChild() = runBlocking { + fun testContextIsClonedWhenPassedToChildCurrentScope() = runBlocking { Sentry.setTag("parent", "parentValue") launch(SentryContext()) { Sentry.setTag("c1", "c1value") @@ -102,10 +132,44 @@ class SentryContextTest { c2.join() assertNotNull(getTag("c1")) - assertNull(getTag("c2")) + assertNotNull(getTag("c2")) + }.join() + assertNotNull(getTag("parent")) + assertNotNull(getTag("c1")) + assertNotNull(getTag("c2")) + return@runBlocking + } + + @Test + fun testContextIsClonedWhenPassedToChild() = runBlocking { + Sentry.configureScope(ScopeType.CURRENT) { scope -> + scope.setTag("parent", "parentValue") } - assertNull(getTag("c1")) - assertNull(getTag("c2")) + launch(SentryContext()) { + Sentry.configureScope(ScopeType.CURRENT) { scope -> + scope.setTag("c1", "c1value") + } + assertEquals("c1value", getTag("c1", ScopeType.CURRENT)) + assertEquals("parentValue", getTag("parent", ScopeType.CURRENT)) + assertNull(getTag("c2", ScopeType.CURRENT)) + + val c2 = launch() { + Sentry.configureScope(ScopeType.CURRENT) { scope -> + scope.setTag("c2", "c2value") + } + assertEquals("c2value", getTag("c2", ScopeType.CURRENT)) + assertEquals("parentValue", getTag("parent", ScopeType.CURRENT)) + assertNotNull(getTag("c1", ScopeType.CURRENT)) + } + + c2.join() + + assertNotNull(getTag("c1", ScopeType.CURRENT)) + assertNull(getTag("c2", ScopeType.CURRENT)) + }.join() + assertNotNull(getTag("parent", ScopeType.CURRENT)) + assertNull(getTag("c1", ScopeType.CURRENT)) + assertNull(getTag("c2", ScopeType.CURRENT)) } @Test @@ -120,7 +184,7 @@ class SentryContextTest { val c2 = launch( SentryContext( Sentry.getCurrentScopes().clone().also { - it.setTag("cloned", "clonedValue") + Sentry.setTag("cloned", "clonedValue") } ) ) { @@ -134,12 +198,56 @@ class SentryContextTest { c2.join() assertNotNull(getTag("c1")) - assertNull(getTag("c2")) - assertNull(getTag("cloned")) + assertNotNull(getTag("c2")) + assertNotNull(getTag("cloned")) + }.join() + + assertNotNull(getTag("c1")) + assertNotNull(getTag("c2")) + assertNotNull(getTag("cloned")) + return@runBlocking + } + + @Test + fun testExplicitlyPassedContextOverridesPropagatedContextCurrentScope() = runBlocking { + Sentry.configureScope(ScopeType.CURRENT) { scope -> + scope.setTag("parent", "parentValue") + } + launch(SentryContext()) { + Sentry.configureScope(ScopeType.CURRENT) { scope -> + scope.setTag("c1", "c1value") + } + assertEquals("c1value", getTag("c1", ScopeType.CURRENT)) + assertEquals("parentValue", getTag("parent", ScopeType.CURRENT)) + assertNull(getTag("c2", ScopeType.CURRENT)) + + val c2 = launch( + SentryContext( + Sentry.getCurrentScopes().clone().also { + it.configureScope(ScopeType.CURRENT) { scope -> + scope.setTag("cloned", "clonedValue") + } + } + ) + ) { + Sentry.configureScope(ScopeType.CURRENT) { scope -> + scope.setTag("c2", "c2value") + } + assertEquals("c2value", getTag("c2", ScopeType.CURRENT)) + assertEquals("parentValue", getTag("parent", ScopeType.CURRENT)) + assertNotNull(getTag("c1", ScopeType.CURRENT)) + assertNotNull(getTag("cloned", ScopeType.CURRENT)) + } + + c2.join() + + assertNotNull(getTag("c1", ScopeType.CURRENT)) + assertNull(getTag("c2", ScopeType.CURRENT)) + assertNull(getTag("cloned", ScopeType.CURRENT)) } - assertNull(getTag("c1")) - assertNull(getTag("c2")) - assertNull(getTag("cloned")) + assertNull(getTag("c1", ScopeType.CURRENT)) + assertNull(getTag("c2", ScopeType.CURRENT)) + assertNull(getTag("cloned", ScopeType.CURRENT)) } @Test @@ -167,9 +275,9 @@ class SentryContextTest { assertEquals(initialContextElement, mergedContextElement) } - private fun getTag(tag: String): String? { + private fun getTag(tag: String, scopeType: ScopeType = ScopeType.ISOLATION): String? { var value: String? = null - Sentry.configureScope { + Sentry.configureScope(scopeType) { value = it.tags[tag] } return value diff --git a/sentry/src/main/java/io/sentry/SentryWrapper.java b/sentry/src/main/java/io/sentry/SentryWrapper.java index 165ace7c83..d0f2cd8017 100644 --- a/sentry/src/main/java/io/sentry/SentryWrapper.java +++ b/sentry/src/main/java/io/sentry/SentryWrapper.java @@ -27,18 +27,23 @@ public final class SentryWrapper { * @return the wrapped {@link Callable} * @param - the result type of the {@link Callable} */ - @SuppressWarnings("deprecation") + // TODO adapt javadoc public static Callable wrapCallable(final @NotNull Callable callable) { - // TODO replace with forking - final IScopes newHub = Sentry.getCurrentScopes().clone(); + final IScopes newScopes = Sentry.getCurrentScopes().forkedCurrentScope("wrapCallable"); + + return () -> { + try (ISentryLifecycleToken ignored = newScopes.makeCurrent()) { + return callable.call(); + } + }; + } + + public static Callable wrapCallableIsolated(final @NotNull Callable callable) { + final IScopes newScopes = Sentry.getCurrentScopes().forkedScopes("wrapCallable"); return () -> { - final IScopes oldState = Sentry.getCurrentScopes(); - Sentry.setCurrentScopes(newHub); - try { + try (ISentryLifecycleToken ignored = newScopes.makeCurrent()) { return callable.call(); - } finally { - Sentry.setCurrentScopes(oldState); } }; } @@ -55,16 +60,21 @@ public static Callable wrapCallable(final @NotNull Callable callable) */ @SuppressWarnings("deprecation") public static Supplier wrapSupplier(final @NotNull Supplier supplier) { - // TODO replace with forking - final IScopes newHub = Sentry.getCurrentScopes().clone(); + final IScopes newScopes = Sentry.forkedCurrentScope("wrapSupplier"); + + return () -> { + try (ISentryLifecycleToken ignore = newScopes.makeCurrent()) { + return supplier.get(); + } + }; + } + + public static Supplier wrapSupplierIsolated(final @NotNull Supplier supplier) { + final IScopes newScopes = Sentry.forkedScopes("wrapSupplier"); return () -> { - final IScopes oldState = Sentry.getCurrentScopes(); - Sentry.setCurrentScopes(newHub); - try { + try (ISentryLifecycleToken ignore = newScopes.makeCurrent()) { return supplier.get(); - } finally { - Sentry.setCurrentScopes(oldState); } }; } diff --git a/sentry/src/test/java/io/sentry/SentryWrapperTest.kt b/sentry/src/test/java/io/sentry/SentryWrapperTest.kt index 2fb9b38566..a3511450f0 100644 --- a/sentry/src/test/java/io/sentry/SentryWrapperTest.kt +++ b/sentry/src/test/java/io/sentry/SentryWrapperTest.kt @@ -36,7 +36,7 @@ class SentryWrapperTest { } val mainHub = Sentry.getCurrentScopes() - val threadedHub = Sentry.getCurrentScopes().clone() + val threadedHub = mainHub.forkedCurrentScope("test") executor.submit { Sentry.setCurrentScopes(threadedHub) @@ -46,7 +46,7 @@ class SentryWrapperTest { val callableFuture = CompletableFuture.supplyAsync( - SentryWrapper.wrapSupplier { + SentryWrapper.wrapSupplierIsolated { assertNotEquals(mainHub, Sentry.getCurrentScopes()) assertNotEquals(threadedHub, Sentry.getCurrentScopes()) "Result 1" @@ -63,7 +63,7 @@ class SentryWrapperTest { } @Test - fun `wrapped supply async isolates Hubs`() { + fun `wrapped supply async does not isolate Scopes`() { val capturedEvents = mutableListOf() Sentry.init { @@ -110,12 +110,12 @@ class SentryWrapperTest { val clonedEvent2 = capturedEvents.firstOrNull { it.message?.formatted == "ClonedMessage2" } assertEquals(2, mainEvent?.breadcrumbs?.size) - assertEquals(2, clonedEvent?.breadcrumbs?.size) - assertEquals(2, clonedEvent2?.breadcrumbs?.size) + assertEquals(3, clonedEvent?.breadcrumbs?.size) + assertEquals(4, clonedEvent2?.breadcrumbs?.size) } @Test - fun `wrapped callable isolates Hubs`() { + fun `wrapped callable does not isolate Scopes`() { val capturedEvents = mutableListOf() Sentry.init { @@ -159,8 +159,8 @@ class SentryWrapperTest { val clonedEvent2 = capturedEvents.firstOrNull { it.message?.formatted == "ClonedMessage2" } assertEquals(2, mainEvent?.breadcrumbs?.size) - assertEquals(2, clonedEvent?.breadcrumbs?.size) - assertEquals(2, clonedEvent2?.breadcrumbs?.size) + assertEquals(3, clonedEvent?.breadcrumbs?.size) + assertEquals(4, clonedEvent2?.breadcrumbs?.size) } @Test @@ -170,7 +170,7 @@ class SentryWrapperTest { } val mainHub = Sentry.getCurrentScopes() - val threadedHub = Sentry.getCurrentScopes().clone() + val threadedHub = Sentry.getCurrentScopes().forkedCurrentScope("test") executor.submit { Sentry.setCurrentScopes(threadedHub) @@ -194,4 +194,173 @@ class SentryWrapperTest { assertEquals(threadedHub, Sentry.getCurrentScopes()) }.get() } + + @Test + fun `scopes is reset to its state within the thread after isolated supply is done`() { + Sentry.init { + it.dsn = dsn + it.beforeSend = SentryOptions.BeforeSendCallback { event, hint -> + event + } + } + + val mainHub = Sentry.getCurrentScopes() + val threadedHub = Sentry.getCurrentScopes().forkedCurrentScope("test") + + executor.submit { + Sentry.setCurrentScopes(threadedHub) + }.get() + + assertEquals(mainHub, Sentry.getCurrentScopes()) + + val callableFuture = + CompletableFuture.supplyAsync( + SentryWrapper.wrapSupplierIsolated { + assertNotEquals(mainHub, Sentry.getCurrentScopes()) + assertNotEquals(threadedHub, Sentry.getCurrentScopes()) + "Result 1" + }, + executor + ) + + callableFuture.join() + + executor.submit { + assertNotEquals(mainHub, Sentry.getCurrentScopes()) + assertEquals(threadedHub, Sentry.getCurrentScopes()) + }.get() + } + + @Test + fun `wrapped supply async isolates Scopes`() { + val capturedEvents = mutableListOf() + + Sentry.init { + it.dsn = dsn + it.beforeSend = SentryOptions.BeforeSendCallback { event, hint -> + capturedEvents.add(event) + event + } + } + + Sentry.addBreadcrumb("MyOriginalBreadcrumbBefore") + Sentry.captureMessage("OriginalMessageBefore") + + val callableFuture = + CompletableFuture.supplyAsync( + SentryWrapper.wrapSupplierIsolated { + Thread.sleep(20) + Sentry.addBreadcrumb("MyClonedBreadcrumb") + Sentry.captureMessage("ClonedMessage") + "Result 1" + }, + executor + ) + + val callableFuture2 = + CompletableFuture.supplyAsync( + SentryWrapper.wrapSupplierIsolated { + Thread.sleep(10) + Sentry.addBreadcrumb("MyClonedBreadcrumb2") + Sentry.captureMessage("ClonedMessage2") + "Result 2" + }, + executor + ) + + Sentry.addBreadcrumb("MyOriginalBreadcrumb") + Sentry.captureMessage("OriginalMessage") + + callableFuture.join() + callableFuture2.join() + + val mainEvent = capturedEvents.firstOrNull { it.message?.formatted == "OriginalMessage" } + val clonedEvent = capturedEvents.firstOrNull { it.message?.formatted == "ClonedMessage" } + val clonedEvent2 = capturedEvents.firstOrNull { it.message?.formatted == "ClonedMessage2" } + + assertEquals(2, mainEvent?.breadcrumbs?.size) + assertEquals(2, clonedEvent?.breadcrumbs?.size) + assertEquals(2, clonedEvent2?.breadcrumbs?.size) + } + + @Test + fun `wrapped callable isolates Scopes`() { + val capturedEvents = mutableListOf() + + Sentry.init { + it.dsn = dsn + it.beforeSend = SentryOptions.BeforeSendCallback { event, hint -> + capturedEvents.add(event) + event + } + } + + Sentry.addBreadcrumb("MyOriginalBreadcrumbBefore") + Sentry.captureMessage("OriginalMessageBefore") + println(Thread.currentThread().name) + + val future1 = executor.submit( + SentryWrapper.wrapCallableIsolated { + Thread.sleep(20) + Sentry.addBreadcrumb("MyClonedBreadcrumb") + Sentry.captureMessage("ClonedMessage") + "Result 1" + } + ) + + val future2 = executor.submit( + SentryWrapper.wrapCallableIsolated { + Thread.sleep(10) + Sentry.addBreadcrumb("MyClonedBreadcrumb2") + Sentry.captureMessage("ClonedMessage2") + "Result 2" + } + ) + + Sentry.addBreadcrumb("MyOriginalBreadcrumb") + Sentry.captureMessage("OriginalMessage") + + future1.get() + future2.get() + + val mainEvent = capturedEvents.firstOrNull { it.message?.formatted == "OriginalMessage" } + val clonedEvent = capturedEvents.firstOrNull { it.message?.formatted == "ClonedMessage" } + val clonedEvent2 = capturedEvents.firstOrNull { it.message?.formatted == "ClonedMessage2" } + + assertEquals(2, mainEvent?.breadcrumbs?.size) + assertEquals(2, clonedEvent?.breadcrumbs?.size) + assertEquals(2, clonedEvent2?.breadcrumbs?.size) + } + + @Test + fun `scopes is reset to its state within the thread after isolated callable is done`() { + Sentry.init { + it.dsn = dsn + } + + val mainHub = Sentry.getCurrentScopes() + val threadedHub = Sentry.getCurrentScopes().forkedCurrentScope("test") + + executor.submit { + Sentry.setCurrentScopes(threadedHub) + }.get() + + assertEquals(mainHub, Sentry.getCurrentScopes()) + + val callableFuture = + executor.submit( + SentryWrapper.wrapCallableIsolated { + assertNotEquals(mainHub, Sentry.getCurrentScopes()) + assertNotEquals(threadedHub, Sentry.getCurrentScopes()) + "Result 1" + } + ) + + callableFuture.get() + + executor.submit { + assertNotEquals(mainHub, Sentry.getCurrentScopes()) + assertEquals(threadedHub, Sentry.getCurrentScopes()) + }.get() + } }