-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Documentation: Add Grafana dashboard to etcd monitoring mixin #9789
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9789 +/- ##
=========================================
- Coverage 64.04% 63.9% -0.15%
=========================================
Files 376 376
Lines 35236 35236
=========================================
- Hits 22568 22517 -51
- Misses 11068 11116 +48
- Partials 1600 1603 +3
Continue to review full report at Codecov.
|
@@ -6,226 +6,1302 @@ | |||
prometheusAlerts+:: { | |||
groups+: [ | |||
{ | |||
name: "etcd", | |||
name: 'etcd', | |||
rules: [ |
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.
This looks great, just a nit/question. I thought it was JSON standard to use double quotes for strings. Just curious why the choice of single, is this the preference for Prometheus configs?
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.
It’s actually jsonnet convention, I just use the official jsonnet formatting, I have 0 hard feelings for any style things, just doing the best practice offered by the tooling.
The rendered json does use double quotes.
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.
@brancz thanks for explaining, I figured I was missing something.
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.
lgtm. thanks @brancz !
This PR adds the Grafana dashboard to the etcd monitoring mixin, initially added in #9640. It also formats the jsonnet code to the standard form.
Additionally I added datasource and job templating to the dashboard and re-generated the rendered dashboard json in
Documentation/op-guide
with:We may want to add this to CI at some point, but I hear @tomwilkie is building a general mixin-tool, so I'd prefer to just postpone anything regarding CI until that all-in-one tool is ready.
@tomwilkie @metalmatze @gyuho