Skip to content

Support generic postgres custom metrics#224

Merged
truthbk merged 3 commits intoDataDog:masterfrom
sethcleveland:Support_generic_postgres_custom_metrics
Sep 20, 2016
Merged

Support generic postgres custom metrics#224
truthbk merged 3 commits intoDataDog:masterfrom
sethcleveland:Support_generic_postgres_custom_metrics

Conversation

@sethcleveland
Copy link
Copy Markdown
Contributor

No description provided.

@sethcleveland
Copy link
Copy Markdown
Contributor Author

# $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
Copy link
Copy Markdown
Member

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.

@truthbk
Copy link
Copy Markdown
Member

truthbk commented Aug 17, 2016

Thanks a lot @sethcleveland, tackles an open issue which is always greatly appreciated! If you could add some spec tests that'd be fabulous.

@sethcleveland
Copy link
Copy Markdown
Contributor Author

Sure thing. This is an open issue for us as well. I'll add documentation and spec tests.

@sethcleveland sethcleveland force-pushed the Support_generic_postgres_custom_metrics branch from 15108ed to fc575c1 Compare August 31, 2016 18:00
@sethcleveland
Copy link
Copy Markdown
Contributor Author

sethcleveland commented Aug 31, 2016

I did a brief scan on the failures and most things appear to fail this. --

bundle exec rake test
---> syntax:manifests
---> syntax:templates
---> syntax:hiera:yaml
manifests/integrations/mysql.pp:41:parameter_order:WARNING:optional parameter listed before required parameter
manifests/integrations/pgbouncer.pp:31:parameter_order:WARNING:optional parameter listed before required parameter
manifests/integrations/postgres.pp:56:parameter_order:WARNING:optional parameter listed before required parameter

I didn't make those changes. As far as I can tell they are the status quo.

@sethcleveland
Copy link
Copy Markdown
Contributor Author

@truthbk: How are these changes?

@truthbk
Copy link
Copy Markdown
Member

truthbk commented Sep 6, 2016

@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 master we can get rid of them..... :)

  - Adding resource definition for postgres custom metrics (for validation)
  - Added custom metrics example and test specs
@sethcleveland sethcleveland force-pushed the Support_generic_postgres_custom_metrics branch from 4374d8a to 411488d Compare September 9, 2016 17:02
@sethcleveland
Copy link
Copy Markdown
Contributor Author

@truthbk just rebased against master. Hopefully that fixes the problems.

@sethcleveland
Copy link
Copy Markdown
Contributor Author

@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.

@rooprob
Copy link
Copy Markdown
Contributor

rooprob commented Sep 12, 2016

Hi,

Can we split out and merge 5cd38a4 as it's blocking PRs such as #231 and #229.

@sethcleveland
Copy link
Copy Markdown
Contributor Author

@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.

@sethcleveland
Copy link
Copy Markdown
Contributor Author

@rooprob I cherry-picked that commit and made a PR here -- #232.

Comment thread manifests/integrations/postgres.pp Outdated
# $tags
# Optional array of tags
# $custom_metrics
# An array of custom metrics with the following hash keys - query, metrics,
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

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.

@truthbk
Copy link
Copy Markdown
Member

truthbk commented Sep 20, 2016

@sethcleveland thank you so much for this and #232 - merging this in. Again, apologies for the long MTTR.

@truthbk truthbk merged commit 8218721 into DataDog:master Sep 20, 2016
cegeka-jenkins pushed a commit to cegeka/puppet-datadog_agent that referenced this pull request Jan 31, 2018
…tgres_custom_metrics

Support generic postgres custom metrics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants