diff --git a/CHANGELOG.md b/CHANGELOG.md index 4126d1706dd..10f53cef310 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- Send a monitor config derived from Spring `@Scheduled` with `@SentryCheckIn` check-ins by default, so Sentry creates or updates the monitor from code. Set `@SentryCheckIn(upsertMonitorConfig = false)` to turn this off ([#6215](https://github.com/getsentry/sentry-java/pull/6215)) - Report the cellular network technology generation in `device.connection_effective_type`, for example `4g` or `5g` ([#6146](https://github.com/getsentry/sentry-java/pull/6146)) ## 8.59.0 diff --git a/sentry-spring-7/api/sentry-spring-7.api b/sentry-spring-7/api/sentry-spring-7.api index c9250b550fd..c5aaf0a5d1b 100644 --- a/sentry-spring-7/api/sentry-spring-7.api +++ b/sentry-spring-7/api/sentry-spring-7.api @@ -136,6 +136,7 @@ public final class io/sentry/spring7/cache/SentryCacheWrapper : org/springframew public abstract interface annotation class io/sentry/spring7/checkin/SentryCheckIn : java/lang/annotation/Annotation { public abstract fun heartbeat ()Z public abstract fun monitorSlug ()Ljava/lang/String; + public abstract fun upsertMonitorConfig ()Z public abstract fun value ()Ljava/lang/String; } diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/checkin/SentryCheckIn.java b/sentry-spring-7/src/main/java/io/sentry/spring7/checkin/SentryCheckIn.java index 47cd80ab5b2..38491436786 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/checkin/SentryCheckIn.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/checkin/SentryCheckIn.java @@ -29,6 +29,17 @@ */ boolean heartbeat() default false; + /** + * Whether to send the schedule and zone from the method's {@code @Scheduled} with check-ins, so + * Sentry creates or updates the monitor. On by default. Set to false to manage the monitor's + * schedule in Sentry instead. + * + *

Heartbeat check-ins never send a monitor config. + * + * @return true to send a monitor config, true by default + */ + boolean upsertMonitorConfig() default true; + /** * Monitor slug. If not set, no check-in will be sent. * diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/checkin/SentryCheckInAdvice.java b/sentry-spring-7/src/main/java/io/sentry/spring7/checkin/SentryCheckInAdvice.java index e58e0e9dd97..3051acbb607 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/checkin/SentryCheckInAdvice.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/checkin/SentryCheckInAdvice.java @@ -6,13 +6,21 @@ import io.sentry.DateUtils; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; +import io.sentry.MonitorConfig; import io.sentry.ScopesAdapter; import io.sentry.SentryLevel; import io.sentry.protocol.SentryId; import io.sentry.time.Stopwatch; +import io.sentry.util.MonitorConfigUtils; import io.sentry.util.Objects; import io.sentry.util.TracingUtils; import java.lang.reflect.Method; +import java.time.Duration; +import java.time.format.DateTimeParseException; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.jetbrains.annotations.ApiStatus; @@ -20,7 +28,10 @@ import org.jetbrains.annotations.Nullable; import org.springframework.aop.support.AopUtils; import org.springframework.context.EmbeddedValueResolverAware; +import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringValueResolver; @@ -31,10 +42,19 @@ @ApiStatus.Internal @Open public class SentryCheckInAdvice implements MethodInterceptor, EmbeddedValueResolverAware { + // Spring before 5.3 parses crons with CronSequenceGenerator + private static final boolean LEGACY_CRON_PARSER = + !ClassUtils.isPresent( + "org.springframework.scheduling.support.CronExpression", + SentryCheckInAdvice.class.getClassLoader()); + private final @NotNull IScopes scopes; private @Nullable StringValueResolver resolver; + private final @NotNull Map monitorConfigs = + new ConcurrentHashMap<>(); + public SentryCheckInAdvice() { this(ScopesAdapter.getInstance()); } @@ -87,6 +107,11 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl return invocation.proceed(); } + final @Nullable MonitorConfig monitorConfig = + !isHeartbeatOnly && checkInAnnotation.upsertMonitorConfig() + ? monitorConfig(mostSpecificMethod) + : null; + try (final @NotNull ISentryLifecycleToken ignored = scopes.forkedScopes("SentryCheckInAdvice").makeCurrent()) { TracingUtils.startNewTrace(scopes); @@ -98,7 +123,9 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl try { if (!isHeartbeatOnly) { - checkInId = scopes.captureCheckIn(new CheckIn(monitorSlug, CheckInStatus.IN_PROGRESS)); + final @NotNull CheckIn inProgress = new CheckIn(monitorSlug, CheckInStatus.IN_PROGRESS); + inProgress.setMonitorConfig(monitorConfig); + checkInId = scopes.captureCheckIn(inProgress); } return invocation.proceed(); } catch (Throwable e) { @@ -113,6 +140,96 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl } } + private @Nullable MonitorConfig monitorConfig(final @NotNull Method method) { + return monitorConfigs.computeIfAbsent( + method, key -> new CachedMonitorConfig(monitorConfigFromScheduled(key))) + .config; + } + + private @Nullable MonitorConfig monitorConfigFromScheduled(final @NotNull Method method) { + try { + final @NotNull Set schedules = + AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Scheduled.class); + if (schedules.size() != 1) { + return null; + } + final @NotNull Scheduled scheduled = schedules.iterator().next(); + // timeUnit only exists from Spring 5.3.10, so read it as an attribute + final @Nullable Object timeUnitAttribute = + AnnotationUtils.getAnnotationAttributes(scheduled).get("timeUnit"); + final @NotNull TimeUnit timeUnit = + timeUnitAttribute instanceof TimeUnit + ? (TimeUnit) timeUnitAttribute + : TimeUnit.MILLISECONDS; + final @Nullable String cron = resolve(scheduled.cron()); + // Spring only reads the zone for a cron + final @Nullable String zone = + cron == null || cron.isEmpty() ? null : resolve(scheduled.zone()); + return MonitorConfigUtils.fromSpringScheduled( + cron, + zone, + periodMillis(scheduled.fixedRate(), scheduled.fixedRateString(), timeUnit), + periodMillis(scheduled.fixedDelay(), scheduled.fixedDelayString(), timeUnit), + LEGACY_CRON_PARSER); + } catch (RuntimeException e) { + scopes + .getOptions() + .getLogger() + .log( + SentryLevel.WARNING, + "Could not derive a monitor config from @Scheduled for method annotated with @SentryCheckIn.", + e); + return null; + } + } + + private @Nullable Long periodMillis( + final long value, final @NotNull String valueString, final @NotNull TimeUnit timeUnit) { + if (value >= 0) { + return timeUnit.toMillis(value); + } + final @Nullable String resolved = resolve(valueString); + if (resolved == null || resolved.isEmpty()) { + return null; + } + final @NotNull String trimmed = resolved.trim(); + @Nullable Long millis = null; + if (trimmed.startsWith("P") || trimmed.startsWith("p")) { + try { + millis = Duration.parse(trimmed).toMillis(); + } catch (DateTimeParseException | ArithmeticException e) { + // logged below + } + } else { + millis = MonitorConfigUtils.parsePeriodMillis(trimmed, timeUnit); + } + if (millis == null) { + scopes + .getOptions() + .getLogger() + .log( + SentryLevel.DEBUG, + "Not sending a monitor config for @SentryCheckIn because the @Scheduled period '%s' could not be parsed.", + trimmed); + } + return millis; + } + + private @Nullable String resolve(final @NotNull String value) { + if (resolver == null || value.isEmpty()) { + return value; + } + return resolver.resolveStringValue(value); + } + + private static final class CachedMonitorConfig { + private final @Nullable MonitorConfig config; + + private CachedMonitorConfig(final @Nullable MonitorConfig config) { + this.config = config; + } + } + @Override public void setEmbeddedValueResolver(StringValueResolver resolver) { this.resolver = resolver; diff --git a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryCheckInAdviceTest.kt b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryCheckInAdviceTest.kt index e15affdbdd5..a071ba293ca 100644 --- a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryCheckInAdviceTest.kt +++ b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryCheckInAdviceTest.kt @@ -4,16 +4,21 @@ import io.sentry.CheckIn import io.sentry.CheckInStatus import io.sentry.IScopes import io.sentry.ISentryLifecycleToken +import io.sentry.MonitorConfig import io.sentry.Sentry import io.sentry.SentryOptions import io.sentry.protocol.SentryId import io.sentry.spring7.checkin.SentryCheckIn import io.sentry.spring7.checkin.SentryCheckInAdviceConfiguration import io.sentry.spring7.checkin.SentryCheckInPointcutConfiguration +import java.util.TimeZone +import java.util.concurrent.TimeUnit import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertSame import org.junit.jupiter.api.assertThrows import org.junit.runner.RunWith import org.mockito.kotlin.any @@ -32,6 +37,8 @@ import org.springframework.context.annotation.Configuration import org.springframework.context.annotation.EnableAspectJAutoProxy import org.springframework.context.annotation.Import import org.springframework.context.support.PropertySourcesPlaceholderConfigurer +import org.springframework.scheduling.annotation.Scheduled +import org.springframework.scheduling.annotation.Schedules import org.springframework.test.context.TestPropertySource import org.springframework.test.context.junit.jupiter.SpringJUnitConfig import org.springframework.test.context.junit4.SpringRunner @@ -39,7 +46,15 @@ import org.springframework.util.StringValueResolver @RunWith(SpringRunner::class) @SpringJUnitConfig(SentryCheckInAdviceTest.Config::class) -@TestPropertySource(properties = ["my.cron.slug = mypropertycronslug"]) +@TestPropertySource( + properties = + [ + "my.cron.slug = mypropertycronslug", + "my.cron.schedule = 0 30 2 * * *", + "my.cron.zone = America/New_York", + "my.cron.empty.zone = ", + ] +) class SentryCheckInAdviceTest { @Autowired lateinit var sampleService: SampleService @@ -50,6 +65,8 @@ class SentryCheckInAdviceTest { @Autowired lateinit var sampleServiceSpringProperties: SampleServiceSpringProperties + @Autowired lateinit var sampleServiceScheduled: SampleServiceScheduled + @Autowired lateinit var scopes: IScopes val lifecycleToken = mock() @@ -74,6 +91,7 @@ class SentryCheckInAdviceTest { val inProgressCheckIn = checkInCaptor.firstValue assertEquals("monitor_slug_1", inProgressCheckIn.monitorSlug) assertEquals(CheckInStatus.IN_PROGRESS.apiName(), inProgressCheckIn.status) + assertNull(inProgressCheckIn.monitorConfig) val doneCheckIn = checkInCaptor.lastValue assertEquals("monitor_slug_1", doneCheckIn.monitorSlug) @@ -228,6 +246,150 @@ class SentryCheckInAdviceTest { order.verify(lifecycleToken).close() } + @Test + fun `cron with zone is sent as crontab monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.cronWithZone() } + assertNotNull(config) + assertEquals("crontab", config.schedule.type) + assertEquals("15 10 * * 1-5", config.schedule.value) + assertNull(config.schedule.unit) + assertEquals("Europe/Vienna", config.timezone) + } + + @Test + fun `cron without zone is sent in the JVM default zone`() { + val defaultTimeZone = TimeZone.getDefault() + TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")) + try { + val config = inProgressMonitorConfig { sampleServiceScheduled.cron() } + assertEquals("0 2 * * *", config?.schedule?.value) + assertEquals("Asia/Tokyo", config?.timezone) + } finally { + TimeZone.setDefault(defaultTimeZone) + } + } + + @Test + fun `cron and zone placeholders are resolved`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.cronFromProperties() } + assertEquals("30 2 * * *", config?.schedule?.value) + assertEquals("America/New_York", config?.timezone) + } + + @Test + fun `fixed rate with time unit is sent as interval monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.fixedRateHours() } + assertNotNull(config) + assertEquals("interval", config.schedule.type) + assertEquals("2", config.schedule.value) + assertEquals("hour", config.schedule.unit) + } + + @Test + fun `ISO-8601 fixed rate string is sent as interval monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.fixedRateIso() } + assertEquals("10", config?.schedule?.value) + assertEquals("minute", config?.schedule?.unit) + } + + @Test + fun `simple duration fixed rate string is sent as interval monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.fixedRateSimpleDuration() } + assertEquals("interval", config?.schedule?.type) + assertEquals("5", config?.schedule?.value) + assertEquals("minute", config?.schedule?.unit) + } + + @Test + fun `fixed delay sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.fixedDelay() }) + assertNull(inProgressMonitorConfig { sampleServiceScheduled.fixedDelayIso() }) + } + + @Test + fun `disabled cron sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.disabledCron() }) + } + + @Test + fun `zone placeholder resolving to empty uses the JVM default zone`() { + val defaultTimeZone = TimeZone.getDefault() + TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")) + try { + val config = inProgressMonitorConfig { sampleServiceScheduled.cronWithEmptyZone() } + assertEquals("0 3 * * *", config?.schedule?.value) + assertEquals("Asia/Tokyo", config?.timezone) + } finally { + TimeZone.setDefault(defaultTimeZone) + } + } + + @Test + fun `cron zone overrides the default timezone from options`() { + val options = + SentryOptions().apply { + cron = SentryOptions.Cron().apply { defaultTimezone = "Europe/Berlin" } + } + whenever(scopes.options).thenReturn(options) + val config = inProgressMonitorConfig { sampleServiceScheduled.cronWithZoneAndDefaults() } + assertEquals("Europe/Vienna", config?.timezone) + } + + @Test + fun `unresolvable cron placeholder sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.unresolvableCron() }) + } + + @Test + fun `exception while deriving monitor config sends no monitor config and runs the job`() { + var result = 0 + assertNull(inProgressMonitorConfig { result = sampleServiceScheduled.derivationThrows() }) + assertEquals(1, result) + } + + @Test + fun `monitor config is derived once per method`() { + val first = inProgressMonitorConfig { sampleServiceScheduled.fixedRateHours() } + val second = inProgressMonitorConfig { sampleServiceScheduled.fixedRateHours() } + assertNotNull(first) + assertSame(first, second) + } + + @Test + fun `multiple schedules send no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.multipleSchedules() }) + } + + @Test + fun `schedules container sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.schedulesContainer() }) + } + + @Test + fun `upsertMonitorConfig false sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDisabled() }) + } + + @Test + fun `heartbeat with @Scheduled sends a single check-in without monitor config`() { + val checkInCaptor = argumentCaptor() + whenever(scopes.captureCheckIn(checkInCaptor.capture())).thenReturn(SentryId()) + sampleServiceScheduled.heartbeat() + assertEquals(1, checkInCaptor.allValues.size) + assertEquals(CheckInStatus.OK.apiName(), checkInCaptor.firstValue.status) + assertNull(checkInCaptor.firstValue.monitorConfig) + } + + private fun inProgressMonitorConfig(block: () -> Unit): MonitorConfig? { + val checkInCaptor = argumentCaptor() + whenever(scopes.captureCheckIn(checkInCaptor.capture())).thenReturn(SentryId()) + block() + assertEquals(2, checkInCaptor.allValues.size) + assertEquals(CheckInStatus.IN_PROGRESS.apiName(), checkInCaptor.firstValue.status) + assertNull(checkInCaptor.lastValue.monitorConfig) + return checkInCaptor.firstValue.monitorConfig + } + @Configuration @EnableAspectJAutoProxy(proxyTargetClass = true) @Import(SentryCheckInAdviceConfiguration::class, SentryCheckInPointcutConfiguration::class) @@ -241,6 +403,8 @@ class SentryCheckInAdviceTest { @Bean open fun sampleServiceSpringProperties() = SampleServiceSpringProperties() + @Bean open fun sampleServiceScheduled() = SampleServiceScheduled() + @Bean open fun scopes(): IScopes { val scopes = mock() @@ -291,6 +455,72 @@ class SentryCheckInAdviceTest { open fun helloExceptionProperty() = 1 } + open class SampleServiceScheduled { + + @SentryCheckIn("cron_zone") + @Scheduled(cron = "0 15 10 * * MON-FRI", zone = "Europe/Vienna") + open fun cronWithZone() {} + + @SentryCheckIn("cron") @Scheduled(cron = "0 0 2 * * *") open fun cron() {} + + @SentryCheckIn("cron_properties") + @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") + open fun cronFromProperties() {} + + @SentryCheckIn("fixed_rate_hours") + @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) + open fun fixedRateHours() {} + + @SentryCheckIn("fixed_rate_iso") + @Scheduled(fixedRateString = "PT10M") + open fun fixedRateIso() {} + + @SentryCheckIn("fixed_rate_simple") + @Scheduled(fixedRateString = "5m") + open fun fixedRateSimpleDuration() {} + + @SentryCheckIn("fixed_delay") @Scheduled(fixedDelay = 300_000) open fun fixedDelay() {} + + @SentryCheckIn("fixed_delay_iso") + @Scheduled(fixedDelayString = "PT10M") + open fun fixedDelayIso() {} + + @SentryCheckIn("disabled_cron") @Scheduled(cron = "-") open fun disabledCron() {} + + @SentryCheckIn("cron_empty_zone") + @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") + open fun cronWithEmptyZone() {} + + @SentryCheckIn("cron_zone_defaults") + @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") + open fun cronWithZoneAndDefaults() {} + + @SentryCheckIn("schedules_container") + @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) + open fun schedulesContainer() {} + + @SentryCheckIn("unresolvable_cron") + @Scheduled(cron = "\${my.cron.missing}") + open fun unresolvableCron() {} + + @SentryCheckIn("derivation_throws") + @Scheduled(cron = "\${my.cron.exception.property}") + open fun derivationThrows() = 1 + + @SentryCheckIn("multiple") + @Scheduled(cron = "0 0 1 * * *") + @Scheduled(cron = "0 0 13 * * *") + open fun multipleSchedules() {} + + @SentryCheckIn("upsert_disabled", upsertMonitorConfig = false) + @Scheduled(cron = "0 0 1 * * *") + open fun upsertDisabled() {} + + @SentryCheckIn("heartbeat", heartbeat = true) + @Scheduled(cron = "0 0 1 * * *") + open fun heartbeat() {} + } + class MyPropertyPlaceholderConfigurer : PropertySourcesPlaceholderConfigurer() { override fun doProcessProperties( diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api index 24b9af7e14b..aa1dae669ba 100644 --- a/sentry-spring-jakarta/api/sentry-spring-jakarta.api +++ b/sentry-spring-jakarta/api/sentry-spring-jakarta.api @@ -136,6 +136,7 @@ public final class io/sentry/spring/jakarta/cache/SentryCacheWrapper : org/sprin public abstract interface annotation class io/sentry/spring/jakarta/checkin/SentryCheckIn : java/lang/annotation/Annotation { public abstract fun heartbeat ()Z public abstract fun monitorSlug ()Ljava/lang/String; + public abstract fun upsertMonitorConfig ()Z public abstract fun value ()Ljava/lang/String; } diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckIn.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckIn.java index bfb6b724b8f..5e180daa0e6 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckIn.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckIn.java @@ -29,6 +29,17 @@ */ boolean heartbeat() default false; + /** + * Whether to send the schedule and zone from the method's {@code @Scheduled} with check-ins, so + * Sentry creates or updates the monitor. On by default. Set to false to manage the monitor's + * schedule in Sentry instead. + * + *

Heartbeat check-ins never send a monitor config. + * + * @return true to send a monitor config, true by default + */ + boolean upsertMonitorConfig() default true; + /** * Monitor slug. If not set, no check-in will be sent. * diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckInAdvice.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckInAdvice.java index 1e3697e781d..1260e9ba05a 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckInAdvice.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckInAdvice.java @@ -6,13 +6,21 @@ import io.sentry.DateUtils; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; +import io.sentry.MonitorConfig; import io.sentry.ScopesAdapter; import io.sentry.SentryLevel; import io.sentry.protocol.SentryId; import io.sentry.time.Stopwatch; +import io.sentry.util.MonitorConfigUtils; import io.sentry.util.Objects; import io.sentry.util.TracingUtils; import java.lang.reflect.Method; +import java.time.Duration; +import java.time.format.DateTimeParseException; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.jetbrains.annotations.ApiStatus; @@ -20,7 +28,10 @@ import org.jetbrains.annotations.Nullable; import org.springframework.aop.support.AopUtils; import org.springframework.context.EmbeddedValueResolverAware; +import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringValueResolver; @@ -31,10 +42,19 @@ @ApiStatus.Internal @Open public class SentryCheckInAdvice implements MethodInterceptor, EmbeddedValueResolverAware { + // Spring before 5.3 parses crons with CronSequenceGenerator + private static final boolean LEGACY_CRON_PARSER = + !ClassUtils.isPresent( + "org.springframework.scheduling.support.CronExpression", + SentryCheckInAdvice.class.getClassLoader()); + private final @NotNull IScopes scopes; private @Nullable StringValueResolver resolver; + private final @NotNull Map monitorConfigs = + new ConcurrentHashMap<>(); + public SentryCheckInAdvice() { this(ScopesAdapter.getInstance()); } @@ -87,6 +107,11 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl return invocation.proceed(); } + final @Nullable MonitorConfig monitorConfig = + !isHeartbeatOnly && checkInAnnotation.upsertMonitorConfig() + ? monitorConfig(mostSpecificMethod) + : null; + try (final @NotNull ISentryLifecycleToken ignored = scopes.forkedScopes("SentryCheckInAdvice").makeCurrent()) { TracingUtils.startNewTrace(scopes); @@ -98,7 +123,9 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl try { if (!isHeartbeatOnly) { - checkInId = scopes.captureCheckIn(new CheckIn(monitorSlug, CheckInStatus.IN_PROGRESS)); + final @NotNull CheckIn inProgress = new CheckIn(monitorSlug, CheckInStatus.IN_PROGRESS); + inProgress.setMonitorConfig(monitorConfig); + checkInId = scopes.captureCheckIn(inProgress); } return invocation.proceed(); } catch (Throwable e) { @@ -113,6 +140,96 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl } } + private @Nullable MonitorConfig monitorConfig(final @NotNull Method method) { + return monitorConfigs.computeIfAbsent( + method, key -> new CachedMonitorConfig(monitorConfigFromScheduled(key))) + .config; + } + + private @Nullable MonitorConfig monitorConfigFromScheduled(final @NotNull Method method) { + try { + final @NotNull Set schedules = + AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Scheduled.class); + if (schedules.size() != 1) { + return null; + } + final @NotNull Scheduled scheduled = schedules.iterator().next(); + // timeUnit only exists from Spring 5.3.10, so read it as an attribute + final @Nullable Object timeUnitAttribute = + AnnotationUtils.getAnnotationAttributes(scheduled).get("timeUnit"); + final @NotNull TimeUnit timeUnit = + timeUnitAttribute instanceof TimeUnit + ? (TimeUnit) timeUnitAttribute + : TimeUnit.MILLISECONDS; + final @Nullable String cron = resolve(scheduled.cron()); + // Spring only reads the zone for a cron + final @Nullable String zone = + cron == null || cron.isEmpty() ? null : resolve(scheduled.zone()); + return MonitorConfigUtils.fromSpringScheduled( + cron, + zone, + periodMillis(scheduled.fixedRate(), scheduled.fixedRateString(), timeUnit), + periodMillis(scheduled.fixedDelay(), scheduled.fixedDelayString(), timeUnit), + LEGACY_CRON_PARSER); + } catch (RuntimeException e) { + scopes + .getOptions() + .getLogger() + .log( + SentryLevel.WARNING, + "Could not derive a monitor config from @Scheduled for method annotated with @SentryCheckIn.", + e); + return null; + } + } + + private @Nullable Long periodMillis( + final long value, final @NotNull String valueString, final @NotNull TimeUnit timeUnit) { + if (value >= 0) { + return timeUnit.toMillis(value); + } + final @Nullable String resolved = resolve(valueString); + if (resolved == null || resolved.isEmpty()) { + return null; + } + final @NotNull String trimmed = resolved.trim(); + @Nullable Long millis = null; + if (trimmed.startsWith("P") || trimmed.startsWith("p")) { + try { + millis = Duration.parse(trimmed).toMillis(); + } catch (DateTimeParseException | ArithmeticException e) { + // logged below + } + } else { + millis = MonitorConfigUtils.parsePeriodMillis(trimmed, timeUnit); + } + if (millis == null) { + scopes + .getOptions() + .getLogger() + .log( + SentryLevel.DEBUG, + "Not sending a monitor config for @SentryCheckIn because the @Scheduled period '%s' could not be parsed.", + trimmed); + } + return millis; + } + + private @Nullable String resolve(final @NotNull String value) { + if (resolver == null || value.isEmpty()) { + return value; + } + return resolver.resolveStringValue(value); + } + + private static final class CachedMonitorConfig { + private final @Nullable MonitorConfig config; + + private CachedMonitorConfig(final @Nullable MonitorConfig config) { + this.config = config; + } + } + @Override public void setEmbeddedValueResolver(StringValueResolver resolver) { this.resolver = resolver; diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryCheckInAdviceTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryCheckInAdviceTest.kt index c6edd834530..7a7e5b81d04 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryCheckInAdviceTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryCheckInAdviceTest.kt @@ -4,16 +4,21 @@ import io.sentry.CheckIn import io.sentry.CheckInStatus import io.sentry.IScopes import io.sentry.ISentryLifecycleToken +import io.sentry.MonitorConfig import io.sentry.Sentry import io.sentry.SentryOptions import io.sentry.protocol.SentryId import io.sentry.spring.jakarta.checkin.SentryCheckIn import io.sentry.spring.jakarta.checkin.SentryCheckInAdviceConfiguration import io.sentry.spring.jakarta.checkin.SentryCheckInPointcutConfiguration +import java.util.TimeZone +import java.util.concurrent.TimeUnit import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertSame import org.junit.jupiter.api.assertThrows import org.junit.runner.RunWith import org.mockito.kotlin.any @@ -32,6 +37,8 @@ import org.springframework.context.annotation.Configuration import org.springframework.context.annotation.EnableAspectJAutoProxy import org.springframework.context.annotation.Import import org.springframework.context.support.PropertySourcesPlaceholderConfigurer +import org.springframework.scheduling.annotation.Scheduled +import org.springframework.scheduling.annotation.Schedules import org.springframework.test.context.TestPropertySource import org.springframework.test.context.junit.jupiter.SpringJUnitConfig import org.springframework.test.context.junit4.SpringRunner @@ -39,7 +46,15 @@ import org.springframework.util.StringValueResolver @RunWith(SpringRunner::class) @SpringJUnitConfig(SentryCheckInAdviceTest.Config::class) -@TestPropertySource(properties = ["my.cron.slug = mypropertycronslug"]) +@TestPropertySource( + properties = + [ + "my.cron.slug = mypropertycronslug", + "my.cron.schedule = 0 30 2 * * *", + "my.cron.zone = America/New_York", + "my.cron.empty.zone = ", + ] +) class SentryCheckInAdviceTest { @Autowired lateinit var sampleService: SampleService @@ -50,6 +65,8 @@ class SentryCheckInAdviceTest { @Autowired lateinit var sampleServiceSpringProperties: SampleServiceSpringProperties + @Autowired lateinit var sampleServiceScheduled: SampleServiceScheduled + @Autowired lateinit var scopes: IScopes val lifecycleToken = mock() @@ -74,6 +91,7 @@ class SentryCheckInAdviceTest { val inProgressCheckIn = checkInCaptor.firstValue assertEquals("monitor_slug_1", inProgressCheckIn.monitorSlug) assertEquals(CheckInStatus.IN_PROGRESS.apiName(), inProgressCheckIn.status) + assertNull(inProgressCheckIn.monitorConfig) val doneCheckIn = checkInCaptor.lastValue assertEquals("monitor_slug_1", doneCheckIn.monitorSlug) @@ -228,6 +246,150 @@ class SentryCheckInAdviceTest { order.verify(lifecycleToken).close() } + @Test + fun `cron with zone is sent as crontab monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.cronWithZone() } + assertNotNull(config) + assertEquals("crontab", config.schedule.type) + assertEquals("15 10 * * 1-5", config.schedule.value) + assertNull(config.schedule.unit) + assertEquals("Europe/Vienna", config.timezone) + } + + @Test + fun `cron without zone is sent in the JVM default zone`() { + val defaultTimeZone = TimeZone.getDefault() + TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")) + try { + val config = inProgressMonitorConfig { sampleServiceScheduled.cron() } + assertEquals("0 2 * * *", config?.schedule?.value) + assertEquals("Asia/Tokyo", config?.timezone) + } finally { + TimeZone.setDefault(defaultTimeZone) + } + } + + @Test + fun `cron and zone placeholders are resolved`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.cronFromProperties() } + assertEquals("30 2 * * *", config?.schedule?.value) + assertEquals("America/New_York", config?.timezone) + } + + @Test + fun `fixed rate with time unit is sent as interval monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.fixedRateHours() } + assertNotNull(config) + assertEquals("interval", config.schedule.type) + assertEquals("2", config.schedule.value) + assertEquals("hour", config.schedule.unit) + } + + @Test + fun `ISO-8601 fixed rate string is sent as interval monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.fixedRateIso() } + assertEquals("10", config?.schedule?.value) + assertEquals("minute", config?.schedule?.unit) + } + + @Test + fun `simple duration fixed rate string is sent as interval monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.fixedRateSimpleDuration() } + assertEquals("interval", config?.schedule?.type) + assertEquals("5", config?.schedule?.value) + assertEquals("minute", config?.schedule?.unit) + } + + @Test + fun `fixed delay sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.fixedDelay() }) + assertNull(inProgressMonitorConfig { sampleServiceScheduled.fixedDelayIso() }) + } + + @Test + fun `disabled cron sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.disabledCron() }) + } + + @Test + fun `zone placeholder resolving to empty uses the JVM default zone`() { + val defaultTimeZone = TimeZone.getDefault() + TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")) + try { + val config = inProgressMonitorConfig { sampleServiceScheduled.cronWithEmptyZone() } + assertEquals("0 3 * * *", config?.schedule?.value) + assertEquals("Asia/Tokyo", config?.timezone) + } finally { + TimeZone.setDefault(defaultTimeZone) + } + } + + @Test + fun `cron zone overrides the default timezone from options`() { + val options = + SentryOptions().apply { + cron = SentryOptions.Cron().apply { defaultTimezone = "Europe/Berlin" } + } + whenever(scopes.options).thenReturn(options) + val config = inProgressMonitorConfig { sampleServiceScheduled.cronWithZoneAndDefaults() } + assertEquals("Europe/Vienna", config?.timezone) + } + + @Test + fun `unresolvable cron placeholder sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.unresolvableCron() }) + } + + @Test + fun `exception while deriving monitor config sends no monitor config and runs the job`() { + var result = 0 + assertNull(inProgressMonitorConfig { result = sampleServiceScheduled.derivationThrows() }) + assertEquals(1, result) + } + + @Test + fun `monitor config is derived once per method`() { + val first = inProgressMonitorConfig { sampleServiceScheduled.fixedRateHours() } + val second = inProgressMonitorConfig { sampleServiceScheduled.fixedRateHours() } + assertNotNull(first) + assertSame(first, second) + } + + @Test + fun `multiple schedules send no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.multipleSchedules() }) + } + + @Test + fun `schedules container sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.schedulesContainer() }) + } + + @Test + fun `upsertMonitorConfig false sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDisabled() }) + } + + @Test + fun `heartbeat with @Scheduled sends a single check-in without monitor config`() { + val checkInCaptor = argumentCaptor() + whenever(scopes.captureCheckIn(checkInCaptor.capture())).thenReturn(SentryId()) + sampleServiceScheduled.heartbeat() + assertEquals(1, checkInCaptor.allValues.size) + assertEquals(CheckInStatus.OK.apiName(), checkInCaptor.firstValue.status) + assertNull(checkInCaptor.firstValue.monitorConfig) + } + + private fun inProgressMonitorConfig(block: () -> Unit): MonitorConfig? { + val checkInCaptor = argumentCaptor() + whenever(scopes.captureCheckIn(checkInCaptor.capture())).thenReturn(SentryId()) + block() + assertEquals(2, checkInCaptor.allValues.size) + assertEquals(CheckInStatus.IN_PROGRESS.apiName(), checkInCaptor.firstValue.status) + assertNull(checkInCaptor.lastValue.monitorConfig) + return checkInCaptor.firstValue.monitorConfig + } + @Configuration @EnableAspectJAutoProxy(proxyTargetClass = true) @Import(SentryCheckInAdviceConfiguration::class, SentryCheckInPointcutConfiguration::class) @@ -241,6 +403,8 @@ class SentryCheckInAdviceTest { @Bean open fun sampleServiceSpringProperties() = SampleServiceSpringProperties() + @Bean open fun sampleServiceScheduled() = SampleServiceScheduled() + @Bean open fun scopes(): IScopes { val scopes = mock() @@ -291,6 +455,72 @@ class SentryCheckInAdviceTest { open fun helloExceptionProperty() = 1 } + open class SampleServiceScheduled { + + @SentryCheckIn("cron_zone") + @Scheduled(cron = "0 15 10 * * MON-FRI", zone = "Europe/Vienna") + open fun cronWithZone() {} + + @SentryCheckIn("cron") @Scheduled(cron = "0 0 2 * * *") open fun cron() {} + + @SentryCheckIn("cron_properties") + @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") + open fun cronFromProperties() {} + + @SentryCheckIn("fixed_rate_hours") + @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) + open fun fixedRateHours() {} + + @SentryCheckIn("fixed_rate_iso") + @Scheduled(fixedRateString = "PT10M") + open fun fixedRateIso() {} + + @SentryCheckIn("fixed_rate_simple") + @Scheduled(fixedRateString = "5m") + open fun fixedRateSimpleDuration() {} + + @SentryCheckIn("fixed_delay") @Scheduled(fixedDelay = 300_000) open fun fixedDelay() {} + + @SentryCheckIn("fixed_delay_iso") + @Scheduled(fixedDelayString = "PT10M") + open fun fixedDelayIso() {} + + @SentryCheckIn("disabled_cron") @Scheduled(cron = "-") open fun disabledCron() {} + + @SentryCheckIn("cron_empty_zone") + @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") + open fun cronWithEmptyZone() {} + + @SentryCheckIn("cron_zone_defaults") + @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") + open fun cronWithZoneAndDefaults() {} + + @SentryCheckIn("schedules_container") + @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) + open fun schedulesContainer() {} + + @SentryCheckIn("unresolvable_cron") + @Scheduled(cron = "\${my.cron.missing}") + open fun unresolvableCron() {} + + @SentryCheckIn("derivation_throws") + @Scheduled(cron = "\${my.cron.exception.property}") + open fun derivationThrows() = 1 + + @SentryCheckIn("multiple") + @Scheduled(cron = "0 0 1 * * *") + @Scheduled(cron = "0 0 13 * * *") + open fun multipleSchedules() {} + + @SentryCheckIn("upsert_disabled", upsertMonitorConfig = false) + @Scheduled(cron = "0 0 1 * * *") + open fun upsertDisabled() {} + + @SentryCheckIn("heartbeat", heartbeat = true) + @Scheduled(cron = "0 0 1 * * *") + open fun heartbeat() {} + } + class MyPropertyPlaceholderConfigurer : PropertySourcesPlaceholderConfigurer() { override fun doProcessProperties( diff --git a/sentry-spring/api/sentry-spring.api b/sentry-spring/api/sentry-spring.api index 4e1bea84288..8b8bfa69c70 100644 --- a/sentry-spring/api/sentry-spring.api +++ b/sentry-spring/api/sentry-spring.api @@ -134,6 +134,7 @@ public final class io/sentry/spring/cache/SentryCacheWrapper : org/springframewo public abstract interface annotation class io/sentry/spring/checkin/SentryCheckIn : java/lang/annotation/Annotation { public abstract fun heartbeat ()Z public abstract fun monitorSlug ()Ljava/lang/String; + public abstract fun upsertMonitorConfig ()Z public abstract fun value ()Ljava/lang/String; } diff --git a/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryCheckIn.java b/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryCheckIn.java index 805f4bd5464..03df99385c2 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryCheckIn.java +++ b/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryCheckIn.java @@ -29,6 +29,17 @@ */ boolean heartbeat() default false; + /** + * Whether to send the schedule and zone from the method's {@code @Scheduled} with check-ins, so + * Sentry creates or updates the monitor. On by default. Set to false to manage the monitor's + * schedule in Sentry instead. + * + *

Heartbeat check-ins never send a monitor config. + * + * @return true to send a monitor config, true by default + */ + boolean upsertMonitorConfig() default true; + /** * Monitor slug. If not set, no check-in will be sent. * diff --git a/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryCheckInAdvice.java b/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryCheckInAdvice.java index 2abd2cfb994..202bd30e56a 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryCheckInAdvice.java +++ b/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryCheckInAdvice.java @@ -6,13 +6,21 @@ import io.sentry.DateUtils; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; +import io.sentry.MonitorConfig; import io.sentry.ScopesAdapter; import io.sentry.SentryLevel; import io.sentry.protocol.SentryId; import io.sentry.time.Stopwatch; +import io.sentry.util.MonitorConfigUtils; import io.sentry.util.Objects; import io.sentry.util.TracingUtils; import java.lang.reflect.Method; +import java.time.Duration; +import java.time.format.DateTimeParseException; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.jetbrains.annotations.ApiStatus; @@ -20,7 +28,10 @@ import org.jetbrains.annotations.Nullable; import org.springframework.aop.support.AopUtils; import org.springframework.context.EmbeddedValueResolverAware; +import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringValueResolver; @@ -31,10 +42,19 @@ @ApiStatus.Internal @Open public class SentryCheckInAdvice implements MethodInterceptor, EmbeddedValueResolverAware { + // Spring before 5.3 parses crons with CronSequenceGenerator + private static final boolean LEGACY_CRON_PARSER = + !ClassUtils.isPresent( + "org.springframework.scheduling.support.CronExpression", + SentryCheckInAdvice.class.getClassLoader()); + private final @NotNull IScopes scopes; private @Nullable StringValueResolver resolver; + private final @NotNull Map monitorConfigs = + new ConcurrentHashMap<>(); + public SentryCheckInAdvice() { this(ScopesAdapter.getInstance()); } @@ -90,6 +110,11 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl return invocation.proceed(); } + final @Nullable MonitorConfig monitorConfig = + !isHeartbeatOnly && checkInAnnotation.upsertMonitorConfig() + ? monitorConfig(mostSpecificMethod) + : null; + try (final @NotNull ISentryLifecycleToken ignored = scopes.forkedScopes("SentryCheckInAdvice").makeCurrent()) { TracingUtils.startNewTrace(scopes); @@ -101,7 +126,9 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl try { if (!isHeartbeatOnly) { - checkInId = scopes.captureCheckIn(new CheckIn(monitorSlug, CheckInStatus.IN_PROGRESS)); + final @NotNull CheckIn inProgress = new CheckIn(monitorSlug, CheckInStatus.IN_PROGRESS); + inProgress.setMonitorConfig(monitorConfig); + checkInId = scopes.captureCheckIn(inProgress); } return invocation.proceed(); } catch (Throwable e) { @@ -116,6 +143,96 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl } } + private @Nullable MonitorConfig monitorConfig(final @NotNull Method method) { + return monitorConfigs.computeIfAbsent( + method, key -> new CachedMonitorConfig(monitorConfigFromScheduled(key))) + .config; + } + + private @Nullable MonitorConfig monitorConfigFromScheduled(final @NotNull Method method) { + try { + final @NotNull Set schedules = + AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Scheduled.class); + if (schedules.size() != 1) { + return null; + } + final @NotNull Scheduled scheduled = schedules.iterator().next(); + // timeUnit only exists from Spring 5.3.10, so read it as an attribute + final @Nullable Object timeUnitAttribute = + AnnotationUtils.getAnnotationAttributes(scheduled).get("timeUnit"); + final @NotNull TimeUnit timeUnit = + timeUnitAttribute instanceof TimeUnit + ? (TimeUnit) timeUnitAttribute + : TimeUnit.MILLISECONDS; + final @Nullable String cron = resolve(scheduled.cron()); + // Spring only reads the zone for a cron + final @Nullable String zone = + cron == null || cron.isEmpty() ? null : resolve(scheduled.zone()); + return MonitorConfigUtils.fromSpringScheduled( + cron, + zone, + periodMillis(scheduled.fixedRate(), scheduled.fixedRateString(), timeUnit), + periodMillis(scheduled.fixedDelay(), scheduled.fixedDelayString(), timeUnit), + LEGACY_CRON_PARSER); + } catch (RuntimeException e) { + scopes + .getOptions() + .getLogger() + .log( + SentryLevel.WARNING, + "Could not derive a monitor config from @Scheduled for method annotated with @SentryCheckIn.", + e); + return null; + } + } + + private @Nullable Long periodMillis( + final long value, final @NotNull String valueString, final @NotNull TimeUnit timeUnit) { + if (value >= 0) { + return timeUnit.toMillis(value); + } + final @Nullable String resolved = resolve(valueString); + if (resolved == null || resolved.isEmpty()) { + return null; + } + final @NotNull String trimmed = resolved.trim(); + @Nullable Long millis = null; + if (trimmed.startsWith("P") || trimmed.startsWith("p")) { + try { + millis = Duration.parse(trimmed).toMillis(); + } catch (DateTimeParseException | ArithmeticException e) { + // logged below + } + } else { + millis = MonitorConfigUtils.parsePeriodMillis(trimmed, timeUnit); + } + if (millis == null) { + scopes + .getOptions() + .getLogger() + .log( + SentryLevel.DEBUG, + "Not sending a monitor config for @SentryCheckIn because the @Scheduled period '%s' could not be parsed.", + trimmed); + } + return millis; + } + + private @Nullable String resolve(final @NotNull String value) { + if (resolver == null || value.isEmpty()) { + return value; + } + return resolver.resolveStringValue(value); + } + + private static final class CachedMonitorConfig { + private final @Nullable MonitorConfig config; + + private CachedMonitorConfig(final @Nullable MonitorConfig config) { + this.config = config; + } + } + @Override public void setEmbeddedValueResolver(StringValueResolver resolver) { this.resolver = resolver; diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryCheckInAdviceTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryCheckInAdviceTest.kt index 2c5e7d0cd06..b6e8a4e3de2 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryCheckInAdviceTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryCheckInAdviceTest.kt @@ -4,17 +4,22 @@ import io.sentry.CheckIn import io.sentry.CheckInStatus import io.sentry.IScopes import io.sentry.ISentryLifecycleToken +import io.sentry.MonitorConfig import io.sentry.Sentry import io.sentry.SentryOptions import io.sentry.protocol.SentryId import io.sentry.spring.checkin.SentryCheckIn import io.sentry.spring.checkin.SentryCheckInAdviceConfiguration import io.sentry.spring.checkin.SentryCheckInPointcutConfiguration +import java.util.TimeZone +import java.util.concurrent.TimeUnit import kotlin.RuntimeException import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertSame import org.junit.jupiter.api.assertThrows import org.junit.runner.RunWith import org.mockito.kotlin.any @@ -33,6 +38,8 @@ import org.springframework.context.annotation.Configuration import org.springframework.context.annotation.EnableAspectJAutoProxy import org.springframework.context.annotation.Import import org.springframework.context.support.PropertySourcesPlaceholderConfigurer +import org.springframework.scheduling.annotation.Scheduled +import org.springframework.scheduling.annotation.Schedules import org.springframework.test.context.TestPropertySource import org.springframework.test.context.junit.jupiter.SpringJUnitConfig import org.springframework.test.context.junit4.SpringRunner @@ -40,7 +47,15 @@ import org.springframework.util.StringValueResolver @RunWith(SpringRunner::class) @SpringJUnitConfig(SentryCheckInAdviceTest.Config::class) -@TestPropertySource(properties = ["my.cron.slug = mypropertycronslug"]) +@TestPropertySource( + properties = + [ + "my.cron.slug = mypropertycronslug", + "my.cron.schedule = 0 30 2 * * *", + "my.cron.zone = America/New_York", + "my.cron.empty.zone = ", + ] +) class SentryCheckInAdviceTest { @Autowired lateinit var sampleService: SampleService @@ -51,6 +66,8 @@ class SentryCheckInAdviceTest { @Autowired lateinit var sampleServiceSpringProperties: SampleServiceSpringProperties + @Autowired lateinit var sampleServiceScheduled: SampleServiceScheduled + @Autowired lateinit var scopes: IScopes val lifecycleToken = mock() @@ -75,6 +92,7 @@ class SentryCheckInAdviceTest { val inProgressCheckIn = checkInCaptor.firstValue assertEquals("monitor_slug_1", inProgressCheckIn.monitorSlug) assertEquals(CheckInStatus.IN_PROGRESS.apiName(), inProgressCheckIn.status) + assertNull(inProgressCheckIn.monitorConfig) val doneCheckIn = checkInCaptor.lastValue assertEquals("monitor_slug_1", doneCheckIn.monitorSlug) @@ -229,6 +247,150 @@ class SentryCheckInAdviceTest { order.verify(lifecycleToken).close() } + @Test + fun `cron with zone is sent as crontab monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.cronWithZone() } + assertNotNull(config) + assertEquals("crontab", config.schedule.type) + assertEquals("15 10 * * 1-5", config.schedule.value) + assertNull(config.schedule.unit) + assertEquals("Europe/Vienna", config.timezone) + } + + @Test + fun `cron without zone is sent in the JVM default zone`() { + val defaultTimeZone = TimeZone.getDefault() + TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")) + try { + val config = inProgressMonitorConfig { sampleServiceScheduled.cron() } + assertEquals("0 2 * * *", config?.schedule?.value) + assertEquals("Asia/Tokyo", config?.timezone) + } finally { + TimeZone.setDefault(defaultTimeZone) + } + } + + @Test + fun `cron and zone placeholders are resolved`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.cronFromProperties() } + assertEquals("30 2 * * *", config?.schedule?.value) + assertEquals("America/New_York", config?.timezone) + } + + @Test + fun `fixed rate with time unit is sent as interval monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.fixedRateHours() } + assertNotNull(config) + assertEquals("interval", config.schedule.type) + assertEquals("2", config.schedule.value) + assertEquals("hour", config.schedule.unit) + } + + @Test + fun `ISO-8601 fixed rate string is sent as interval monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.fixedRateIso() } + assertEquals("10", config?.schedule?.value) + assertEquals("minute", config?.schedule?.unit) + } + + @Test + fun `simple duration fixed rate string is sent as interval monitor config`() { + val config = inProgressMonitorConfig { sampleServiceScheduled.fixedRateSimpleDuration() } + assertEquals("interval", config?.schedule?.type) + assertEquals("5", config?.schedule?.value) + assertEquals("minute", config?.schedule?.unit) + } + + @Test + fun `fixed delay sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.fixedDelay() }) + assertNull(inProgressMonitorConfig { sampleServiceScheduled.fixedDelayIso() }) + } + + @Test + fun `disabled cron sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.disabledCron() }) + } + + @Test + fun `zone placeholder resolving to empty uses the JVM default zone`() { + val defaultTimeZone = TimeZone.getDefault() + TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")) + try { + val config = inProgressMonitorConfig { sampleServiceScheduled.cronWithEmptyZone() } + assertEquals("0 3 * * *", config?.schedule?.value) + assertEquals("Asia/Tokyo", config?.timezone) + } finally { + TimeZone.setDefault(defaultTimeZone) + } + } + + @Test + fun `cron zone overrides the default timezone from options`() { + val options = + SentryOptions().apply { + cron = SentryOptions.Cron().apply { defaultTimezone = "Europe/Berlin" } + } + whenever(scopes.options).thenReturn(options) + val config = inProgressMonitorConfig { sampleServiceScheduled.cronWithZoneAndDefaults() } + assertEquals("Europe/Vienna", config?.timezone) + } + + @Test + fun `unresolvable cron placeholder sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.unresolvableCron() }) + } + + @Test + fun `exception while deriving monitor config sends no monitor config and runs the job`() { + var result = 0 + assertNull(inProgressMonitorConfig { result = sampleServiceScheduled.derivationThrows() }) + assertEquals(1, result) + } + + @Test + fun `monitor config is derived once per method`() { + val first = inProgressMonitorConfig { sampleServiceScheduled.fixedRateHours() } + val second = inProgressMonitorConfig { sampleServiceScheduled.fixedRateHours() } + assertNotNull(first) + assertSame(first, second) + } + + @Test + fun `multiple schedules send no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.multipleSchedules() }) + } + + @Test + fun `schedules container sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.schedulesContainer() }) + } + + @Test + fun `upsertMonitorConfig false sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDisabled() }) + } + + @Test + fun `heartbeat with @Scheduled sends a single check-in without monitor config`() { + val checkInCaptor = argumentCaptor() + whenever(scopes.captureCheckIn(checkInCaptor.capture())).thenReturn(SentryId()) + sampleServiceScheduled.heartbeat() + assertEquals(1, checkInCaptor.allValues.size) + assertEquals(CheckInStatus.OK.apiName(), checkInCaptor.firstValue.status) + assertNull(checkInCaptor.firstValue.monitorConfig) + } + + private fun inProgressMonitorConfig(block: () -> Unit): MonitorConfig? { + val checkInCaptor = argumentCaptor() + whenever(scopes.captureCheckIn(checkInCaptor.capture())).thenReturn(SentryId()) + block() + assertEquals(2, checkInCaptor.allValues.size) + assertEquals(CheckInStatus.IN_PROGRESS.apiName(), checkInCaptor.firstValue.status) + assertNull(checkInCaptor.lastValue.monitorConfig) + return checkInCaptor.firstValue.monitorConfig + } + @Configuration @EnableAspectJAutoProxy(proxyTargetClass = true) @Import(SentryCheckInAdviceConfiguration::class, SentryCheckInPointcutConfiguration::class) @@ -242,6 +404,8 @@ class SentryCheckInAdviceTest { @Bean open fun sampleServiceSpringProperties() = SampleServiceSpringProperties() + @Bean open fun sampleServiceScheduled() = SampleServiceScheduled() + @Bean open fun scopes(): IScopes { val scopes = mock() @@ -292,6 +456,72 @@ class SentryCheckInAdviceTest { open fun helloExceptionProperty() = 1 } + open class SampleServiceScheduled { + + @SentryCheckIn("cron_zone") + @Scheduled(cron = "0 15 10 * * MON-FRI", zone = "Europe/Vienna") + open fun cronWithZone() {} + + @SentryCheckIn("cron") @Scheduled(cron = "0 0 2 * * *") open fun cron() {} + + @SentryCheckIn("cron_properties") + @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") + open fun cronFromProperties() {} + + @SentryCheckIn("fixed_rate_hours") + @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) + open fun fixedRateHours() {} + + @SentryCheckIn("fixed_rate_iso") + @Scheduled(fixedRateString = "PT10M") + open fun fixedRateIso() {} + + @SentryCheckIn("fixed_rate_simple") + @Scheduled(fixedRateString = "5m") + open fun fixedRateSimpleDuration() {} + + @SentryCheckIn("fixed_delay") @Scheduled(fixedDelay = 300_000) open fun fixedDelay() {} + + @SentryCheckIn("fixed_delay_iso") + @Scheduled(fixedDelayString = "PT10M") + open fun fixedDelayIso() {} + + @SentryCheckIn("disabled_cron") @Scheduled(cron = "-") open fun disabledCron() {} + + @SentryCheckIn("cron_empty_zone") + @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") + open fun cronWithEmptyZone() {} + + @SentryCheckIn("cron_zone_defaults") + @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") + open fun cronWithZoneAndDefaults() {} + + @SentryCheckIn("schedules_container") + @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) + open fun schedulesContainer() {} + + @SentryCheckIn("unresolvable_cron") + @Scheduled(cron = "\${my.cron.missing}") + open fun unresolvableCron() {} + + @SentryCheckIn("derivation_throws") + @Scheduled(cron = "\${my.cron.exception.property}") + open fun derivationThrows() = 1 + + @SentryCheckIn("multiple") + @Scheduled(cron = "0 0 1 * * *") + @Scheduled(cron = "0 0 13 * * *") + open fun multipleSchedules() {} + + @SentryCheckIn("upsert_disabled", upsertMonitorConfig = false) + @Scheduled(cron = "0 0 1 * * *") + open fun upsertDisabled() {} + + @SentryCheckIn("heartbeat", heartbeat = true) + @Scheduled(cron = "0 0 1 * * *") + open fun heartbeat() {} + } + class MyPropertyPlaceholderConfigurer : PropertySourcesPlaceholderConfigurer() { override fun doProcessProperties(