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.

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

rapu: fire shutdown metric on app shutdown #940

Merged
merged 1 commit into from
Aug 26, 2024
Merged

Conversation

nosahama
Copy link
Contributor

@nosahama nosahama commented Aug 26, 2024

About this change - What it does

  • we add the karapace_shutdown_count metric, to the base app and fire the metric on app shutdown
  • the close_by_app() is added to the app shutdown hooks and thus is called on app shutdown

Shutdown logs

Screenshot 2024-08-26 at 16 18 22

Active shutdown alert after 2 manual shutdowns

Screenshot 2024-08-26 at 16 20 37

Firing shutdown alert after 2 manual shutdowns

Screenshot 2024-08-26 at 16 27 31

Why this way

This way we have a metric to count the app shutdown ands this is handled on the app server level, rather than in application code.

- we add the `karapace_shutdown_count` metric, to
the base app and fire the metric on app shutdown
- the `close_by_app()` is added to the app shutdown hooks and thus is
called on app shutdown
@nosahama nosahama requested review from a team as code owners August 26, 2024 13:52
Copy link
Contributor

@eliax1996 eliax1996 left a comment

Choose a reason for hiding this comment

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

LGTM, nice great job!

@eliax1996 eliax1996 enabled auto-merge August 26, 2024 15:00
@nosahama
Copy link
Contributor Author

LGTM, nice great job!

Thanks man, I was thinking in another iteration, we can add labels to the shutdown metric, i.e. the reason for the shutdown, maybe set the error on the app object and reference the error when we fire the shutdown metric or leave it empty which will denote a normal expected shutdown, we can improve 👍

@eliax1996 eliax1996 merged commit dff48ce into main Aug 26, 2024
10 checks passed
@eliax1996 eliax1996 deleted the nosahama/shutdown-metric branch August 26, 2024 16:49
Copy link

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  karapace
  config.py
  rapu.py
  statsd.py
Project Total  

This report was generated by python-coverage-comment-action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants