Skip to content

Commit d7a2468

Browse files
committed
fix(quartz): Skip name steps and calendars, send 7/n as Saturday
Quartz ignores a step after a day or month name, and Sentry can't express calendar exclusions, so send no config for either. 7/n only fires on Saturday.
1 parent ea073b2 commit d7a2468

2 files changed

Lines changed: 38 additions & 5 deletions

File tree

‎sentry-quartz/src/main/java/io/sentry/quartz/SentryJobListener.java‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.List;
1919
import java.util.Locale;
2020
import java.util.TimeZone;
21+
import java.util.regex.Pattern;
2122
import org.jetbrains.annotations.ApiStatus;
2223
import org.jetbrains.annotations.NotNull;
2324
import org.jetbrains.annotations.Nullable;
@@ -47,6 +48,10 @@ public final class SentryJobListener implements JobListener {
4748
private static final @NotNull List<String> DAY_NAMES =
4849
Arrays.asList("SUN", "MON", "TUE", "WED", "THU", "FRI", "SAT");
4950

51+
// Quartz ignores a step after a name, so SUN/2 is every Sunday
52+
private static final @NotNull Pattern NAME_STEP =
53+
Pattern.compile("(^|,)[A-Za-z]{3}(-[A-Za-z]{3})?/");
54+
5055
private final @NotNull IScopes scopes;
5156

5257
public SentryJobListener() {
@@ -115,6 +120,10 @@ private boolean shouldUpsertMonitorConfig(final @NotNull JobExecutionContext con
115120

116121
private @Nullable MonitorConfig monitorConfigFromTrigger(final @Nullable Trigger trigger) {
117122
try {
123+
// Sentry can't express calendar exclusions
124+
if (trigger == null || trigger.getCalendarName() != null) {
125+
return null;
126+
}
118127
if (trigger instanceof CronTrigger) {
119128
final @NotNull CronTrigger cronTrigger = (CronTrigger) trigger;
120129
final @Nullable String cron = toSixFieldCron(cronTrigger.getCronExpression());
@@ -123,14 +132,14 @@ private boolean shouldUpsertMonitorConfig(final @NotNull JobExecutionContext con
123132
}
124133
final @Nullable TimeZone timeZone = cronTrigger.getTimeZone();
125134
return MonitorConfigUtils.fromSchedule(
126-
cron, timeZone == null ? null : timeZone.getID(), null, null);
135+
cron, timeZone == null ? null : timeZone.getID(), null);
127136
}
128137
if (trigger instanceof SimpleTrigger) {
129138
final @NotNull SimpleTrigger simpleTrigger = (SimpleTrigger) trigger;
130139
if (simpleTrigger.getRepeatCount() != SimpleTrigger.REPEAT_INDEFINITELY) {
131140
return null;
132141
}
133-
return MonitorConfigUtils.fromSchedule(null, null, simpleTrigger.getRepeatInterval(), null);
142+
return MonitorConfigUtils.fromSchedule(null, null, simpleTrigger.getRepeatInterval());
134143
}
135144
return null;
136145
} catch (RuntimeException e) {
@@ -143,7 +152,10 @@ private boolean shouldUpsertMonitorConfig(final @NotNull JobExecutionContext con
143152
}
144153
}
145154

146-
/** Quartz numbers days of week 1-7 from Sunday; crontab uses 0-6. Null if a year is set. */
155+
/**
156+
* Quartz numbers days of week 1-7 from Sunday; crontab uses 0-6. Null if a year is set or a name
157+
* has a step.
158+
*/
147159
static @Nullable String toSixFieldCron(final @Nullable String quartzCron) {
148160
if (quartzCron == null) {
149161
return null;
@@ -155,6 +167,9 @@ private boolean shouldUpsertMonitorConfig(final @NotNull JobExecutionContext con
155167
if (fields.length != 6 && fields.length != 7) {
156168
return null;
157169
}
170+
if (NAME_STEP.matcher(fields[4]).find() || NAME_STEP.matcher(fields[5]).find()) {
171+
return null;
172+
}
158173
final @Nullable String dayOfWeek = toZeroBasedDayOfWeek(fields[5]);
159174
if (dayOfWeek == null) {
160175
return null;
@@ -210,8 +225,11 @@ private boolean shouldUpsertMonitorConfig(final @NotNull JobExecutionContext con
210225
final @Nullable Integer end = toZeroBasedDay(range[1]);
211226
return end == null ? null : start + "-" + end + step;
212227
}
228+
if (slash < 0 || start == 6) {
229+
return String.valueOf(start);
230+
}
213231
// a step from a single day ends on Saturday in Quartz, but on Sunday (7) in crontab
214-
return slash >= 0 ? start + "-6" + step : String.valueOf(start);
232+
return start + "-6" + step;
215233
}
216234

217235
private static @Nullable Integer toZeroBasedDay(final @NotNull String day) {

‎sentry-quartz/src/test/kotlin/io/sentry/quartz/SentryJobListenerTest.kt‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,17 @@ class SentryJobListenerTest {
165165
assertThat(inProgressMonitorConfig(trigger)).isNull()
166166
}
167167

168+
@Test
169+
fun `trigger with a calendar sends no monitor config`() {
170+
val trigger =
171+
TriggerBuilder.newTrigger()
172+
.withSchedule(CronScheduleBuilder.cronSchedule("0 0 2 * * ?"))
173+
.modifiedByCalendar("holidays")
174+
.build()
175+
176+
assertThat(inProgressMonitorConfig(trigger)).isNull()
177+
}
178+
168179
@Test
169180
fun `without upsert monitor config key sends no monitor config`() {
170181
assertThat(inProgressMonitorConfig(cronTrigger("0 0 2 * * ?"), JobDataMap())).isNull()
@@ -209,7 +220,7 @@ class SentryJobListenerTest {
209220
"0 0 12 ? * FRIL" to "0 0 12 ? * 5L",
210221
"0 0 12 ? * 1/2" to "0 0 12 ? * 0-6/2",
211222
"0 0 12 ? * 2/2" to "0 0 12 ? * 1-6/2",
212-
"0 0 12 ? * MON/2" to "0 0 12 ? * 1-6/2",
223+
"0 0 12 ? * 7/2" to "0 0 12 ? * 6",
213224
"0 0 12 ? * 2-6/2" to "0 0 12 ? * 1-5/2",
214225
"0 0 12 ? * */2" to "0 0 12 ? * */2",
215226
"0 0 12 L * ?" to "0 0 12 L * ?",
@@ -227,6 +238,10 @@ class SentryJobListenerTest {
227238
"0 0 12 ? * L",
228239
"0 0 12 ? * 8",
229240
"0 0 12 ? * XYZL",
241+
"0 0 12 ? * SUN/2",
242+
"0 0 12 ? * mon-wed/3",
243+
"0 0 12 ? * 1,MON/2",
244+
"0 0 12 * JUN/3 ?",
230245
"0 12 * * *",
231246
)) {
232247
assertThat(SentryJobListener.toSixFieldCron(quartz)).isNull()

0 commit comments

Comments
 (0)