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.

Sign up for GitHub

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

Jump to bottom

Make ignore_metrics support * wildcard for OpenMetrics #5759

Merged
merged 1 commit into from
Feb 17, 2020
Merged

Make ignore_metrics support * wildcard for OpenMetrics #5759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion datadog_checks_base/datadog_checks/base/checks/openmetrics/mixins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,15 @@ def create_scraper_configuration(self, instance=None):
# very high cardinality. Metrics included in this list will be silently
# skipped without a 'Unable to handle metric' debug line in the logs
config['ignore_metrics'] = instance.get('ignore_metrics', default_instance.get('ignore_metrics', []))
config['_ignored_metrics'] = set()
config['_ignored_patterns'] = set()

# Separate ignored metric names and ignored patterns in different sets for faster lookup later
for metric in config['ignore_metrics']:
if '*' in metric:
config['_ignored_patterns'].add(metric)
else:
config['_ignored_metrics'].add(metric)

# If you want to send the buckets as tagged values when dealing with histograms,
# set send_histograms_buckets to True, set to False otherwise.
Expand Down Expand Up @@ -506,12 +515,22 @@ def process_metric(self, metric, scraper_config, metric_transformers=None):
# If targeted metric, store labels
self._store_labels(metric, scraper_config)

if metric.name in scraper_config['ignore_metrics']:
if metric.name in scraper_config['_ignored_metrics']:
self._send_telemetry_counter(
self.TELEMETRY_COUNTER_METRICS_IGNORE_COUNT, len(metric.samples), scraper_config
)
return # Ignore the metric

for ignore_pattern in scraper_config['_ignored_patterns']:
if fnmatchcase(metric.name, ignore_pattern):
Copy link
Contributor

Choose a reason for hiding this comment

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

This will cause a large slow down. No way around it I guess, just saying...

Copy link
Contributor

Choose a reason for hiding this comment

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

is it worth adding a if config['ignore_metrics'] or the for on the empty set if fast enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here is a small benchmark - it's almost the same

>>> timeit('for x in s:    print("found")', setup='s = set(); x = "randomstring"',number=100000)
0.004889011383056641
>>>
>>> timeit('if map:    print("found")', setup='map = dict()', number=100000)
0.003404855728149414

Copy link
Contributor

Choose a reason for hiding this comment

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

in absolute it's fast yes, but looks like we could use the 30% speedup 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here is a follow-up PR #5764

# Metric must be ignored
# Cache the ignored metric name to avoid calling fnmatchcase in the next check run
scraper_config['_ignored_metrics'].add(metric.name)
self._send_telemetry_counter(
self.TELEMETRY_COUNTER_METRICS_IGNORE_COUNT, len(metric.samples), scraper_config
)
return # Ignore the metric

self._send_telemetry_counter(self.TELEMETRY_COUNTER_METRICS_PROCESS_COUNT, len(metric.samples), scraper_config)

if self._filter_metric(metric, scraper_config):
Expand Down
69 changes: 69 additions & 0 deletions datadog_checks_base/tests/test_openmetrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,75 @@ def test_ignore_metric(aggregator, mocked_prometheus_check, ref_gauge):
aggregator.assert_metric('prometheus.process.vm.bytes', count=0)


def test_ignore_metric_wildcard(aggregator, mocked_prometheus_check, ref_gauge):
"""
Test that metric that matched the ignored metrics pattern is properly discarded.
"""
check = mocked_prometheus_check
instance = copy.deepcopy(PROMETHEUS_CHECK_INSTANCE)
instance['ignore_metrics'] = ['process_virtual_*']

config = check.get_scraper_config(instance)
config['_dry_run'] = False

check.process_metric(ref_gauge, config)

aggregator.assert_metric('prometheus.process.vm.bytes', count=0)


def test_ignore_metrics_multiple_wildcards(
aggregator, mocked_prometheus_check, mocked_prometheus_scraper_config, text_data
):
"""
Test that metrics that matched an ignored metrics pattern is properly discarded.
"""
check = mocked_prometheus_check
instance = copy.deepcopy(PROMETHEUS_CHECK_INSTANCE)
instance['_dry_run'] = False
instance['metrics'] = [
{
# Ignored
'go_memstats_mspan_inuse_bytes': 'go_memstats.mspan.inuse_bytes',
'go_memstats_mallocs_total': 'go_memstats.mallocs.total',
'go_memstats_mspan_sys_bytes': 'go_memstats.mspan.sys_bytes',
'go_memstats_alloc_bytes': 'go_memstats.alloc_bytes',
'go_memstats_gc_sys_bytes': 'go_memstats.gc.sys_bytes',
'go_memstats_buck_hash_sys_bytes': 'go_memstats.buck_hash.sys_bytes',
# Not ignored
'go_memstats_mcache_sys_bytes': 'go_memstats.mcache.sys_bytes',
'go_memstats_heap_released_bytes_total': 'go_memstats.heap.released.bytes_total',
}
]
instance['ignore_metrics'] = [
'go_memstats_mallocs_total',
'go_memstats_mspan_*',
'*alloc*',
'*gc_sys_bytes',
'go_memstats_*_hash_sys_bytes',
]

config = check.create_scraper_configuration(instance)

mock_response = mock.MagicMock(
status_code=200, iter_lines=lambda **kwargs: text_data.split("\n"), headers={'Content-Type': text_content_type}
)
with mock.patch('requests.get', return_value=mock_response, __name__="get"):
check.process(config)

# Make sure metrics are ignored
aggregator.assert_metric('prometheus.go_memstats.mspan.inuse_bytes', count=0)
aggregator.assert_metric('prometheus.go_memstats.mallocs.total', count=0)
aggregator.assert_metric('prometheus.go_memstats.mspan.sys_bytes', count=0)
aggregator.assert_metric('prometheus.go_memstats.alloc_bytes', count=0)
aggregator.assert_metric('prometheus.go_memstats.gc.sys_bytes', count=0)
aggregator.assert_metric('prometheus.go_memstats.buck_hash.sys_bytes', count=0)

# Make sure we don't ignore other metrics
aggregator.assert_metric('prometheus.go_memstats.mcache.sys_bytes', count=1)
aggregator.assert_metric('prometheus.go_memstats.heap.released.bytes_total', count=1)
aggregator.assert_all_metrics_covered()


def test_label_joins(aggregator, mocked_prometheus_check, mocked_prometheus_scraper_config, mock_get):
""" Tests label join on text format """
check = mocked_prometheus_check
Expand Down
10 changes: 10 additions & 0 deletions openmetrics/datadog_checks/openmetrics/data/conf.yaml.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,13 @@ instances:
## Note: bearer_token_auth should be set to true to enable adding the token to HTTP headers for authentication.
#
# bearer_token_path: "<TOKEN_PATH>"

## @param ignore_metrics - list of strings - optional
## List of metrics to be ignored, the "*" wildcard can be used to match multiple metric names.
#
# ignore_metrics:
# - <IGNORED_METRIC_NAME>
# - <PREFIX_*>
# - <*_SUFFIX>
# - <PREFIX_*_SUFFIX>
# - <*_SUBSTRING_*>