Add `metric_patterns` options to filter all metric submission with a list of regexes by sarah-witt · Pull Request #11508 · DataDog/integrations-core

sarah-witt · 2022-02-15T21:47:10Z

What does this PR do?

Adds instance-level options to the base check to filter all metric submission with a regex. The main option is metric_patterns with two sub-options are exclude and include.

Motivation

Allow users to have more control over what metrics are submitted for each integration

Additional Notes

These behave very similarly to exclude_labels and include_labels, where exclude takes precedence over include
These can either be strings or regexes.
They also use the same regex construction as exclude_metrics
The namespace is included in the regex
I could add a debug log line if a metric is skipped, but that would be noisy

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

codecov · 2022-02-15T21:57:55Z

Codecov Report

Merging #11508 (02b30f8) into master (31548f9) will increase coverage by 0.26%.
The diff coverage is 98.19%.

Flag	Coverage Δ
activemq_xml	`82.31% <ø> (ø)`
ambari	`85.75% <ø> (ø)`
avi_vantage	`91.92% <ø> (ø)`
btrfs	`82.91% <ø> (ø)`
coredns	`94.54% <ø> (ø)`
directory	`94.87% <ø> (ø)`
ibm_i	`80.65% <ø> (ø)`
kube_proxy	`100.00% <ø> (ø)`
kube_scheduler	`96.20% <ø> (ø)`
kubelet	`90.42% <ø> (+0.79%)`	⬆️
kyototycoon	`85.96% <ø> (ø)`
linkerd	`85.14% <ø> (+1.14%)`	⬆️
nagios	`89.53% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

vickenty

Writing a single regular expression to match a collection of strings might not be the best UX for this. Would it make sense to let the user specify a list of glob patterns instead? (We could still use a compiled regular expression under the hood).

ofek

We should have both an inclusion and exclusion option
They should be a list of patterns as @vickenty and @fanny-jiang suggested
We should save like this

integrations-core/datadog_checks_base/datadog_checks/base/checks/openmetrics/v2/scraper.py

Lines 149 to 150 in 40a66fe

if exclude_metrics_patterns:

self.exclude_metrics_pattern = re.compile('|'.join(exclude_metrics_patterns))

(see #5764 (comment))

sarah-witt · 2022-02-16T21:59:50Z

Ah thanks everyone- I'll update the field to be a list and also model the behavior of exclude_labels and include_labels by having both an inclusion and exclusion list as well as compiling everything faster.

ofek

^

vickenty

LGTM, just a small nit / suggestion.

vickenty · 2022-03-10T10:47:21Z

+        if not isinstance(filters, list):
+            raise ConfigurationError('Setting `{}` must be an array'.format(option_name))
+
+        metrics_pattern = None


Suggested change

metrics_pattern = None

vickenty · 2022-03-10T10:47:38Z

+        if metrics_patterns:
+            metrics_pattern = re.compile('|'.join(metrics_patterns))
+        return metrics_pattern


Would it make sense to return the values directly, instead of going through a variable?

Suggested change

if metrics_patterns:

metrics_pattern = re.compile('|'.join(metrics_patterns))

return metrics_pattern

if metrics_patterns:

return re.compile('|'.join(metrics_patterns))

return None

Sure, I agree it's a little more readable this way

ofek

🔥

Last thing, WDYTA naming them (ex|in)clude_metric_patterns?

sarah-witt · 2022-03-10T19:33:17Z

Thanks, updated with the suggestion to have an overall param metric_filters

metric_filters:
  include:
    - ...
  exclude:

…list of regexes (#11508) * Add option for global metrics filter * Filter before context limiter * Compile regex and check for invalid regex * Add test for invalid regex * Add exclude and include filters * move regex logic down, add more tests * Remove space * Add more invalid tests * Refactor to helper functions, address comments, and add more tests * Add another test case * Add test case for include glob * Remove duplicate logic and further extract * Don't use metric_pattern veriable * Add overall metrics_filters param * Update message and tests * Rename to metric_patterns ddcf5dc

CrashLaker · 2024-09-18T11:39:19Z

hi all. i'm struggling to find how can this be added to the prometheusScrape section in helm values.yaml
can anyone please point me to the right direction/documentation? @fanny-jiang @ofek @sarah-witt
thanks,c.

Add option for global metrics filter

63d1d88

ghost added the integration/datadog_checks_base label Feb 15, 2022

sarah-witt added the changelog/Added label Feb 15, 2022

sarah-witt marked this pull request as ready for review February 15, 2022 22:24

sarah-witt requested review from a team as code owners February 15, 2022 22:24

Filter before context limiter

87e2933

vickenty reviewed Feb 16, 2022

View reviewed changes

Comment thread datadog_checks_base/datadog_checks/base/checks/base.py Outdated

Comment thread datadog_checks_base/datadog_checks/base/checks/base.py Outdated

sarah-witt added 2 commits February 16, 2022 14:51

Compile regex and check for invalid regex

785d0e5

Add test for invalid regex

595e5f1

sarah-witt requested a review from vickenty February 16, 2022 20:58

fanny-jiang reviewed Feb 16, 2022

View reviewed changes

Comment thread datadog_checks_base/datadog_checks/base/checks/base.py Outdated

ofek requested changes Feb 16, 2022

View reviewed changes

sarah-witt added 4 commits March 1, 2022 10:31

Add exclude and include filters

0dec9ab

move regex logic down, add more tests

0550b10

Remove space

49db9ff

Add more invalid tests

7d82582

sarah-witt commented Mar 2, 2022

View reviewed changes

Comment thread datadog_checks_base/datadog_checks/base/checks/base.py Outdated

sarah-witt changed the title ~~Add option to filter all metric submission with a regex~~ Add options to filter all metric submission with a list of regexes Mar 2, 2022

sarah-witt changed the title ~~Add options to filter all metric submission with a list of regexes~~ Add exclude_metrics_filters and include_metrics_filters options to filter all metric submission with a list of regexes Mar 2, 2022

ofek approved these changes Mar 4, 2022

View reviewed changes

Comment thread datadog_checks_base/datadog_checks/base/checks/base.py Outdated

ofek requested changes Mar 4, 2022

View reviewed changes

vickenty reviewed Mar 4, 2022

View reviewed changes

ofek requested changes Mar 4, 2022

View reviewed changes

Comment thread datadog_checks_base/datadog_checks/base/checks/base.py Outdated

Comment thread datadog_checks_base/datadog_checks/base/checks/base.py Outdated

sarah-witt added 3 commits March 9, 2022 13:44

Refactor to helper functions, address comments, and add more tests

8c289e3

Add another test case

2f6ab9c

Add test case for include glob

63ad480

ofek requested changes Mar 9, 2022

View reviewed changes

Comment thread datadog_checks_base/datadog_checks/base/checks/base.py Outdated

Remove duplicate logic and further extract

16129b5

vickenty previously approved these changes Mar 10, 2022

View reviewed changes

Don't use metric_pattern veriable

090d93c

sarah-witt dismissed vickenty’s stale review via 090d93c March 10, 2022 14:51

ofek requested changes Mar 10, 2022

View reviewed changes

Add overall metrics_filters param

d852539

sarah-witt changed the title ~~Add exclude_metrics_filters and include_metrics_filters options to filter all metric submission with a list of regexes~~ Add metric_filters options to filter all metric submission with a list of regexes Mar 10, 2022

sarah-witt added 2 commits March 10, 2022 14:43

Update message and tests

dced482

Rename to metric_patterns

02b30f8

sarah-witt changed the title ~~Add metric_filters options to filter all metric submission with a list of regexes~~ Add metric_patterns options to filter all metric submission with a list of regexes Mar 10, 2022

ofek approved these changes Mar 11, 2022

View reviewed changes

sarah-witt merged commit ddcf5dc into master Mar 11, 2022

sarah-witt deleted the sarah/filter-metrics-regex branch March 11, 2022 19:12

ofek mentioned this pull request Apr 8, 2022

Fix metric_patterns option to support namespaces #11795

Merged

fanny-jiang mentioned this pull request Aug 8, 2022

Add ignore_metrics to Prometheus scraper #12713

Closed

4 tasks

	if exclude_metrics_patterns:
	self.exclude_metrics_pattern = re.compile('\|'.join(exclude_metrics_patterns))

Uh oh!

Conversation

sarah-witt commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

Uh oh!

codecov Bot commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vickenty left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ofek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarah-witt commented Feb 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ofek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vickenty left a comment

Choose a reason for hiding this comment

Uh oh!

vickenty Mar 10, 2022

Choose a reason for hiding this comment

Uh oh!

vickenty Mar 10, 2022

Choose a reason for hiding this comment

Uh oh!

sarah-witt Mar 10, 2022

Choose a reason for hiding this comment

Uh oh!

ofek left a comment

Choose a reason for hiding this comment

Uh oh!

sarah-witt commented Mar 10, 2022

Uh oh!

CrashLaker commented Sep 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sarah-witt commented Feb 15, 2022 •

edited

Loading

codecov Bot commented Feb 15, 2022 •

edited

Loading

ofek left a comment •

edited

Loading

sarah-witt commented Feb 16, 2022 •

edited

Loading