Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add team to trigger monitoring record PROD-2741 #110

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Add team to trigger monitoring record PROD-2741 #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions admin/app/com/lucidchart/piezo/admin/controllers/Jobs.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,16 +234,17 @@ class Jobs(schedulerFactory: WorkerSchedulerFactory, jobView: html.job, cc: Cont
logger.error(errorMessage)
ImportFailure(Some(jobDetail.getKey), "Trigger Import Error:"+errorMessage)
} else {
val triggers = triggersBinding.map(_.value).flatten
triggers.map { case (trigger, monitoringPriority, errorTime) =>
scheduler.scheduleJob(trigger)
triggerMonitoringPriorityModel.setTriggerMonitoringRecord(
trigger.getKey,
monitoringPriority,
errorTime
)
}
ImportSuccess(Some(jobDetail.getKey))
val triggers = triggersBinding.flatMap(_.value)
triggers.foreach { case (trigger, monitoringPriority, errorTime, monitoringTeam) =>
scheduler.scheduleJob(trigger)
triggerMonitoringPriorityModel.setTriggerMonitoringRecord(
trigger.getKey,
monitoringPriority,
errorTime,
monitoringTeam,
)
}
ImportSuccess(Some(jobDetail.getKey))
}
} catch {
case _: ObjectAlreadyExistsException =>
Expand Down
17 changes: 13 additions & 4 deletions admin/app/com/lucidchart/piezo/admin/controllers/TriggerFormHelper.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ class TriggerFormHelper(scheduler: Scheduler) extends JobDataHelper {
jobDataMap: Option[JobDataMap],
triggerMonitoringPriority: String,
triggerMaxErrorTime: Int,
): (Trigger, TriggerMonitoringPriority, Int) = {
triggerMonitoringTeam: Option[String],
): (Trigger, TriggerMonitoringPriority, Int, Option[String]) = {
val newTrigger: Trigger = TriggerBuilder
.newTrigger()
.withIdentity(name, group)
Expand All @@ -58,10 +59,15 @@ class TriggerFormHelper(scheduler: Scheduler) extends JobDataHelper {
.forJob(jobName, jobGroup)
.usingJobData(jobDataMap.getOrElse(new JobDataMap()))
.build()
(newTrigger, TriggerMonitoringPriority.withName(triggerMonitoringPriority), triggerMaxErrorTime)
(
newTrigger,
TriggerMonitoringPriority.withName(triggerMonitoringPriority),
triggerMaxErrorTime,
triggerMonitoringTeam,
)
}

private def triggerFormUnapply(tp: (Trigger, TriggerMonitoringPriority, Int)): Option[
private def triggerFormUnapply(tp: (Trigger, TriggerMonitoringPriority, Int, Option[String])): Option[
(
String,
String,
Expand All @@ -74,6 +80,7 @@ class TriggerFormHelper(scheduler: Scheduler) extends JobDataHelper {
Option[JobDataMap],
String,
Int,
Option[String],
),
] = {
val trigger = tp._1
Expand All @@ -95,6 +102,7 @@ class TriggerFormHelper(scheduler: Scheduler) extends JobDataHelper {
Some(trigger.getJobDataMap),
tp._2.toString,
tp._3,
tp._4,
),
)
}
Expand Down Expand Up @@ -124,7 +132,7 @@ class TriggerFormHelper(scheduler: Scheduler) extends JobDataHelper {
}
}

def buildTriggerForm = Form[(Trigger, TriggerMonitoringPriority, Int)](
def buildTriggerForm: Form[(Trigger, TriggerMonitoringPriority, Int, Option[String])] = Form(
mapping(
"triggerType" -> nonEmptyText(),
"group" -> nonEmptyText(),
Expand All @@ -146,6 +154,7 @@ class TriggerFormHelper(scheduler: Scheduler) extends JobDataHelper {
"job-data-map" -> jobDataMap,
"triggerMonitoringPriority" -> nonEmptyText(),
"triggerMaxErrorTime" -> of(MaxSecondsBetweenSuccessesFormatter).verifying(Constraints.min(0)),
"triggerMonitoringTeam" -> optional(text()),
)(triggerFormApply)(triggerFormUnapply)
.verifying(
"Job does not exist",
Expand Down
9 changes: 5 additions & 4 deletions admin/app/com/lucidchart/piezo/admin/controllers/TriggerHelper.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ object TriggerHelper {
case _ => Json.obj()
}

val (monitoringPriority, maxSecondsInError) = monitoringModel.getTriggerMonitoringRecord(
val (monitoringPriority, maxSecondsInError, monitoringTeam) = monitoringModel.getTriggerMonitoringRecord(
trigger.getKey,
).map { monitoringRecord =>
(monitoringRecord.priority, monitoringRecord.maxSecondsInError)
}.getOrElse((TriggerMonitoringPriority.Low, 300))
(monitoringRecord.priority, monitoringRecord.maxSecondsInError, monitoringRecord.monitoringTeam)
}.getOrElse((TriggerMonitoringPriority.Low, 300, None))
val jobDataMap = trigger.getJobDataMap
val job = trigger.getJobKey
Json.obj(
Expand All @@ -66,7 +66,8 @@ object TriggerHelper {
"description" -> trigger.getDescription,
"job-data-map" -> JsObject(jobDataMap.getKeys.toSeq.map(key => key -> JsString(jobDataMap.getString(key)))),
"triggerMonitoringPriority" -> monitoringPriority.name,
"triggerMaxErrorTime" -> maxSecondsInError
"triggerMaxErrorTime" -> maxSecondsInError,
"triggerMonitoringTeam" -> monitoringTeam,
) ++ schedule
}

Expand Down
33 changes: 18 additions & 15 deletions admin/app/com/lucidchart/piezo/admin/controllers/Triggers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ class Triggers(schedulerFactory: WorkerSchedulerFactory, cc: ControllerComponent
}
}

val (triggerMonitoringPriority, triggerMaxErrorTime) = Try {
val (triggerMonitoringPriority, triggerMaxErrorTime, triggerMonitoringTeam) = Try {
triggerMonitoringPriorityModel.getTriggerMonitoringRecord(
triggerDetail.getKey
).map { triggerMonitoringRecord =>
(triggerMonitoringRecord.priority, triggerMonitoringRecord.maxSecondsInError)
}.getOrElse((TriggerMonitoringPriority.Low, 300))
(triggerMonitoringRecord.priority, triggerMonitoringRecord.maxSecondsInError, triggerMonitoringRecord.monitoringTeam)
}.getOrElse((TriggerMonitoringPriority.Low, 300, None))
}.getOrElse {
logger.error("Failed to get trigger monitoring info")
(TriggerMonitoringPriority.Low, 300)
(TriggerMonitoringPriority.Low, 300, None)
}

Ok(
Expand All @@ -87,6 +87,7 @@ class Triggers(schedulerFactory: WorkerSchedulerFactory, cc: ControllerComponent
None,
Some(triggerMonitoringPriority),
triggerMaxErrorTime,
triggerMonitoringTeam,
triggerState = Some(scheduler.getTriggerState(triggerKey))
)(request)
)
Expand Down Expand Up @@ -159,7 +160,7 @@ class Triggers(schedulerFactory: WorkerSchedulerFactory, cc: ControllerComponent
case "simple" => new DummySimpleTrigger(jobGroup, jobName)
}
val newTriggerForm = triggerFormHelper.buildTriggerForm.fill(
(dummyTrigger, TriggerMonitoringPriority.Low, 300)
(dummyTrigger, TriggerMonitoringPriority.Low, 300, None)
)
Ok(
com.lucidchart.piezo.admin.views.html.editTrigger(
Expand All @@ -182,18 +183,18 @@ class Triggers(schedulerFactory: WorkerSchedulerFactory, cc: ControllerComponent
NotFound(com.lucidchart.piezo.admin.views.html.trigger(mutable.Buffer(), None, None, errorMsg)(request))
} else {
val triggerDetail: Trigger = scheduler.getTrigger(triggerKey)
val (triggerMonitoringPriority, triggerMaxErrorTime) = Try {
val (triggerMonitoringPriority, triggerMaxErrorTime, triggerMonitoringTeam) = Try {
triggerMonitoringPriorityModel.getTriggerMonitoringRecord(
triggerDetail.getKey,
).map { triggerMonitoringRecord =>
(triggerMonitoringRecord.priority, triggerMonitoringRecord.maxSecondsInError)
}.getOrElse((TriggerMonitoringPriority.Low, 300))
(triggerMonitoringRecord.priority, triggerMonitoringRecord.maxSecondsInError, triggerMonitoringRecord.monitoringTeam)
}.getOrElse((TriggerMonitoringPriority.Low, 300, None))
}.getOrElse {
logger.error("Failed to get trigger monitoring info")
(TriggerMonitoringPriority.Low, 300)
(TriggerMonitoringPriority.Low, 300, None)
}
val editTriggerForm = triggerFormHelper.buildTriggerForm.fill(
(triggerDetail, triggerMonitoringPriority, triggerMaxErrorTime)
(triggerDetail, triggerMonitoringPriority, triggerMaxErrorTime, triggerMonitoringTeam)
)
if (isTemplate) {
Ok(
Expand Down Expand Up @@ -237,12 +238,13 @@ class Triggers(schedulerFactory: WorkerSchedulerFactory, cc: ControllerComponent
)
),
value => {
val (trigger, triggerMonitoringPriority, triggerMaxErrorTime) = value
val (trigger, triggerMonitoringPriority, triggerMaxErrorTime, triggerMonitoringTeam) = value
scheduler.rescheduleJob(trigger.getKey(), trigger)
triggerMonitoringPriorityModel.setTriggerMonitoringRecord(
trigger.getKey,
triggerMonitoringPriority,
triggerMaxErrorTime
triggerMaxErrorTime,
triggerMonitoringTeam
)
Redirect(routes.Triggers.getTrigger(trigger.getKey.getGroup(), trigger.getKey.getName()))
.flashing("message" -> "Successfully added trigger.", "class" -> "")
Expand All @@ -263,20 +265,21 @@ class Triggers(schedulerFactory: WorkerSchedulerFactory, cc: ControllerComponent
)
),
value => {
val (trigger, triggerMonitoringPriority, triggerMaxErrorTime) = value
val (trigger, triggerMonitoringPriority, triggerMaxErrorTime, triggerMonitoringTeam) = value
try {
scheduler.scheduleJob(trigger)
triggerMonitoringPriorityModel.setTriggerMonitoringRecord(
trigger.getKey,
triggerMonitoringPriority,
triggerMaxErrorTime
triggerMaxErrorTime,
triggerMonitoringTeam
)
Redirect(routes.Triggers.getTrigger(trigger.getKey.getGroup(), trigger.getKey.getName()))
.flashing("message" -> "Successfully added trigger.", "class" -> "")
} catch {
case alreadyExists: ObjectAlreadyExistsException =>
val form = triggerFormHelper.buildTriggerForm.fill(
(trigger, triggerMonitoringPriority, triggerMaxErrorTime)
(trigger, triggerMonitoringPriority, triggerMaxErrorTime, triggerMonitoringTeam)
)
Ok(
com.lucidchart.piezo.admin.views.html.editTrigger(
Expand Down
5 changes: 4 additions & 1 deletion admin/app/com/lucidchart/piezo/admin/views/editTrigger.scala.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
@(
triggersByGroup: scala.collection.mutable.Buffer[(String, scala.collection.immutable.List[org.quartz.TriggerKey])],
triggerForm: Form[(org.quartz.Trigger, com.lucidchart.piezo.TriggerMonitoringPriority.Value, Int)],
triggerForm: Form[(org.quartz.Trigger, com.lucidchart.piezo.TriggerMonitoringPriority.Value, Int, Option[String])],
formAction: play.api.mvc.Call,
existing: Boolean,
isTemplate: Boolean,
Expand Down Expand Up @@ -70,6 +70,9 @@ <h4 class="text-danger">@triggerForm.errors.filter(_.key == "").map(_.message).m
@helper.input(triggerForm("triggerMaxErrorTime"), Symbol("_label") -> "Monitoring - Max Seconds Between Successes", Symbol("labelClass") -> "col-sm-2 text-right", Symbol("inputDivClass") -> "col-sm-4", Symbol("placeholder") -> "", Symbol("value") -> triggerForm.data.get("triggerMaxErrorTime").getOrElse(300)) { (id, name, value, args) =>
<input type="number" class="form-control form-inline-control " name="@name" id="@id" @toHtmlArgs(args)>
}
@helper.input(triggerForm("triggerMonitoringTeam"), Symbol("_label") -> "Monitoring team", Symbol("labelClass") -> "col-sm-2 text-right", Symbol("inputDivClass") -> "col-sm-4", Symbol("placeholder") -> "", Symbol("value") -> triggerForm.data.get("triggerMonitoringTeam").getOrElse(None)) { (id, name, value, args) =>
<input type="text" class="form-control form-inline-control" name="@name" id="@id" @toHtmlArgs(args)>
}

@helper.input(triggerForm("description"), Symbol("_label") -> "Description", Symbol("labelClass") -> "col-sm-2 text-right", Symbol("inputDivClass") -> "col-sm-10", Symbol("placeholder") -> "Description", Symbol("value")-> triggerForm.data.get("description").getOrElse("")) { (id, name, value, args) =>
<input type="text" class="form-control form-inline-control " name="@name" id="@id" @toHtmlArgs(args)>
Expand Down
5 changes: 5 additions & 0 deletions admin/app/com/lucidchart/piezo/admin/views/trigger.scala.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
errorMessage: Option[String] = None,
triggerMonitoringPriority: Option[com.lucidchart.piezo.TriggerMonitoringPriority.Value] = None,
triggerMaxErrorTime: Integer = 300,
triggerMonitoringTeam: Option[String] = None,
triggerState: Option[TriggerState] = None
)(
implicit
Expand Down Expand Up @@ -167,6 +168,10 @@ <h4>This will permanently delete the trigger!</h4>
<td class="text-right">Monitoring - max seconds between successes:</td>
<td>@triggerMaxErrorTime seconds</td>
</tr>
<tr>
<td class="text-right">Monitoring team:</td>
<td>@triggerMonitoringTeam.getOrElse("")</td>
</tr>
}
<tr>
<td class="text-right">Description:</td>
Expand Down
1 change: 1 addition & 0 deletions worker/src/main/resources/piezo_mysql_8.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE trigger_monitoring_priority ADD monitoring_team VARCHAR(100) DEFAULT NULL;
16 changes: 12 additions & 4 deletions worker/src/main/scala/com/lucidchart/piezo/TriggerMonitoringModel.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ case class TriggerMonitoringRecord (
triggerGroup: String,
priority: TriggerMonitoringPriority,
maxSecondsInError: Int,
monitoringTeam: Option[String],
created: Date,
modified: Date
)
Expand All @@ -39,23 +40,29 @@ class TriggerMonitoringModel(props: Properties) {
def setTriggerMonitoringRecord(
triggerKey: TriggerKey,
triggerMonitoringPriority: TriggerMonitoringPriority,
maxSecondsInError: Int
maxSecondsInError: Int,
monitoringTeam: Option[String]
): Int = {
val connection = connectionProvider.getConnection
try {
val prepared = connection.prepareStatement("""
INSERT INTO trigger_monitoring_priority
(trigger_name, trigger_group, priority, max_error_time)
(trigger_name, trigger_group, priority, max_error_time, monitoring_team)
VALUES
(?, ?, ?, ?)
(?, ?, ?, ?, ?)
ON DUPLICATE KEY UPDATE
priority = values(priority),
max_error_time = values(max_error_time)
max_error_time = values(max_error_time),
monitoring_team = values(monitoring_team)
""")
prepared.setString(1, triggerKey.getName)
prepared.setString(2, triggerKey.getGroup)
prepared.setInt(3, triggerMonitoringPriority.id)
prepared.setInt(4, maxSecondsInError)
monitoringTeam match {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

monitoringTeam.fold(prepared.setNull(5, java.sql.Types.VARCHAR))(team => prepared.setString(5, team))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally tend to agree with this article by jhenline that argues that Option#fold is inconsistent and other ways of writing it are more readable

case Some(team) => prepared.setString(5, team)
case None => prepared.setNull(5, java.sql.Types.VARCHAR)
}
prepared.executeUpdate()
} catch {
case e: Exception => logger.error(
Expand Down Expand Up @@ -117,6 +124,7 @@ class TriggerMonitoringModel(props: Properties) {
rs.getString("trigger_group"),
priority,
rs.getInt("max_error_time"),
Option(rs.getString("monitoring_team")),
rs.getDate("created"),
rs.getDate("modified")
)
Expand Down
6 changes: 3 additions & 3 deletions worker/src/test/scala/com/lucidchart/piezo/ModelTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class ModelTest extends Specification with BeforeAll with AfterAll {
}

override def beforeAll(): Unit = {
val piezoSchema = for (num <- 0 to 7) yield getPatchFile(s"piezo_mysql_$num.sql")
val piezoSchema = for (num <- 0 to 8) yield getPatchFile(s"piezo_mysql_$num.sql")
val quartzSchema = getPatchFile("quartz_mysql_0.sql")
val schema = (quartzSchema +: piezoSchema).map { path =>
Files.readAllLines(path).asScala.mkString("\n")
Expand Down Expand Up @@ -81,7 +81,7 @@ class ModelTest extends Specification with BeforeAll with AfterAll {
val triggerMonitoringPriorityModel = new TriggerMonitoringModel(properties)
val triggerKey = new TriggerKey("blahj", "blahg")
triggerMonitoringPriorityModel.getTriggerMonitoringRecord(triggerKey) must beNone
triggerMonitoringPriorityModel.setTriggerMonitoringRecord(triggerKey, TriggerMonitoringPriority.Low, 1800)
triggerMonitoringPriorityModel.setTriggerMonitoringRecord(triggerKey, TriggerMonitoringPriority.Low, 1800, Some("my-team"))
triggerMonitoringPriorityModel.getTriggerMonitoringRecord(triggerKey) must beSome
triggerMonitoringPriorityModel.deleteTriggerMonitoringRecord(triggerKey) mustEqual 1
triggerMonitoringPriorityModel.getTriggerMonitoringRecord(triggerKey) must beNone
Expand All @@ -107,4 +107,4 @@ class ModelTest extends Specification with BeforeAll with AfterAll {
triggerHistoryModel.deleteTriggers(new Date().getTime+1000) mustEqual 1
}
}
}
}
Loading