Skip to content

feat(crons): Report "true" transition timestamp in DecisionResult#80864

Merged
evanpurkhiser merged 1 commit into
masterfrom
evanpurkhiser/feat-crons-report-true-transition-timestamp-in-decisionresult
Nov 15, 2024
Merged

feat(crons): Report "true" transition timestamp in DecisionResult#80864
evanpurkhiser merged 1 commit into
masterfrom
evanpurkhiser/feat-crons-report-true-transition-timestamp-in-decisionresult

Conversation

@evanpurkhiser

@evanpurkhiser evanpurkhiser commented Nov 15, 2024

Copy link
Copy Markdown
Member

When transition into and out of an incident we want to know the true start time and true recovery time of the incident. We can do this with the following logic:

  • When starting an incident we backfill the ABNORMAL decisions to INCIDENT. Record the timestamp of the very first abnormal and report that back with the DecisionResult that is transition=INCIDENT_STARTED

  • When recovering from an incident we backfill the RECOVERING decisions as NORMAL. Record the timestamp of the very first normal decision and report that back with the DecisionResult that is transition=INCIDENT_RECOVERED

For all other decisions (including abnormal recovery and incident recovery failed) the timestamp is always just the tick that we're making the decision for, since these do not have

The resulting time range is an inclusive start and exclusive end to determine the datetime range of the incident.

Part of GH-79328

@evanpurkhiser evanpurkhiser requested a review from a team as a code owner November 15, 2024 22:07
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 15, 2024
Comment thread src/sentry/monitors/system_incidents.py Outdated
Comment thread src/sentry/monitors/system_incidents.py Outdated
Comment thread src/sentry/monitors/system_incidents.py Outdated
Comment thread src/sentry/monitors/system_incidents.py Outdated

@JoshFerge JoshFerge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good!

@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/feat-crons-report-true-transition-timestamp-in-decisionresult branch from 9e29855 to 905090d Compare November 15, 2024 22:34
When transition into and out of an incident we want to know the _true_
start time and true recovery time of the incident. We can do this with
the following logic:

 - When starting an incident we backfill the ABNORMAL decisions to
   INCIDENT. Record the timestamp of the very first abnormal and report
   that back with the DecisionResult that is transition=INCIDENT_STARTED

 - When recovering from an incident we backfill the RECOVERING decisions
   as NORMAL. Record the timestamp of the very first normal decision and
   report that back with the DecisionResult that is
   transition=INCIDENT_RECOVERED

For all other decisions (including abnormal recovery and incident
recovery failed) the timestamp is always just the tick that we're making
the decision for, since these do not have

The resulting time range is an inclusive start and exclusive end to
determine the datetime range of the incident.
@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/feat-crons-report-true-transition-timestamp-in-decisionresult branch from 905090d to b6cf28b Compare November 15, 2024 22:40
@codecov

codecov Bot commented Nov 15, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/monitors/system_incidents.py 95.65% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #80864      +/-   ##
==========================================
- Coverage   78.42%   78.42%   -0.01%     
==========================================
  Files        7211     7211              
  Lines      319706   319718      +12     
  Branches    44004    44005       +1     
==========================================
+ Hits       250716   250725       +9     
- Misses      62601    62609       +8     
+ Partials     6389     6384       -5     

@evanpurkhiser evanpurkhiser merged commit 5776728 into master Nov 15, 2024
@evanpurkhiser evanpurkhiser deleted the evanpurkhiser/feat-crons-report-true-transition-timestamp-in-decisionresult branch November 15, 2024 23:38
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants