-
Notifications
You must be signed in to change notification settings - Fork 18
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 metric for pipelines indicating if they are up #94
Add metric for pipelines indicating if they are up #94
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #94 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 12 12
Lines 480 496 +16
=========================================
+ Hits 480 496 +16
☔ View full report in Codecov by Sentry. |
0ac4f87
to
3627bec
Compare
Other than the test coverage issue on the impossible-to-reach return statement, this should be ready to merge. It's rebased from master. |
@excalq could you rebase this PR and update metric names after that (if required)? |
@excalq after merging this PR I will release next version as a tag |
dcff8c1
to
ea9b0b9
Compare
ea9b0b9
to
518ca46
Compare
518ca46
to
fb72979
Compare
@excalq I allowed myself to do rebase, force-push and a little refactor, could you take a last look on the code before I merge it? |
The show stopper is the duplicated last_timestamps metrics. I can fix that now. |
@excalq can I merge it after CI is passed? I will squash the commits therefore I changed the title to be a commit message |
The duplication is fixed (might be a good future test, btw), so everything's ready to merge. None of my comments are blockers btw, so please resolve them. |
Resolves #93 by adding
logstash_stats_pipeline_up{pipeline_id}
, with a boolean gauge metric.Uses
sort --version-sort
inscripts/add_metrics_to_readme.sh
for correct mixed string/number sorting of README metrics.