Skip to content

migration(crons): Add MonitorIncident table #56403

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.

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

Closed
wants to merge 4 commits into from

Conversation

rjo100
Copy link
Contributor

@rjo100 rjo100 commented Sep 18, 2023

Adds MonitorIncidentTable to keep track of open monitor incidents + display them on the timeline view.

If a failure threshold is set, we will open an incident and tag it with the appropriate hash and reference the checkin that caused it to pass the threshold.

If a recovery threshold is set, we will find any incident with the appropriate hash and set it as the closing_checkin.

In the future, if no thresholds are set, we will move to this behavior. But for now, we will keep existing behavior as-is.

The thresholds can be derived from the checkin configs, though we may want to add them as top level fields.

Closes #56402

@rjo100 rjo100 requested a review from a team as a code owner September 18, 2023 17:54
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 18, 2023
Comment on lines 602 to 603
opening_checkin = FlexibleForeignKey("sentry.MonitorCheckIn", related_name="created_incidents")
closing_checkin = FlexibleForeignKey(
Copy link
Member

Choose a reason for hiding this comment

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

These feel like misnomers, I think we probably want 'starting_checkinandresolved_checkin`

and for good measure, we should probably add some comments to these fields to clearly indicate which checkins in the lifecycle of an incident these would be.

Also Important, these should probably be nullablabe since the checkins will be removed, in which case, I think we almost definitely would probably want to either

  1. Also remove the incident when any checkins are removed
  2. Keep start and end date and allow empty start and end check ins

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense

closing_checkin = FlexibleForeignKey(
"sentry.MonitorCheckIn", related_name="closed_incidents", null=True
)
grouphash = models.CharField(max_length=32)
Copy link
Member

Choose a reason for hiding this comment

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

Can we just use a UUID field?

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 guess but wanted to keep it consistent with

hash = models.CharField(max_length=32)

Copy link
Member

Choose a reason for hiding this comment

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

👍

@rjo100
Copy link
Contributor Author

rjo100 commented Sep 18, 2023

Closing in favor of #56435

@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2023
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.

Create Table migration
2 participants