fix(metrics): record drift from last resource check (#976)

* fix(metrics): record drift from last resource check Calculate the resource check drift metric (keel.resource.check.drift) by using the last time resource check succeeded as the start time. Previously, it was calculating based on the beginning of the actuation loop, which would drop to zero when the loop restarted due to timeout. Fixes #950 * fix(pr): move ResourceCheckCompleted, remove ScheduledResourceCheckStarting Remove ScheduledResourceCheckStarting since it's no longer used. Move ResourceCheckCompleted to a separate file. * fix(pr): add missing import
spinnaker · Apr 7, 2020 · 399d102 · 399d102
1 parent 8987784
commit 399d102
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 6 deletions.
diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/actuation/CheckScheduler.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/actuation/CheckScheduler.kt
@@ -60,7 +60,6 @@ class CheckScheduler(
   fun checkResources() {
     if (enabled.get()) {
       log.debug("Starting scheduled resource validation…")
-      publisher.publishEvent(ScheduledResourceCheckStarting)
 
       val job = launch {
         supervisorScope {
@@ -79,7 +78,10 @@ class CheckScheduler(
                    * to prevent the cancellation of all coroutines under [job]
                    */
                   withTimeout(checkTimeout.toMillis()) {
-                    launch { resourceActuator.checkResource(it) }
+                    launch {
+                      resourceActuator.checkResource(it)
+                      publisher.publishEvent(ResourceCheckCompleted)
+                    }
                   }
                 } catch (e: TimeoutCancellationException) {
                   log.error("Timed out checking resource ${it.id}", e)

diff --git a/...re/src/main/kotlin/com/netflix/spinnaker/keel/actuation/ScheduledResourceCheckStarting.kt b/...re/src/main/kotlin/com/netflix/spinnaker/keel/actuation/ScheduledResourceCheckStarting.kt
@@ -1,6 +1,6 @@
 package com.netflix.spinnaker.keel.actuation
 
-object ScheduledResourceCheckStarting
+object ResourceCheckCompleted
 
 object ScheduledEnvironmentCheckStarting
 

diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/telemetry/TelemetryListener.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/telemetry/TelemetryListener.kt
@@ -4,9 +4,9 @@ import com.netflix.spectator.api.BasicTag
 import com.netflix.spectator.api.Counter
 import com.netflix.spectator.api.Registry
 import com.netflix.spectator.api.patterns.PolledMeter
+import com.netflix.spinnaker.keel.actuation.ResourceCheckCompleted
 import com.netflix.spinnaker.keel.actuation.ScheduledArtifactCheckStarting
 import com.netflix.spinnaker.keel.actuation.ScheduledEnvironmentCheckStarting
-import com.netflix.spinnaker.keel.actuation.ScheduledResourceCheckStarting
 import com.netflix.spinnaker.keel.events.ResourceActuationLaunched
 import com.netflix.spinnaker.keel.events.ResourceCheckResult
 import java.time.Clock
@@ -115,8 +115,8 @@ class TelemetryListener(
     ).safeIncrement()
   }
 
-  @EventListener(ScheduledResourceCheckStarting::class)
-  fun onScheduledCheckStarting(event: ScheduledResourceCheckStarting) {
+  @EventListener(ResourceCheckCompleted::class)
+  fun onResourceCheckCompleted(event: ResourceCheckCompleted) {
     lastResourceCheck.set(clock.instant())
   }