-
Notifications
You must be signed in to change notification settings - Fork 28
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #106 from mattspataro/guardrails-for-monitoring
Change Piezo UI to Prevent Job Trigger Periods that are Longer than the Monitoring Period
- Loading branch information
Showing
3 changed files
with
308 additions
and
38 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
169 changes: 169 additions & 0 deletions
169
admin/app/com/lucidchart/piezo/admin/utils/CronHelper.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| package com.lucidchart.piezo.admin.utils | ||
|
|
||
| import java.time.temporal.{ChronoUnit, TemporalUnit} | ||
| import java.time.{Instant, LocalDate, Month, ZoneOffset} | ||
| import java.util.{Date, TimeZone} | ||
| import org.quartz.CronExpression | ||
| import play.api.Logging | ||
| import scala.annotation.tailrec | ||
| import scala.util.control.NonFatal | ||
|
|
||
| object CronHelper extends Logging { | ||
| val IMPOSSIBLE_MAX_INTERVAL: Long = Long.MaxValue | ||
| val DEFAULT_MAX_INTERVAL = 0 | ||
| val NON_EXISTENT: Int = -1 | ||
|
|
||
| /** | ||
| * Approximates the largest interval between two trigger events for a given cron expression. This is a difficult | ||
| * problem to solve perfectly, so this represents a "best effort approach" - the goal is to handle the most | ||
| * expressions with the least amount of complexity. | ||
| * | ||
| * Known limitations: | ||
| * 1. Daylight savings | ||
| * 1. Complex year subexpressions | ||
| * @param cronExpression | ||
| */ | ||
| def getMaxInterval(cronExpression: String): Long = { | ||
| try { | ||
| val (secondsMinutesHourStrings, dayStrings) = cronExpression.split("\\s+").splitAt(3) | ||
| val subexpressions = getSubexpressions(secondsMinutesHourStrings :+ dayStrings.mkString(" ")).reverse | ||
|
|
||
| // find the largest subexpression that is not continuously triggering (*) | ||
| val outermostIndex = subexpressions.indexWhere(!_.isContinuouslyTriggering) | ||
| if (outermostIndex == NON_EXISTENT) 1 | ||
| else { | ||
| // get the max interval for this expression | ||
| val outermost = subexpressions(outermostIndex) | ||
| if (outermost.maxInterval == IMPOSSIBLE_MAX_INTERVAL) IMPOSSIBLE_MAX_INTERVAL | ||
| else { | ||
| // subtract the inner intervals of the smaller, nested subexpressions | ||
| val nested = subexpressions.slice(outermostIndex + 1, subexpressions.size) | ||
| val innerIntervalsOfNested = nested.collect { case expr: BoundSubexpression => expr.innerInterval }.sum | ||
| outermost.maxInterval - innerIntervalsOfNested | ||
| } | ||
| } | ||
|
|
||
| } catch { | ||
| case NonFatal(e) => | ||
| logger.error("Failed to validate cron expression", e) | ||
| DEFAULT_MAX_INTERVAL | ||
| } | ||
| } | ||
|
|
||
| private def getSubexpressions(parts: Array[String]): IndexedSeq[Subexpression] = { | ||
| parts | ||
| .zip(List(Seconds, Minutes, Hours, Days)) | ||
| .map { case (str, cronType) => cronType(str) } | ||
| .toIndexedSeq | ||
| } | ||
| } | ||
|
|
||
| case class Seconds(str: String) extends BoundSubexpression(str, x => s"$x * * ? * *", ChronoUnit.SECONDS, 60) | ||
| case class Minutes(str: String) extends BoundSubexpression(str, x => s"0 $x * ? * *", ChronoUnit.MINUTES, 60) | ||
| case class Hours(str: String) extends BoundSubexpression(str, x => s"0 0 $x ? * *", ChronoUnit.HOURS, 24) | ||
| case class Days(str: String) extends UnboundSubexpression(str, x => s"0 0 0 $x", 400) | ||
|
|
||
| abstract class Subexpression(str: String, getSimplifiedCron: String => String) { | ||
| def maxInterval: Long | ||
| def isContinuouslyTriggering: Boolean | ||
|
|
||
| protected def startDate: Date | ||
| final protected lazy val cron: CronExpression = { | ||
| val newCron = new CronExpression(getSimplifiedCron(str)) | ||
| newCron.setTimeZone(TimeZone.getTimeZone("UTC")) // use a timezone without daylight savings | ||
| newCron | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Represents a subexpression in which the range over which the triggers occur is bound or fixed. For example, seconds | ||
| * always occur within a minute, minutes always occur within an hour, and hours always occur within a day. Because the | ||
| * range is fixed, we can determine all possibilities by sampling over the entire range. | ||
| */ | ||
| abstract class BoundSubexpression( | ||
| str: String, | ||
| getSimplifiedCron: String => String, | ||
| temporalUnit: TemporalUnit, | ||
| val numUnitsInContainer: Long, | ||
| ) extends Subexpression(str, getSimplifiedCron) { | ||
|
|
||
| final override protected val startDate = new Date(BoundSubexpression.startInstant.toEpochMilli) | ||
| final protected val endDate = Date.from( | ||
| BoundSubexpression.startInstant.plus(numUnitsInContainer, temporalUnit), | ||
| ) | ||
| final override lazy val maxInterval: Long = getMaxInterval(cron, startDate, endDate, 0) | ||
| final override lazy val isContinuouslyTriggering: Boolean = maxInterval == temporalUnit.getDuration.getSeconds | ||
|
|
||
| /** | ||
| * The interval between the first and last trigger within the range, or "everything but the ends". Should encompass | ||
| * every trigger produced by the subexpression. | ||
| */ | ||
| final lazy val innerInterval: Long = getInnerInterval(cron, startDate, endDate) | ||
|
|
||
| @tailrec | ||
| private def getMaxInterval(expr: CronExpression, prev: Date, end: Date, maxInterval: Long): Long = { | ||
| Option(expr.getTimeAfter(prev)) match { | ||
| case Some(curr) if !prev.after(end) => // iterate once past the "end" in order to wrap around | ||
| val currentInterval = (curr.getTime - prev.getTime) / 1000 | ||
| val newMax = Math.max(currentInterval, maxInterval) | ||
| getMaxInterval(expr, curr, end, newMax) | ||
| case _ => maxInterval | ||
| } | ||
| } | ||
|
|
||
| private def getInnerInterval(expr: CronExpression, prev: Date, end: Date): Long = { | ||
| Option(expr.getTimeAfter(prev)).fold(Long.MaxValue) { firstTriggerDate => | ||
| val firstTriggerTime = firstTriggerDate.getTime / 1000 | ||
| val lastTriggerTime = getLastTriggerTime(expr, firstTriggerDate, end) | ||
| lastTriggerTime - firstTriggerTime | ||
| } | ||
| } | ||
|
|
||
| @tailrec | ||
| private def getLastTriggerTime(expr: CronExpression, prev: Date, end: Date): Long = { | ||
| Option(expr.getTimeAfter(prev)) match { // stop iterating before going past the "end" | ||
| case Some(curr) if !curr.after(end) => getLastTriggerTime(expr, curr, end) | ||
| case _ => prev.getTime / 1000 | ||
| } | ||
| } | ||
| } | ||
|
|
||
| object BoundSubexpression { | ||
| final protected val startInstant: Instant = LocalDate | ||
| .of(2010, Month.SEPTEMBER, 3) | ||
| .atStartOfDay | ||
| .toInstant(ZoneOffset.UTC) | ||
| .minus(1, ChronoUnit.SECONDS) | ||
| } | ||
|
|
||
| /** | ||
| * Represents a subexpression that is unbound, meaning that the range over which triggers occur is unknown, or is | ||
| * variable. For example, days can occur within a week, month, or year, and each of these ranges can vary in size. | ||
| * Because we can't determine the range over which days are triggered, we estimate the max interval by sampling a | ||
| * certain number of times. The larger the number of samples, the more accurate the estimate. | ||
| */ | ||
| abstract class UnboundSubexpression( | ||
| str: String, | ||
| getSimplifiedCron: String => String, | ||
| val maxNumSamples: Long, | ||
| ) extends Subexpression(str, getSimplifiedCron) | ||
| with Logging { | ||
|
|
||
| final override protected val startDate = new Date | ||
| final override lazy val maxInterval: Long = getSampledMaxInterval(startDate, maxNumSamples, cron) | ||
| final override lazy val isContinuouslyTriggering: Boolean = str.split(" ").forall(expr => expr == "*" || expr == "?") | ||
|
|
||
| @tailrec | ||
| private def getSampledMaxInterval(prev: Date, numSamples: Long, expr: CronExpression, maxInterval: Long = 0): Long = { | ||
| Option(expr.getTimeAfter(prev)) match { | ||
| case Some(next) if numSamples > 0 => | ||
| val intervalInSeconds = (next.getTime - prev.getTime) / 1000 | ||
| if (intervalInSeconds > maxInterval) { | ||
| val sampleId = maxNumSamples - numSamples | ||
| logger.debug(s"Seconds:$intervalInSeconds Sample:$sampleId Interval:$prev -> $next") | ||
| } | ||
| getSampledMaxInterval(next, numSamples - 1, expr, Math.max(intervalInSeconds, maxInterval)) | ||
| case _ => if (prev.equals(startDate)) CronHelper.IMPOSSIBLE_MAX_INTERVAL else maxInterval | ||
| } | ||
| } | ||
| } |
60 changes: 60 additions & 0 deletions
60
admin/test/com/lucidchart/piezo/admin/util/CronHelperTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| package com.lucidchart.piezo.admin.util | ||
|
|
||
| import com.lucidchart.piezo.admin.utils.CronHelper | ||
| import org.specs2.mutable.Specification | ||
|
|
||
| class CronHelperTest extends Specification { | ||
|
|
||
| val SECOND: Int = 1 | ||
| val MINUTE: Int = 60 * SECOND | ||
| val HOUR: Int = 60 * MINUTE | ||
| val DAY: Int = 24 * HOUR | ||
| val WEEK: Int = 7 * DAY | ||
| val YEAR: Int = 365 * DAY | ||
| val LEAP_YEAR: Int = YEAR + DAY | ||
| val IMPOSSIBLE: Long = Long.MaxValue | ||
|
|
||
| def maxInterval(str: String): Long = CronHelper.getMaxInterval(str) | ||
|
|
||
| "CronHelper" should { | ||
| "validate basic cron expressions" in { | ||
| maxInterval("* * * * * ?") mustEqual SECOND // every second | ||
| maxInterval("0 * * * * ?") mustEqual MINUTE // second 0 of every minute | ||
| maxInterval("0 0 * * * ?") mustEqual HOUR // second 0 during minute 0 of every hour | ||
| maxInterval("0 0 0 * * ?") mustEqual DAY // second 0 during minute 0 during hour 0 of every day | ||
| maxInterval("* 0 * * * ?") mustEqual (HOUR - MINUTE + SECOND) // every second during minute 0 | ||
| maxInterval("* * 0 * * ?") mustEqual (DAY - HOUR + SECOND) | ||
| } | ||
|
|
||
| "validate more basic cron expressions" in { | ||
| maxInterval("0/1 0-59 */1 * * ?") mustEqual SECOND // variations on 1 second | ||
| maxInterval("* * 0-23 * * ?") mustEqual SECOND | ||
| maxInterval("22 2/6 * * * ?") mustEqual 6 * MINUTE // 22nd second of every 6th minute after minute 2 | ||
| maxInterval("*/15 * * * * ?") mustEqual 15 * SECOND | ||
| maxInterval("30 10 */1 * * ?") mustEqual HOUR | ||
| maxInterval("15 * * * * ?") mustEqual MINUTE | ||
| maxInterval("3,2,1,0 45,44,16,15 6,5,4 * * ? *") mustEqual (21 * HOUR + 29 * MINUTE + 57 * SECOND) | ||
| maxInterval("50-0 30-40 14-12 * * ?") mustEqual (1 * HOUR + 49 * MINUTE + 1 * SECOND) | ||
| maxInterval("0 0 8-4 * * ?") mustEqual 4 * HOUR | ||
| maxInterval("0 0 0/6 * * ? *") mustEqual 6 * HOUR | ||
| maxInterval("0 10,20,30 * * ? *") mustEqual 40 * MINUTE | ||
| maxInterval("0-10/2 0-5,20-25 0,5-11/2,20-23 * ? *") mustEqual 8 * HOUR + 34 * MINUTE + 50 * SECOND | ||
| } | ||
|
|
||
| "validate complex cron expressions" in { | ||
| maxInterval("0/15 * * 1-12 * ?") mustEqual 19 * DAY + 15 * SECOND // every 15 seconds on days 1-12 of the month | ||
| maxInterval("* * * * 1-11 ?") mustEqual 31 * DAY + SECOND // every second of every month except for december | ||
| maxInterval("* * * * * ? 1998") mustEqual IMPOSSIBLE // every second of 1998 | ||
| maxInterval("0 0 0 29 2 ? *") mustEqual 8 * YEAR + DAY // 8 years since we skip leap day roughly every 100 years | ||
| maxInterval("* * * 29 2 ? *") mustEqual 8 * YEAR + SECOND // every second on leap day | ||
| maxInterval("0 11 11 11 11 ?") mustEqual LEAP_YEAR // every november 11th at 11:11am | ||
| maxInterval("1 2 3 ? * 6") mustEqual WEEK // every saturday | ||
| maxInterval("0 15 10 ? * 6#3") mustEqual 5 * WEEK // third saturday of every month | ||
| maxInterval("0 15 10 ? * MON-FRI") mustEqual 3 * DAY // every weekday | ||
| maxInterval("0 0 0/6 * 1,2,3,4,5,6,7,8,9,10,11,12 ? *") mustEqual DAY - (18 * HOUR) | ||
| maxInterval("* * * 1-31 * ?") mustEqual SECOND | ||
| maxInterval("* * * * 1-12 ?") mustEqual SECOND | ||
| maxInterval("* * * ? * 1-7") mustEqual SECOND | ||
| } | ||
| } | ||
| } |