Skip to content
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

Collect statement metrics & samples from all databases on host #9252

Merged
merged 5 commits into from
May 10, 2021

Conversation

djova
Copy link
Contributor

@djova djova commented Apr 27, 2021

What does this PR do?

Update the collection of postgres statement metrics & samples to automatically collect data for all databases on a host.

Changes:

  • collection of statement metrics & samples now respects the dbstrict setting
  • in order to be able to collect telemetry from multiple databases the statement sampler thread now maintains a collection pool with one connection per database
  • added a new ignore_databases configuration to enable users to define which databases on the host will be ignored. It defaults to the same exclusion list that was previously hardcoded in the "instance metrics" query. This setting is now shared across the whole check (instance metrics, statement metrics, statement samples)

Follow-up to #8627

Motivation

  • Simplify configuration for collection of statement metrics, samples & execution plans for users by enabling collection from all databases on a host with only a single configured "check instance." Previously users had to enumerate each database in a host separately.
  • Ensure that collection of statement samples & plans respects the dbstrict setting

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@ghost ghost added the integration/postgres label Apr 27, 2021
@djova djova changed the title initial change postgres statement samples: automatically connect to all databases on host Apr 27, 2021
@djova djova force-pushed the djova/postgres-samples-automatic-db-detection branch from 4c5bcd2 to 92819bb Compare April 29, 2021 21:11
@djova djova changed the title postgres statement samples: automatically connect to all databases on host postgres statement metrics & samples: collect from all databases on host Apr 29, 2021
@djova djova force-pushed the djova/postgres-samples-automatic-db-detection branch from 6657ac8 to 2f3f9fe Compare April 29, 2021 22:31
@djova djova marked this pull request as ready for review April 29, 2021 22:33
@djova djova requested a review from a team as a code owner April 29, 2021 22:33
postgres/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
postgres/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
postgres/datadog_checks/postgres/data/conf.yaml.example Outdated Show resolved Hide resolved
postgres/datadog_checks/postgres/data/conf.yaml.example Outdated Show resolved Hide resolved
@djova djova force-pushed the djova/postgres-samples-automatic-db-detection branch from 35a136e to 7488cf4 Compare April 30, 2021 19:11
Copy link
Contributor

@ofek ofek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Have we considered preventing all other collection from the instance that runs this feature?

postgres/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
postgres/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
postgres/datadog_checks/postgres/statement_samples.py Outdated Show resolved Hide resolved
postgres/datadog_checks/postgres/statement_samples.py Outdated Show resolved Hide resolved
@djova djova force-pushed the djova/postgres-samples-automatic-db-detection branch 3 times, most recently from 4544097 to be13787 Compare May 7, 2021 20:12
Update the collection of postgres statement metrics & samples to automatically collect data for all databases on a host.

Changes:

* collection of statement metrics & samples now respects the `dbstrict` setting
* in order to be able to collect telemetry from multiple databases the statement sampler thread now maintains a collection pool with one connection per database
* added a new `ignore_databases` configuration to enable users to define which databases on the host will be ignored. It defaults to the same exclusion list that was previously hardcoded in the "instance metrics" query. This setting is now shared across the whole check (instance metrics, statement metrics, statement samples)

Follow-up to #8627

Motivation:

* Simplify configuration for collection of statement metrics, samples & execution plans for users by enabling collection from all databases on a host with only a single configured "check instance." Previously users had to enumerate each database in a host separately.
* Ensure that collection of statement samples & plans respects the `dbstrict` setting
@djova djova force-pushed the djova/postgres-samples-automatic-db-detection branch from be13787 to 60258a3 Compare May 7, 2021 20:15
@ofek ofek changed the title postgres statement metrics & samples: collect from all databases on host Collect statement metrics & samples from all databases on host May 10, 2021
@djova
Copy link
Contributor Author

djova commented May 10, 2021

Have we considered preventing all other collection from the instance that runs this feature?

Summary from offline discussion: the intended deployment configuration for the postgres check is to have only a single check instance for each postgres host. That one check instance will collect data from all databases on the host by default. If a user decides to configure multiple check instances for different databases on the same host then it's expected that there will be duplicate data collected by the check instances. In that case the user will need to set dbstrict: true to restrict the check instance to collecting data only from the specific database configured for that check instance.

@djova djova merged commit 0d73de6 into master May 10, 2021
@djova djova deleted the djova/postgres-samples-automatic-db-detection branch May 10, 2021 19:30
djova added a commit that referenced this pull request Jun 3, 2021
Remove the incorrect `db` tag inherited from the check instance tags from postgres DBM statement metrics & FQT events. This tag needs to be excluded because statement metrics & FQT events are collected from all databases on the host as of #9252. DBM statement samples are already doing the right thing here.
ofek pushed a commit that referenced this pull request Jun 3, 2021
Remove the incorrect `db` tag inherited from the check instance tags from postgres DBM statement metrics & FQT events. This tag needs to be excluded because statement metrics & FQT events are collected from all databases on the host as of #9252. DBM statement samples are already doing the right thing here.
ofek pushed a commit that referenced this pull request Jun 3, 2021
Remove the incorrect `db` tag inherited from the check instance tags from postgres DBM statement metrics & FQT events. This tag needs to be excluded because statement metrics & FQT events are collected from all databases on the host as of #9252. DBM statement samples are already doing the right thing here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants