-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add Prometheus metrics for DarkBlue Service #48
Conversation
jayamala17
commented
May 1, 2024
•
edited
Loading
edited
- Added Prometheus pushgateway used for monitoring batch or cron jobs.
- Through the gateway url endpoint the metrics are pushed to prometheus
- Added metrics for job duration, last job run. and metrics regards to success and failure count on each job run.
Hi @jayamala17, could you resolve the conflicts, improve the title, and add a short description? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Releasing some comments so you can see what I've thought about so far. I'm going to think a bit more about the tests (the stubbing and mocking), but probably tomorrow. I will also give it a whirl then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Releasing some comments. Still looking at this and I will work on that separate branch with my config suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, more comments, we're getting closer.
I got this working on my local machine, looks cool! 🙂 |
@jayamala17, FYI, I started experimenting with the Mock stuff, found a couple other things worth considering. I'm going to open a PR with the changes so it's easier to talk about. We can discuss tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! A couple more ideas to consider, but you can feel free to merge when you would like.
@@ -35,7 +36,7 @@ def initialize(config) | |||
type: config.aptrust.remote.type, | |||
settings: config.aptrust.remote.settings | |||
), | |||
status_event_repo: StatusEventRepository::StatusEventRepositoryFactory.for(use_db: DB), | |||
status_event_repo: S.status_event_repo, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not do this now, but I'd like to get to a place where we only use S
in the top-level scope, meaning we'd have to add a new constructor parameter.