diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c3ec23dd2..f25c8c7412c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features -- Monitor every Spring `@Scheduled` method with Sentry Crons by setting `sentry.monitor-scheduled-tasks=true`, without adding `@SentryCheckIn` to each method. Exclude methods with `sentry.exclude-scheduled-tasks` ([#6275](https://github.com/getsentry/sentry-java/pull/6275)) +- Monitor every Spring `@Scheduled` method with Sentry Crons by setting `sentry.monitor-scheduled-tasks=true`, without adding `@SentryCheckIn` to each method. Exclude methods with `sentry.exclude-scheduled-tasks` ([#6275](https://github.com/getsentry/sentry-java/pull/6275), [#6276](https://github.com/getsentry/sentry-java/pull/6276)) - 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)) diff --git a/sentry-spring-boot/api/sentry-spring-boot.api b/sentry-spring-boot/api/sentry-spring-boot.api index fe13689faf5..4fafe9d695b 100644 --- a/sentry-spring-boot/api/sentry-spring-boot.api +++ b/sentry-spring-boot/api/sentry-spring-boot.api @@ -32,16 +32,20 @@ public class io/sentry/spring/boot/SentryProperties : io/sentry/SentryOptions { public fun ()V public fun getClients ()Lio/sentry/spring/boot/SentryProperties$Clients; public fun getExceptionResolverOrder ()I + public fun getExcludeScheduledTasks ()Ljava/util/List; public fun getGraphql ()Lio/sentry/spring/boot/SentryProperties$Graphql; public fun getLogging ()Lio/sentry/spring/boot/SentryProperties$Logging; public fun getUserFilterOrder ()Ljava/lang/Integer; public fun isKeepTransactionsOpenForAsyncResponses ()Z + public fun isMonitorScheduledTasks ()Z public fun isUseGitCommitIdAsRelease ()Z public fun setClients (Lio/sentry/spring/boot/SentryProperties$Clients;)V public fun setExceptionResolverOrder (I)V + public fun setExcludeScheduledTasks (Ljava/util/List;)V public fun setGraphql (Lio/sentry/spring/boot/SentryProperties$Graphql;)V public fun setKeepTransactionsOpenForAsyncResponses (Z)V public fun setLogging (Lio/sentry/spring/boot/SentryProperties$Logging;)V + public fun setMonitorScheduledTasks (Z)V public fun setUseGitCommitIdAsRelease (Z)V public fun setUserFilterOrder (Ljava/lang/Integer;)V } diff --git a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index a405e007cbd..744fa9d739a 100644 --- a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -24,9 +24,11 @@ import io.sentry.spring.SpringSecuritySentryUserProvider; import io.sentry.spring.boot.graphql.SentryGraphqlAutoConfiguration; import io.sentry.spring.cache.SentryCacheBeanPostProcessor; +import io.sentry.spring.checkin.SentryCheckInAdvice; import io.sentry.spring.checkin.SentryCheckInAdviceConfiguration; import io.sentry.spring.checkin.SentryCheckInPointcutConfiguration; import io.sentry.spring.checkin.SentryQuartzConfiguration; +import io.sentry.spring.checkin.SentryScheduledCheckInPointcut; import io.sentry.spring.exception.SentryCaptureExceptionParameterPointcutConfiguration; import io.sentry.spring.exception.SentryExceptionParameterAdviceConfiguration; import io.sentry.spring.kafka.SentryKafkaConsumerBeanPostProcessor; @@ -47,11 +49,16 @@ import java.util.List; import java.util.Optional; import javax.servlet.http.HttpServletRequest; +import org.aopalliance.aop.Advice; import org.aspectj.lang.ProceedingJoinPoint; import org.jetbrains.annotations.NotNull; import org.quartz.core.QuartzScheduler; import org.slf4j.MDC; +import org.springframework.aop.Advisor; +import org.springframework.aop.support.DefaultPointcutAdvisor; import org.springframework.beans.factory.ObjectProvider; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; @@ -70,6 +77,7 @@ import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.Role; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.core.env.Environment; @@ -292,6 +300,27 @@ static class SentryCheckInAspectsConfiguration { @Import(SentryCheckInPointcutConfiguration.class) @Open static class SentryCheckInPointcutAutoConfiguration {} + + @Configuration(proxyBeanMethods = false) + @ConditionalOnProperty(name = "sentry.monitor-scheduled-tasks", havingValue = "true") + @Open + static class SentryScheduledCheckInConfiguration { + @Bean + @Role(BeanDefinition.ROLE_INFRASTRUCTURE) + public @NotNull Advice sentryScheduledCheckInAdvice() { + return new SentryCheckInAdvice(ScopesAdapter.getInstance(), true); + } + + @Bean + @Role(BeanDefinition.ROLE_INFRASTRUCTURE) + public @NotNull Advisor sentryScheduledCheckInAdvisor( + final @NotNull @Qualifier("sentryScheduledCheckInAdvice") Advice advice, + final @NotNull SentryProperties sentryProperties) { + return new DefaultPointcutAdvisor( + new SentryScheduledCheckInPointcut(sentryProperties.getExcludeScheduledTasks()), + advice); + } + } } /** Registers beans specific to Spring MVC. */ diff --git a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java index 879b4e2ada6..cf8c6934c50 100644 --- a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java +++ b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java @@ -40,6 +40,15 @@ public class SentryProperties extends SentryOptions { /** Clients integration properties. */ private @NotNull Clients clients = new Clients(); + /** Monitor every {@code @Scheduled} method with Sentry Crons. */ + private boolean monitorScheduledTasks = false; + + /** + * {@code @Scheduled} methods not to monitor when {@code monitorScheduledTasks} is on, as {@code + * ClassName.methodName} or a regex matching it. + */ + private @NotNull List excludeScheduledTasks = new ArrayList<>(); + public boolean isUseGitCommitIdAsRelease() { return useGitCommitIdAsRelease; } @@ -109,6 +118,22 @@ public void setClients(@NotNull Clients clients) { this.clients = clients; } + public boolean isMonitorScheduledTasks() { + return monitorScheduledTasks; + } + + public void setMonitorScheduledTasks(boolean monitorScheduledTasks) { + this.monitorScheduledTasks = monitorScheduledTasks; + } + + public @NotNull List getExcludeScheduledTasks() { + return excludeScheduledTasks; + } + + public void setExcludeScheduledTasks(@NotNull List excludeScheduledTasks) { + this.excludeScheduledTasks = excludeScheduledTasks; + } + @Open public static class Clients { /** Enable automatic instrumentation of Spring-managed OkHttp clients. Disabled by default. */ diff --git a/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index 31be74477fd..20215ee145f 100644 --- a/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -20,6 +20,7 @@ import io.sentry.NoOpTransportFactory import io.sentry.ProfileLifecycle import io.sentry.SamplingContext import io.sentry.Sentry +import io.sentry.SentryEnvelope import io.sentry.SentryEvent import io.sentry.SentryIntegrationPackageStorage import io.sentry.SentryLevel @@ -50,6 +51,7 @@ import io.sentry.transport.ITransportGate import io.sentry.transport.apache.ApacheHttpClientTransportFactory import java.io.File import java.lang.RuntimeException +import java.util.concurrent.TimeUnit import javax.servlet.Filter import kotlin.test.Test import kotlin.test.assertEquals @@ -58,9 +60,12 @@ import kotlin.test.assertTrue import org.aspectj.lang.ProceedingJoinPoint import org.assertj.core.api.Assertions.assertThat import org.mockito.internal.util.MockUtil.isMock +import org.mockito.kotlin.after import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.mock +import org.mockito.kotlin.timeout import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.quartz.JobExecutionContext @@ -71,6 +76,7 @@ import org.quartz.core.QuartzScheduler import org.slf4j.MDC import org.springframework.aop.support.NameMatchMethodPointcut import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.autoconfigure.aop.AopAutoConfiguration import org.springframework.boot.autoconfigure.quartz.QuartzAutoConfiguration import org.springframework.boot.autoconfigure.quartz.SchedulerFactoryBeanCustomizer import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration @@ -85,6 +91,8 @@ import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration import org.springframework.core.Ordered import org.springframework.core.annotation.Order +import org.springframework.scheduling.annotation.EnableScheduling +import org.springframework.scheduling.annotation.Scheduled import org.springframework.scheduling.quartz.SchedulerFactoryBean import org.springframework.security.core.context.SecurityContextHolder import org.springframework.web.client.RestTemplate @@ -1126,6 +1134,44 @@ class SentryAutoConfigurationTest { .run { assertThat(it).doesNotHaveBean(SchedulerFactoryBeanCustomizer::class.java) } } + @Test + fun `does not monitor @Scheduled methods by default`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj").run { + assertThat(it).doesNotHaveBean("sentryScheduledCheckInAdvisor") + } + } + + @Test + fun `monitors @Scheduled methods when monitor-scheduled-tasks is enabled`() { + baseContextRunner + .withPropertyValues( + "sentry.dsn=http://key@localhost/proj", + "sentry.monitor-scheduled-tasks=true", + "sentry.exclude-scheduled-tasks[0]=SentryAutoConfigurationTest.ScheduledJobs.excluded", + ) + .withConfiguration(AutoConfigurations.of(AopAutoConfiguration::class.java)) + .withUserConfiguration( + MockTransportConfiguration::class.java, + ScheduledJobsConfiguration::class.java, + ) + .run { + assertThat(it).hasBean("sentryScheduledCheckInAdvisor") + val transport = it.getBean(ITransport::class.java) + val captor = argumentCaptor() + verify(transport, timeout(5000).times(2)).send(any(), anyOrNull()) + verify(transport, after(500).times(2)).send(captor.capture(), anyOrNull()) + val payloads = + captor.allValues.map { envelope -> String(envelope.items.first().data, Charsets.UTF_8) } + assertThat(payloads).allMatch { payload -> + payload.contains( + "\"monitor_slug\":\"sentry-auto-configuration-test-scheduled-jobs-sync\"" + ) + } + assertThat(payloads[0]).contains("\"value\":1,\"unit\":\"hour\"") + assertThat(payloads[1]).contains("\"status\":\"ok\"") + } + } + @Test fun `Sentry quartz job listener is added`() { contextRunner @@ -1241,6 +1287,18 @@ class SentryAutoConfigurationTest { @Bean open fun sentryTransport() = transport } + @Configuration(proxyBeanMethods = false) + @EnableScheduling + open class ScheduledJobsConfiguration { + @Bean open fun scheduledJobs() = ScheduledJobs() + } + + open class ScheduledJobs { + @Scheduled(fixedRate = 1, timeUnit = TimeUnit.HOURS) open fun sync() {} + + @Scheduled(fixedRate = 1, timeUnit = TimeUnit.HOURS) open fun excluded() {} + } + @Configuration(proxyBeanMethods = false) open class NoOpTransportConfiguration { diff --git a/sentry-spring/api/sentry-spring.api b/sentry-spring/api/sentry-spring.api index 8b8bfa69c70..edd99b8fc1c 100644 --- a/sentry-spring/api/sentry-spring.api +++ b/sentry-spring/api/sentry-spring.api @@ -141,6 +141,7 @@ public abstract interface annotation class io/sentry/spring/checkin/SentryCheckI public class io/sentry/spring/checkin/SentryCheckInAdvice : org/aopalliance/intercept/MethodInterceptor, org/springframework/context/EmbeddedValueResolverAware { public fun ()V public fun (Lio/sentry/IScopes;)V + public fun (Lio/sentry/IScopes;Z)V public fun invoke (Lorg/aopalliance/intercept/MethodInvocation;)Ljava/lang/Object; public fun setEmbeddedValueResolver (Lorg/springframework/util/StringValueResolver;)V } @@ -161,6 +162,11 @@ public class io/sentry/spring/checkin/SentryQuartzConfiguration { public fun schedulerFactoryBeanCustomizer ()Lorg/springframework/boot/autoconfigure/quartz/SchedulerFactoryBeanCustomizer; } +public final class io/sentry/spring/checkin/SentryScheduledCheckInPointcut : org/springframework/aop/support/StaticMethodMatcherPointcut { + public fun (Ljava/util/List;)V + public fun matches (Ljava/lang/reflect/Method;Ljava/lang/Class;)Z +} + public final class io/sentry/spring/checkin/SentrySchedulerFactoryBeanCustomizer : org/springframework/boot/autoconfigure/quartz/SchedulerFactoryBeanCustomizer { public fun ()V public fun customize (Lorg/springframework/scheduling/quartz/SchedulerFactoryBean;)V 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 202bd30e56a..ca9cfedda03 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 @@ -15,12 +15,16 @@ import io.sentry.util.Objects; import io.sentry.util.TracingUtils; import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.format.DateTimeParseException; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import java.util.regex.Pattern; +import java.util.zip.CRC32; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.jetbrains.annotations.ApiStatus; @@ -48,8 +52,14 @@ public class SentryCheckInAdvice implements MethodInterceptor, EmbeddedValueReso "org.springframework.scheduling.support.CronExpression", SentryCheckInAdvice.class.getClassLoader()); + private static final @NotNull Pattern WORD_BOUNDARY = + Pattern.compile("(?<=[a-z0-9])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])"); + private static final int MAX_SLUG_LENGTH = 50; + private final @NotNull IScopes scopes; + private final boolean monitorScheduledTasks; + private @Nullable StringValueResolver resolver; private final @NotNull Map monitorConfigs = @@ -60,7 +70,16 @@ public SentryCheckInAdvice() { } public SentryCheckInAdvice(final @NotNull IScopes scopes) { + this(scopes, false); + } + + /** + * @param monitorScheduledTasks whether to also monitor {@code @Scheduled} methods without {@link + * SentryCheckIn}, using a slug derived from the class and method name + */ + public SentryCheckInAdvice(final @NotNull IScopes scopes, final boolean monitorScheduledTasks) { this.scopes = Objects.requireNonNull(scopes, "scopes are required"); + this.monitorScheduledTasks = monitorScheduledTasks; } @Override @@ -72,6 +91,14 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl SentryCheckIn checkInAnnotation = AnnotationUtils.findAnnotation(mostSpecificMethod, SentryCheckIn.class); if (checkInAnnotation == null) { + if (monitorScheduledTasks && isScheduled(mostSpecificMethod)) { + return captureCheckIns( + invocation, + scheduledTaskSlug( + scheduledTaskName(invocation.getThis().getClass(), mostSpecificMethod)), + false, + monitorConfig(mostSpecificMethod)); + } return invocation.proceed(); } @@ -114,7 +141,15 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl !isHeartbeatOnly && checkInAnnotation.upsertMonitorConfig() ? monitorConfig(mostSpecificMethod) : null; + return captureCheckIns(invocation, monitorSlug, isHeartbeatOnly, monitorConfig); + } + private Object captureCheckIns( + final @NotNull MethodInvocation invocation, + final @NotNull String monitorSlug, + final boolean isHeartbeatOnly, + final @Nullable MonitorConfig monitorConfig) + throws Throwable { try (final @NotNull ISentryLifecycleToken ignored = scopes.forkedScopes("SentryCheckInAdvice").makeCurrent()) { TracingUtils.startNewTrace(scopes); @@ -143,6 +178,37 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl } } + static boolean isScheduled(final @NotNull Method method) { + return !AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Scheduled.class) + .isEmpty(); + } + + /** {@code ClassName.methodName}, with nested classes as {@code Outer.Inner}. */ + static @NotNull String scheduledTaskName( + final @NotNull Class targetClass, final @NotNull Method method) { + return ClassUtils.getShortName(ClassUtils.getUserClass(targetClass)) + "." + method.getName(); + } + + static @NotNull String scheduledTaskSlug(final @NotNull String taskName) { + final @NotNull String slug = + WORD_BOUNDARY + .matcher(taskName) + .replaceAll("-") + .toLowerCase(Locale.ROOT) + .replaceAll("[^a-z0-9]+", "-") + .replaceAll("^-+|-+$", ""); + if (slug.length() <= MAX_SLUG_LENGTH) { + return slug; + } + // keep truncated slugs unique + final @NotNull CRC32 crc = new CRC32(); + crc.update(slug.getBytes(StandardCharsets.UTF_8)); + final @NotNull String hash = String.format(Locale.ROOT, "%08x", crc.getValue()); + return slug.substring(0, MAX_SLUG_LENGTH - hash.length() - 1).replaceAll("-+$", "") + + "-" + + hash; + } + private @Nullable MonitorConfig monitorConfig(final @NotNull Method method) { return monitorConfigs.computeIfAbsent( method, key -> new CachedMonitorConfig(monitorConfigFromScheduled(key))) diff --git a/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryScheduledCheckInPointcut.java b/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryScheduledCheckInPointcut.java new file mode 100644 index 00000000000..9d231240b02 --- /dev/null +++ b/sentry-spring/src/main/java/io/sentry/spring/checkin/SentryScheduledCheckInPointcut.java @@ -0,0 +1,55 @@ +package io.sentry.spring.checkin; + +import io.sentry.FilterString; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.springframework.aop.support.AopUtils; +import org.springframework.aop.support.StaticMethodMatcherPointcut; +import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.util.ClassUtils; + +/** + * Matches {@code @Scheduled} methods without {@link SentryCheckIn}, except excluded ones. Excludes + * match the task name ({@code ClassName.methodName}) exactly or as a regex. + */ +@ApiStatus.Experimental +public final class SentryScheduledCheckInPointcut extends StaticMethodMatcherPointcut { + private final @NotNull List excludedScheduledTasks = new ArrayList<>(); + + public SentryScheduledCheckInPointcut(final @Nullable List excludedScheduledTasks) { + if (excludedScheduledTasks != null) { + for (final @NotNull String task : excludedScheduledTasks) { + if (!task.trim().isEmpty()) { + this.excludedScheduledTasks.add(new FilterString(task.trim())); + } + } + } + } + + @Override + public boolean matches(final @NotNull Method method, final @NotNull Class targetClass) { + final @NotNull Class userClass = ClassUtils.getUserClass(targetClass); + // CGLIB can't intercept these + if (Modifier.isFinal(userClass.getModifiers()) || Modifier.isFinal(method.getModifiers())) { + return false; + } + final @NotNull Method specificMethod = AopUtils.getMostSpecificMethod(method, userClass); + if (!SentryCheckInAdvice.isScheduled(specificMethod) + || AnnotationUtils.findAnnotation(specificMethod, SentryCheckIn.class) != null) { + return false; + } + final @NotNull String taskName = + SentryCheckInAdvice.scheduledTaskName(userClass, specificMethod); + for (final @NotNull FilterString excluded : excludedScheduledTasks) { + if (excluded.getFilterString().equals(taskName) || excluded.matches(taskName)) { + return false; + } + } + return true; + } +} diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/checkin/SentryScheduledCheckInTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/checkin/SentryScheduledCheckInTest.kt new file mode 100644 index 00000000000..83102dbaf91 --- /dev/null +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/checkin/SentryScheduledCheckInTest.kt @@ -0,0 +1,238 @@ +package io.sentry.spring.checkin + +import io.sentry.CheckIn +import io.sentry.CheckInStatus +import io.sentry.IScopes +import io.sentry.ISentryLifecycleToken +import io.sentry.SentryOptions +import io.sentry.protocol.SentryId +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue +import org.junit.jupiter.api.assertThrows +import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.atLeast +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.reset +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.springframework.aop.Advisor +import org.springframework.aop.Pointcut +import org.springframework.aop.support.AopUtils +import org.springframework.aop.support.DefaultPointcutAdvisor +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.beans.factory.annotation.Qualifier +import org.springframework.context.annotation.Bean +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.test.context.TestPropertySource +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig +import org.springframework.test.context.junit4.SpringRunner + +@RunWith(SpringRunner::class) +@SpringJUnitConfig(SentryScheduledCheckInTest.Config::class) +@TestPropertySource(properties = ["my.cron.schedule = 0 30 2 * * *"]) +class SentryScheduledCheckInTest { + private val SLUG = "sentry-scheduled-check-in-test-jobs-sync-users" + + @Autowired lateinit var jobs: Jobs + + @Autowired lateinit var excludedJobs: ExcludedJobs + + @Autowired lateinit var finalJobs: FinalJobs + + @Autowired lateinit var scopes: IScopes + + @BeforeTest + fun setup() { + reset(scopes) + whenever(scopes.options).thenReturn(SentryOptions()) + whenever(scopes.forkedScopes(any())).thenReturn(scopes) + whenever(scopes.makeCurrent()).thenReturn(mock()) + whenever(scopes.captureCheckIn(any())).thenReturn(SentryId()) + } + + @Test + fun `scheduled method without @SentryCheckIn is monitored with a derived slug and config`() { + val checkIns = capture { jobs.syncUsers() } + assertEquals(2, checkIns.size) + assertEquals(SLUG, checkIns[0].monitorSlug) + assertEquals(CheckInStatus.IN_PROGRESS.apiName(), checkIns[0].status) + val config = assertNotNull(checkIns[0].monitorConfig) + assertEquals("0 2 * * *", config.schedule.value) + assertEquals("Europe/Vienna", config.timezone) + assertEquals(SLUG, checkIns[1].monitorSlug) + assertEquals(CheckInStatus.OK.apiName(), checkIns[1].status) + } + + @Test + fun `cron placeholder is resolved`() { + val checkIns = capture { jobs.fromProperty() } + assertEquals("30 2 * * *", checkIns[0].monitorConfig!!.schedule.value) + } + + @Test + fun `fixed delay is monitored without config`() { + val checkIns = capture { jobs.fixedDelay() } + assertEquals(2, checkIns.size) + assertNull(checkIns[0].monitorConfig) + } + + @Test + fun `failing job sends an error check-in and rethrows`() { + val checkIns = capture { assertThrows { jobs.oops() } } + assertEquals(CheckInStatus.ERROR.apiName(), checkIns[1].status) + } + + @Test + fun `@SentryCheckIn keeps its slug and is only checked in once`() { + val checkIns = capture { jobs.explicit() } + assertEquals(listOf("explicit", "explicit"), checkIns.map { it.monitorSlug }) + } + + @Test + fun `methods without @Scheduled are not monitored`() { + jobs.notScheduled() + verify(scopes, never()).captureCheckIn(any()) + } + + @Test + fun `excluded tasks are not monitored`() { + jobs.excluded() + excludedJobs.run() + verify(scopes, never()).captureCheckIn(any()) + assertFalse(AopUtils.isAopProxy(excludedJobs)) + } + + @Test + fun `final classes are not proxied`() { + finalJobs.run() + verify(scopes, never()).captureCheckIn(any()) + assertFalse(AopUtils.isAopProxy(finalJobs)) + assertTrue(AopUtils.isAopProxy(jobs)) + } + + @Test + fun `slugs are kebab case and at most 50 characters`() { + assertEquals( + "report-jobs-send-daily-report", + SentryCheckInAdvice.scheduledTaskSlug("ReportJobs.sendDailyReport"), + ) + assertEquals( + "http-client-jobs-run", + SentryCheckInAdvice.scheduledTaskSlug("HTTPClientJobs.run"), + ) + assertEquals("outer-inner-run2-fa", SentryCheckInAdvice.scheduledTaskSlug("Outer.Inner.run2FA")) + val first = + SentryCheckInAdvice.scheduledTaskSlug( + "AVeryLongScheduledJobClassNameForReporting.firstMethod" + ) + val second = + SentryCheckInAdvice.scheduledTaskSlug( + "AVeryLongScheduledJobClassNameForReporting.secondMethod" + ) + assertTrue(first.length <= 50) + assertTrue(first.startsWith("a-very-long-scheduled-job-class-name-for-")) + assertTrue(first != second) + assertEquals( + first, + SentryCheckInAdvice.scheduledTaskSlug( + "AVeryLongScheduledJobClassNameForReporting.firstMethod" + ), + ) + } + + @Test + fun `task name uses the user class`() { + assertEquals( + "SentryScheduledCheckInTest.Jobs.syncUsers", + SentryCheckInAdvice.scheduledTaskName( + jobs.javaClass, + Jobs::class.java.getMethod("syncUsers"), + ), + ) + } + + private fun capture(block: () -> Unit): List { + val captor = argumentCaptor() + block() + verify(scopes, atLeast(0)).captureCheckIn(captor.capture()) + return captor.allValues + } + + @Configuration + @EnableAspectJAutoProxy(proxyTargetClass = true) + @Import(SentryCheckInPointcutConfiguration::class) + open class Config { + + @Bean open fun jobs() = Jobs() + + @Bean open fun excludedJobs() = ExcludedJobs() + + @Bean open fun finalJobs() = FinalJobs() + + @Bean open fun scopes(): IScopes = mock() + + @Bean + open fun sentryCheckInAdvisor( + @Qualifier("sentryCheckInPointcut") pointcut: Pointcut, + scopes: IScopes, + ): Advisor = DefaultPointcutAdvisor(pointcut, SentryCheckInAdvice(scopes)) + + @Bean open fun sentryScheduledCheckInAdvice(scopes: IScopes) = SentryCheckInAdvice(scopes, true) + + @Bean + open fun sentryScheduledCheckInAdvisor( + @Qualifier("sentryScheduledCheckInAdvice") advice: SentryCheckInAdvice + ): Advisor = + DefaultPointcutAdvisor( + SentryScheduledCheckInPointcut( + listOf("SentryScheduledCheckInTest.Jobs.excluded", ".*\\.ExcludedJobs\\..*", " ") + ), + advice, + ) + + companion object { + @Bean + @JvmStatic + fun propertySourcesPlaceholderConfigurer() = PropertySourcesPlaceholderConfigurer() + } + } + + open class Jobs { + @Scheduled(cron = "0 0 2 * * *", zone = "Europe/Vienna") open fun syncUsers() {} + + @Scheduled(cron = "\${my.cron.schedule}") open fun fromProperty() {} + + @Scheduled(fixedDelay = 1000) open fun fixedDelay() {} + + @Scheduled(cron = "0 0 2 * * *") + open fun oops() { + throw IllegalStateException("thrown on purpose") + } + + @SentryCheckIn("explicit") @Scheduled(cron = "0 0 2 * * *") open fun explicit() {} + + @Scheduled(cron = "0 0 2 * * *") open fun excluded() {} + + open fun notScheduled() {} + } + + open class ExcludedJobs { + @Scheduled(cron = "0 0 2 * * *") open fun run() {} + } + + class FinalJobs { + @Scheduled(cron = "0 0 2 * * *") fun run() {} + } +}