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

Support "*" wildcard in type_overrides configuration #7071

Merged
merged 1 commit into from
Jul 20, 2020
Merged

Support "*" wildcard in type_overrides configuration #7071

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
23 changes: 22 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 @@ -247,6 +247,20 @@ def create_scraper_configuration(self, instance=None):
config['type_overrides'] = default_instance.get('type_overrides', {})
config['type_overrides'].update(instance.get('type_overrides', {}))

# `_type_override_patterns` is a dictionary where we store Pattern objects
# that match metric names as keys, and their corresponding metric type overrrides as values.
config['_type_override_patterns'] = {}

with_wildcards = set()
for metric, type in iteritems(config['type_overrides']):
if '*' in metric:
config['_type_override_patterns'][compile(translate(metric))] = type
Comment on lines +255 to +257
Copy link
Member

@L3n41c L3n41c Jul 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

From a complexity and performance perspective, I think we can do even better.

Here, we have a map with one entry per “pattern” type_override even if several entries have the same type.
It means that, in the loop below, we’ll do several regex matching where a single regex match could be enough.

The idea would be for config['_type_override_patterns'] to have a single entry per type.

That would be something like this:

Suggested change
for metric, type in iteritems(config['type_overrides']):
if '*' in metric:
config['_type_override_patterns'][compile(translate(metric))] = type
overrides_per_type = {}
for metric, type in iteritems(config['type_overrides']):
if '*' in metric:
if type not in overrides_per_type:
overrides_per_type[type] = []
overrides_per_type[type].append(fnmatch.translate(metric))
del(config['type_overrides'][metric])
for type, metric_patterns in iteritems(overrides_per_type):
config['_type_override_patterns'][compile('|'.join(metrics_patterns))] = type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @L3n41c for the suggestion!
I actually thought about this approach at first but I'm not sure if it's worth it, as this path is executed only once (at the class creation) and more importantly this makes the code less straight forward and more difficult to read/understand.
@ofek what do you think about the tradeoff perf/code-readability here? is it worth it?

Copy link
Contributor

Choose a reason for hiding this comment

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

If only at class creation time, then yes might not be worth it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you, mind approving the PR please if looks good then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ofek a friendly bump!

L3n41c marked this conversation as resolved.
Show resolved Hide resolved
with_wildcards.add(metric)

# cleanup metric names with wildcards from the 'type_overrides' dict
for metric in with_wildcards:
del config['type_overrides'][metric]

# Some metrics are retrieved from differents hosts and often
# a label can hold this information, this transfers it to the hostname
config['label_to_hostname'] = instance.get('label_to_hostname', default_instance.get('label_to_hostname', None))
Expand Down Expand Up @@ -392,7 +406,14 @@ def parse_metric_family(self, response, scraper_config):
self._send_telemetry_counter(
self.TELEMETRY_COUNTER_METRICS_INPUT_COUNT, len(metric.samples), scraper_config
)
metric.type = scraper_config['type_overrides'].get(metric.name, metric.type)
type_override = scraper_config['type_overrides'].get(metric.name)
if type_override:
metric.type = type_override
elif scraper_config['_type_override_patterns']:
for pattern, new_type in iteritems(scraper_config['_type_override_patterns']):
if pattern.search(metric.name):
metric.type = new_type
break
if metric.type not in self.METRIC_TYPES:
continue
metric.name = self._remove_metric_prefix(metric.name, scraper_config)
Expand Down
53 changes: 52 additions & 1 deletion datadog_checks_base/tests/test_openmetrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,11 @@ def close(self):
pass


FAKE_ENDPOINT = 'http://fake.endpoint:10055/metrics'


PROMETHEUS_CHECK_INSTANCE = {
'prometheus_url': 'http://fake.endpoint:10055/metrics',
'prometheus_url': FAKE_ENDPOINT,
'metrics': [{'process_virtual_memory_bytes': 'process.vm.bytes'}],
'namespace': 'prometheus',
# Defaults for checks that were based on PrometheusCheck
Expand Down Expand Up @@ -2534,3 +2537,51 @@ def test_http_handler(mocked_openmetrics_check_factory):

assert http_handler.options['headers']['accept-encoding'] == 'gzip'
assert http_handler.options['headers']['accept'] == 'text/plain'


def test_simple_type_overrides(aggregator, mocked_prometheus_check, text_data):
"""
Test that metric type is overridden correctly.
"""
check = mocked_prometheus_check
instance = copy.deepcopy(PROMETHEUS_CHECK_INSTANCE)
instance['type_overrides'] = {"process_virtual_memory_bytes": "counter"}

# Make sure we don't send counters as gauges
instance['send_monotonic_counter'] = True

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

check.poll = mock.MagicMock(return_value=MockResponse(text_data, text_content_type))
check.process(config)

aggregator.assert_metric('prometheus.process.vm.bytes', count=1, metric_type=aggregator.MONOTONIC_COUNT)

assert len(check.config_map[FAKE_ENDPOINT]['_type_override_patterns']) == 0
assert len(check.config_map[FAKE_ENDPOINT]['type_overrides']) == 1


def test_wildcard_type_overrides(aggregator, mocked_prometheus_check, text_data):
"""
Test that metric type is overridden correctly with wildcard.
"""
check = mocked_prometheus_check
instance = copy.deepcopy(PROMETHEUS_CHECK_INSTANCE)
instance['type_overrides'] = {"*_virtual_memory_*": "counter"}

# Make sure we don't send counters as gauges
instance['send_monotonic_counter'] = True

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

check.poll = mock.MagicMock(return_value=MockResponse(text_data, text_content_type))
check.process(config)

aggregator.assert_metric('prometheus.process.vm.bytes', count=1, metric_type=aggregator.MONOTONIC_COUNT)

# assert the pattern was stored correctly
assert len(check.config_map[FAKE_ENDPOINT]['_type_override_patterns']) == 1
assert list(check.config_map[FAKE_ENDPOINT]['_type_override_patterns'].values())[0] == 'counter'
assert len(check.config_map[FAKE_ENDPOINT]['type_overrides']) == 0
1 change: 1 addition & 0 deletions ..._checks_dev/datadog_checks/dev/tooling/templates/configuration/instances/openmetrics.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
Type override allows you to override a type in the prometheus payload
or type an untyped metrics (they're ignored by default).
Supported <METRIC_TYPE> are `gauge`, `counter`, `histogram`, `summary`
The "*" wildcard can be used to match multiple metric names.
value:
example:
<METRIC_NAME>: <METRIC_TYPE>
Expand Down
1 change: 1 addition & 0 deletions gitlab/datadog_checks/gitlab/data/conf.yaml.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ instances:
## Type override allows you to override a type in the prometheus payload
## or type an untyped metrics (they're ignored by default).
## Supported <METRIC_TYPE> are `gauge`, `counter`, `histogram`, `summary`
## The "*" wildcard can be used to match multiple metric names.
#
# type_overrides:
# <METRIC_NAME>: <METRIC_TYPE>
Expand Down
1 change: 1 addition & 0 deletions istio/datadog_checks/istio/data/conf.yaml.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ instances:
## Type override allows you to override a type in the prometheus payload
## or type an untyped metrics (they're ignored by default).
## Supported <METRIC_TYPE> are `gauge`, `counter`, `histogram`, `summary`
## The "*" wildcard can be used to match multiple metric names.
#
# type_overrides:
# <METRIC_NAME>: <METRIC_TYPE>
Expand Down
1 change: 1 addition & 0 deletions kube_apiserver_metrics/datadog_checks/kube_apiserver_metrics/data/conf.yaml.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ instances:
## Type override allows you to override a type in the prometheus payload
## or type an untyped metrics (they're ignored by default).
## Supported <METRIC_TYPE> are `gauge`, `counter`, `histogram`, `summary`
## The "*" wildcard can be used to match multiple metric names.
#
# type_overrides:
# <METRIC_NAME>: <METRIC_TYPE>
Expand Down
1 change: 1 addition & 0 deletions nginx_ingress_controller/datadog_checks/nginx_ingress_controller/data/conf.yaml.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ instances:
## Type override allows you to override a type in the prometheus payload
## or type an untyped metrics (they're ignored by default).
## Supported <METRIC_TYPE> are `gauge`, `counter`, `histogram`, `summary`
## The "*" wildcard can be used to match multiple metric names.
#
# type_overrides:
# <METRIC_NAME>: <METRIC_TYPE>
Expand Down
1 change: 1 addition & 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 @@ -101,6 +101,7 @@ instances:
## Type override allows you to override a type in the prometheus payload
## or type an untyped metrics (they're ignored by default).
## Supported <METRIC_TYPE> are `gauge`, `counter`, `histogram`, `summary`
## The "*" wildcard can be used to match multiple metric names.
#
# type_overrides:
# <METRIC_NAME>: <METRIC_TYPE>
Expand Down