-
Notifications
You must be signed in to change notification settings - Fork 261
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
Support generic postgres custom metrics #224
Support generic postgres custom metrics #224
Conversation
Dropping in puppet support for the following -- https://help.datadoghq.com/hc/en-us/articles/208385813-Postgres-custom-metric-collection-explained |
# $custom_metrics | ||
# An array of custom metrics with the following hash keys - query, metrics, | ||
# relation, descriptors. Refer to this guide for details on those fields: | ||
# https://help.datadoghq.com/hc/en-us/articles/208385813-Postgres-custom-metric-collection-explained |
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.
Could you please add an example with custom metrics below? Should help a bit with usability.
Thanks a lot @sethcleveland, tackles an open issue which is always greatly appreciated! If you could add some spec tests that'd be fabulous. |
Sure thing. This is an open issue for us as well. I'll add documentation and spec tests. |
15108ed
to
fc575c1
Compare
I did a brief scan on the failures and most things appear to fail this. --
I didn't make those changes. As far as I can tell they are the status quo. |
@truthbk: How are these changes? |
@sethcleveland this is great, thanks a lot. I'll review in a little bit more depth before merging, but it looks good. I'll keep you posted! I need to understand where those failures are coming from. Maybe if you rebase to the latest |
- Adding resource definition for postgres custom metrics (for validation) - Added custom metrics example and test specs
4374d8a
to
411488d
Compare
@truthbk just rebased against master. Hopefully that fixes the problems. |
@truthbk turns out rebasing against master didn't resolve the failures. I'm assuming the linters were added after the fact. I fixed the failing builds as part of this PR commit -- 5cd38a4. FWIW's I'm not clear why parameter optional/non-optional parameter ordering matters. I've only see puppet modules used with explicit parameter values. |
@truthbk do these changes work? Other people can be unblocked by one of the commits. I can cherry pick the fix out into another PR while you spend more time reviewing these changes. Let me know where you'll stand before I make another PR. |
# $tags | ||
# Optional array of tags | ||
# $custom_metrics | ||
# An array of custom metrics with the following hash keys - query, metrics, |
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.
Isn't this a hash of custom metrics?
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.
Yup. I'll make an adjustment.
@sethcleveland thank you so much for this and #232 - merging this in. Again, apologies for the long MTTR. |
…tgres_custom_metrics Support generic postgres custom metrics
No description provided.