From ea62aa186978c0b0c993f44a9bec5806acfcb518 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 2 Oct 2026 12:53:47 -0700 Subject: [PATCH 01/18] ref(core): Add MonitorConfigUtils to convert schedules to monitor configs Framework-free conversion of a 6 field cron expression, fixed rate or fixed delay into a MonitorConfig, plus period parsing, so the Spring and Quartz integrations can share it. --- sentry/api/sentry.api | 5 + .../io/sentry/util/MonitorConfigUtils.java | 155 +++++++++++++++ .../io/sentry/util/MonitorConfigUtilsTest.kt | 185 ++++++++++++++++++ 3 files changed, 345 insertions(+) create mode 100644 sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java create mode 100644 sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 01b068ee689..bd41c383406 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -8120,6 +8120,11 @@ public final class io/sentry/util/MapObjectWriter : io/sentry/ObjectWriter { public fun value (Z)Lio/sentry/util/MapObjectWriter; } +public final class io/sentry/util/MonitorConfigUtils { + public static fun fromSchedule (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Ljava/lang/Long;)Lio/sentry/MonitorConfig; + public static fun parsePeriodMillis (Ljava/lang/String;Ljava/util/concurrent/TimeUnit;)Ljava/lang/Long; +} + public final class io/sentry/util/Objects { public static fun equals (Ljava/lang/Object;Ljava/lang/Object;)Z public static fun hash ([Ljava/lang/Object;)I diff --git a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java new file mode 100644 index 00000000000..584227c4e7e --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java @@ -0,0 +1,155 @@ +package io.sentry.util; + +import io.sentry.MonitorConfig; +import io.sentry.MonitorSchedule; +import io.sentry.MonitorScheduleUnit; +import java.util.Collections; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** Converts scheduler definitions into a {@link MonitorConfig}. */ +@ApiStatus.Internal +public final class MonitorConfigUtils { + private static final @NotNull String CRON_DISABLED = "-"; + private static final long MINUTE_MILLIS = TimeUnit.MINUTES.toMillis(1); + private static final @NotNull Map CRON_MACROS; + private static final @NotNull Pattern SIMPLE_DURATION = + Pattern.compile("^([+-]?\\d+)([a-zA-Z]{0,2})$"); + + static { + final @NotNull Map macros = new HashMap<>(); + macros.put("@yearly", "0 0 1 1 *"); + macros.put("@annually", "0 0 1 1 *"); + macros.put("@monthly", "0 0 1 * *"); + macros.put("@weekly", "0 0 * * 0"); + macros.put("@daily", "0 0 * * *"); + macros.put("@midnight", "0 0 * * *"); + macros.put("@hourly", "0 * * * *"); + CRON_MACROS = Collections.unmodifiableMap(macros); + } + + private MonitorConfigUtils() {} + + /** + * Builds a monitor config from exactly one of a cron, fixed rate or fixed delay. + * + * @param cron 6 field cron (seconds first) or macro, null or empty if unset + * @param zone cron time zone, ignored for intervals + * @param fixedRateMillis null if unset + * @param fixedDelayMillis null if unset + * @return null if the schedule can't be expressed as a Sentry monitor schedule + */ + public static @Nullable MonitorConfig fromSchedule( + final @Nullable String cron, + final @Nullable String zone, + final @Nullable Long fixedRateMillis, + final @Nullable Long fixedDelayMillis) { + final boolean hasCron = cron != null && !cron.isEmpty(); + + if (hasCron && cron != null && fixedRateMillis == null && fixedDelayMillis == null) { + final @Nullable String crontab = toCrontab(cron); + if (crontab == null) { + return null; + } + final @NotNull MonitorConfig config = new MonitorConfig(MonitorSchedule.crontab(crontab)); + if (zone != null && !zone.isEmpty()) { + config.setTimezone(zone); + } + return config; + } + + final @Nullable Long period = fixedRateMillis != null ? fixedRateMillis : fixedDelayMillis; + if (!hasCron && period != null && (fixedRateMillis == null || fixedDelayMillis == null)) { + final long millis = period; + if (millis < MINUTE_MILLIS || millis % MINUTE_MILLIS != 0) { + return null; + } + final long minutes = millis / MINUTE_MILLIS; + if (minutes > Integer.MAX_VALUE) { + return null; + } + return new MonitorConfig(MonitorSchedule.interval((int) minutes, MonitorScheduleUnit.MINUTE)); + } + return null; + } + + /** + * Parses a plain number in {@code defaultUnit} or a Spring 6.1 simple duration like {@code 30s}. + * ISO-8601 is not supported. + * + * @return milliseconds, or null if unparseable + */ + public static @Nullable Long parsePeriodMillis( + final @Nullable String value, final @NotNull TimeUnit defaultUnit) { + if (value == null) { + return null; + } + final @NotNull Matcher matcher = SIMPLE_DURATION.matcher(value.trim()); + if (!matcher.matches()) { + return null; + } + final long amount; + try { + amount = Long.parseLong(matcher.group(1)); + } catch (NumberFormatException e) { + return null; + } + final @Nullable TimeUnit unit = timeUnit(matcher.group(2), defaultUnit); + return unit == null ? null : unit.toMillis(amount); + } + + private static @Nullable TimeUnit timeUnit( + final @NotNull String suffix, final @NotNull TimeUnit defaultUnit) { + switch (suffix.toLowerCase(Locale.ROOT)) { + case "": + return defaultUnit; + case "ns": + return TimeUnit.NANOSECONDS; + case "us": + return TimeUnit.MICROSECONDS; + case "ms": + return TimeUnit.MILLISECONDS; + case "s": + return TimeUnit.SECONDS; + case "m": + return TimeUnit.MINUTES; + case "h": + return TimeUnit.HOURS; + case "d": + return TimeUnit.DAYS; + default: + return null; + } + } + + /** Drops a fixed seconds field; returns null otherwise, as crontab has no seconds. */ + static @Nullable String toCrontab(final @NotNull String cron) { + final @NotNull String trimmed = cron.trim(); + if (CRON_DISABLED.equals(trimmed)) { + return null; + } + if (trimmed.startsWith("@")) { + return CRON_MACROS.get(trimmed.toLowerCase(Locale.ROOT)); + } + final @NotNull String[] fields = trimmed.split("\\s+", -1); + if (fields.length != 6 || !fields[0].matches("\\d{1,2}")) { + return null; + } + final @NotNull StringBuilder crontab = new StringBuilder(); + for (int i = 1; i < fields.length; i++) { + if (i > 1) { + crontab.append(' '); + } + // '?' means '*' in Spring and Quartz cron + crontab.append("?".equals(fields[i]) ? "*" : fields[i]); + } + return crontab.toString(); + } +} diff --git a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt new file mode 100644 index 00000000000..a6c471763fa --- /dev/null +++ b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt @@ -0,0 +1,185 @@ +package io.sentry.util + +import com.google.common.truth.Truth.assertThat +import java.util.concurrent.TimeUnit +import kotlin.test.Test + +class MonitorConfigUtilsTest { + + @Test + fun `cron with fixed seconds drops the seconds field`() { + val config = MonitorConfigUtils.fromSchedule("0 15 10 * * MON-FRI", null, null, null) + + assertThat(config).isNotNull() + assertThat(config!!.schedule.type).isEqualTo("crontab") + assertThat(config.schedule.value).isEqualTo("15 10 * * MON-FRI") + assertThat(config.schedule.unit).isNull() + assertThat(config.timezone).isNull() + } + + @Test + fun `cron whitespace is normalized`() { + val config = MonitorConfigUtils.fromSchedule(" 30 0 2 * * * ", null, null, null) + + assertThat(config?.schedule?.value).isEqualTo("0 2 * * *") + } + + @Test + fun `cron question mark is converted to asterisk`() { + val config = MonitorConfigUtils.fromSchedule("0 0 9 ? * MON", null, null, null) + + assertThat(config?.schedule?.value).isEqualTo("0 9 * * MON") + } + + @Test + fun `cron macros are expanded`() { + val expected = + mapOf( + "@yearly" to "0 0 1 1 *", + "@annually" to "0 0 1 1 *", + "@monthly" to "0 0 1 * *", + "@weekly" to "0 0 * * 0", + "@daily" to "0 0 * * *", + "@midnight" to "0 0 * * *", + "@hourly" to "0 * * * *", + "@HOURLY" to "0 * * * *", + ) + for ((macro, crontab) in expected) { + assertThat(MonitorConfigUtils.fromSchedule(macro, null, null, null)?.schedule?.value) + .isEqualTo(crontab) + } + } + + @Test + fun `unknown cron macro returns null`() { + assertThat(MonitorConfigUtils.fromSchedule("@reboot", null, null, null)).isNull() + } + + @Test + fun `cron with variable seconds returns null`() { + assertThat(MonitorConfigUtils.fromSchedule("*/30 * * * * *", null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("0,30 * * * * *", null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("0-10 * * * * *", null, null, null)).isNull() + } + + @Test + fun `cron without 6 fields returns null`() { + assertThat(MonitorConfigUtils.fromSchedule("0 * * * *", null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("0 0 * * * * 2030", null, null, null)).isNull() + } + + @Test + fun `disabled cron returns null`() { + assertThat(MonitorConfigUtils.fromSchedule("-", null, null, null)).isNull() + } + + @Test + fun `zone is set for cron`() { + val config = MonitorConfigUtils.fromSchedule("0 0 2 * * *", "Europe/Vienna", null, null) + + assertThat(config?.schedule?.value).isEqualTo("0 2 * * *") + assertThat(config?.timezone).isEqualTo("Europe/Vienna") + } + + @Test + fun `empty zone is not set`() { + assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", "", null, null)?.timezone).isNull() + } + + @Test + fun `fixed rate in whole minutes is an interval`() { + val config = MonitorConfigUtils.fromSchedule(null, null, TimeUnit.MINUTES.toMillis(5), null) + + assertThat(config).isNotNull() + assertThat(config!!.schedule.type).isEqualTo("interval") + assertThat(config.schedule.value).isEqualTo("5") + assertThat(config.schedule.unit).isEqualTo("minute") + } + + @Test + fun `fixed delay in whole minutes is an interval`() { + val config = MonitorConfigUtils.fromSchedule("", null, null, TimeUnit.HOURS.toMillis(2)) + + assertThat(config?.schedule?.value).isEqualTo("120") + assertThat(config?.schedule?.unit).isEqualTo("minute") + } + + @Test + fun `zone is ignored for intervals`() { + val config = + MonitorConfigUtils.fromSchedule(null, "Europe/Vienna", TimeUnit.MINUTES.toMillis(1), null) + + assertThat(config?.schedule?.value).isEqualTo("1") + assertThat(config?.timezone).isNull() + } + + @Test + fun `sub-minute period returns null`() { + assertThat(MonitorConfigUtils.fromSchedule(null, null, 30_000L, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule(null, null, 0L, null)).isNull() + } + + @Test + fun `period not in whole minutes returns null`() { + assertThat(MonitorConfigUtils.fromSchedule(null, null, 90_000L, null)).isNull() + } + + @Test + fun `period too large for an int returns null`() { + assertThat( + MonitorConfigUtils.fromSchedule( + null, + null, + TimeUnit.MINUTES.toMillis(Int.MAX_VALUE.toLong() + 1), + null, + ) + ) + .isNull() + } + + @Test + fun `more than one schedule kind returns null`() { + assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", null, 60_000L, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule(null, null, 60_000L, 60_000L)).isNull() + } + + @Test + fun `no schedule returns null`() { + assertThat(MonitorConfigUtils.fromSchedule(null, null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("", null, null, null)).isNull() + } + + @Test + fun `parsePeriodMillis parses plain numbers in the default unit`() { + assertThat(MonitorConfigUtils.parsePeriodMillis("300000", TimeUnit.MILLISECONDS)) + .isEqualTo(300_000L) + assertThat(MonitorConfigUtils.parsePeriodMillis(" 5 ", TimeUnit.MINUTES)).isEqualTo(300_000L) + } + + @Test + fun `parsePeriodMillis parses the simple duration style`() { + val expected = + mapOf( + "2000000ns" to 2L, + "3000us" to 3L, + "500ms" to 500L, + "30s" to 30_000L, + "5m" to 300_000L, + "1h" to 3_600_000L, + "1d" to 86_400_000L, + "5M" to 300_000L, + "+5m" to 300_000L, + ) + for ((value, millis) in expected) { + assertThat(MonitorConfigUtils.parsePeriodMillis(value, TimeUnit.SECONDS)).isEqualTo(millis) + } + } + + @Test + fun `parsePeriodMillis returns null for unparseable values`() { + for (value in listOf("", "abc", "5x", "5 m", "1.5m", "PT5M", "99999999999999999999")) { + assertThat(MonitorConfigUtils.parsePeriodMillis(value, TimeUnit.MILLISECONDS)).isNull() + } + assertThat(MonitorConfigUtils.parsePeriodMillis(null, TimeUnit.MILLISECONDS)).isNull() + } +} From 28761ae677b0c8fdb43bfab4e3404d905168e402 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 2 Oct 2026 13:30:26 -0700 Subject: [PATCH 02/18] fix(core): Only send monitor configs Sentry reads the same way Return no config when day of month and day of week are both set, for cron syntax Sentry rejects, and for zones that are not IANA region IDs. Whole-hour fixed offsets map to Etc/GMT zones. Intervals use hours or days when they divide evenly. --- .../io/sentry/util/MonitorConfigUtils.java | 250 +++++++++++++++++- .../io/sentry/util/MonitorConfigUtilsTest.kt | 137 +++++++++- 2 files changed, 372 insertions(+), 15 deletions(-) diff --git a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java index 584227c4e7e..a96def263a0 100644 --- a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java +++ b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java @@ -3,10 +3,16 @@ import io.sentry.MonitorConfig; import io.sentry.MonitorSchedule; import io.sentry.MonitorScheduleUnit; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; +import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; +import java.util.TimeZone; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -22,6 +28,34 @@ public final class MonitorConfigUtils { private static final @NotNull Map CRON_MACROS; private static final @NotNull Pattern SIMPLE_DURATION = Pattern.compile("^([+-]?\\d+)([a-zA-Z]{0,2})$"); + private static final @NotNull Pattern FIXED_OFFSET = + Pattern.compile("^(?:GMT|UTC)?([+-])(\\d{1,2})(?::?(\\d{2}))?$"); + private static final @NotNull Pattern DIGITS = Pattern.compile("\\d+"); + + private static final int DAY_OF_MONTH = 2; + private static final int MONTH = 3; + private static final int DAY_OF_WEEK = 4; + private static final int[] FIELD_MIN = {0, 0, 1, 1, 0}; + private static final int[] FIELD_MAX = {59, 23, 31, 12, 7}; + private static final int LAST = -1; + private static final int[] DAYS_IN_MONTH = {31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; + private static final @NotNull List MONTH_NAMES = + Arrays.asList( + "JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC"); + private static final @NotNull List DAY_NAMES = + Arrays.asList("SUN", "MON", "TUE", "WED", "THU", "FRI", "SAT"); + + /** IANA zones without a slash; Java also knows non-IANA short IDs like PST. */ + private static final @NotNull Set TOP_LEVEL_ZONES = + new HashSet<>( + Arrays.asList( + ("CET CST6CDT Cuba EET EST EST5EDT Egypt Eire GB GB-Eire GMT GMT+0 GMT-0 GMT0 " + + "Greenwich HST Hongkong Iceland Iran Israel Jamaica Japan Kwajalein Libya " + + "MET MST MST7MDT NZ NZ-CHAT Navajo PRC PST8PDT Poland Portugal ROC ROK " + + "Singapore Turkey UCT UTC Universal W-SU WET Zulu") + .split(" "))); + + private static volatile @Nullable Set availableZoneIds; static { final @NotNull Map macros = new HashMap<>(); @@ -60,7 +94,11 @@ private MonitorConfigUtils() {} } final @NotNull MonitorConfig config = new MonitorConfig(MonitorSchedule.crontab(crontab)); if (zone != null && !zone.isEmpty()) { - config.setTimezone(zone); + final @Nullable String timezone = toTimezone(zone); + if (timezone == null) { + return null; + } + config.setTimezone(timezone); } return config; } @@ -71,11 +109,19 @@ private MonitorConfigUtils() {} if (millis < MINUTE_MILLIS || millis % MINUTE_MILLIS != 0) { return null; } - final long minutes = millis / MINUTE_MILLIS; - if (minutes > Integer.MAX_VALUE) { + long value = millis / MINUTE_MILLIS; + @NotNull MonitorScheduleUnit unit = MonitorScheduleUnit.MINUTE; + if (value % (24 * 60) == 0) { + value /= 24 * 60; + unit = MonitorScheduleUnit.DAY; + } else if (value % 60 == 0) { + value /= 60; + unit = MonitorScheduleUnit.HOUR; + } + if (value > Integer.MAX_VALUE) { return null; } - return new MonitorConfig(MonitorSchedule.interval((int) minutes, MonitorScheduleUnit.MINUTE)); + return new MonitorConfig(MonitorSchedule.interval((int) value, unit)); } return null; } @@ -129,7 +175,50 @@ private MonitorConfigUtils() {} } } - /** Drops a fixed seconds field; returns null otherwise, as crontab has no seconds. */ + /** + * Returns a zone ID Sentry accepts: IANA region IDs as is, whole-hour fixed offsets like {@code + * GMT+2} as {@code Etc/GMT-2}, null otherwise. + */ + static @Nullable String toTimezone(final @NotNull String zone) { + final @NotNull Matcher offset = FIXED_OFFSET.matcher(zone); + if (offset.matches()) { + final @Nullable String minutes = offset.group(3); + if (minutes != null && !"00".equals(minutes)) { + return null; + } + final int hours = Integer.parseInt(offset.group(2)); + if (hours == 0) { + return "Etc/GMT"; + } + final boolean east = "+".equals(offset.group(1)); + if (hours > (east ? 14 : 12)) { + return null; + } + // Etc/GMT signs are inverted + return "Etc/GMT" + (east ? "-" : "+") + hours; + } + if (TOP_LEVEL_ZONES.contains(zone)) { + return zone; + } + if (zone.indexOf('/') > 0 && !zone.startsWith("SystemV/") && availableZones().contains(zone)) { + return zone; + } + return null; + } + + private static @NotNull Set availableZones() { + @Nullable Set zones = availableZoneIds; + if (zones == null) { + zones = new HashSet<>(Arrays.asList(TimeZone.getAvailableIDs())); + availableZoneIds = zones; + } + return zones; + } + + /** + * Drops a fixed seconds field. Returns null if Sentry would reject the result or read it + * differently, as crontab has no seconds and ORs day of month and day of week. + */ static @Nullable String toCrontab(final @NotNull String cron) { final @NotNull String trimmed = cron.trim(); if (CRON_DISABLED.equals(trimmed)) { @@ -139,17 +228,152 @@ private MonitorConfigUtils() {} return CRON_MACROS.get(trimmed.toLowerCase(Locale.ROOT)); } final @NotNull String[] fields = trimmed.split("\\s+", -1); - if (fields.length != 6 || !fields[0].matches("\\d{1,2}")) { + if (fields.length != 6 || !fields[0].matches("\\d{1,2}") || Integer.parseInt(fields[0]) > 59) { return null; } - final @NotNull StringBuilder crontab = new StringBuilder(); - for (int i = 1; i < fields.length; i++) { - if (i > 1) { - crontab.append(' '); - } + final @NotNull String[] crontab = new String[5]; + for (int i = 0; i < crontab.length; i++) { + final @NotNull String field = fields[i + 1]; // '?' means '*' in Spring and Quartz cron - crontab.append("?".equals(fields[i]) ? "*" : fields[i]); + final boolean dayField = i == DAY_OF_MONTH || i == DAY_OF_WEEK; + crontab[i] = dayField && "?".equals(field) ? "*" : field; + } + // crontab ORs day of month and day of week unless one starts with '*', Spring and Quartz AND + if (!crontab[DAY_OF_MONTH].startsWith("*") && !crontab[DAY_OF_WEEK].startsWith("*")) { + return null; + } + final @NotNull List> values = new ArrayList<>(); + for (int i = 0; i < crontab.length; i++) { + final @Nullable Set parsed = parseField(crontab[i].toUpperCase(Locale.ROOT), i); + if (parsed == null) { + return null; + } + values.add(parsed); + } + if (!dayExistsInMonths(values.get(DAY_OF_MONTH), values.get(MONTH))) { + return null; + } + final @NotNull StringBuilder result = new StringBuilder(); + for (int i = 0; i < crontab.length; i++) { + if (i > 0) { + result.append(' '); + } + result.append(crontab[i]); + } + return result.toString(); + } + + // Mirrors the parser Sentry uses (cronsim). Returns null for anything it rejects or that we + // can't send as is. Day of week L and # entries are not expanded, as only the count matters. + private static @Nullable Set parseField(final @NotNull String field, final int index) { + final @NotNull Set result = new HashSet<>(); + for (final @NotNull String term : field.split(",", -1)) { + final @Nullable Set parsed = parseTerm(term, index); + if (parsed == null) { + return null; + } + result.addAll(parsed); + } + return result; + } + + private static @Nullable Set parseTerm(final @NotNull String term, final int index) { + if ("*".equals(term)) { + return range(FIELD_MIN[index], FIELD_MAX[index], 1); + } + if (index == DAY_OF_WEEK && term.indexOf('L') >= 0) { + final @NotNull String day = term.substring(0, term.length() - 1); + if (!term.endsWith("L") || !DIGITS.matcher(day).matches() || value(day, index) == null) { + return null; + } + return Collections.singleton(LAST); + } + if (index == DAY_OF_WEEK && term.indexOf('#') >= 0) { + final int hash = term.indexOf('#'); + final @Nullable Integer nth = number(term.substring(hash + 1)); + if (nth == null || nth < 1 || nth > 5 || value(term.substring(0, hash), index) == null) { + return null; + } + return Collections.singleton(LAST); + } + if (term.indexOf('/') >= 0) { + final int slash = term.indexOf('/'); + final @Nullable Integer step = number(term.substring(slash + 1)); + final @NotNull String base = term.substring(0, slash); + if (step == null || step == 0 || "L".equals(base) || "LW".equals(base)) { + return null; + } + final @Nullable Set items = parseTerm(base, index); + if (items == null) { + return null; + } + if (items.size() == 1) { + return range(items.iterator().next(), FIELD_MAX[index], step); + } + final @NotNull List sorted = new ArrayList<>(items); + Collections.sort(sorted); + final @NotNull Set result = new HashSet<>(); + for (int i = 0; i < sorted.size(); i += step) { + result.add(sorted.get(i)); + } + return result; + } + if (term.indexOf('-') >= 0) { + final int dash = term.indexOf('-'); + final @Nullable Integer start = value(term.substring(0, dash), index); + final @Nullable Integer end = value(term.substring(dash + 1), index); + if (start == null || end == null || end < start) { + return null; + } + return range(start, end, 1); + } + if (index == DAY_OF_MONTH && ("L".equals(term) || "LW".equals(term))) { + return Collections.singleton(LAST); + } + final @Nullable Integer value = value(term, index); + return value == null ? null : Collections.singleton(value); + } + + private static @Nullable Integer value(final @NotNull String value, final int index) { + if (index == MONTH && MONTH_NAMES.contains(value)) { + return MONTH_NAMES.indexOf(value) + 1; + } + if (index == DAY_OF_WEEK && DAY_NAMES.contains(value)) { + return DAY_NAMES.indexOf(value); + } + final @Nullable Integer number = number(value); + if (number == null || number < FIELD_MIN[index] || number > FIELD_MAX[index]) { + return null; + } + return number; + } + + private static @Nullable Integer number(final @NotNull String value) { + if (value.isEmpty() || value.length() > 9 || !DIGITS.matcher(value).matches()) { + return null; + } + return Integer.parseInt(value); + } + + private static @NotNull Set range(final int start, final int end, final int step) { + final @NotNull Set result = new HashSet<>(); + for (int i = start; i <= end; i += step) { + result.add(i); + } + return result; + } + + private static boolean dayExistsInMonths( + final @NotNull Set days, final @NotNull Set months) { + final int firstDay = Collections.min(days); + if (firstDay <= 29) { + return true; + } + for (final int month : months) { + if (firstDay <= DAYS_IN_MONTH[month - 1]) { + return true; + } } - return crontab.toString(); + return false; } } diff --git a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt index a6c471763fa..cee9d2a7141 100644 --- a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt @@ -68,6 +68,88 @@ class MonitorConfigUtilsTest { assertThat(MonitorConfigUtils.fromSchedule("0 0 * * * * 2030", null, null, null)).isNull() } + @Test + fun `cron with both day of month and day of week returns null`() { + for (cron in listOf("0 0 9 1-7 * MON", "0 0 9 15 * 1-5", "0 0 9 L * 5L", "0 0 9 1 * 1/2")) { + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null, null)).isNull() + } + } + + @Test + fun `cron with a day field starting with asterisk is kept`() { + val expected = + mapOf( + "0 0 9 */2 * MON" to "0 9 */2 * MON", + "0 0 9 15 * */2" to "0 9 15 * */2", + "0 0 9 1 * ?" to "0 9 1 * *", + ) + for ((cron, crontab) in expected) { + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null, null)?.schedule?.value) + .isEqualTo(crontab) + } + } + + @Test + fun `cron syntax Sentry accepts is kept`() { + val expected = + listOf( + "0 */15 * * * *", + "0 5/15 * * * *", + "0 0,30 8-18/2 * * *", + "0 0 0 1,15 JAN-MAR,dec *", + "0 0 0 L * *", + "0 0 0 lw * *", + "0 0 0 31 1-12 *", + "0 0 0 * * 0,7", + "0 0 0 * * sat", + "0 0 0 * * MON-FRI/2", + "0 0 0 * * 5L", + "0 0 0 * * 5#3", + "0 0 0 * * MON#2", + "0 0 0 * * 1-7/2", + ) + for (cron in expected) { + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null, null)?.schedule?.value) + .isEqualTo(cron.substringAfter(' ')) + } + } + + @Test + fun `cron syntax Sentry rejects returns null`() { + val rejected = + listOf( + "60 * * * * *", + "0 60 * * * *", + "0 0 24 * * *", + "0 0 22-2 * * *", + "0 */0 * * * *", + "0 0,,30 * * * *", + "0 ? * * * *", + "0 0 0 0 * *", + "0 0 0 32 * *", + "0 0 0 15W * *", + "0 0 0 LW/2 * *", + "0 0 0 L-3 * *", + "0 0 0 5L * *", + "0 0 0 1#2 * *", + "0 0 0 30 2 *", + "0 0 0 31 4,6 *", + "0 0 0 * 0 *", + "0 0 0 * 13 *", + "0 0 0 * FOO *", + "0 0 0 * * 8", + "0 0 0 * * SAT-SUN", + "0 0 0 * * L", + "0 0 0 * * FRIL", + "0 0 0 * * L5", + "0 0 0 * * 5#6", + "0 0 0 * * 5#0", + ) + for (cron in rejected) { + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null, null)).isNull() + } + } + @Test fun `disabled cron returns null`() { assertThat(MonitorConfigUtils.fromSchedule("-", null, null, null)).isNull() @@ -81,6 +163,41 @@ class MonitorConfigUtilsTest { assertThat(config?.timezone).isEqualTo("Europe/Vienna") } + @Test + fun `IANA zones are kept`() { + for (zone in + listOf("UTC", "GMT", "Etc/GMT+5", "US/Eastern", "America/Argentina/Buenos_Aires")) { + assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", zone, null, null)?.timezone) + .isEqualTo(zone) + } + } + + @Test + fun `whole hour fixed offsets are converted to Etc zones`() { + val expected = + mapOf( + "GMT+2" to "Etc/GMT-2", + "GMT+02:00" to "Etc/GMT-2", + "UTC-5" to "Etc/GMT+5", + "+03:00" to "Etc/GMT-3", + "-1200" to "Etc/GMT+12", + "GMT+14" to "Etc/GMT-14", + "GMT-00:00" to "Etc/GMT", + ) + for ((zone, timezone) in expected) { + assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", zone, null, null)?.timezone) + .isEqualTo(timezone) + } + } + + @Test + fun `zones Sentry rejects return null`() { + for (zone in + listOf("GMT+05:30", "GMT+15", "UTC-13", "PST", "IST", "Nowhere/Land", "SystemV/EST5")) { + assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", zone, null, null)).isNull() + } + } + @Test fun `empty zone is not set`() { assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", "", null, null)?.timezone).isNull() @@ -98,12 +215,28 @@ class MonitorConfigUtilsTest { @Test fun `fixed delay in whole minutes is an interval`() { - val config = MonitorConfigUtils.fromSchedule("", null, null, TimeUnit.HOURS.toMillis(2)) + val config = MonitorConfigUtils.fromSchedule("", null, null, TimeUnit.MINUTES.toMillis(90)) - assertThat(config?.schedule?.value).isEqualTo("120") + assertThat(config?.schedule?.value).isEqualTo("90") assertThat(config?.schedule?.unit).isEqualTo("minute") } + @Test + fun `period in whole hours or days uses the largest unit`() { + val expected = + mapOf( + TimeUnit.MINUTES.toMillis(60) to ("1" to "hour"), + TimeUnit.HOURS.toMillis(36) to ("36" to "hour"), + TimeUnit.HOURS.toMillis(24) to ("1" to "day"), + TimeUnit.DAYS.toMillis(14) to ("14" to "day"), + ) + for ((millis, interval) in expected) { + val config = MonitorConfigUtils.fromSchedule(null, null, millis, null) + assertThat(config?.schedule?.value).isEqualTo(interval.first) + assertThat(config?.schedule?.unit).isEqualTo(interval.second) + } + } + @Test fun `zone is ignored for intervals`() { val config = From bd470268326fee4d8b6cc7594b5217160887e84d Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 2 Oct 2026 15:38:46 -0700 Subject: [PATCH 03/18] ref(core): Add MonitorConfigUtils.fromSpringScheduled Move the Spring day-of-week rewrite and the JVM default zone rule into core so the three Spring modules share them. --- sentry/api/sentry.api | 1 + .../io/sentry/util/MonitorConfigUtils.java | 62 +++++++++++++++++ .../io/sentry/util/MonitorConfigUtilsTest.kt | 66 +++++++++++++++++++ 3 files changed, 129 insertions(+) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index bd41c383406..57510ebf0a3 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -8122,6 +8122,7 @@ public final class io/sentry/util/MapObjectWriter : io/sentry/ObjectWriter { public final class io/sentry/util/MonitorConfigUtils { public static fun fromSchedule (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Ljava/lang/Long;)Lio/sentry/MonitorConfig; + public static fun fromSpringScheduled (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Ljava/lang/Long;)Lio/sentry/MonitorConfig; public static fun parsePeriodMillis (Ljava/lang/String;Ljava/util/concurrent/TimeUnit;)Ljava/lang/Long; } diff --git a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java index a96def263a0..b1ddedd3d4f 100644 --- a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java +++ b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java @@ -126,6 +126,68 @@ private MonitorConfigUtils() {} return null; } + /** + * Builds a monitor config from Spring {@code @Scheduled} values with placeholders resolved. Reads + * days of week and a cron without zone the way Spring does. + * + * @param cron Spring cron, null or empty if unset + * @param zone cron time zone, null or empty for the JVM default zone + * @param fixedRateMillis null if unset + * @param fixedDelayMillis null if unset + * @return null if the schedule can't be expressed as a Sentry monitor schedule + */ + public static @Nullable MonitorConfig fromSpringScheduled( + final @Nullable String cron, + final @Nullable String zone, + final @Nullable Long fixedRateMillis, + final @Nullable Long fixedDelayMillis) { + final @Nullable String crontabCron = toCrontabDaysOfWeek(cron); + @Nullable String cronZone = null; + if (crontabCron != null && !crontabCron.isEmpty()) { + cronZone = zone == null || zone.isEmpty() ? TimeZone.getDefault().getID() : zone; + } + return fromSchedule(crontabCron, cronZone, fixedRateMillis, fixedDelayMillis); + } + + /** + * Spring numbers days of week from Monday with 0 or 7 for Sunday, and starts {@code *} on Monday. + * Rewrites them so crontab reads them the same way. + */ + static @Nullable String toCrontabDaysOfWeek(final @Nullable String cron) { + if (cron == null) { + return null; + } + final @NotNull String[] fields = cron.trim().split("\\s+", -1); + if (fields.length != 6) { + return cron; + } + final @NotNull StringBuilder daysOfWeek = new StringBuilder(); + for (@NotNull String item : fields[5].toUpperCase(Locale.ROOT).split(",", -1)) { + for (int i = 0; i < DAY_NAMES.size(); i++) { + item = item.replace(DAY_NAMES.get(i), String.valueOf(i == 0 ? 7 : i)); + } + if (item.startsWith("*/")) { + item = "1-7" + item.substring(1); + } else if (item.startsWith("7-")) { + // Sunday starting a range is 0 in Spring + item = "0" + item.substring(1); + } + if (daysOfWeek.length() > 0) { + daysOfWeek.append(','); + } + daysOfWeek.append(item); + } + fields[5] = daysOfWeek.toString(); + final @NotNull StringBuilder result = new StringBuilder(); + for (int i = 0; i < fields.length; i++) { + if (i > 0) { + result.append(' '); + } + result.append(fields[i]); + } + return result.toString(); + } + /** * Parses a plain number in {@code defaultUnit} or a Spring 6.1 simple duration like {@code 30s}. * ISO-8601 is not supported. diff --git a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt index cee9d2a7141..fd476ceb4ea 100644 --- a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt @@ -1,6 +1,7 @@ package io.sentry.util import com.google.common.truth.Truth.assertThat +import java.util.TimeZone import java.util.concurrent.TimeUnit import kotlin.test.Test @@ -282,6 +283,71 @@ class MonitorConfigUtilsTest { assertThat(MonitorConfigUtils.fromSchedule("", null, null, null)).isNull() } + @Test + fun `Spring days of week are rewritten to how crontab reads them`() { + val expected = + mapOf( + "0 15 10 * * MON-FRI" to "15 10 * * 1-5", + "0 0 9 * * */2" to "0 9 * * 1-7/2", + "0 0 9 ? * SUN/2" to "0 9 * * 7/2", + "0 0 9 ? * SUN-TUE,fri-sun" to "0 9 * * 0-2,5-7", + "0 0 9 ? * 7-2" to "0 9 * * 0-2", + "0 0 9 ? * 0,7" to "0 9 * * 0,7", + "0 0 9 ? * FRIL" to "0 9 * * 5L", + "0 0 9 ? * MON#2" to "0 9 * * 1#2", + "0 0 9 * * *" to "0 9 * * *", + ) + for ((cron, crontab) in expected) { + assertThat(MonitorConfigUtils.fromSpringScheduled(cron, "UTC", null, null)?.schedule?.value) + .isEqualTo(crontab) + } + } + + @Test + fun `Spring cron without zone uses the JVM default zone`() { + val defaultTimeZone = TimeZone.getDefault() + TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")) + try { + assertThat(MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", null, null, null)?.timezone) + .isEqualTo("Asia/Tokyo") + assertThat(MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", "", null, null)?.timezone) + .isEqualTo("Asia/Tokyo") + } finally { + TimeZone.setDefault(defaultTimeZone) + } + } + + @Test + fun `Spring cron zone is converted`() { + val config = MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", "GMT+2", null, null) + + assertThat(config?.schedule?.value).isEqualTo("0 2 * * *") + assertThat(config?.timezone).isEqualTo("Etc/GMT-2") + } + + @Test + fun `Spring schedules Sentry can't express return null`() { + for (cron in listOf("0 0 9 1-7 * MON", "-", "\${my.cron.missing}", "*/30 * * * * *")) { + assertThat(MonitorConfigUtils.fromSpringScheduled(cron, "UTC", null, null)).isNull() + } + assertThat(MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", "GMT+05:30", null, null)) + .isNull() + assertThat(MonitorConfigUtils.fromSpringScheduled(null, null, 30_000L, null)).isNull() + } + + @Test + fun `Spring periods are intervals without zone`() { + val rate = MonitorConfigUtils.fromSpringScheduled("", null, TimeUnit.MINUTES.toMillis(5), null) + assertThat(rate?.schedule?.type).isEqualTo("interval") + assertThat(rate?.schedule?.value).isEqualTo("5") + assertThat(rate?.schedule?.unit).isEqualTo("minute") + assertThat(rate?.timezone).isNull() + + val delay = MonitorConfigUtils.fromSpringScheduled(null, null, null, TimeUnit.HOURS.toMillis(2)) + assertThat(delay?.schedule?.value).isEqualTo("2") + assertThat(delay?.schedule?.unit).isEqualTo("hour") + } + @Test fun `parsePeriodMillis parses plain numbers in the default unit`() { assertThat(MonitorConfigUtils.parsePeriodMillis("300000", TimeUnit.MILLISECONDS)) From fca421637cd3ac3514606c56bb9cd5ee10db7131 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 2 Oct 2026 16:29:03 -0700 Subject: [PATCH 04/18] fix(core): Return no config for stepped single value ranges Spring and Quartz run 10-10/2 only at 10, cronsim steps to the field max. --- .../src/main/java/io/sentry/util/MonitorConfigUtils.java | 4 ++++ .../test/java/io/sentry/util/MonitorConfigUtilsTest.kt | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java index b1ddedd3d4f..8d4bd832d49 100644 --- a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java +++ b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java @@ -370,6 +370,10 @@ private MonitorConfigUtils() {} return null; } if (items.size() == 1) { + // cronsim steps a single value range like 10-10/2 to the field max + if (base.indexOf('-') >= 0) { + return null; + } return range(items.iterator().next(), FIELD_MAX[index], step); } final @NotNull List sorted = new ArrayList<>(items); diff --git a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt index fd476ceb4ea..d6976f7d148 100644 --- a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt @@ -151,6 +151,15 @@ class MonitorConfigUtilsTest { } } + @Test + fun `stepped single value range returns null`() { + // Spring and Quartz run only at the value, cronsim steps to the field max + for (cron in + listOf("0 10-10/2 * * * *", "0 0 0-0/3 * * *", "0 0 0 ? * 4-4/3", "0 0 0 * 6-6/1 *")) { + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null, null)).isNull() + } + } + @Test fun `disabled cron returns null`() { assertThat(MonitorConfigUtils.fromSchedule("-", null, null, null)).isNull() From 780c60501bc140b5164f094243f91f58870b3ce4 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 2 Oct 2026 16:47:19 -0700 Subject: [PATCH 05/18] fix(core): Read Spring crons per parser and skip fixed delays Spring before 5.3 steps day of month */n from 0 and can run at midnight when both day fields are set; read its crons that way. Spring 5.3 runs #5 in months without a fifth weekday, so return null for it. A fixed delay drifts from any interval, so it gets no config. --- sentry/api/sentry.api | 4 +- .../io/sentry/util/MonitorConfigUtils.java | 76 +++++++-- .../io/sentry/util/MonitorConfigUtilsTest.kt | 154 ++++++++++++------ 3 files changed, 166 insertions(+), 68 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 57510ebf0a3..a6f573385f7 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -8121,8 +8121,8 @@ public final class io/sentry/util/MapObjectWriter : io/sentry/ObjectWriter { } public final class io/sentry/util/MonitorConfigUtils { - public static fun fromSchedule (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Ljava/lang/Long;)Lio/sentry/MonitorConfig; - public static fun fromSpringScheduled (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Ljava/lang/Long;)Lio/sentry/MonitorConfig; + public static fun fromSchedule (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;)Lio/sentry/MonitorConfig; + public static fun fromSpringScheduled (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Ljava/lang/Long;Z)Lio/sentry/MonitorConfig; public static fun parsePeriodMillis (Ljava/lang/String;Ljava/util/concurrent/TimeUnit;)Ljava/lang/Long; } diff --git a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java index 8d4bd832d49..fd4b2675690 100644 --- a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java +++ b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java @@ -72,22 +72,20 @@ public final class MonitorConfigUtils { private MonitorConfigUtils() {} /** - * Builds a monitor config from exactly one of a cron, fixed rate or fixed delay. + * Builds a monitor config from exactly one of a cron or a fixed interval. * * @param cron 6 field cron (seconds first) or macro, null or empty if unset * @param zone cron time zone, ignored for intervals - * @param fixedRateMillis null if unset - * @param fixedDelayMillis null if unset + * @param intervalMillis fixed rate, null if unset * @return null if the schedule can't be expressed as a Sentry monitor schedule */ public static @Nullable MonitorConfig fromSchedule( final @Nullable String cron, final @Nullable String zone, - final @Nullable Long fixedRateMillis, - final @Nullable Long fixedDelayMillis) { + final @Nullable Long intervalMillis) { final boolean hasCron = cron != null && !cron.isEmpty(); - if (hasCron && cron != null && fixedRateMillis == null && fixedDelayMillis == null) { + if (hasCron && cron != null && intervalMillis == null) { final @Nullable String crontab = toCrontab(cron); if (crontab == null) { return null; @@ -103,9 +101,8 @@ private MonitorConfigUtils() {} return config; } - final @Nullable Long period = fixedRateMillis != null ? fixedRateMillis : fixedDelayMillis; - if (!hasCron && period != null && (fixedRateMillis == null || fixedDelayMillis == null)) { - final long millis = period; + if (!hasCron && intervalMillis != null) { + final long millis = intervalMillis; if (millis < MINUTE_MILLIS || millis % MINUTE_MILLIS != 0) { return null; } @@ -128,30 +125,72 @@ private MonitorConfigUtils() {} /** * Builds a monitor config from Spring {@code @Scheduled} values with placeholders resolved. Reads - * days of week and a cron without zone the way Spring does. + * the cron and a cron without zone the way Spring does. * * @param cron Spring cron, null or empty if unset * @param zone cron time zone, null or empty for the JVM default zone * @param fixedRateMillis null if unset - * @param fixedDelayMillis null if unset + * @param fixedDelayMillis null if unset; a fixed delay never gets a config + * @param legacyCronParser true for Spring before 5.3, which parses crons with {@code + * CronSequenceGenerator} * @return null if the schedule can't be expressed as a Sentry monitor schedule */ public static @Nullable MonitorConfig fromSpringScheduled( final @Nullable String cron, final @Nullable String zone, final @Nullable Long fixedRateMillis, - final @Nullable Long fixedDelayMillis) { - final @Nullable String crontabCron = toCrontabDaysOfWeek(cron); + final @Nullable Long fixedDelayMillis, + final boolean legacyCronParser) { + // runs start a delay after the previous run ends, so they drift from any interval + if (fixedDelayMillis != null) { + return null; + } + final @Nullable String crontabCron = + legacyCronParser ? toCrontabLegacy(cron) : toCrontabDaysOfWeek(cron); @Nullable String cronZone = null; if (crontabCron != null && !crontabCron.isEmpty()) { cronZone = zone == null || zone.isEmpty() ? TimeZone.getDefault().getID() : zone; } - return fromSchedule(crontabCron, cronZone, fixedRateMillis, fixedDelayMillis); + return fromSchedule(crontabCron, cronZone, fixedRateMillis); + } + + /** + * Spring before 5.3 steps day of month {@code *}/n from 0, so it runs on n, 2n, ... Days of week + * already match crontab. Null if both day fields are set, as it then sometimes runs at midnight. + */ + static @Nullable String toCrontabLegacy(final @Nullable String cron) { + if (cron == null) { + return null; + } + final @NotNull String[] fields = cron.trim().split("\\s+", -1); + if (fields.length != 6) { + return cron; + } + if (!isAnyDay(fields[3]) && !isAnyDay(fields[5])) { + return null; + } + final @NotNull StringBuilder daysOfMonth = new StringBuilder(); + for (@NotNull String item : fields[3].split(",", -1)) { + if (item.startsWith("*/")) { + item = item.substring(2) + "-31" + item.substring(1); + } + if (daysOfMonth.length() > 0) { + daysOfMonth.append(','); + } + daysOfMonth.append(item); + } + fields[3] = daysOfMonth.toString(); + return join(fields); + } + + private static boolean isAnyDay(final @NotNull String field) { + return "*".equals(field) || "?".equals(field); } /** * Spring numbers days of week from Monday with 0 or 7 for Sunday, and starts {@code *} on Monday. - * Rewrites them so crontab reads them the same way. + * Rewrites them so crontab reads them the same way. Null for {@code #5}, which Spring 5.3 also + * runs in months without a fifth weekday. */ static @Nullable String toCrontabDaysOfWeek(final @Nullable String cron) { if (cron == null) { @@ -166,6 +205,9 @@ private MonitorConfigUtils() {} for (int i = 0; i < DAY_NAMES.size(); i++) { item = item.replace(DAY_NAMES.get(i), String.valueOf(i == 0 ? 7 : i)); } + if (item.matches(".*#0*5")) { + return null; + } if (item.startsWith("*/")) { item = "1-7" + item.substring(1); } else if (item.startsWith("7-")) { @@ -178,6 +220,10 @@ private MonitorConfigUtils() {} daysOfWeek.append(item); } fields[5] = daysOfWeek.toString(); + return join(fields); + } + + private static @NotNull String join(final @NotNull String[] fields) { final @NotNull StringBuilder result = new StringBuilder(); for (int i = 0; i < fields.length; i++) { if (i > 0) { diff --git a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt index d6976f7d148..f3e08ac99fa 100644 --- a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt @@ -9,7 +9,7 @@ class MonitorConfigUtilsTest { @Test fun `cron with fixed seconds drops the seconds field`() { - val config = MonitorConfigUtils.fromSchedule("0 15 10 * * MON-FRI", null, null, null) + val config = MonitorConfigUtils.fromSchedule("0 15 10 * * MON-FRI", null, null) assertThat(config).isNotNull() assertThat(config!!.schedule.type).isEqualTo("crontab") @@ -20,14 +20,14 @@ class MonitorConfigUtilsTest { @Test fun `cron whitespace is normalized`() { - val config = MonitorConfigUtils.fromSchedule(" 30 0 2 * * * ", null, null, null) + val config = MonitorConfigUtils.fromSchedule(" 30 0 2 * * * ", null, null) assertThat(config?.schedule?.value).isEqualTo("0 2 * * *") } @Test fun `cron question mark is converted to asterisk`() { - val config = MonitorConfigUtils.fromSchedule("0 0 9 ? * MON", null, null, null) + val config = MonitorConfigUtils.fromSchedule("0 0 9 ? * MON", null, null) assertThat(config?.schedule?.value).isEqualTo("0 9 * * MON") } @@ -46,33 +46,33 @@ class MonitorConfigUtilsTest { "@HOURLY" to "0 * * * *", ) for ((macro, crontab) in expected) { - assertThat(MonitorConfigUtils.fromSchedule(macro, null, null, null)?.schedule?.value) + assertThat(MonitorConfigUtils.fromSchedule(macro, null, null)?.schedule?.value) .isEqualTo(crontab) } } @Test fun `unknown cron macro returns null`() { - assertThat(MonitorConfigUtils.fromSchedule("@reboot", null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("@reboot", null, null)).isNull() } @Test fun `cron with variable seconds returns null`() { - assertThat(MonitorConfigUtils.fromSchedule("*/30 * * * * *", null, null, null)).isNull() - assertThat(MonitorConfigUtils.fromSchedule("0,30 * * * * *", null, null, null)).isNull() - assertThat(MonitorConfigUtils.fromSchedule("0-10 * * * * *", null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("*/30 * * * * *", null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("0,30 * * * * *", null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("0-10 * * * * *", null, null)).isNull() } @Test fun `cron without 6 fields returns null`() { - assertThat(MonitorConfigUtils.fromSchedule("0 * * * *", null, null, null)).isNull() - assertThat(MonitorConfigUtils.fromSchedule("0 0 * * * * 2030", null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("0 * * * *", null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("0 0 * * * * 2030", null, null)).isNull() } @Test fun `cron with both day of month and day of week returns null`() { for (cron in listOf("0 0 9 1-7 * MON", "0 0 9 15 * 1-5", "0 0 9 L * 5L", "0 0 9 1 * 1/2")) { - assertThat(MonitorConfigUtils.fromSchedule(cron, null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null)).isNull() } } @@ -85,7 +85,7 @@ class MonitorConfigUtilsTest { "0 0 9 1 * ?" to "0 9 1 * *", ) for ((cron, crontab) in expected) { - assertThat(MonitorConfigUtils.fromSchedule(cron, null, null, null)?.schedule?.value) + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null)?.schedule?.value) .isEqualTo(crontab) } } @@ -110,7 +110,7 @@ class MonitorConfigUtilsTest { "0 0 0 * * 1-7/2", ) for (cron in expected) { - assertThat(MonitorConfigUtils.fromSchedule(cron, null, null, null)?.schedule?.value) + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null)?.schedule?.value) .isEqualTo(cron.substringAfter(' ')) } } @@ -147,7 +147,7 @@ class MonitorConfigUtilsTest { "0 0 0 * * 5#0", ) for (cron in rejected) { - assertThat(MonitorConfigUtils.fromSchedule(cron, null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null)).isNull() } } @@ -156,18 +156,18 @@ class MonitorConfigUtilsTest { // Spring and Quartz run only at the value, cronsim steps to the field max for (cron in listOf("0 10-10/2 * * * *", "0 0 0-0/3 * * *", "0 0 0 ? * 4-4/3", "0 0 0 * 6-6/1 *")) { - assertThat(MonitorConfigUtils.fromSchedule(cron, null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null)).isNull() } } @Test fun `disabled cron returns null`() { - assertThat(MonitorConfigUtils.fromSchedule("-", null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("-", null, null)).isNull() } @Test fun `zone is set for cron`() { - val config = MonitorConfigUtils.fromSchedule("0 0 2 * * *", "Europe/Vienna", null, null) + val config = MonitorConfigUtils.fromSchedule("0 0 2 * * *", "Europe/Vienna", null) assertThat(config?.schedule?.value).isEqualTo("0 2 * * *") assertThat(config?.timezone).isEqualTo("Europe/Vienna") @@ -177,7 +177,7 @@ class MonitorConfigUtilsTest { fun `IANA zones are kept`() { for (zone in listOf("UTC", "GMT", "Etc/GMT+5", "US/Eastern", "America/Argentina/Buenos_Aires")) { - assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", zone, null, null)?.timezone) + assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", zone, null)?.timezone) .isEqualTo(zone) } } @@ -195,7 +195,7 @@ class MonitorConfigUtilsTest { "GMT-00:00" to "Etc/GMT", ) for ((zone, timezone) in expected) { - assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", zone, null, null)?.timezone) + assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", zone, null)?.timezone) .isEqualTo(timezone) } } @@ -204,18 +204,18 @@ class MonitorConfigUtilsTest { fun `zones Sentry rejects return null`() { for (zone in listOf("GMT+05:30", "GMT+15", "UTC-13", "PST", "IST", "Nowhere/Land", "SystemV/EST5")) { - assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", zone, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", zone, null)).isNull() } } @Test fun `empty zone is not set`() { - assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", "", null, null)?.timezone).isNull() + assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", "", null)?.timezone).isNull() } @Test fun `fixed rate in whole minutes is an interval`() { - val config = MonitorConfigUtils.fromSchedule(null, null, TimeUnit.MINUTES.toMillis(5), null) + val config = MonitorConfigUtils.fromSchedule(null, null, TimeUnit.MINUTES.toMillis(5)) assertThat(config).isNotNull() assertThat(config!!.schedule.type).isEqualTo("interval") @@ -223,14 +223,6 @@ class MonitorConfigUtilsTest { assertThat(config.schedule.unit).isEqualTo("minute") } - @Test - fun `fixed delay in whole minutes is an interval`() { - val config = MonitorConfigUtils.fromSchedule("", null, null, TimeUnit.MINUTES.toMillis(90)) - - assertThat(config?.schedule?.value).isEqualTo("90") - assertThat(config?.schedule?.unit).isEqualTo("minute") - } - @Test fun `period in whole hours or days uses the largest unit`() { val expected = @@ -241,7 +233,7 @@ class MonitorConfigUtilsTest { TimeUnit.DAYS.toMillis(14) to ("14" to "day"), ) for ((millis, interval) in expected) { - val config = MonitorConfigUtils.fromSchedule(null, null, millis, null) + val config = MonitorConfigUtils.fromSchedule(null, null, millis) assertThat(config?.schedule?.value).isEqualTo(interval.first) assertThat(config?.schedule?.unit).isEqualTo(interval.second) } @@ -250,7 +242,7 @@ class MonitorConfigUtilsTest { @Test fun `zone is ignored for intervals`() { val config = - MonitorConfigUtils.fromSchedule(null, "Europe/Vienna", TimeUnit.MINUTES.toMillis(1), null) + MonitorConfigUtils.fromSchedule(null, "Europe/Vienna", TimeUnit.MINUTES.toMillis(1)) assertThat(config?.schedule?.value).isEqualTo("1") assertThat(config?.timezone).isNull() @@ -258,13 +250,13 @@ class MonitorConfigUtilsTest { @Test fun `sub-minute period returns null`() { - assertThat(MonitorConfigUtils.fromSchedule(null, null, 30_000L, null)).isNull() - assertThat(MonitorConfigUtils.fromSchedule(null, null, 0L, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule(null, null, 30_000L)).isNull() + assertThat(MonitorConfigUtils.fromSchedule(null, null, 0L)).isNull() } @Test fun `period not in whole minutes returns null`() { - assertThat(MonitorConfigUtils.fromSchedule(null, null, 90_000L, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule(null, null, 90_000L)).isNull() } @Test @@ -274,7 +266,6 @@ class MonitorConfigUtilsTest { null, null, TimeUnit.MINUTES.toMillis(Int.MAX_VALUE.toLong() + 1), - null, ) ) .isNull() @@ -282,14 +273,13 @@ class MonitorConfigUtilsTest { @Test fun `more than one schedule kind returns null`() { - assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", null, 60_000L, null)).isNull() - assertThat(MonitorConfigUtils.fromSchedule(null, null, 60_000L, 60_000L)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("0 0 2 * * *", null, 60_000L)).isNull() } @Test fun `no schedule returns null`() { - assertThat(MonitorConfigUtils.fromSchedule(null, null, null, null)).isNull() - assertThat(MonitorConfigUtils.fromSchedule("", null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule(null, null, null)).isNull() + assertThat(MonitorConfigUtils.fromSchedule("", null, null)).isNull() } @Test @@ -307,7 +297,9 @@ class MonitorConfigUtilsTest { "0 0 9 * * *" to "0 9 * * *", ) for ((cron, crontab) in expected) { - assertThat(MonitorConfigUtils.fromSpringScheduled(cron, "UTC", null, null)?.schedule?.value) + assertThat( + MonitorConfigUtils.fromSpringScheduled(cron, "UTC", null, null, false)?.schedule?.value + ) .isEqualTo(crontab) } } @@ -317,9 +309,13 @@ class MonitorConfigUtilsTest { val defaultTimeZone = TimeZone.getDefault() TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")) try { - assertThat(MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", null, null, null)?.timezone) + assertThat( + MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", null, null, null, false)?.timezone + ) .isEqualTo("Asia/Tokyo") - assertThat(MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", "", null, null)?.timezone) + assertThat( + MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", "", null, null, false)?.timezone + ) .isEqualTo("Asia/Tokyo") } finally { TimeZone.setDefault(defaultTimeZone) @@ -328,7 +324,7 @@ class MonitorConfigUtilsTest { @Test fun `Spring cron zone is converted`() { - val config = MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", "GMT+2", null, null) + val config = MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", "GMT+2", null, null, false) assertThat(config?.schedule?.value).isEqualTo("0 2 * * *") assertThat(config?.timezone).isEqualTo("Etc/GMT-2") @@ -337,24 +333,80 @@ class MonitorConfigUtilsTest { @Test fun `Spring schedules Sentry can't express return null`() { for (cron in listOf("0 0 9 1-7 * MON", "-", "\${my.cron.missing}", "*/30 * * * * *")) { - assertThat(MonitorConfigUtils.fromSpringScheduled(cron, "UTC", null, null)).isNull() + assertThat(MonitorConfigUtils.fromSpringScheduled(cron, "UTC", null, null, false)).isNull() } - assertThat(MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", "GMT+05:30", null, null)) + assertThat( + MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", "GMT+05:30", null, null, false) + ) .isNull() - assertThat(MonitorConfigUtils.fromSpringScheduled(null, null, 30_000L, null)).isNull() + assertThat(MonitorConfigUtils.fromSpringScheduled(null, null, 30_000L, null, false)).isNull() } @Test fun `Spring periods are intervals without zone`() { - val rate = MonitorConfigUtils.fromSpringScheduled("", null, TimeUnit.MINUTES.toMillis(5), null) + val rate = + MonitorConfigUtils.fromSpringScheduled("", null, TimeUnit.MINUTES.toMillis(5), null, false) assertThat(rate?.schedule?.type).isEqualTo("interval") assertThat(rate?.schedule?.value).isEqualTo("5") assertThat(rate?.schedule?.unit).isEqualTo("minute") assertThat(rate?.timezone).isNull() + } + + @Test + fun `Spring fixed delay returns null`() { + for (legacy in listOf(false, true)) { + assertThat(MonitorConfigUtils.fromSpringScheduled(null, null, null, 300_000L, legacy)) + .isNull() + assertThat(MonitorConfigUtils.fromSpringScheduled("", null, 300_000L, 300_000L, legacy)) + .isNull() + } + } + + @Test + fun `Spring fifth weekday returns null`() { + assertThat(MonitorConfigUtils.fromSpringScheduled("0 0 9 ? * MON#5", "UTC", null, null, false)) + .isNull() + } + + @Test + fun `legacy Spring cron is rewritten to how crontab reads it`() { + val expected = + mapOf( + "0 0 9 */2 * *" to "0 9 2-31/2 * *", + "0 0 9 */1,15 * ?" to "0 9 1-31/1,15 * *", + "0 0 9 ? * */2" to "0 9 * * */2", + "0 0 9 ? * SUN/2" to "0 9 * * SUN/2", + "0 0 9 ? * 5-7" to "0 9 * * 5-7", + "0 0 9 1-15/3 * *" to "0 9 1-15/3 * *", + ) + for ((cron, crontab) in expected) { + assertThat( + MonitorConfigUtils.fromSpringScheduled(cron, "UTC", null, null, true)?.schedule?.value + ) + .isEqualTo(crontab) + } + } - val delay = MonitorConfigUtils.fromSpringScheduled(null, null, null, TimeUnit.HOURS.toMillis(2)) - assertThat(delay?.schedule?.value).isEqualTo("2") - assertThat(delay?.schedule?.unit).isEqualTo("hour") + @Test + fun `legacy Spring cron with both day fields returns null`() { + // CronSequenceGenerator then sometimes runs at midnight + for (cron in listOf("0 0 9 1 * */2", "0 0 9 */2 * */3", "0 0 9 */2 * MON")) { + assertThat(MonitorConfigUtils.fromSpringScheduled(cron, "UTC", null, null, true)).isNull() + } + } + + @Test + fun `legacy Spring cron without zone uses the JVM default zone`() { + val defaultTimeZone = TimeZone.getDefault() + TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")) + try { + assertThat( + MonitorConfigUtils.fromSpringScheduled("0 0 2 * * *", null, null, null, true)?.timezone + ) + .isEqualTo("Asia/Tokyo") + } finally { + TimeZone.setDefault(defaultTimeZone) + } } @Test From 5d745c70b33f217843902ea68ca13fe3f59fba26 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Mon, 5 Oct 2026 15:54:08 -0700 Subject: [PATCH 06/18] feat(spring): Send monitor config from @Scheduled with @SentryCheckIn Co-Authored-By: Claude --- CHANGELOG.md | 1 + sentry-spring-7/api/sentry-spring-7.api | 1 + .../sentry/spring7/checkin/SentryCheckIn.java | 8 + .../spring7/checkin/SentryCheckInAdvice.java | 119 ++++++++- .../sentry/spring7/SentryCheckInAdviceTest.kt | 238 +++++++++++++++++- .../api/sentry-spring-jakarta.api | 1 + .../spring/jakarta/checkin/SentryCheckIn.java | 8 + .../jakarta/checkin/SentryCheckInAdvice.java | 119 ++++++++- .../spring/jakarta/SentryCheckInAdviceTest.kt | 238 +++++++++++++++++- sentry-spring/api/sentry-spring.api | 1 + .../sentry/spring/checkin/SentryCheckIn.java | 8 + .../spring/checkin/SentryCheckInAdvice.java | 119 ++++++++- .../sentry/spring/SentryCheckInAdviceTest.kt | 238 +++++++++++++++++- 13 files changed, 1093 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4126d1706dd..17d332f8011 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- 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)) ## 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..f840fa7c44b 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,14 @@ */ boolean heartbeat() default false; + /** + * Set to true to send the schedule from the method's {@code @Scheduled}, so Sentry creates or + * updates the monitor. + * + * @return true to send a monitor config, false by default + */ + boolean upsertMonitorConfig() default false; + /** * 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..920ef6a707d 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 (Throwable 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..8f0921bd78e 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 defaults to false and sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.defaultNoConfig() }) + } + + @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,78 @@ class SentryCheckInAdviceTest { open fun helloExceptionProperty() = 1 } + open class SampleServiceScheduled { + + @SentryCheckIn("cron_zone", upsertMonitorConfig = true) + @Scheduled(cron = "0 15 10 * * MON-FRI", zone = "Europe/Vienna") + open fun cronWithZone() {} + + @SentryCheckIn("cron", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 2 * * *") + open fun cron() {} + + @SentryCheckIn("cron_properties", upsertMonitorConfig = true) + @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") + open fun cronFromProperties() {} + + @SentryCheckIn("fixed_rate_hours", upsertMonitorConfig = true) + @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) + open fun fixedRateHours() {} + + @SentryCheckIn("fixed_rate_iso", upsertMonitorConfig = true) + @Scheduled(fixedRateString = "PT10M") + open fun fixedRateIso() {} + + @SentryCheckIn("fixed_rate_simple", upsertMonitorConfig = true) + @Scheduled(fixedRateString = "5m") + open fun fixedRateSimpleDuration() {} + + @SentryCheckIn("fixed_delay", upsertMonitorConfig = true) + @Scheduled(fixedDelay = 300_000) + open fun fixedDelay() {} + + @SentryCheckIn("fixed_delay_iso", upsertMonitorConfig = true) + @Scheduled(fixedDelayString = "PT10M") + open fun fixedDelayIso() {} + + @SentryCheckIn("disabled_cron", upsertMonitorConfig = true) + @Scheduled(cron = "-") + open fun disabledCron() {} + + @SentryCheckIn("cron_empty_zone", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") + open fun cronWithEmptyZone() {} + + @SentryCheckIn("cron_zone_defaults", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") + open fun cronWithZoneAndDefaults() {} + + @SentryCheckIn("schedules_container", upsertMonitorConfig = true) + @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) + open fun schedulesContainer() {} + + @SentryCheckIn("unresolvable_cron", upsertMonitorConfig = true) + @Scheduled(cron = "\${my.cron.missing}") + open fun unresolvableCron() {} + + @SentryCheckIn("derivation_throws", upsertMonitorConfig = true) + @Scheduled(cron = "\${my.cron.exception.property}") + open fun derivationThrows() = 1 + + @SentryCheckIn("multiple", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 1 * * *") + @Scheduled(cron = "0 0 13 * * *") + open fun multipleSchedules() {} + + @SentryCheckIn("default_no_config") + @Scheduled(cron = "0 0 1 * * *") + open fun defaultNoConfig() {} + + @SentryCheckIn("heartbeat", heartbeat = true, upsertMonitorConfig = 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..4eff0b0edf4 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,14 @@ */ boolean heartbeat() default false; + /** + * Set to true to send the schedule from the method's {@code @Scheduled}, so Sentry creates or + * updates the monitor. + * + * @return true to send a monitor config, false by default + */ + boolean upsertMonitorConfig() default false; + /** * 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..221bb1bed93 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 (Throwable 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..c50ca7979a6 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 defaults to false and sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.defaultNoConfig() }) + } + + @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,78 @@ class SentryCheckInAdviceTest { open fun helloExceptionProperty() = 1 } + open class SampleServiceScheduled { + + @SentryCheckIn("cron_zone", upsertMonitorConfig = true) + @Scheduled(cron = "0 15 10 * * MON-FRI", zone = "Europe/Vienna") + open fun cronWithZone() {} + + @SentryCheckIn("cron", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 2 * * *") + open fun cron() {} + + @SentryCheckIn("cron_properties", upsertMonitorConfig = true) + @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") + open fun cronFromProperties() {} + + @SentryCheckIn("fixed_rate_hours", upsertMonitorConfig = true) + @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) + open fun fixedRateHours() {} + + @SentryCheckIn("fixed_rate_iso", upsertMonitorConfig = true) + @Scheduled(fixedRateString = "PT10M") + open fun fixedRateIso() {} + + @SentryCheckIn("fixed_rate_simple", upsertMonitorConfig = true) + @Scheduled(fixedRateString = "5m") + open fun fixedRateSimpleDuration() {} + + @SentryCheckIn("fixed_delay", upsertMonitorConfig = true) + @Scheduled(fixedDelay = 300_000) + open fun fixedDelay() {} + + @SentryCheckIn("fixed_delay_iso", upsertMonitorConfig = true) + @Scheduled(fixedDelayString = "PT10M") + open fun fixedDelayIso() {} + + @SentryCheckIn("disabled_cron", upsertMonitorConfig = true) + @Scheduled(cron = "-") + open fun disabledCron() {} + + @SentryCheckIn("cron_empty_zone", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") + open fun cronWithEmptyZone() {} + + @SentryCheckIn("cron_zone_defaults", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") + open fun cronWithZoneAndDefaults() {} + + @SentryCheckIn("schedules_container", upsertMonitorConfig = true) + @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) + open fun schedulesContainer() {} + + @SentryCheckIn("unresolvable_cron", upsertMonitorConfig = true) + @Scheduled(cron = "\${my.cron.missing}") + open fun unresolvableCron() {} + + @SentryCheckIn("derivation_throws", upsertMonitorConfig = true) + @Scheduled(cron = "\${my.cron.exception.property}") + open fun derivationThrows() = 1 + + @SentryCheckIn("multiple", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 1 * * *") + @Scheduled(cron = "0 0 13 * * *") + open fun multipleSchedules() {} + + @SentryCheckIn("default_no_config") + @Scheduled(cron = "0 0 1 * * *") + open fun defaultNoConfig() {} + + @SentryCheckIn("heartbeat", heartbeat = true, upsertMonitorConfig = 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..e3cbb2a7c7b 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,14 @@ */ boolean heartbeat() default false; + /** + * Set to true to send the schedule from the method's {@code @Scheduled}, so Sentry creates or + * updates the monitor. + * + * @return true to send a monitor config, false by default + */ + boolean upsertMonitorConfig() default false; + /** * 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..83594da523f 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 (Throwable 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..62bab3973d2 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 defaults to false and sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.defaultNoConfig() }) + } + + @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,78 @@ class SentryCheckInAdviceTest { open fun helloExceptionProperty() = 1 } + open class SampleServiceScheduled { + + @SentryCheckIn("cron_zone", upsertMonitorConfig = true) + @Scheduled(cron = "0 15 10 * * MON-FRI", zone = "Europe/Vienna") + open fun cronWithZone() {} + + @SentryCheckIn("cron", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 2 * * *") + open fun cron() {} + + @SentryCheckIn("cron_properties", upsertMonitorConfig = true) + @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") + open fun cronFromProperties() {} + + @SentryCheckIn("fixed_rate_hours", upsertMonitorConfig = true) + @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) + open fun fixedRateHours() {} + + @SentryCheckIn("fixed_rate_iso", upsertMonitorConfig = true) + @Scheduled(fixedRateString = "PT10M") + open fun fixedRateIso() {} + + @SentryCheckIn("fixed_rate_simple", upsertMonitorConfig = true) + @Scheduled(fixedRateString = "5m") + open fun fixedRateSimpleDuration() {} + + @SentryCheckIn("fixed_delay", upsertMonitorConfig = true) + @Scheduled(fixedDelay = 300_000) + open fun fixedDelay() {} + + @SentryCheckIn("fixed_delay_iso", upsertMonitorConfig = true) + @Scheduled(fixedDelayString = "PT10M") + open fun fixedDelayIso() {} + + @SentryCheckIn("disabled_cron", upsertMonitorConfig = true) + @Scheduled(cron = "-") + open fun disabledCron() {} + + @SentryCheckIn("cron_empty_zone", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") + open fun cronWithEmptyZone() {} + + @SentryCheckIn("cron_zone_defaults", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") + open fun cronWithZoneAndDefaults() {} + + @SentryCheckIn("schedules_container", upsertMonitorConfig = true) + @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) + open fun schedulesContainer() {} + + @SentryCheckIn("unresolvable_cron", upsertMonitorConfig = true) + @Scheduled(cron = "\${my.cron.missing}") + open fun unresolvableCron() {} + + @SentryCheckIn("derivation_throws", upsertMonitorConfig = true) + @Scheduled(cron = "\${my.cron.exception.property}") + open fun derivationThrows() = 1 + + @SentryCheckIn("multiple", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 1 * * *") + @Scheduled(cron = "0 0 13 * * *") + open fun multipleSchedules() {} + + @SentryCheckIn("default_no_config") + @Scheduled(cron = "0 0 1 * * *") + open fun defaultNoConfig() {} + + @SentryCheckIn("heartbeat", heartbeat = true, upsertMonitorConfig = true) + @Scheduled(cron = "0 0 1 * * *") + open fun heartbeat() {} + } + class MyPropertyPlaceholderConfigurer : PropertySourcesPlaceholderConfigurer() { override fun doProcessProperties( From 44ad1c796e7ec10aaa3b03a02b65c075ae006303 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Mon, 5 Oct 2026 15:54:15 -0700 Subject: [PATCH 07/18] feat(quartz): Send monitor config from the trigger with check-ins Co-Authored-By: Claude --- CHANGELOG.md | 1 + sentry-quartz/api/sentry-quartz.api | 1 + sentry-quartz/build.gradle.kts | 2 + .../io/sentry/quartz/SentryJobListener.java | 158 +++++++++- .../io/sentry/quartz/SentryJobListenerTest.kt | 288 ++++++++++++++++++ 5 files changed, 449 insertions(+), 1 deletion(-) create mode 100644 sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt 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..3b51735257d 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 (Throwable 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..178e282f76d --- /dev/null +++ b/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt @@ -0,0 +1,288 @@ +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 `trigger throwing an error still sends the check-in without monitor config`() { + val trigger = mock() + whenever(trigger.cronExpression).thenThrow(NoSuchMethodError("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 + } +} From bcd5682e0295f43efb14b9dd86e2c1e40093bbdd Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Mon, 5 Oct 2026 16:07:25 -0700 Subject: [PATCH 08/18] feat(spring): Send @Scheduled monitor config by default Co-Authored-By: Claude --- CHANGELOG.md | 2 +- .../sentry/spring7/checkin/SentryCheckIn.java | 11 +++-- .../sentry/spring7/SentryCheckInAdviceTest.kt | 46 ++++++++----------- .../spring/jakarta/checkin/SentryCheckIn.java | 11 +++-- .../spring/jakarta/SentryCheckInAdviceTest.kt | 46 ++++++++----------- .../sentry/spring/checkin/SentryCheckIn.java | 11 +++-- .../sentry/spring/SentryCheckInAdviceTest.kt | 46 ++++++++----------- 7 files changed, 82 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17d332f8011..10f53cef310 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features -- 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)) +- 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/src/main/java/io/sentry/spring7/checkin/SentryCheckIn.java b/sentry-spring-7/src/main/java/io/sentry/spring7/checkin/SentryCheckIn.java index f840fa7c44b..09c8e6f290d 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 @@ -30,12 +30,15 @@ boolean heartbeat() default false; /** - * Set to true to send the schedule from the method's {@code @Scheduled}, so Sentry creates or - * updates the monitor. + * 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, for example if its timezone differs from the JVM's. * - * @return true to send a monitor config, false by default + *

Heartbeat check-ins never send a monitor config. + * + * @return true to send a monitor config, true by default */ - boolean upsertMonitorConfig() default false; + boolean upsertMonitorConfig() default true; /** * Monitor slug. If not set, no check-in will be sent. 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 8f0921bd78e..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 @@ -366,8 +366,8 @@ class SentryCheckInAdviceTest { } @Test - fun `upsertMonitorConfig defaults to false and sends no monitor config`() { - assertNull(inProgressMonitorConfig { sampleServiceScheduled.defaultNoConfig() }) + fun `upsertMonitorConfig false sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDisabled() }) } @Test @@ -457,72 +457,66 @@ class SentryCheckInAdviceTest { open class SampleServiceScheduled { - @SentryCheckIn("cron_zone", upsertMonitorConfig = true) + @SentryCheckIn("cron_zone") @Scheduled(cron = "0 15 10 * * MON-FRI", zone = "Europe/Vienna") open fun cronWithZone() {} - @SentryCheckIn("cron", upsertMonitorConfig = true) - @Scheduled(cron = "0 0 2 * * *") - open fun cron() {} + @SentryCheckIn("cron") @Scheduled(cron = "0 0 2 * * *") open fun cron() {} - @SentryCheckIn("cron_properties", upsertMonitorConfig = true) + @SentryCheckIn("cron_properties") @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") open fun cronFromProperties() {} - @SentryCheckIn("fixed_rate_hours", upsertMonitorConfig = true) + @SentryCheckIn("fixed_rate_hours") @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) open fun fixedRateHours() {} - @SentryCheckIn("fixed_rate_iso", upsertMonitorConfig = true) + @SentryCheckIn("fixed_rate_iso") @Scheduled(fixedRateString = "PT10M") open fun fixedRateIso() {} - @SentryCheckIn("fixed_rate_simple", upsertMonitorConfig = true) + @SentryCheckIn("fixed_rate_simple") @Scheduled(fixedRateString = "5m") open fun fixedRateSimpleDuration() {} - @SentryCheckIn("fixed_delay", upsertMonitorConfig = true) - @Scheduled(fixedDelay = 300_000) - open fun fixedDelay() {} + @SentryCheckIn("fixed_delay") @Scheduled(fixedDelay = 300_000) open fun fixedDelay() {} - @SentryCheckIn("fixed_delay_iso", upsertMonitorConfig = true) + @SentryCheckIn("fixed_delay_iso") @Scheduled(fixedDelayString = "PT10M") open fun fixedDelayIso() {} - @SentryCheckIn("disabled_cron", upsertMonitorConfig = true) - @Scheduled(cron = "-") - open fun disabledCron() {} + @SentryCheckIn("disabled_cron") @Scheduled(cron = "-") open fun disabledCron() {} - @SentryCheckIn("cron_empty_zone", upsertMonitorConfig = true) + @SentryCheckIn("cron_empty_zone") @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") open fun cronWithEmptyZone() {} - @SentryCheckIn("cron_zone_defaults", upsertMonitorConfig = true) + @SentryCheckIn("cron_zone_defaults") @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") open fun cronWithZoneAndDefaults() {} - @SentryCheckIn("schedules_container", upsertMonitorConfig = true) + @SentryCheckIn("schedules_container") @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) open fun schedulesContainer() {} - @SentryCheckIn("unresolvable_cron", upsertMonitorConfig = true) + @SentryCheckIn("unresolvable_cron") @Scheduled(cron = "\${my.cron.missing}") open fun unresolvableCron() {} - @SentryCheckIn("derivation_throws", upsertMonitorConfig = true) + @SentryCheckIn("derivation_throws") @Scheduled(cron = "\${my.cron.exception.property}") open fun derivationThrows() = 1 - @SentryCheckIn("multiple", upsertMonitorConfig = true) + @SentryCheckIn("multiple") @Scheduled(cron = "0 0 1 * * *") @Scheduled(cron = "0 0 13 * * *") open fun multipleSchedules() {} - @SentryCheckIn("default_no_config") + @SentryCheckIn("upsert_disabled", upsertMonitorConfig = false) @Scheduled(cron = "0 0 1 * * *") - open fun defaultNoConfig() {} + open fun upsertDisabled() {} - @SentryCheckIn("heartbeat", heartbeat = true, upsertMonitorConfig = true) + @SentryCheckIn("heartbeat", heartbeat = true) @Scheduled(cron = "0 0 1 * * *") open fun heartbeat() {} } 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 4eff0b0edf4..54bd182976b 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 @@ -30,12 +30,15 @@ boolean heartbeat() default false; /** - * Set to true to send the schedule from the method's {@code @Scheduled}, so Sentry creates or - * updates the monitor. + * 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, for example if its timezone differs from the JVM's. * - * @return true to send a monitor config, false by default + *

Heartbeat check-ins never send a monitor config. + * + * @return true to send a monitor config, true by default */ - boolean upsertMonitorConfig() default false; + boolean upsertMonitorConfig() default true; /** * Monitor slug. If not set, no check-in will be sent. 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 c50ca7979a6..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 @@ -366,8 +366,8 @@ class SentryCheckInAdviceTest { } @Test - fun `upsertMonitorConfig defaults to false and sends no monitor config`() { - assertNull(inProgressMonitorConfig { sampleServiceScheduled.defaultNoConfig() }) + fun `upsertMonitorConfig false sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDisabled() }) } @Test @@ -457,72 +457,66 @@ class SentryCheckInAdviceTest { open class SampleServiceScheduled { - @SentryCheckIn("cron_zone", upsertMonitorConfig = true) + @SentryCheckIn("cron_zone") @Scheduled(cron = "0 15 10 * * MON-FRI", zone = "Europe/Vienna") open fun cronWithZone() {} - @SentryCheckIn("cron", upsertMonitorConfig = true) - @Scheduled(cron = "0 0 2 * * *") - open fun cron() {} + @SentryCheckIn("cron") @Scheduled(cron = "0 0 2 * * *") open fun cron() {} - @SentryCheckIn("cron_properties", upsertMonitorConfig = true) + @SentryCheckIn("cron_properties") @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") open fun cronFromProperties() {} - @SentryCheckIn("fixed_rate_hours", upsertMonitorConfig = true) + @SentryCheckIn("fixed_rate_hours") @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) open fun fixedRateHours() {} - @SentryCheckIn("fixed_rate_iso", upsertMonitorConfig = true) + @SentryCheckIn("fixed_rate_iso") @Scheduled(fixedRateString = "PT10M") open fun fixedRateIso() {} - @SentryCheckIn("fixed_rate_simple", upsertMonitorConfig = true) + @SentryCheckIn("fixed_rate_simple") @Scheduled(fixedRateString = "5m") open fun fixedRateSimpleDuration() {} - @SentryCheckIn("fixed_delay", upsertMonitorConfig = true) - @Scheduled(fixedDelay = 300_000) - open fun fixedDelay() {} + @SentryCheckIn("fixed_delay") @Scheduled(fixedDelay = 300_000) open fun fixedDelay() {} - @SentryCheckIn("fixed_delay_iso", upsertMonitorConfig = true) + @SentryCheckIn("fixed_delay_iso") @Scheduled(fixedDelayString = "PT10M") open fun fixedDelayIso() {} - @SentryCheckIn("disabled_cron", upsertMonitorConfig = true) - @Scheduled(cron = "-") - open fun disabledCron() {} + @SentryCheckIn("disabled_cron") @Scheduled(cron = "-") open fun disabledCron() {} - @SentryCheckIn("cron_empty_zone", upsertMonitorConfig = true) + @SentryCheckIn("cron_empty_zone") @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") open fun cronWithEmptyZone() {} - @SentryCheckIn("cron_zone_defaults", upsertMonitorConfig = true) + @SentryCheckIn("cron_zone_defaults") @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") open fun cronWithZoneAndDefaults() {} - @SentryCheckIn("schedules_container", upsertMonitorConfig = true) + @SentryCheckIn("schedules_container") @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) open fun schedulesContainer() {} - @SentryCheckIn("unresolvable_cron", upsertMonitorConfig = true) + @SentryCheckIn("unresolvable_cron") @Scheduled(cron = "\${my.cron.missing}") open fun unresolvableCron() {} - @SentryCheckIn("derivation_throws", upsertMonitorConfig = true) + @SentryCheckIn("derivation_throws") @Scheduled(cron = "\${my.cron.exception.property}") open fun derivationThrows() = 1 - @SentryCheckIn("multiple", upsertMonitorConfig = true) + @SentryCheckIn("multiple") @Scheduled(cron = "0 0 1 * * *") @Scheduled(cron = "0 0 13 * * *") open fun multipleSchedules() {} - @SentryCheckIn("default_no_config") + @SentryCheckIn("upsert_disabled", upsertMonitorConfig = false) @Scheduled(cron = "0 0 1 * * *") - open fun defaultNoConfig() {} + open fun upsertDisabled() {} - @SentryCheckIn("heartbeat", heartbeat = true, upsertMonitorConfig = true) + @SentryCheckIn("heartbeat", heartbeat = true) @Scheduled(cron = "0 0 1 * * *") open fun heartbeat() {} } 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 e3cbb2a7c7b..fcd53debaeb 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 @@ -30,12 +30,15 @@ boolean heartbeat() default false; /** - * Set to true to send the schedule from the method's {@code @Scheduled}, so Sentry creates or - * updates the monitor. + * 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, for example if its timezone differs from the JVM's. * - * @return true to send a monitor config, false by default + *

Heartbeat check-ins never send a monitor config. + * + * @return true to send a monitor config, true by default */ - boolean upsertMonitorConfig() default false; + boolean upsertMonitorConfig() default true; /** * Monitor slug. If not set, no check-in will be sent. 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 62bab3973d2..b6e8a4e3de2 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryCheckInAdviceTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryCheckInAdviceTest.kt @@ -367,8 +367,8 @@ class SentryCheckInAdviceTest { } @Test - fun `upsertMonitorConfig defaults to false and sends no monitor config`() { - assertNull(inProgressMonitorConfig { sampleServiceScheduled.defaultNoConfig() }) + fun `upsertMonitorConfig false sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDisabled() }) } @Test @@ -458,72 +458,66 @@ class SentryCheckInAdviceTest { open class SampleServiceScheduled { - @SentryCheckIn("cron_zone", upsertMonitorConfig = true) + @SentryCheckIn("cron_zone") @Scheduled(cron = "0 15 10 * * MON-FRI", zone = "Europe/Vienna") open fun cronWithZone() {} - @SentryCheckIn("cron", upsertMonitorConfig = true) - @Scheduled(cron = "0 0 2 * * *") - open fun cron() {} + @SentryCheckIn("cron") @Scheduled(cron = "0 0 2 * * *") open fun cron() {} - @SentryCheckIn("cron_properties", upsertMonitorConfig = true) + @SentryCheckIn("cron_properties") @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") open fun cronFromProperties() {} - @SentryCheckIn("fixed_rate_hours", upsertMonitorConfig = true) + @SentryCheckIn("fixed_rate_hours") @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) open fun fixedRateHours() {} - @SentryCheckIn("fixed_rate_iso", upsertMonitorConfig = true) + @SentryCheckIn("fixed_rate_iso") @Scheduled(fixedRateString = "PT10M") open fun fixedRateIso() {} - @SentryCheckIn("fixed_rate_simple", upsertMonitorConfig = true) + @SentryCheckIn("fixed_rate_simple") @Scheduled(fixedRateString = "5m") open fun fixedRateSimpleDuration() {} - @SentryCheckIn("fixed_delay", upsertMonitorConfig = true) - @Scheduled(fixedDelay = 300_000) - open fun fixedDelay() {} + @SentryCheckIn("fixed_delay") @Scheduled(fixedDelay = 300_000) open fun fixedDelay() {} - @SentryCheckIn("fixed_delay_iso", upsertMonitorConfig = true) + @SentryCheckIn("fixed_delay_iso") @Scheduled(fixedDelayString = "PT10M") open fun fixedDelayIso() {} - @SentryCheckIn("disabled_cron", upsertMonitorConfig = true) - @Scheduled(cron = "-") - open fun disabledCron() {} + @SentryCheckIn("disabled_cron") @Scheduled(cron = "-") open fun disabledCron() {} - @SentryCheckIn("cron_empty_zone", upsertMonitorConfig = true) + @SentryCheckIn("cron_empty_zone") @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") open fun cronWithEmptyZone() {} - @SentryCheckIn("cron_zone_defaults", upsertMonitorConfig = true) + @SentryCheckIn("cron_zone_defaults") @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") open fun cronWithZoneAndDefaults() {} - @SentryCheckIn("schedules_container", upsertMonitorConfig = true) + @SentryCheckIn("schedules_container") @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) open fun schedulesContainer() {} - @SentryCheckIn("unresolvable_cron", upsertMonitorConfig = true) + @SentryCheckIn("unresolvable_cron") @Scheduled(cron = "\${my.cron.missing}") open fun unresolvableCron() {} - @SentryCheckIn("derivation_throws", upsertMonitorConfig = true) + @SentryCheckIn("derivation_throws") @Scheduled(cron = "\${my.cron.exception.property}") open fun derivationThrows() = 1 - @SentryCheckIn("multiple", upsertMonitorConfig = true) + @SentryCheckIn("multiple") @Scheduled(cron = "0 0 1 * * *") @Scheduled(cron = "0 0 13 * * *") open fun multipleSchedules() {} - @SentryCheckIn("default_no_config") + @SentryCheckIn("upsert_disabled", upsertMonitorConfig = false) @Scheduled(cron = "0 0 1 * * *") - open fun defaultNoConfig() {} + open fun upsertDisabled() {} - @SentryCheckIn("heartbeat", heartbeat = true, upsertMonitorConfig = true) + @SentryCheckIn("heartbeat", heartbeat = true) @Scheduled(cron = "0 0 1 * * *") open fun heartbeat() {} } From a82b34ce5e964f53a63026ea23070ff60cc5fb5f Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Mon, 5 Oct 2026 16:09:38 -0700 Subject: [PATCH 09/18] feat(quartz): Send trigger monitor config by default Co-Authored-By: Claude --- CHANGELOG.md | 2 +- .../io/sentry/quartz/SentryJobListener.java | 34 ++++++++--- .../io/sentry/quartz/SentryJobListenerTest.kt | 56 ++++++++++++++----- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0859bb625bf..8a2b11dedb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +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)) +- Quartz `SentryJobListener` now sends a monitor config derived from the trigger with check-ins by default, so Sentry can create or update the monitor from code. Existing monitors take their schedule and time zone from the trigger; set the `sentry-upsert-monitor-config` job or trigger data key to `"false"` to keep managing them in Sentry ([#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/src/main/java/io/sentry/quartz/SentryJobListener.java b/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java index 3b51735257d..c9683a7942f 100644 --- a/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java +++ b/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java @@ -24,9 +24,12 @@ import org.jetbrains.annotations.Nullable; import org.quartz.CronTrigger; import org.quartz.JobDataMap; +import org.quartz.JobDetail; import org.quartz.JobExecutionContext; import org.quartz.JobExecutionException; import org.quartz.JobListener; +import org.quartz.Scheduler; +import org.quartz.SchedulerException; import org.quartz.SimpleTrigger; import org.quartz.Trigger; @@ -42,7 +45,10 @@ 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. */ + /** + * Job or trigger data key. The listener sends a monitor config from the trigger with check-ins by + * default; set this to {@code "false"} to manage the monitor's schedule in Sentry instead. + */ public static final String SENTRY_UPSERT_MONITOR_CONFIG_KEY = "sentry-upsert-monitor-config"; private static final @NotNull List DAY_NAMES = @@ -76,9 +82,7 @@ public void jobToBeExecuted(final @NotNull JobExecutionContext context) { return; } final @Nullable MonitorConfig monitorConfig = - shouldUpsertMonitorConfig(context) - ? monitorConfigFromTrigger(context.getTrigger()) - : null; + shouldUpsertMonitorConfig(context) ? monitorConfigFromTrigger(context) : null; final @NotNull ISentryLifecycleToken lifecycleToken = scopes.forkedScopes("SentryJobListener").makeCurrent(); context.put(SENTRY_SCOPE_LIFECYCLE_TOKEN_KEY, lifecycleToken); @@ -112,18 +116,24 @@ public void jobToBeExecuted(final @NotNull JobExecutionContext context) { private boolean shouldUpsertMonitorConfig(final @NotNull JobExecutionContext context) { final @Nullable JobDataMap jobDataMap = context.getMergedJobDataMap(); if (jobDataMap == null) { - return false; + return true; } final @Nullable Object o = jobDataMap.get(SENTRY_UPSERT_MONITOR_CONFIG_KEY); - return o != null && "true".equalsIgnoreCase(o.toString()); + return o == null || !"false".equalsIgnoreCase(o.toString().trim()); } - private @Nullable MonitorConfig monitorConfigFromTrigger(final @Nullable Trigger trigger) { + private @Nullable MonitorConfig monitorConfigFromTrigger( + final @NotNull JobExecutionContext context) { try { + final @Nullable Trigger trigger = context.getTrigger(); // Sentry can't express calendar exclusions if (trigger == null || trigger.getCalendarName() != null) { return null; } + // each trigger would overwrite the others' schedule + if (hasSeveralTriggers(context)) { + return null; + } if (trigger instanceof CronTrigger) { final @NotNull CronTrigger cronTrigger = (CronTrigger) trigger; final @Nullable String cron = toSixFieldCron(cronTrigger.getCronExpression()); @@ -152,6 +162,16 @@ private boolean shouldUpsertMonitorConfig(final @NotNull JobExecutionContext con } } + private static boolean hasSeveralTriggers(final @NotNull JobExecutionContext context) + throws SchedulerException { + final @Nullable Scheduler scheduler = context.getScheduler(); + final @Nullable JobDetail jobDetail = context.getJobDetail(); + if (scheduler == null || jobDetail == null) { + return false; + } + return scheduler.getTriggersOfJob(jobDetail.getKey()).size() > 1; + } + /** * 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. diff --git a/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt b/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt index 178e282f76d..9be624dceb2 100644 --- a/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt +++ b/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt @@ -19,7 +19,10 @@ import org.mockito.kotlin.whenever import org.quartz.CalendarIntervalScheduleBuilder import org.quartz.CronScheduleBuilder import org.quartz.JobDataMap +import org.quartz.JobDetail import org.quartz.JobExecutionContext +import org.quartz.JobKey +import org.quartz.Scheduler import org.quartz.SimpleScheduleBuilder import org.quartz.Trigger import org.quartz.TriggerBuilder @@ -141,7 +144,7 @@ class SentryJobListenerTest { @Test fun `scope token is stored even if capturing the check-in fails`() { - val jobDataMap = upsertJobDataMap() + val jobDataMap = JobDataMap() jobDataMap[SentryJobListener.SENTRY_SLUG_KEY] = "my-job" val context = mock() whenever(context.mergedJobDataMap).thenReturn(jobDataMap) @@ -177,8 +180,16 @@ class SentryJobListenerTest { } @Test - fun `without upsert monitor config key sends no monitor config`() { - assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ?"), JobDataMap())).isNull() + fun `sends monitor config by default`() { + assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ?"), JobDataMap())).isNotNull() + } + + @Test + fun `upsert monitor config true sends monitor config`() { + val jobDataMap = JobDataMap() + jobDataMap[SentryJobListener.SENTRY_UPSERT_MONITOR_CONFIG_KEY] = "true" + + assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ?"), jobDataMap)).isNotNull() } @Test @@ -190,11 +201,26 @@ class SentryJobListenerTest { } @Test - fun `upsert monitor config as boolean true sends monitor config`() { + fun `upsert monitor config as boolean false sends no monitor config`() { val jobDataMap = JobDataMap() - jobDataMap[SentryJobListener.SENTRY_UPSERT_MONITOR_CONFIG_KEY] = true + jobDataMap[SentryJobListener.SENTRY_UPSERT_MONITOR_CONFIG_KEY] = false - assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ?"), jobDataMap)).isNotNull() + assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ?"), jobDataMap)).isNull() + } + + @Test + fun `job with one trigger sends monitor config`() { + val trigger = cronTrigger("0 0 2 * * ?") + + assertThat(inProgressMonitorConfig(trigger, triggersOfJob = listOf(trigger))).isNotNull() + } + + @Test + fun `job with several triggers sends no monitor config`() { + val trigger = cronTrigger("0 0 2 * * ?") + val triggers = listOf(trigger, cronTrigger("0 0 3 * * ?")) + + assertThat(inProgressMonitorConfig(trigger, triggersOfJob = triggers)).isNull() } @Test @@ -261,20 +287,24 @@ class SentryJobListenerTest { .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(), + jobDataMap: JobDataMap = JobDataMap(), + triggersOfJob: List? = null, ): MonitorConfig? { jobDataMap[SentryJobListener.SENTRY_SLUG_KEY] = "my-job" val context = mock() whenever(context.mergedJobDataMap).thenReturn(jobDataMap) whenever(context.trigger).thenReturn(trigger) + if (triggersOfJob != null) { + val jobKey = JobKey.jobKey("my-job") + val jobDetail = mock() + whenever(jobDetail.key).thenReturn(jobKey) + val scheduler = mock() + whenever(scheduler.getTriggersOfJob(jobKey)).thenReturn(triggersOfJob) + whenever(context.jobDetail).thenReturn(jobDetail) + whenever(context.scheduler).thenReturn(scheduler) + } val checkInCaptor = argumentCaptor() whenever(scopes.captureCheckIn(checkInCaptor.capture())).thenReturn(SentryId()) From 6f5ad4e207b188fd341bc5002b58c252c6f9accc Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Mon, 5 Oct 2026 16:30:30 -0700 Subject: [PATCH 10/18] ref(spring): Shorten upsertMonitorConfig Javadoc Co-Authored-By: Claude --- .../src/main/java/io/sentry/spring7/checkin/SentryCheckIn.java | 2 +- .../java/io/sentry/spring/jakarta/checkin/SentryCheckIn.java | 2 +- .../src/main/java/io/sentry/spring/checkin/SentryCheckIn.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 09c8e6f290d..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 @@ -32,7 +32,7 @@ /** * 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, for example if its timezone differs from the JVM's. + * schedule in Sentry instead. * *

Heartbeat check-ins never send a monitor config. * 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 54bd182976b..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 @@ -32,7 +32,7 @@ /** * 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, for example if its timezone differs from the JVM's. + * schedule in Sentry instead. * *

Heartbeat check-ins never send a monitor config. * 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 fcd53debaeb..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 @@ -32,7 +32,7 @@ /** * 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, for example if its timezone differs from the JVM's. + * schedule in Sentry instead. * *

Heartbeat check-ins never send a monitor config. * From d758789134152e23dd2cfd4f443c2137e5b510fe Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Mon, 5 Oct 2026 17:43:42 -0700 Subject: [PATCH 11/18] fix(core): Keep Sunday-only day of week ranges SUN-SUN was rewritten to 0-7, which crontab reads as every day. Co-Authored-By: Claude --- sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java | 3 +++ sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt | 2 ++ 2 files changed, 5 insertions(+) diff --git a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java index fd4b2675690..1e033d335e3 100644 --- a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java +++ b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java @@ -210,6 +210,9 @@ private static boolean isAnyDay(final @NotNull String field) { } if (item.startsWith("*/")) { item = "1-7" + item.substring(1); + } else if (item.startsWith("7-7")) { + // Sunday only; 0-7 would be every day + item = "7" + item.substring(3); } else if (item.startsWith("7-")) { // Sunday starting a range is 0 in Spring item = "0" + item.substring(1); diff --git a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt index f3e08ac99fa..69b8f8833ed 100644 --- a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt @@ -291,6 +291,8 @@ class MonitorConfigUtilsTest { "0 0 9 ? * SUN/2" to "0 9 * * 7/2", "0 0 9 ? * SUN-TUE,fri-sun" to "0 9 * * 0-2,5-7", "0 0 9 ? * 7-2" to "0 9 * * 0-2", + "0 0 9 ? * SUN-SUN" to "0 9 * * 7", + "0 0 9 ? * 7-7/2" to "0 9 * * 7/2", "0 0 9 ? * 0,7" to "0 9 * * 0,7", "0 0 9 ? * FRIL" to "0 9 * * 5L", "0 0 9 ? * MON#2" to "0 9 * * 1#2", From d903a79dbe34a15948084f0cd050c87d5c155b1d Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Mon, 5 Oct 2026 17:43:45 -0700 Subject: [PATCH 12/18] fix(spring): Only catch runtime exceptions when deriving monitor config Co-Authored-By: Claude --- .../java/io/sentry/spring7/checkin/SentryCheckInAdvice.java | 2 +- .../io/sentry/spring/jakarta/checkin/SentryCheckInAdvice.java | 2 +- .../main/java/io/sentry/spring/checkin/SentryCheckInAdvice.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 920ef6a707d..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 @@ -171,7 +171,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl periodMillis(scheduled.fixedRate(), scheduled.fixedRateString(), timeUnit), periodMillis(scheduled.fixedDelay(), scheduled.fixedDelayString(), timeUnit), LEGACY_CRON_PARSER); - } catch (Throwable e) { + } catch (RuntimeException e) { scopes .getOptions() .getLogger() 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 221bb1bed93..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 @@ -171,7 +171,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl periodMillis(scheduled.fixedRate(), scheduled.fixedRateString(), timeUnit), periodMillis(scheduled.fixedDelay(), scheduled.fixedDelayString(), timeUnit), LEGACY_CRON_PARSER); - } catch (Throwable e) { + } catch (RuntimeException e) { scopes .getOptions() .getLogger() 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 83594da523f..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 @@ -174,7 +174,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl periodMillis(scheduled.fixedRate(), scheduled.fixedRateString(), timeUnit), periodMillis(scheduled.fixedDelay(), scheduled.fixedDelayString(), timeUnit), LEGACY_CRON_PARSER); - } catch (Throwable e) { + } catch (RuntimeException e) { scopes .getOptions() .getLogger() From 43e6900b154e8d79baccaedb643edb0e306ccd58 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Mon, 5 Oct 2026 17:43:48 -0700 Subject: [PATCH 13/18] fix(quartz): Catch only expected errors when deriving monitor config Co-Authored-By: Claude --- .../src/main/java/io/sentry/quartz/SentryJobListener.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 c9683a7942f..70bded39c06 100644 --- a/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java +++ b/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java @@ -152,7 +152,8 @@ private boolean shouldUpsertMonitorConfig(final @NotNull JobExecutionContext con return MonitorConfigUtils.fromSchedule(null, null, simpleTrigger.getRepeatInterval()); } return null; - } catch (Throwable e) { + } catch (RuntimeException | SchedulerException | NoSuchMethodError | AbstractMethodError e) { + // Trigger implementations may come from other Quartz versions or third parties scopes .getOptions() .getLogger() From 5db58a4ac5000ec920505be72814fbcb7ec20140 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Thu, 15 Oct 2026 12:02:32 -0700 Subject: [PATCH 14/18] ref(spring): Shorten upsertMonitorConfig Javadoc Co-Authored-By: Claude --- .../main/java/io/sentry/spring7/checkin/SentryCheckIn.java | 4 ++-- .../java/io/sentry/spring/jakarta/checkin/SentryCheckIn.java | 4 ++-- .../src/main/java/io/sentry/spring/checkin/SentryCheckIn.java | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) 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 38491436786..92bd8046ae7 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 @@ -31,8 +31,8 @@ /** * 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. + * Sentry creates or updates the monitor. Set to false to manage the monitor's schedule in Sentry + * instead. * *

Heartbeat check-ins never send a monitor config. * 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 5e180daa0e6..6eb1786bae8 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 @@ -31,8 +31,8 @@ /** * 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. + * Sentry creates or updates the monitor. Set to false to manage the monitor's schedule in Sentry + * instead. * *

Heartbeat check-ins never send a monitor config. * 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 03df99385c2..e3fbb018511 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 @@ -31,8 +31,8 @@ /** * 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. + * Sentry creates or updates the monitor. Set to false to manage the monitor's schedule in Sentry + * instead. * *

Heartbeat check-ins never send a monitor config. * From 1e7aa247acd399c733e889d190b07208c1d5d420 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Thu, 29 Oct 2026 11:54:43 -0700 Subject: [PATCH 15/18] ref(spring): Use pattern-matching instanceof for the @Scheduled time unit --- .../java/io/sentry/spring7/checkin/SentryCheckInAdvice.java | 4 +--- .../io/sentry/spring/jakarta/checkin/SentryCheckInAdvice.java | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) 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 6bcb823032f..651079fc812 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 @@ -160,9 +160,7 @@ public SentryCheckInAdvice(final @NotNull IScopes scopes) { final @Nullable Object timeUnitAttribute = AnnotationUtils.getAnnotationAttributes(scheduled).get("timeUnit"); final @NotNull TimeUnit timeUnit = - timeUnitAttribute instanceof TimeUnit - ? (TimeUnit) timeUnitAttribute - : TimeUnit.MILLISECONDS; + timeUnitAttribute instanceof TimeUnit unit ? unit : TimeUnit.MILLISECONDS; final @Nullable String cron = resolve(scheduled.cron()); // Spring only reads the zone for a cron final @Nullable String zone = 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 1260e9ba05a..f0394964fa1 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 @@ -158,9 +158,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl final @Nullable Object timeUnitAttribute = AnnotationUtils.getAnnotationAttributes(scheduled).get("timeUnit"); final @NotNull TimeUnit timeUnit = - timeUnitAttribute instanceof TimeUnit - ? (TimeUnit) timeUnitAttribute - : TimeUnit.MILLISECONDS; + timeUnitAttribute instanceof TimeUnit unit ? unit : TimeUnit.MILLISECONDS; final @Nullable String cron = resolve(scheduled.cron()); // Spring only reads the zone for a cron final @Nullable String zone = From 0b9b8527584295d78798b7c07018b12cb6dde4d0 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Thu, 5 Nov 2026 11:31:51 -0500 Subject: [PATCH 16/18] fix(core): Use a 2 minute check-in margin for crons with fixed seconds Dropping a non-zero seconds field makes check-ins arrive up to 59s after the expected minute, which a 1 minute margin barely covers. --- .../io/sentry/util/MonitorConfigUtils.java | 13 +++++ .../io/sentry/util/MonitorConfigUtilsTest.kt | 50 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java index 1e033d335e3..4058529b12f 100644 --- a/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java +++ b/sentry/src/main/java/io/sentry/util/MonitorConfigUtils.java @@ -25,6 +25,7 @@ public final class MonitorConfigUtils { private static final @NotNull String CRON_DISABLED = "-"; private static final long MINUTE_MILLIS = TimeUnit.MINUTES.toMillis(1); + private static final long MIN_CHECKIN_MARGIN_WITH_SECONDS = 2; private static final @NotNull Map CRON_MACROS; private static final @NotNull Pattern SIMPLE_DURATION = Pattern.compile("^([+-]?\\d+)([a-zA-Z]{0,2})$"); @@ -91,6 +92,13 @@ private MonitorConfigUtils() {} return null; } final @NotNull MonitorConfig config = new MonitorConfig(MonitorSchedule.crontab(crontab)); + // Runs check in up to 59s after the minute Sentry expects, so a 1 minute margin is too tight + if (hasNonZeroSeconds(cron)) { + final @Nullable Long margin = config.getCheckinMargin(); + if (margin == null || margin < MIN_CHECKIN_MARGIN_WITH_SECONDS) { + config.setCheckinMargin(MIN_CHECKIN_MARGIN_WITH_SECONDS); + } + } if (zone != null && !zone.isEmpty()) { final @Nullable String timezone = toTimezone(zone); if (timezone == null) { @@ -123,6 +131,11 @@ private MonitorConfigUtils() {} return null; } + private static boolean hasNonZeroSeconds(final @NotNull String cron) { + final @NotNull String seconds = cron.trim().split("\\s+", -1)[0]; + return DIGITS.matcher(seconds).matches() && Integer.parseInt(seconds) > 0; + } + /** * Builds a monitor config from Spring {@code @Scheduled} values with placeholders resolved. Reads * the cron and a cron without zone the way Spring does. diff --git a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt index 69b8f8833ed..e882b3ed070 100644 --- a/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/MonitorConfigUtilsTest.kt @@ -1,9 +1,15 @@ package io.sentry.util import com.google.common.truth.Truth.assertThat +import io.sentry.IScopes +import io.sentry.Sentry +import io.sentry.SentryOptions import java.util.TimeZone import java.util.concurrent.TimeUnit import kotlin.test.Test +import org.mockito.Mockito +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever class MonitorConfigUtilsTest { @@ -18,6 +24,50 @@ class MonitorConfigUtilsTest { assertThat(config.timezone).isNull() } + @Test + fun `cron at second 0 keeps the default check-in margin`() { + val config = MonitorConfigUtils.fromSchedule("0 * * * * *", null, null) + + assertThat(config!!.checkinMargin).isNull() + } + + @Test + fun `cron with non-zero seconds gets a 2 minute check-in margin`() { + for (cron in listOf("1 * * * * *", "59 * * * * *", "30 15 10 * * *")) { + assertThat(MonitorConfigUtils.fromSchedule(cron, null, null)!!.checkinMargin).isEqualTo(2L) + } + } + + @Test + fun `cron with non-zero seconds raises a smaller default check-in margin`() { + withDefaultCheckinMargin(1L) { + assertThat(MonitorConfigUtils.fromSchedule("59 * * * * *", null, null)!!.checkinMargin) + .isEqualTo(2L) + } + } + + @Test + fun `cron with non-zero seconds keeps a larger default check-in margin`() { + withDefaultCheckinMargin(5L) { + assertThat(MonitorConfigUtils.fromSchedule("59 * * * * *", null, null)!!.checkinMargin) + .isEqualTo(5L) + } + } + + private fun withDefaultCheckinMargin(margin: Long, block: () -> Unit) { + Mockito.mockStatic(Sentry::class.java).use { sentry -> + val scopes = mock() + sentry.`when` { Sentry.getCurrentScopes() }.thenReturn(scopes) + whenever(scopes.options) + .thenReturn( + SentryOptions().apply { + cron = SentryOptions.Cron().apply { defaultCheckinMargin = margin } + } + ) + block() + } + } + @Test fun `cron whitespace is normalized`() { val config = MonitorConfigUtils.fromSchedule(" 30 0 2 * * * ", null, null) From 129cb735b56551cc5df5ed028642ab9feeaa947c Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Thu, 5 Nov 2026 11:37:19 -0500 Subject: [PATCH 17/18] fix(spring): Default upsertMonitorConfig to false and read @Scheduled like Spring Existing @SentryCheckIn users shouldn't have their monitors changed on upgrade, so sending the schedule is opt-in. Find @Scheduled with Spring's own lookup so an overriding method doesn't inherit a parent's schedule. --- CHANGELOG.md | 2 +- .../sentry/spring7/checkin/SentryCheckIn.java | 7 +-- .../spring7/checkin/SentryCheckInAdvice.java | 4 +- .../sentry/spring7/SentryCheckInAdviceTest.kt | 62 ++++++++++++++----- .../spring/jakarta/checkin/SentryCheckIn.java | 7 +-- .../jakarta/checkin/SentryCheckInAdvice.java | 4 +- .../spring/jakarta/SentryCheckInAdviceTest.kt | 62 ++++++++++++++----- .../sentry/spring/checkin/SentryCheckIn.java | 7 +-- .../spring/checkin/SentryCheckInAdvice.java | 4 +- .../sentry/spring/SentryCheckInAdviceTest.kt | 62 ++++++++++++++----- 10 files changed, 157 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2434542f46..50cb3134dfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +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)) +- Send a monitor config derived from Spring `@Scheduled` with `@SentryCheckIn` check-ins, so Sentry creates or updates the monitor from code. Set `@SentryCheckIn(upsertMonitorConfig = true)` to send the schedule ([#6215](https://github.com/getsentry/sentry-java/pull/6215)) - Added `Environment` to the `Scope`, allowing you to overwrite the `Environment` set on the options ([#5772](https://github.com/getsentry/sentry-java/pull/5772)) - The scope `Environment` is also applied to the trace envelope header (DSC), logs, metrics and profiles, so all telemetry of a trace reports the same environment - Add an `attachTracingHeaders` flag to `SentryApolloInterceptor` to stop attaching `sentry-trace` and `baggage` headers to Apollo v2 requests ([#6414](https://github.com/getsentry/sentry-java/pull/6414)) 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 92bd8046ae7..ac17f85a985 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 @@ -31,14 +31,13 @@ /** * Whether to send the schedule and zone from the method's {@code @Scheduled} with check-ins, so - * Sentry creates or updates the monitor. Set to false to manage the monitor's schedule in Sentry - * instead. + * Sentry creates or updates the monitor. * *

Heartbeat check-ins never send a monitor config. * - * @return true to send a monitor config, true by default + * @return true to send a monitor config, false by default */ - boolean upsertMonitorConfig() default true; + boolean upsertMonitorConfig() default false; /** * 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 651079fc812..ca0137d7714 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 @@ -31,6 +31,7 @@ import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.scheduling.annotation.Schedules; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringValueResolver; @@ -151,7 +152,8 @@ public SentryCheckInAdvice(final @NotNull IScopes scopes) { private @Nullable MonitorConfig monitorConfigFromScheduled(final @NotNull Method method) { try { final @NotNull Set schedules = - AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Scheduled.class); + AnnotatedElementUtils.getMergedRepeatableAnnotations( + method, Scheduled.class, Schedules.class); if (schedules.size() != 1) { return null; } 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 a071ba293ca..22e02b0b297 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 @@ -67,6 +67,8 @@ class SentryCheckInAdviceTest { @Autowired lateinit var sampleServiceScheduled: SampleServiceScheduled + @Autowired lateinit var sampleServiceInheritedSchedule: SampleServiceInheritedSchedule + @Autowired lateinit var scopes: IScopes val lifecycleToken = mock() @@ -370,6 +372,16 @@ class SentryCheckInAdviceTest { assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDisabled() }) } + @Test + fun `upsertMonitorConfig defaults to false`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDefault() }) + } + + @Test + fun `@Scheduled on an overridden parent method sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceInheritedSchedule.job() }) + } + @Test fun `heartbeat with @Scheduled sends a single check-in without monitor config`() { val checkInCaptor = argumentCaptor() @@ -405,6 +417,8 @@ class SentryCheckInAdviceTest { @Bean open fun sampleServiceScheduled() = SampleServiceScheduled() + @Bean open fun sampleServiceInheritedSchedule() = SampleServiceInheritedSchedule() + @Bean open fun scopes(): IScopes { val scopes = mock() @@ -457,57 +471,63 @@ class SentryCheckInAdviceTest { open class SampleServiceScheduled { - @SentryCheckIn("cron_zone") + @SentryCheckIn("cron_zone", upsertMonitorConfig = true) @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", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 2 * * *") + open fun cron() {} - @SentryCheckIn("cron_properties") + @SentryCheckIn("cron_properties", upsertMonitorConfig = true) @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") open fun cronFromProperties() {} - @SentryCheckIn("fixed_rate_hours") + @SentryCheckIn("fixed_rate_hours", upsertMonitorConfig = true) @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) open fun fixedRateHours() {} - @SentryCheckIn("fixed_rate_iso") + @SentryCheckIn("fixed_rate_iso", upsertMonitorConfig = true) @Scheduled(fixedRateString = "PT10M") open fun fixedRateIso() {} - @SentryCheckIn("fixed_rate_simple") + @SentryCheckIn("fixed_rate_simple", upsertMonitorConfig = true) @Scheduled(fixedRateString = "5m") open fun fixedRateSimpleDuration() {} - @SentryCheckIn("fixed_delay") @Scheduled(fixedDelay = 300_000) open fun fixedDelay() {} + @SentryCheckIn("fixed_delay", upsertMonitorConfig = true) + @Scheduled(fixedDelay = 300_000) + open fun fixedDelay() {} - @SentryCheckIn("fixed_delay_iso") + @SentryCheckIn("fixed_delay_iso", upsertMonitorConfig = true) @Scheduled(fixedDelayString = "PT10M") open fun fixedDelayIso() {} - @SentryCheckIn("disabled_cron") @Scheduled(cron = "-") open fun disabledCron() {} + @SentryCheckIn("disabled_cron", upsertMonitorConfig = true) + @Scheduled(cron = "-") + open fun disabledCron() {} - @SentryCheckIn("cron_empty_zone") + @SentryCheckIn("cron_empty_zone", upsertMonitorConfig = true) @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") open fun cronWithEmptyZone() {} - @SentryCheckIn("cron_zone_defaults") + @SentryCheckIn("cron_zone_defaults", upsertMonitorConfig = true) @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") open fun cronWithZoneAndDefaults() {} - @SentryCheckIn("schedules_container") + @SentryCheckIn("schedules_container", upsertMonitorConfig = true) @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) open fun schedulesContainer() {} - @SentryCheckIn("unresolvable_cron") + @SentryCheckIn("unresolvable_cron", upsertMonitorConfig = true) @Scheduled(cron = "\${my.cron.missing}") open fun unresolvableCron() {} - @SentryCheckIn("derivation_throws") + @SentryCheckIn("derivation_throws", upsertMonitorConfig = true) @Scheduled(cron = "\${my.cron.exception.property}") open fun derivationThrows() = 1 - @SentryCheckIn("multiple") + @SentryCheckIn("multiple", upsertMonitorConfig = true) @Scheduled(cron = "0 0 1 * * *") @Scheduled(cron = "0 0 13 * * *") open fun multipleSchedules() {} @@ -516,11 +536,21 @@ class SentryCheckInAdviceTest { @Scheduled(cron = "0 0 1 * * *") open fun upsertDisabled() {} - @SentryCheckIn("heartbeat", heartbeat = true) + @SentryCheckIn("upsert_default") @Scheduled(cron = "0 0 1 * * *") open fun upsertDefault() {} + + @SentryCheckIn("heartbeat", heartbeat = true, upsertMonitorConfig = true) @Scheduled(cron = "0 0 1 * * *") open fun heartbeat() {} } + open class ScheduledParent { + @Scheduled(cron = "0 0 1 * * *") open fun job() {} + } + + open class SampleServiceInheritedSchedule : ScheduledParent() { + @SentryCheckIn("inherited", upsertMonitorConfig = true) override fun job() {} + } + class MyPropertyPlaceholderConfigurer : PropertySourcesPlaceholderConfigurer() { override fun doProcessProperties( 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 6eb1786bae8..89b04aa561a 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 @@ -31,14 +31,13 @@ /** * Whether to send the schedule and zone from the method's {@code @Scheduled} with check-ins, so - * Sentry creates or updates the monitor. Set to false to manage the monitor's schedule in Sentry - * instead. + * Sentry creates or updates the monitor. * *

Heartbeat check-ins never send a monitor config. * - * @return true to send a monitor config, true by default + * @return true to send a monitor config, false by default */ - boolean upsertMonitorConfig() default true; + boolean upsertMonitorConfig() default false; /** * 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 f0394964fa1..76b03a917bc 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 @@ -31,6 +31,7 @@ import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.scheduling.annotation.Schedules; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringValueResolver; @@ -149,7 +150,8 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl private @Nullable MonitorConfig monitorConfigFromScheduled(final @NotNull Method method) { try { final @NotNull Set schedules = - AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Scheduled.class); + AnnotatedElementUtils.getMergedRepeatableAnnotations( + method, Scheduled.class, Schedules.class); if (schedules.size() != 1) { return null; } 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 7a7e5b81d04..84f6e2a8118 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 @@ -67,6 +67,8 @@ class SentryCheckInAdviceTest { @Autowired lateinit var sampleServiceScheduled: SampleServiceScheduled + @Autowired lateinit var sampleServiceInheritedSchedule: SampleServiceInheritedSchedule + @Autowired lateinit var scopes: IScopes val lifecycleToken = mock() @@ -370,6 +372,16 @@ class SentryCheckInAdviceTest { assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDisabled() }) } + @Test + fun `upsertMonitorConfig defaults to false`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDefault() }) + } + + @Test + fun `@Scheduled on an overridden parent method sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceInheritedSchedule.job() }) + } + @Test fun `heartbeat with @Scheduled sends a single check-in without monitor config`() { val checkInCaptor = argumentCaptor() @@ -405,6 +417,8 @@ class SentryCheckInAdviceTest { @Bean open fun sampleServiceScheduled() = SampleServiceScheduled() + @Bean open fun sampleServiceInheritedSchedule() = SampleServiceInheritedSchedule() + @Bean open fun scopes(): IScopes { val scopes = mock() @@ -457,57 +471,63 @@ class SentryCheckInAdviceTest { open class SampleServiceScheduled { - @SentryCheckIn("cron_zone") + @SentryCheckIn("cron_zone", upsertMonitorConfig = true) @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", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 2 * * *") + open fun cron() {} - @SentryCheckIn("cron_properties") + @SentryCheckIn("cron_properties", upsertMonitorConfig = true) @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") open fun cronFromProperties() {} - @SentryCheckIn("fixed_rate_hours") + @SentryCheckIn("fixed_rate_hours", upsertMonitorConfig = true) @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) open fun fixedRateHours() {} - @SentryCheckIn("fixed_rate_iso") + @SentryCheckIn("fixed_rate_iso", upsertMonitorConfig = true) @Scheduled(fixedRateString = "PT10M") open fun fixedRateIso() {} - @SentryCheckIn("fixed_rate_simple") + @SentryCheckIn("fixed_rate_simple", upsertMonitorConfig = true) @Scheduled(fixedRateString = "5m") open fun fixedRateSimpleDuration() {} - @SentryCheckIn("fixed_delay") @Scheduled(fixedDelay = 300_000) open fun fixedDelay() {} + @SentryCheckIn("fixed_delay", upsertMonitorConfig = true) + @Scheduled(fixedDelay = 300_000) + open fun fixedDelay() {} - @SentryCheckIn("fixed_delay_iso") + @SentryCheckIn("fixed_delay_iso", upsertMonitorConfig = true) @Scheduled(fixedDelayString = "PT10M") open fun fixedDelayIso() {} - @SentryCheckIn("disabled_cron") @Scheduled(cron = "-") open fun disabledCron() {} + @SentryCheckIn("disabled_cron", upsertMonitorConfig = true) + @Scheduled(cron = "-") + open fun disabledCron() {} - @SentryCheckIn("cron_empty_zone") + @SentryCheckIn("cron_empty_zone", upsertMonitorConfig = true) @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") open fun cronWithEmptyZone() {} - @SentryCheckIn("cron_zone_defaults") + @SentryCheckIn("cron_zone_defaults", upsertMonitorConfig = true) @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") open fun cronWithZoneAndDefaults() {} - @SentryCheckIn("schedules_container") + @SentryCheckIn("schedules_container", upsertMonitorConfig = true) @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) open fun schedulesContainer() {} - @SentryCheckIn("unresolvable_cron") + @SentryCheckIn("unresolvable_cron", upsertMonitorConfig = true) @Scheduled(cron = "\${my.cron.missing}") open fun unresolvableCron() {} - @SentryCheckIn("derivation_throws") + @SentryCheckIn("derivation_throws", upsertMonitorConfig = true) @Scheduled(cron = "\${my.cron.exception.property}") open fun derivationThrows() = 1 - @SentryCheckIn("multiple") + @SentryCheckIn("multiple", upsertMonitorConfig = true) @Scheduled(cron = "0 0 1 * * *") @Scheduled(cron = "0 0 13 * * *") open fun multipleSchedules() {} @@ -516,11 +536,21 @@ class SentryCheckInAdviceTest { @Scheduled(cron = "0 0 1 * * *") open fun upsertDisabled() {} - @SentryCheckIn("heartbeat", heartbeat = true) + @SentryCheckIn("upsert_default") @Scheduled(cron = "0 0 1 * * *") open fun upsertDefault() {} + + @SentryCheckIn("heartbeat", heartbeat = true, upsertMonitorConfig = true) @Scheduled(cron = "0 0 1 * * *") open fun heartbeat() {} } + open class ScheduledParent { + @Scheduled(cron = "0 0 1 * * *") open fun job() {} + } + + open class SampleServiceInheritedSchedule : ScheduledParent() { + @SentryCheckIn("inherited", upsertMonitorConfig = true) override fun job() {} + } + class MyPropertyPlaceholderConfigurer : PropertySourcesPlaceholderConfigurer() { override fun doProcessProperties( 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 e3fbb018511..55fedd93c2e 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 @@ -31,14 +31,13 @@ /** * Whether to send the schedule and zone from the method's {@code @Scheduled} with check-ins, so - * Sentry creates or updates the monitor. Set to false to manage the monitor's schedule in Sentry - * instead. + * Sentry creates or updates the monitor. * *

Heartbeat check-ins never send a monitor config. * - * @return true to send a monitor config, true by default + * @return true to send a monitor config, false by default */ - boolean upsertMonitorConfig() default true; + boolean upsertMonitorConfig() default false; /** * 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 202bd30e56a..d43d72a03bb 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 @@ -31,6 +31,7 @@ import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.scheduling.annotation.Schedules; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringValueResolver; @@ -152,7 +153,8 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl private @Nullable MonitorConfig monitorConfigFromScheduled(final @NotNull Method method) { try { final @NotNull Set schedules = - AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Scheduled.class); + AnnotatedElementUtils.getMergedRepeatableAnnotations( + method, Scheduled.class, Schedules.class); if (schedules.size() != 1) { return null; } 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 b6e8a4e3de2..0bac0df528c 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryCheckInAdviceTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryCheckInAdviceTest.kt @@ -68,6 +68,8 @@ class SentryCheckInAdviceTest { @Autowired lateinit var sampleServiceScheduled: SampleServiceScheduled + @Autowired lateinit var sampleServiceInheritedSchedule: SampleServiceInheritedSchedule + @Autowired lateinit var scopes: IScopes val lifecycleToken = mock() @@ -371,6 +373,16 @@ class SentryCheckInAdviceTest { assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDisabled() }) } + @Test + fun `upsertMonitorConfig defaults to false`() { + assertNull(inProgressMonitorConfig { sampleServiceScheduled.upsertDefault() }) + } + + @Test + fun `@Scheduled on an overridden parent method sends no monitor config`() { + assertNull(inProgressMonitorConfig { sampleServiceInheritedSchedule.job() }) + } + @Test fun `heartbeat with @Scheduled sends a single check-in without monitor config`() { val checkInCaptor = argumentCaptor() @@ -406,6 +418,8 @@ class SentryCheckInAdviceTest { @Bean open fun sampleServiceScheduled() = SampleServiceScheduled() + @Bean open fun sampleServiceInheritedSchedule() = SampleServiceInheritedSchedule() + @Bean open fun scopes(): IScopes { val scopes = mock() @@ -458,57 +472,63 @@ class SentryCheckInAdviceTest { open class SampleServiceScheduled { - @SentryCheckIn("cron_zone") + @SentryCheckIn("cron_zone", upsertMonitorConfig = true) @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", upsertMonitorConfig = true) + @Scheduled(cron = "0 0 2 * * *") + open fun cron() {} - @SentryCheckIn("cron_properties") + @SentryCheckIn("cron_properties", upsertMonitorConfig = true) @Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}") open fun cronFromProperties() {} - @SentryCheckIn("fixed_rate_hours") + @SentryCheckIn("fixed_rate_hours", upsertMonitorConfig = true) @Scheduled(fixedRate = 2, timeUnit = TimeUnit.HOURS) open fun fixedRateHours() {} - @SentryCheckIn("fixed_rate_iso") + @SentryCheckIn("fixed_rate_iso", upsertMonitorConfig = true) @Scheduled(fixedRateString = "PT10M") open fun fixedRateIso() {} - @SentryCheckIn("fixed_rate_simple") + @SentryCheckIn("fixed_rate_simple", upsertMonitorConfig = true) @Scheduled(fixedRateString = "5m") open fun fixedRateSimpleDuration() {} - @SentryCheckIn("fixed_delay") @Scheduled(fixedDelay = 300_000) open fun fixedDelay() {} + @SentryCheckIn("fixed_delay", upsertMonitorConfig = true) + @Scheduled(fixedDelay = 300_000) + open fun fixedDelay() {} - @SentryCheckIn("fixed_delay_iso") + @SentryCheckIn("fixed_delay_iso", upsertMonitorConfig = true) @Scheduled(fixedDelayString = "PT10M") open fun fixedDelayIso() {} - @SentryCheckIn("disabled_cron") @Scheduled(cron = "-") open fun disabledCron() {} + @SentryCheckIn("disabled_cron", upsertMonitorConfig = true) + @Scheduled(cron = "-") + open fun disabledCron() {} - @SentryCheckIn("cron_empty_zone") + @SentryCheckIn("cron_empty_zone", upsertMonitorConfig = true) @Scheduled(cron = "0 0 3 * * *", zone = "\${my.cron.empty.zone}") open fun cronWithEmptyZone() {} - @SentryCheckIn("cron_zone_defaults") + @SentryCheckIn("cron_zone_defaults", upsertMonitorConfig = true) @Scheduled(cron = "0 0 4 * * *", zone = "Europe/Vienna") open fun cronWithZoneAndDefaults() {} - @SentryCheckIn("schedules_container") + @SentryCheckIn("schedules_container", upsertMonitorConfig = true) @Schedules(Scheduled(cron = "0 0 1 * * *"), Scheduled(cron = "0 0 13 * * *")) open fun schedulesContainer() {} - @SentryCheckIn("unresolvable_cron") + @SentryCheckIn("unresolvable_cron", upsertMonitorConfig = true) @Scheduled(cron = "\${my.cron.missing}") open fun unresolvableCron() {} - @SentryCheckIn("derivation_throws") + @SentryCheckIn("derivation_throws", upsertMonitorConfig = true) @Scheduled(cron = "\${my.cron.exception.property}") open fun derivationThrows() = 1 - @SentryCheckIn("multiple") + @SentryCheckIn("multiple", upsertMonitorConfig = true) @Scheduled(cron = "0 0 1 * * *") @Scheduled(cron = "0 0 13 * * *") open fun multipleSchedules() {} @@ -517,11 +537,21 @@ class SentryCheckInAdviceTest { @Scheduled(cron = "0 0 1 * * *") open fun upsertDisabled() {} - @SentryCheckIn("heartbeat", heartbeat = true) + @SentryCheckIn("upsert_default") @Scheduled(cron = "0 0 1 * * *") open fun upsertDefault() {} + + @SentryCheckIn("heartbeat", heartbeat = true, upsertMonitorConfig = true) @Scheduled(cron = "0 0 1 * * *") open fun heartbeat() {} } + open class ScheduledParent { + @Scheduled(cron = "0 0 1 * * *") open fun job() {} + } + + open class SampleServiceInheritedSchedule : ScheduledParent() { + @SentryCheckIn("inherited", upsertMonitorConfig = true) override fun job() {} + } + class MyPropertyPlaceholderConfigurer : PropertySourcesPlaceholderConfigurer() { override fun doProcessProperties( From 0493cceb68c5eb0804ffb72c512c2dab26a2abf2 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Thu, 5 Nov 2026 11:38:15 -0500 Subject: [PATCH 18/18] fix(quartz): Send the trigger's monitor config only when opted in Match @SentryCheckIn: existing check-in users keep their monitors unchanged unless they set sentry-upsert-monitor-config to "true". --- .../io/sentry/quartz/SentryJobListener.java | 8 ++++---- .../io/sentry/quartz/SentryJobListenerTest.kt | 20 ++++++++++++++++--- 2 files changed, 21 insertions(+), 7 deletions(-) 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 70bded39c06..a548066e157 100644 --- a/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java +++ b/sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java @@ -46,8 +46,8 @@ public final class SentryJobListener implements JobListener { public static final String SENTRY_SCOPE_LIFECYCLE_TOKEN_KEY = "sentry-scope-lifecycle"; /** - * Job or trigger data key. The listener sends a monitor config from the trigger with check-ins by - * default; set this to {@code "false"} to manage the monitor's schedule in Sentry instead. + * Job or trigger data key. Set it to {@code "true"} to send the trigger's schedule with + * check-ins, so Sentry creates or updates the monitor. */ public static final String SENTRY_UPSERT_MONITOR_CONFIG_KEY = "sentry-upsert-monitor-config"; @@ -116,10 +116,10 @@ public void jobToBeExecuted(final @NotNull JobExecutionContext context) { private boolean shouldUpsertMonitorConfig(final @NotNull JobExecutionContext context) { final @Nullable JobDataMap jobDataMap = context.getMergedJobDataMap(); if (jobDataMap == null) { - return true; + return false; } final @Nullable Object o = jobDataMap.get(SENTRY_UPSERT_MONITOR_CONFIG_KEY); - return o == null || !"false".equalsIgnoreCase(o.toString().trim()); + return o != null && "true".equalsIgnoreCase(o.toString().trim()); } private @Nullable MonitorConfig monitorConfigFromTrigger( diff --git a/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt b/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt index 9be624dceb2..7f6ddd68bab 100644 --- a/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt +++ b/sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt @@ -180,8 +180,16 @@ class SentryJobListenerTest { } @Test - fun `sends monitor config by default`() { - assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ?"), JobDataMap())).isNotNull() + fun `sends no monitor config by default`() { + 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 @@ -287,9 +295,15 @@ class SentryJobListenerTest { .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 = JobDataMap(), + jobDataMap: JobDataMap = upsertJobDataMap(), triggersOfJob: List? = null, ): MonitorConfig? { jobDataMap[SentryJobListener.SENTRY_SLUG_KEY] = "my-job"