diff --git a/CHANGELOG.md b/CHANGELOG.md index 17d332f8011..0859bb625bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- Add the `sentry-upsert-monitor-config` job data key to Quartz `SentryJobListener`; set it to `true` to send a monitor config derived from the trigger with check-ins, so Sentry can create the monitor from code ([#6216](https://github.com/getsentry/sentry-java/pull/6216)) - Add `@SentryCheckIn(upsertMonitorConfig = true)` to send a monitor config derived from Spring `@Scheduled` with check-ins, so Sentry can create the monitor from code ([#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-quartz/api/sentry-quartz.api b/sentry-quartz/api/sentry-quartz.api index bb8b142a912..95179351c17 100644 --- a/sentry-quartz/api/sentry-quartz.api +++ b/sentry-quartz/api/sentry-quartz.api @@ -7,6 +7,7 @@ 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_SCOPE_LIFECYCLE_TOKEN_KEY Ljava/lang/String; public static final field SENTRY_SLUG_KEY Ljava/lang/String; + public static final field SENTRY_UPSERT_MONITOR_CONFIG_KEY Ljava/lang/String; public fun ()V public fun (Lio/sentry/IScopes;)V public fun getName ()Ljava/lang/String; diff --git a/sentry-quartz/build.gradle.kts b/sentry-quartz/build.gradle.kts index f4f0d9d07d2..dbf39bc50b8 100644 --- a/sentry-quartz/build.gradle.kts +++ b/sentry-quartz/build.gradle.kts @@ -34,6 +34,8 @@ dependencies { testImplementation(libs.kotlin.test.junit) testImplementation(libs.mockito.kotlin) testImplementation(libs.mockito.inline) + testImplementation(libs.google.truth) + testImplementation(libs.quartz) } tasks.withType().configureEach { diff --git a/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java b/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java index 95b69169b7e..a3f4b74ee0d 100644 --- a/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java +++ b/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java @@ -5,20 +5,30 @@ import io.sentry.CheckInStatus; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; +import io.sentry.MonitorConfig; 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.MonitorConfigUtils; import io.sentry.util.Objects; import io.sentry.util.TracingUtils; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import java.util.TimeZone; +import java.util.regex.Pattern; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.quartz.CronTrigger; import org.quartz.JobDataMap; import org.quartz.JobExecutionContext; import org.quartz.JobExecutionException; import org.quartz.JobListener; +import org.quartz.SimpleTrigger; +import org.quartz.Trigger; @ApiStatus.Experimental public final class SentryJobListener implements JobListener { @@ -32,6 +42,16 @@ public final class SentryJobListener implements JobListener { public static final String SENTRY_SLUG_KEY = "sentry-slug"; public static final String SENTRY_SCOPE_LIFECYCLE_TOKEN_KEY = "sentry-scope-lifecycle"; + /** Job data key; set to {@code true} to send a monitor config from the trigger. */ + public static final String SENTRY_UPSERT_MONITOR_CONFIG_KEY = "sentry-upsert-monitor-config"; + + private static final @NotNull List DAY_NAMES = + Arrays.asList("SUN", "MON", "TUE", "WED", "THU", "FRI", "SAT"); + + // Quartz ignores a step after a name, so SUN/2 is every Sunday + private static final @NotNull Pattern NAME_STEP = + Pattern.compile("(^|,)[A-Za-z]{3}(-[A-Za-z]{3})?/"); + private final @NotNull IScopes scopes; public SentryJobListener() { @@ -55,15 +75,20 @@ public void jobToBeExecuted(final @NotNull JobExecutionContext context) { if (maybeSlug == null) { return; } + final @Nullable MonitorConfig monitorConfig = + shouldUpsertMonitorConfig(context) + ? monitorConfigFromTrigger(context.getTrigger()) + : null; final @NotNull ISentryLifecycleToken lifecycleToken = scopes.forkedScopes("SentryJobListener").makeCurrent(); + context.put(SENTRY_SCOPE_LIFECYCLE_TOKEN_KEY, lifecycleToken); TracingUtils.startNewTrace(scopes); final @NotNull String slug = maybeSlug; final @NotNull CheckIn checkIn = new CheckIn(slug, CheckInStatus.IN_PROGRESS); + checkIn.setMonitorConfig(monitorConfig); final @NotNull SentryId checkInId = scopes.captureCheckIn(checkIn); context.put(SENTRY_CHECK_IN_ID_KEY, checkInId); context.put(SENTRY_SLUG_KEY, slug); - context.put(SENTRY_SCOPE_LIFECYCLE_TOKEN_KEY, lifecycleToken); } catch (Throwable t) { scopes .getOptions() @@ -84,6 +109,137 @@ public void jobToBeExecuted(final @NotNull JobExecutionContext context) { return null; } + private boolean shouldUpsertMonitorConfig(final @NotNull JobExecutionContext context) { + final @Nullable JobDataMap jobDataMap = context.getMergedJobDataMap(); + if (jobDataMap == null) { + return false; + } + final @Nullable Object o = jobDataMap.get(SENTRY_UPSERT_MONITOR_CONFIG_KEY); + return o != null && "true".equalsIgnoreCase(o.toString()); + } + + private @Nullable MonitorConfig monitorConfigFromTrigger(final @Nullable Trigger trigger) { + try { + // Sentry can't express calendar exclusions + if (trigger == null || trigger.getCalendarName() != null) { + return null; + } + if (trigger instanceof CronTrigger) { + final @NotNull CronTrigger cronTrigger = (CronTrigger) trigger; + final @Nullable String cron = toSixFieldCron(cronTrigger.getCronExpression()); + if (cron == null) { + return null; + } + final @Nullable TimeZone timeZone = cronTrigger.getTimeZone(); + return MonitorConfigUtils.fromSchedule( + cron, timeZone == null ? null : timeZone.getID(), null); + } + if (trigger instanceof SimpleTrigger) { + final @NotNull SimpleTrigger simpleTrigger = (SimpleTrigger) trigger; + if (simpleTrigger.getRepeatCount() != SimpleTrigger.REPEAT_INDEFINITELY) { + return null; + } + return MonitorConfigUtils.fromSchedule(null, null, simpleTrigger.getRepeatInterval()); + } + return null; + } catch (RuntimeException e) { + scopes + .getOptions() + .getLogger() + .log( + SentryLevel.WARNING, "Could not derive a monitor config from the Quartz trigger.", e); + return null; + } + } + + /** + * Quartz numbers days of week 1-7 from Sunday; crontab uses 0-6. Null if a year is set or a name + * has a step. + */ + static @Nullable String toSixFieldCron(final @Nullable String quartzCron) { + if (quartzCron == null) { + return null; + } + final @NotNull String[] fields = quartzCron.trim().split("\\s+", -1); + if (fields.length == 7 && !"*".equals(fields[6])) { + return null; + } + if (fields.length != 6 && fields.length != 7) { + return null; + } + if (NAME_STEP.matcher(fields[4]).find() || NAME_STEP.matcher(fields[5]).find()) { + return null; + } + final @Nullable String dayOfWeek = toZeroBasedDayOfWeek(fields[5]); + if (dayOfWeek == null) { + return null; + } + final @NotNull StringBuilder cron = new StringBuilder(); + for (int i = 0; i < 5; i++) { + cron.append(fields[i]).append(' '); + } + return cron.append(dayOfWeek).toString(); + } + + private static @Nullable String toZeroBasedDayOfWeek(final @NotNull String field) { + final @NotNull StringBuilder result = new StringBuilder(); + for (final @NotNull String item : field.split(",", -1)) { + final @Nullable String converted = toZeroBasedDayOfWeekItem(item.toUpperCase(Locale.ROOT)); + if (converted == null) { + return null; + } + if (result.length() > 0) { + result.append(','); + } + result.append(converted); + } + return result.toString(); + } + + private static @Nullable String toZeroBasedDayOfWeekItem(final @NotNull String item) { + if (item.startsWith("*") || item.startsWith("?")) { + // '*' starts on Sunday in both + return item; + } + final int hash = item.indexOf('#'); + if (hash >= 0) { + final @Nullable Integer day = toZeroBasedDay(item.substring(0, hash)); + return day == null ? null : day + item.substring(hash); + } + if (item.length() > 1 && item.endsWith("L")) { + final @Nullable Integer day = toZeroBasedDay(item.substring(0, item.length() - 1)); + return day == null ? null : day + "L"; + } + final int slash = item.indexOf('/'); + final @NotNull String days = slash >= 0 ? item.substring(0, slash) : item; + final @NotNull String step = slash >= 0 ? item.substring(slash) : ""; + final @NotNull String[] range = days.split("-", -1); + if (range.length > 2) { + return null; + } + final @Nullable Integer start = toZeroBasedDay(range[0]); + if (start == null) { + return null; + } + if (range.length == 2) { + final @Nullable Integer end = toZeroBasedDay(range[1]); + return end == null ? null : start + "-" + end + step; + } + if (slash < 0 || start == 6) { + return String.valueOf(start); + } + // a step from a single day ends on Saturday in Quartz, but on Sunday (7) in crontab + return start + "-6" + step; + } + + private static @Nullable Integer toZeroBasedDay(final @NotNull String day) { + if (day.matches("[1-7]")) { + return Integer.parseInt(day) - 1; + } + final int index = DAY_NAMES.indexOf(day); + return index >= 0 ? index : null; + } + @Override public void jobExecutionVetoed(JobExecutionContext context) { // do nothing diff --git a/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt b/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt new file mode 100644 index 00000000000..f61bd26f4e8 --- /dev/null +++ b/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt @@ -0,0 +1,280 @@ +package io.sentry.quartz + +import com.google.common.truth.Truth.assertThat +import io.sentry.CheckIn +import io.sentry.CheckInStatus +import io.sentry.IScopes +import io.sentry.ISentryLifecycleToken +import io.sentry.MonitorConfig +import io.sentry.SentryOptions +import io.sentry.protocol.SentryId +import java.util.TimeZone +import kotlin.test.BeforeTest +import kotlin.test.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.quartz.CalendarIntervalScheduleBuilder +import org.quartz.CronScheduleBuilder +import org.quartz.JobDataMap +import org.quartz.JobExecutionContext +import org.quartz.SimpleScheduleBuilder +import org.quartz.Trigger +import org.quartz.TriggerBuilder + +class SentryJobListenerTest { + + private val scopes = mock() + private val lifecycleToken = mock() + + @BeforeTest + fun setup() { + val forkedScopes = mock() + whenever(scopes.forkedScopes(any())).thenReturn(forkedScopes) + whenever(forkedScopes.makeCurrent()).thenReturn(lifecycleToken) + whenever(scopes.options).thenReturn(SentryOptions()) + } + + @Test + fun `cron trigger is sent as crontab monitor config with timezone`() { + val config = + inProgressMonitorConfig( + cronTrigger("0 15 10 ? * MON-FRI", TimeZone.getTimeZone("America/New_York")) + ) + + assertThat(config).isNotNull() + assertThat(config!!.schedule.type).isEqualTo("crontab") + assertThat(config.schedule.value).isEqualTo("15 10 * * 1-5") + assertThat(config.timezone).isEqualTo("America/New_York") + } + + @Test + fun `cron trigger with wildcard year is sent as crontab`() { + val config = inProgressMonitorConfig(cronTrigger("0 0 2 * * ? *")) + + assertThat(config?.schedule?.value).isEqualTo("0 2 * * *") + } + + @Test + fun `cron trigger with specific year sends no monitor config`() { + assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ? 2030"))).isNull() + } + + @Test + fun `cron trigger with variable seconds sends no monitor config`() { + assertThat(inProgressMonitorConfig(cronTrigger("0/30 * * * * ?"))).isNull() + } + + @Test + fun `cron trigger days of week are converted to start at zero`() { + val config = inProgressMonitorConfig(cronTrigger("0 0 9 ? * 2-6")) + + assertThat(config?.schedule?.value).isEqualTo("0 9 * * 1-5") + } + + @Test + fun `cron trigger with syntax Sentry rejects sends no monitor config`() { + for (cron in + listOf("0 0 12 15W * ?", "0 0 12 L-3 * ?", "0 0 12 ? * FRI-MON", "0 0 22-2 * * ?")) { + assertThat(inProgressMonitorConfig(cronTrigger(cron))).isNull() + } + } + + @Test + fun `cron trigger with whole hour offset zone is sent as Etc zone`() { + val config = inProgressMonitorConfig(cronTrigger("0 0 2 * * ?", TimeZone.getTimeZone("GMT+2"))) + + assertThat(config?.timezone).isEqualTo("Etc/GMT-2") + } + + @Test + fun `cron trigger with zone Sentry rejects sends no monitor config`() { + val trigger = cronTrigger("0 0 2 * * ?", TimeZone.getTimeZone("GMT+05:30")) + + assertThat(inProgressMonitorConfig(trigger)).isNull() + } + + @Test + fun `simple trigger with whole minute interval is sent as interval monitor config`() { + val trigger = + TriggerBuilder.newTrigger() + .withSchedule(SimpleScheduleBuilder.repeatMinutelyForever(5)) + .build() + + val config = inProgressMonitorConfig(trigger) + + assertThat(config).isNotNull() + assertThat(config!!.schedule.type).isEqualTo("interval") + assertThat(config.schedule.value).isEqualTo("5") + assertThat(config.schedule.unit).isEqualTo("minute") + } + + @Test + fun `simple trigger with sub minute interval sends no monitor config`() { + val trigger = + TriggerBuilder.newTrigger() + .withSchedule(SimpleScheduleBuilder.repeatSecondlyForever(90)) + .build() + + assertThat(inProgressMonitorConfig(trigger)).isNull() + } + + @Test + fun `simple trigger without repeat sends no monitor config`() { + val trigger = + TriggerBuilder.newTrigger().withSchedule(SimpleScheduleBuilder.simpleSchedule()).build() + + assertThat(inProgressMonitorConfig(trigger)).isNull() + } + + @Test + fun `simple trigger with a finite repeat count sends no monitor config`() { + val trigger = + TriggerBuilder.newTrigger() + .withSchedule(SimpleScheduleBuilder.repeatMinutelyForTotalCount(5)) + .build() + + assertThat(inProgressMonitorConfig(trigger)).isNull() + } + + @Test + fun `scope token is stored even if capturing the check-in fails`() { + val jobDataMap = upsertJobDataMap() + jobDataMap[SentryJobListener.SENTRY_SLUG_KEY] = "my-job" + val context = mock() + whenever(context.mergedJobDataMap).thenReturn(jobDataMap) + whenever(context.trigger).thenReturn(cronTrigger("0 0 2 * * ?")) + whenever(scopes.captureCheckIn(any())).thenThrow(IllegalStateException("boom")) + + SentryJobListener(scopes).jobToBeExecuted(context) + + verify(context).put(SentryJobListener.SENTRY_SCOPE_LIFECYCLE_TOKEN_KEY, lifecycleToken) + } + + @Test + fun `other triggers send no monitor config`() { + val trigger = + TriggerBuilder.newTrigger() + .withSchedule( + CalendarIntervalScheduleBuilder.calendarIntervalSchedule().withIntervalInDays(1) + ) + .build() + + assertThat(inProgressMonitorConfig(trigger)).isNull() + } + + @Test + fun `trigger with a calendar sends no monitor config`() { + val trigger = + TriggerBuilder.newTrigger() + .withSchedule(CronScheduleBuilder.cronSchedule("0 0 2 * * ?")) + .modifiedByCalendar("holidays") + .build() + + assertThat(inProgressMonitorConfig(trigger)).isNull() + } + + @Test + fun `without upsert monitor config key sends no monitor config`() { + assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ?"), JobDataMap())).isNull() + } + + @Test + fun `upsert monitor config false sends no monitor config`() { + val jobDataMap = JobDataMap() + jobDataMap[SentryJobListener.SENTRY_UPSERT_MONITOR_CONFIG_KEY] = "false" + + assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ?"), jobDataMap)).isNull() + } + + @Test + fun `upsert monitor config as boolean true sends monitor config`() { + val jobDataMap = JobDataMap() + jobDataMap[SentryJobListener.SENTRY_UPSERT_MONITOR_CONFIG_KEY] = true + + assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ?"), jobDataMap)).isNotNull() + } + + @Test + fun `failing trigger still sends the check-in without monitor config`() { + val trigger = mock() + whenever(trigger.cronExpression).thenThrow(IllegalStateException("boom")) + + assertThat(inProgressMonitorConfig(trigger)).isNull() + } + + @Test + fun `quartz cron is converted to six fields`() { + val expected = + mapOf( + "0 0 12 * * ?" to "0 0 12 * * ?", + "0 0 12 * * ? *" to "0 0 12 * * ?", + "0 0 12 ? * 1,7" to "0 0 12 ? * 0,6", + "0 0 12 ? * SUN,sat" to "0 0 12 ? * 0,6", + "0 0 12 ? * MON-FRI" to "0 0 12 ? * 1-5", + "0 0 12 ? * 6#3" to "0 0 12 ? * 5#3", + "0 0 12 ? * FRI#3" to "0 0 12 ? * 5#3", + "0 0 12 ? * 6L" to "0 0 12 ? * 5L", + "0 0 12 ? * FRIL" to "0 0 12 ? * 5L", + "0 0 12 ? * 1/2" to "0 0 12 ? * 0-6/2", + "0 0 12 ? * 2/2" to "0 0 12 ? * 1-6/2", + "0 0 12 ? * 7/2" to "0 0 12 ? * 6", + "0 0 12 ? * 2-6/2" to "0 0 12 ? * 1-5/2", + "0 0 12 ? * */2" to "0 0 12 ? * */2", + "0 0 12 L * ?" to "0 0 12 L * ?", + ) + for ((quartz, cron) in expected) { + assertThat(SentryJobListener.toSixFieldCron(quartz)).isEqualTo(cron) + } + } + + @Test + fun `quartz cron that cannot be converted returns null`() { + for (quartz in + listOf( + "0 0 12 * * ? 2030", + "0 0 12 ? * L", + "0 0 12 ? * 8", + "0 0 12 ? * XYZL", + "0 0 12 ? * SUN/2", + "0 0 12 ? * mon-wed/3", + "0 0 12 ? * 1,MON/2", + "0 0 12 * JUN/3 ?", + "0 12 * * *", + )) { + assertThat(SentryJobListener.toSixFieldCron(quartz)).isNull() + } + } + + private fun cronTrigger(cron: String, timeZone: TimeZone = TimeZone.getTimeZone("UTC")): Trigger = + TriggerBuilder.newTrigger() + .withSchedule(CronScheduleBuilder.cronSchedule(cron).inTimeZone(timeZone)) + .build() + + private fun upsertJobDataMap(): JobDataMap { + val jobDataMap = JobDataMap() + jobDataMap[SentryJobListener.SENTRY_UPSERT_MONITOR_CONFIG_KEY] = "true" + return jobDataMap + } + + private fun inProgressMonitorConfig( + trigger: Trigger, + jobDataMap: JobDataMap = upsertJobDataMap(), + ): MonitorConfig? { + jobDataMap[SentryJobListener.SENTRY_SLUG_KEY] = "my-job" + val context = mock() + whenever(context.mergedJobDataMap).thenReturn(jobDataMap) + whenever(context.trigger).thenReturn(trigger) + val checkInCaptor = argumentCaptor() + whenever(scopes.captureCheckIn(checkInCaptor.capture())).thenReturn(SentryId()) + + SentryJobListener(scopes).jobToBeExecuted(context) + + assertThat(checkInCaptor.allValues).hasSize(1) + assertThat(checkInCaptor.firstValue.monitorSlug).isEqualTo("my-job") + assertThat(checkInCaptor.firstValue.status).isEqualTo(CheckInStatus.IN_PROGRESS.apiName()) + return checkInCaptor.firstValue.monitorConfig + } +}