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

Jump to bottom

Calculate drift metric based on last time resource was checked #950

Closed
lorin opened this issue Mar 31, 2020 · 0 comments · Fixed by #976
Closed

Calculate drift metric based on last time resource was checked #950

lorin opened this issue Mar 31, 2020 · 0 comments · Fixed by #976
Assignees
@lorin
Labels
Core observability
Projects
Declarative delivery 1.0
Milestone
GA

Comments

@lorin
Copy link
Contributor

lorin commented Mar 31, 2020

The way that keel currently computes the drift metric, when the timeout fires the drift metric resets to zero. This is misleading, because it looks like drift has gone down to zero, even though the time since last resource check is still increasing.

It would be better to change TelemetryListener to calculate the drift metric based on the last time a resource was checked, instead of basing it on the start of an actuation loop.

(Suggested by @asher).
@lorin lorin self-assigned this Mar 31, 2020
@luispollo luispollo added this to To do in Declarative delivery 1.0 via automation Apr 6, 2020
@luispollo luispollo modified the milestones: Beta, GA Apr 6, 2020
lorin pushed a commit to lorin/keel that referenced this issue Apr 6, 2020
fix(metrics): record drift from last resource check
180359a 
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 spinnaker#950
@lorin lorin mentioned this issue Apr 6, 2020
Merged
lorin pushed a commit to lorin/keel that referenced this issue Apr 7, 2020
fix(metrics): record drift from last resource check
116cb64 
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 spinnaker#950
lorin pushed a commit to lorin/keel that referenced this issue Apr 7, 2020
fix(metrics): record drift from last resource check
d825811 
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 spinnaker#950
@mergify mergify bot closed this as completed in #976 Apr 7, 2020
Declarative delivery 1.0 automation moved this from To do to Done Apr 7, 2020
mergify bot pushed a commit that referenced this issue Apr 7, 2020
fix(metrics): record drift from last resource check (#976)
399d102 
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lorin lorin

Labels
Core observability
Projects
Declarative delivery 1.0
  
Done
Milestone
GA
Development

Successfully merging a pull request may close this issue.

fix(metrics): record drift from last resource check lorin/keel
2 participants
@lorin @luispollo