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

Add database query utilities #5045

Merged
merged 2 commits into from
Nov 20, 2019
Merged

Add database query utilities #5045

merged 2 commits into from
Nov 20, 2019

Conversation

ofek
Copy link
Contributor

@ofek ofek commented Nov 20, 2019

What does this PR do?

This implements a standard way to define and collect data from arbitrary queries. 2 new classes are introduced under datadog_checks.base.utils.db:

  1. Query - This class accepts a single dict argument which is the necessary data to run the query. The representation is based on our custom_queries format originally designed and implemented way back in Support custom queries #1528. It is now part of all our database integrations and other products have since adopted this idea e.g. https://cloud.google.com/solutions/sap/docs/sap-hana-monitoring-agent-user-guide#defining_custom_queries.
  2. QueryManager - This class is in charge of running any number of Query instances for a single AgentCheck instance given an executor. The executor must accept a single query str and return a sequence of results.

Before queries can be executed, they must be compiled once via query_manager.compile_queries() to validate the input data. After compilation, there is no longer need for error checking nor type look-ups, so we transform every column type to a stateful closure.

We support the standard submission method types, with the addition of:

  • tag - This was already supported, but there is now a boolean argument which will convert values to true or false as a str
  • monotonic_gauge - Given a column named foo, this will submit foo.total as gauge and foo.count as monotonic_count
  • temporal_percent - This maps to Add total_time_to_temporal_percent utility #4924 and will submit of course as rate. For convenience, the scale argument can also be either second, millisecond, microsecond, or nanosecond
  • match - This requires an items argument which is a map of source column values to other types. This is most useful for columnar-oriented databases, see e.g. https://clickhouse.yandex/docs/en/operations/system_tables/#system_tables-metrics

Any arguments not picked up by a transformer will be passed directly to the metric submission methods for overriding tags, setting the hostname, etc.

Motivation

Code reuse

Additional Notes

This satisfies all current use cases for ClickHouse, but we'll need to eventually implement:

  • metadata submission
  • extra_metrics that would be guaranteed to run after every column has been processed to compute things such as percentages
  • the ability to set a row post-processor per Query to do extra complicated things

@codecov
Copy link

codecov bot commented Nov 20, 2019

Codecov Report

Merging #5045 into master will decrease coverage by 4.05%.
The diff coverage is n/a.

Impacted Files Coverage Δ
pdh_check/tests/test_e2e.py 71.42% <0%> (ø) ⬆️
datadog_checks_dev/datadog_checks/dev/docker.py 60.65% <0%> (ø) ⬆️
..._server/datadog_checks/exchange_server/__init__.py 100% <0%> (ø) ⬆️
couchbase/datadog_checks/couchbase/__init__.py 100% <0%> (ø) ⬆️
apache/datadog_checks/apache/__about__.py 100% <0%> (ø) ⬆️
datadog_checks_dev/tests/test_docker.py 100% <0%> (ø) ⬆️
cacti/datadog_checks/cacti/__about__.py 100% <0%> (ø) ⬆️
apache/tests/test_apache.py 84.44% <0%> (ø) ⬆️
kube_apiserver_metrics/tests/common.py 100% <0%> (ø) ⬆️
postfix/tests/conftest.py 94.44% <0%> (ø) ⬆️
... and 822 more

Copy link
Contributor

@therve therve left a comment

Choose a reason for hiding this comment

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

Looks good, some minor comments.

Have you looked how we can adapt this to oracle and others?

datadog_checks_base/datadog_checks/base/utils/db/core.py Outdated Show resolved Hide resolved
datadog_checks_base/datadog_checks/base/utils/db/query.py Outdated Show resolved Hide resolved
self.query = query
self.columns = tuple(column_data)
self.tags = tags
del self.query_data
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to do that instead of setting it to None?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, goal is just to not use memory needlessly

@ofek
Copy link
Contributor Author

ofek commented Nov 20, 2019

Yes others can easily be adapted, though may need things in Additional Notes

@ofek ofek merged commit 1ad533b into master Nov 20, 2019
@ofek ofek deleted the ofek/query branch November 20, 2019 18:59
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.

2 participants