-
Notifications
You must be signed in to change notification settings - Fork 312
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 metrics json #761
Open
Hooloovoo
wants to merge
82
commits into
jimsalterjrs:master
Choose a base branch
from
Hooloovoo:add_metrics_json
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add metrics json #761
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
schema_version and put snapshot info within "snapshot_info" in the JSON
(the $errorlevel for each path/type combination)
deliberately failing
in the JSON output
"overall_snapshot_health_issues" metric
Add and reorganise JSON tests
test_immediately_after_running_sanoid
test_immediately_after_running_sanoid
and tidied up the test README
Tests all pass.
|
I just built a deb package and installed it in a clean VM and it seems to work. |
Is there anything more I can do to help get this merged? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This creates a new
sanoid --monitor-metrics-json
command that produces JSON output.Example output from the scenario in test_two_criticals_hourly_two_warnings_daily is:
That would give a corresponding output from
sanoid --monitor-snapshots
of:The structure of the JSON is as follows:
This does require the perl JSON module. I have made the changes to packages/debian/control to add this dependency, but other package types will have to be updated.
This deliberately builds on my earlier pull request: #729 to ensure that the existing behaviour of
sanoid --monitor-snapshots
is tested and that this new functionality does not alter that behaviour.I have been running this on my servers for most of this year (it was adding the tests that took the most time) with no issues. I use the code at https://gitlab.com/aaron-w/sanoid_prometheus to export these into Prometheus and alert on them. This addresses #675 as far as Sanoid snapshot information is concerned.
In the future it would make sense to add additional top-level objects in the JSON output of the same command (
sanoid --monitor-metrics-json
) for the information returned by--monitor-health
and--monitor-capacity
and the code and JSON structure was designed to make that easier, but these were less urgent for me because there are other ZFS Prometheus exporters for this information. I also wanted to check I could get this merged in before spending more time on additional enhancements.