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.

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

Hubs/Scopes Merge 23 - Use new API for CRONS integrations #3347

Merged
merged 29 commits into from
Apr 22, 2024
Merged

Hubs/Scopes Merge 23 - Use new API for CRONS integrations #3347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
305baf5
replace hub with scopes
adinauer Mar 27, 2024
95f5e1b
Add Scopes
adinauer Apr 2, 2024
27f2398
Introduce `IScopes` interface.
adinauer Apr 2, 2024
ce3c14f
Replace `IHub` with `IScopes` in core
adinauer Apr 2, 2024
ce615f4
Replace `IHub` with `IScopes` in android core
adinauer Apr 2, 2024
22ddc00
Replace `IHub` with `IScopes` in android integrations
adinauer Apr 2, 2024
305c217
Replace `IHub` with `IScopes` in apollo integrations
adinauer Apr 2, 2024
da927bc
Replace `IHub` with `IScopes` in okhttp integration
adinauer Apr 2, 2024
8279276
Replace `IHub` with `IScopes` in graphql integration
adinauer Apr 2, 2024
9bfc086
Replace `IHub` with `IScopes` in logging integrations
adinauer Apr 2, 2024
b998e50
Replace `IHub` with `IScopes` in more integrations
adinauer Apr 2, 2024
739827a
Replace `IHub` with `IScopes` in OTel integration
adinauer Apr 2, 2024
69f2d63
Replace `IHub` with `IScopes` in Spring 5 / Spring Boot 2 integrations
adinauer Apr 2, 2024
792d482
Replace `IHub` with `IScopes` in Spring 6 / Spring Boot 3 integrations
adinauer Apr 2, 2024
9bcbce6
Replace `IHub` with `IScopes` in samples
adinauer Apr 2, 2024
3f25a4b
Merge branch 'feat/hsm-13-replacements-in-samples' into feat/hubs-sco…
adinauer Apr 2, 2024
d6fb40a
gitscopes -> github
adinauer Apr 2, 2024
7752bcc
Replace ThreadLocal with ScopesStorage
adinauer Apr 4, 2024
1e329c5
Move client and throwable to span map to scope
adinauer Apr 4, 2024
b0d89ae
Add global scope
adinauer Apr 4, 2024
cdd414a
use global scope in Scopes
adinauer Apr 4, 2024
98da9ff
Implement pushScope popScope and withScope for Scopes
adinauer Apr 4, 2024
2d26033
Add pushIsolationScope; add fork methods to ISCope
adinauer Apr 12, 2024
bbb6700
Use separate scopes for current, isolation and global scope; rename m…
adinauer Apr 12, 2024
c714b21
Allow controlling which scope configureScope uses
adinauer Apr 12, 2024
a474402
Combine scopes
adinauer Apr 12, 2024
ae93e33
Use new API for CRONS integrations
adinauer Apr 12, 2024
b01298b
Add lifecycle helper
adinauer Apr 12, 2024
1c5eab3
Merge branch '8.x.x' into feat/hsm-23-cron-integrations
adinauer Apr 22, 2024
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 sentry-log4j2/src/test/kotlin/io/sentry/log4j2/SentryAppenderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class SentryAppenderTest {
@BeforeTest
fun `clear MDC`() {
ThreadContext.clearAll()
Sentry.close()
}

@Test
Expand Down
1 change: 1 addition & 0 deletions sentry-quartz/api/sentry-quartz.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ public final class io/sentry/quartz/BuildConfig {

public final class io/sentry/quartz/SentryJobListener : org/quartz/JobListener {
public static final field SENTRY_CHECK_IN_ID_KEY Ljava/lang/String;
public static final field SENTRY_LIFECYCLE_TOKEN_KEY Ljava/lang/String;
public static final field SENTRY_SLUG_KEY Ljava/lang/String;
public fun <init> ()V
public fun <init> (Lio/sentry/IScopes;)V
Expand Down
8 changes: 6 additions & 2 deletions sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
import io.sentry.CheckIn;
import io.sentry.CheckInStatus;
import io.sentry.IScopes;
import io.sentry.ISentryLifecycleToken;
import io.sentry.ScopesAdapter;
import io.sentry.SentryIntegrationPackageStorage;
import io.sentry.SentryLevel;
import io.sentry.protocol.SentryId;
import io.sentry.util.LifecycleHelper;
import io.sentry.util.Objects;
import io.sentry.util.TracingUtils;
import org.jetbrains.annotations.ApiStatus;
Expand All @@ -23,6 +25,7 @@ public final class SentryJobListener implements JobListener {

public static final String SENTRY_CHECK_IN_ID_KEY = "sentry-checkin-id";
public static final String SENTRY_SLUG_KEY = "sentry-slug";
public static final String SENTRY_LIFECYCLE_TOKEN_KEY = "sentry-lifecycle";

private final @NotNull IScopes scopes;

Expand All @@ -49,13 +52,14 @@ public void jobToBeExecuted(final @NotNull JobExecutionContext context) {
if (maybeSlug == null) {
return;
}
scopes.pushScope();
final @NotNull ISentryLifecycleToken lifecycleToken = scopes.pushIsolationScope();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this it likely makes sense to have it isolated.

TracingUtils.startNewTrace(scopes);
final @NotNull String slug = maybeSlug;
final @NotNull CheckIn checkIn = new CheckIn(slug, CheckInStatus.IN_PROGRESS);
final @NotNull SentryId checkInId = scopes.captureCheckIn(checkIn);
context.put(SENTRY_CHECK_IN_ID_KEY, checkInId);
context.put(SENTRY_SLUG_KEY, slug);
context.put(SENTRY_LIFECYCLE_TOKEN_KEY, lifecycleToken);
} catch (Throwable t) {
scopes
.getOptions()
Expand Down Expand Up @@ -103,7 +107,7 @@ public void jobWasExecuted(JobExecutionContext context, JobExecutionException jo
.getLogger()
.log(SentryLevel.ERROR, "Unable to capture check-in in jobWasExecuted.", t);
} finally {
scopes.popScope();
LifecycleHelper.close(context.get(SENTRY_LIFECYCLE_TOKEN_KEY));
}
}
}
5 changes: 3 additions & 2 deletions ...ry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckInAdvice.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import io.sentry.CheckInStatus;
import io.sentry.DateUtils;
import io.sentry.IScopes;
import io.sentry.ISentryLifecycleToken;
import io.sentry.ScopesAdapter;
import io.sentry.SentryLevel;
import io.sentry.protocol.SentryId;
Expand Down Expand Up @@ -86,7 +87,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
return invocation.proceed();
}

scopes.pushScope();
final @NotNull ISentryLifecycleToken lifecycleToken = scopes.pushIsolationScope();
Copy link
Member Author

@adinauer adinauer Apr 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100% sure here, maybe we should make it configurable whether this isolates or not (pushScope vs. pushIsolationScope). The @SentryCheckIn annotation may be used on jobs but can also be used on any method where it may not make sense to isolate but rather keep e.g. the request scope.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default should probably be isolated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah makes sense!
If any user code within invocation.proceed(); performs changes via the static API, e.g. Sentry.setTag(), those will be written to the isolation scope which is created here, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, static API and configureScope default to isolation scope for non Android.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I think about API, the more I think we should just find a sane default and only deliver that as a starting point. We can always add more API if users actually ask for it.

TracingUtils.startNewTrace(scopes);

@Nullable SentryId checkInId = null;
Expand All @@ -106,7 +107,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
CheckIn checkIn = new CheckIn(checkInId, monitorSlug, status);
checkIn.setDuration(DateUtils.millisToSeconds(System.currentTimeMillis() - startTime));
scopes.captureCheckIn(checkIn);
scopes.popScope();
lifecycleToken.close();
lbloder marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
46 changes: 25 additions & 21 deletions sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryCheckInAdviceTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package io.sentry.spring.jakarta
import io.sentry.CheckIn
import io.sentry.CheckInStatus
import io.sentry.IScopes
import io.sentry.ISentryLifecycleToken
import io.sentry.Sentry
import io.sentry.SentryOptions
import io.sentry.protocol.SentryId
Expand Down Expand Up @@ -56,10 +57,13 @@ class SentryCheckInAdviceTest {
@Autowired
lateinit var scopes: IScopes

val lifecycleToken = mock<ISentryLifecycleToken>()

@BeforeTest
fun setup() {
reset(scopes)
whenever(scopes.options).thenReturn(SentryOptions())
whenever(scopes.pushIsolationScope()).thenReturn(lifecycleToken)
}

@Test
Expand All @@ -79,10 +83,10 @@ class SentryCheckInAdviceTest {
assertEquals("monitor_slug_1", doneCheckIn.monitorSlug)
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes, times(2)).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -103,10 +107,10 @@ class SentryCheckInAdviceTest {
assertEquals("monitor_slug_1e", doneCheckIn.monitorSlug)
assertEquals(CheckInStatus.ERROR.apiName(), doneCheckIn.status)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes, times(2)).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -123,10 +127,10 @@ class SentryCheckInAdviceTest {
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
assertNotNull(doneCheckIn.duration)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -144,10 +148,10 @@ class SentryCheckInAdviceTest {
assertEquals(CheckInStatus.ERROR.apiName(), doneCheckIn.status)
assertNotNull(doneCheckIn.duration)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand Down Expand Up @@ -178,10 +182,10 @@ class SentryCheckInAdviceTest {
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
assertNotNull(doneCheckIn.duration)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -198,10 +202,10 @@ class SentryCheckInAdviceTest {
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
assertNotNull(doneCheckIn.duration)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -218,10 +222,10 @@ class SentryCheckInAdviceTest {
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
assertNotNull(doneCheckIn.duration)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Configuration
Expand Down
5 changes: 3 additions & 2 deletions sentry-spring/src/main/java/io/sentry/spring/checkin/SentryCheckInAdvice.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import io.sentry.CheckInStatus;
import io.sentry.DateUtils;
import io.sentry.IScopes;
import io.sentry.ISentryLifecycleToken;
import io.sentry.ScopesAdapter;
import io.sentry.SentryLevel;
import io.sentry.protocol.SentryId;
Expand Down Expand Up @@ -89,7 +90,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
return invocation.proceed();
}

scopes.pushScope();
final @NotNull ISentryLifecycleToken lifecycleToken = scopes.pushIsolationScope();
lbloder marked this conversation as resolved.
Show resolved Hide resolved
TracingUtils.startNewTrace(scopes);

@Nullable SentryId checkInId = null;
Expand All @@ -109,7 +110,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
CheckIn checkIn = new CheckIn(checkInId, monitorSlug, status);
checkIn.setDuration(DateUtils.millisToSeconds(System.currentTimeMillis() - startTime));
scopes.captureCheckIn(checkIn);
scopes.popScope();
lifecycleToken.close();
}
}

Expand Down
50 changes: 27 additions & 23 deletions sentry-spring/src/test/kotlin/io/sentry/spring/SentryCheckInAdviceTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package io.sentry.spring
import io.sentry.CheckIn
import io.sentry.CheckInStatus
import io.sentry.IScopes
import io.sentry.ISentryLifecycleToken
import io.sentry.Sentry
import io.sentry.SentryOptions
import io.sentry.protocol.SentryId
Expand Down Expand Up @@ -57,10 +58,13 @@ class SentryCheckInAdviceTest {
@Autowired
lateinit var scopes: IScopes

val lifecycleToken = mock<ISentryLifecycleToken>()

@BeforeTest
fun setup() {
reset(scopes)
whenever(scopes.options).thenReturn(SentryOptions())
whenever(scopes.pushIsolationScope()).thenReturn(lifecycleToken)
}

@Test
Expand All @@ -80,10 +84,10 @@ class SentryCheckInAdviceTest {
assertEquals("monitor_slug_1", doneCheckIn.monitorSlug)
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes, times(2)).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -104,10 +108,10 @@ class SentryCheckInAdviceTest {
assertEquals("monitor_slug_1e", doneCheckIn.monitorSlug)
assertEquals(CheckInStatus.ERROR.apiName(), doneCheckIn.status)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes, times(2)).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -124,10 +128,10 @@ class SentryCheckInAdviceTest {
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
assertNotNull(doneCheckIn.duration)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -145,10 +149,10 @@ class SentryCheckInAdviceTest {
assertEquals(CheckInStatus.ERROR.apiName(), doneCheckIn.status)
assertNotNull(doneCheckIn.duration)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -160,9 +164,9 @@ class SentryCheckInAdviceTest {
assertEquals(1, result)
assertEquals(0, checkInCaptor.allValues.size)

verify(scopes, never()).pushScope()
verify(scopes, never()).pushIsolationScope()
verify(scopes, never()).captureCheckIn(any())
verify(scopes, never()).popScope()
verify(lifecycleToken, never()).close()
}

@Test
Expand All @@ -179,10 +183,10 @@ class SentryCheckInAdviceTest {
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
assertNotNull(doneCheckIn.duration)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -199,10 +203,10 @@ class SentryCheckInAdviceTest {
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
assertNotNull(doneCheckIn.duration)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Test
Expand All @@ -219,10 +223,10 @@ class SentryCheckInAdviceTest {
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
assertNotNull(doneCheckIn.duration)

val order = inOrder(scopes)
order.verify(scopes).pushScope()
val order = inOrder(scopes, lifecycleToken)
order.verify(scopes).pushIsolationScope()
order.verify(scopes).captureCheckIn(any())
order.verify(scopes).popScope()
order.verify(lifecycleToken).close()
}

@Configuration
Expand Down
6 changes: 3 additions & 3 deletions sentry/src/main/java/io/sentry/util/CheckInUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import io.sentry.CheckInStatus;
import io.sentry.DateUtils;
import io.sentry.IScopes;
import io.sentry.ISentryLifecycleToken;
import io.sentry.MonitorConfig;
import io.sentry.Sentry;
import io.sentry.protocol.SentryId;
Expand All @@ -30,12 +31,11 @@ public static <U> U withCheckIn(
final @Nullable MonitorConfig monitorConfig,
final @NotNull Callable<U> callable)
throws Exception {
final @NotNull ISentryLifecycleToken lifecycleToken = Sentry.pushIsolationScope();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should also be configurable to use pushScope or pushIsolationScope.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default should probably also be isolated. Add overload with isolated: true/false, defaulting to true

final @NotNull IScopes scopes = Sentry.getCurrentScopes();
final long startTime = System.currentTimeMillis();
boolean didError = false;

// TODO fork instead
scopes.pushScope();
TracingUtils.startNewTrace(scopes);

CheckIn inProgressCheckIn = new CheckIn(monitorSlug, CheckInStatus.IN_PROGRESS);
lbloder marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -53,7 +53,7 @@ public static <U> U withCheckIn(
CheckIn checkIn = new CheckIn(checkInId, monitorSlug, status);
checkIn.setDuration(DateUtils.millisToSeconds(System.currentTimeMillis() - startTime));
scopes.captureCheckIn(checkIn);
scopes.popScope();
lifecycleToken.close();
}
}

Expand Down
Loading
Loading