Skip to content

Commit

Permalink
Fix histogram upper bound label name for new OpenMetrics implementati…
Browse files Browse the repository at this point in the history 
…on (#8505)
  • Loading branch information
@ofek
ofek authored Feb 1, 2021
1 parent 60015f3 commit 0f8add2
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 32 deletions.
4 changes: 2 additions & 2 deletions datadog_checks_base/datadog_checks/base/checks/openmetrics/v2/labels.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@ def canonicalize_numeric_label(label):


def normalize_labels_histogram(labels):
upper_bound = labels.get('le')
upper_bound = labels.pop('le', None)
if upper_bound is not None:
labels['le'] = str(canonicalize_numeric_label(upper_bound))
labels['upper_bound'] = str(canonicalize_numeric_label(upper_bound))


def normalize_labels_summary(labels):
Expand Down
6 changes: 3 additions & 3 deletions datadog_checks_base/datadog_checks/base/checks/openmetrics/v2/transformers/histogram.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def histogram(metric, sample_data, runtime_data):
continue

lower_bound = canonicalize_numeric_label(sample.labels['lower_bound'])
upper_bound = canonicalize_numeric_label(sample.labels['le'])
upper_bound = canonicalize_numeric_label(sample.labels['upper_bound'])

if lower_bound == upper_bound:
# this can happen for -inf/-inf bucket that we don't want to send (always 0)
Expand Down Expand Up @@ -65,7 +65,7 @@ def histogram(metric, sample_data, runtime_data):
)
# Skip infinity upper bound as that is otherwise the
# same context as the sample suffixed by `_count`
elif sample_name.endswith('_bucket') and not sample.labels['le'].endswith('inf'):
elif sample_name.endswith('_bucket') and not sample.labels['upper_bound'].endswith('inf'):
monotonic_count_method(
bucket_metric,
sample.value,
Expand Down Expand Up @@ -100,7 +100,7 @@ def histogram(metric, sample_data, runtime_data):
)
# Skip infinity upper bound as that is otherwise the
# same context as the sample suffixed by `_count`
elif sample_name.endswith('_bucket') and not sample.labels['le'].endswith('inf'):
elif sample_name.endswith('_bucket') and not sample.labels['upper_bound'].endswith('inf'):
monotonic_count_method(
bucket_metric,
sample.value,
Expand Down
10 changes: 6 additions & 4 deletions datadog_checks_base/datadog_checks/base/checks/openmetrics/v2/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def decumulate_histogram_buckets(sample_data):
context_key = compute_bucket_hash(sample.labels)
if context_key not in bucket_values_by_context_upper_bound:
bucket_values_by_context_upper_bound[context_key] = {}
bucket_values_by_context_upper_bound[context_key][float(sample.labels['le'])] = sample.value
bucket_values_by_context_upper_bound[context_key][float(sample.labels['upper_bound'])] = sample.value

new_sample_data.append([sample, tags, hostname])

Expand Down Expand Up @@ -64,7 +64,9 @@ def decumulate_histogram_buckets(sample_data):
yield sample, tags, hostname
else:
context_key = compute_bucket_hash(sample.labels)
matching_bucket_tuple = bucket_tuples_by_context_upper_bound[context_key][float(sample.labels['le'])]
matching_bucket_tuple = bucket_tuples_by_context_upper_bound[context_key][
float(sample.labels['upper_bound'])
]

# Prevent 0.0
lower_bound = str(matching_bucket_tuple[0] or 0)
Expand All @@ -76,5 +78,5 @@ def decumulate_histogram_buckets(sample_data):

def compute_bucket_hash(labels):
# we need the unique context for all the buckets
# hence we remove the `le` label
return hash(frozenset(sorted((k, v) for k, v in labels.items() if k != 'le')))
# hence we remove the `upper_bound` label
return hash(frozenset(sorted((k, v) for k, v in labels.items() if k != 'upper_bound')))
46 changes: 23 additions & 23 deletions datadog_checks_base/tests/openmetrics/test_transformers/test_histogram.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_default(aggregator, dd_run_check, mock_http_response):
'test.etcd_disk_wal_fsync_duration_seconds.bucket',
2,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'kind:fs', 'app:vault', 'le:0.001'],
tags=['endpoint:test', 'kind:fs', 'app:vault', 'upper_bound:0.001'],
)
aggregator.assert_metric(
'test.etcd_disk_wal_fsync_duration_seconds.sum',
Expand All @@ -102,7 +102,7 @@ def test_default(aggregator, dd_run_check, mock_http_response):
'test.etcd_disk_wal_fsync_duration_seconds.bucket',
718,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'kind:fs', 'app:kubernetes', 'le:0.001'],
tags=['endpoint:test', 'kind:fs', 'app:kubernetes', 'upper_bound:0.001'],
)
aggregator.assert_metric(
'test.etcd_disk_wal_fsync_duration_seconds.sum',
Expand Down Expand Up @@ -221,61 +221,61 @@ def test_non_cumulative_histogram_buckets(aggregator, dd_run_check, mock_http_re
'test.rest_client_request_latency_seconds.bucket',
81,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.004', 'lower_bound:0.002'],
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.004', 'lower_bound:0.002'],
)
aggregator.assert_metric(
'test.rest_client_request_latency_seconds.bucket',
254,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.001', 'lower_bound:0'],
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.001', 'lower_bound:0'],
)
aggregator.assert_metric(
'test.rest_client_request_latency_seconds.bucket',
367,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.002', 'lower_bound:0.001'],
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.002', 'lower_bound:0.001'],
)
aggregator.assert_metric(
'test.rest_client_request_latency_seconds.bucket',
25,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.008', 'lower_bound:0.004'],
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.008', 'lower_bound:0.004'],
)
aggregator.assert_metric(
'test.rest_client_request_latency_seconds.bucket',
11,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.016', 'lower_bound:0.008'],
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.016', 'lower_bound:0.008'],
)
aggregator.assert_metric(
'test.rest_client_request_latency_seconds.bucket',
6,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.032', 'lower_bound:0.016'],
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.032', 'lower_bound:0.016'],
)
aggregator.assert_metric(
'test.rest_client_request_latency_seconds.bucket',
4,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.064', 'lower_bound:0.032'],
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.064', 'lower_bound:0.032'],
)
aggregator.assert_metric(
'test.rest_client_request_latency_seconds.bucket',
6,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.128', 'lower_bound:0.064'],
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.128', 'lower_bound:0.064'],
)
aggregator.assert_metric(
'test.rest_client_request_latency_seconds.bucket',
1,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.256', 'lower_bound:0.128'],
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.256', 'lower_bound:0.128'],
)
aggregator.assert_metric(
'test.rest_client_request_latency_seconds.bucket',
0,
metric_type=aggregator.MONOTONIC_COUNT,
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.512', 'lower_bound:0.256'],
tags=['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.512', 'lower_bound:0.256'],
)
aggregator.assert_metric(
'test.rest_client_request_latency_seconds.sum',
Expand Down Expand Up @@ -358,7 +358,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
0.004,
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.004', 'lower_bound:0.002'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.004', 'lower_bound:0.002'],
)
aggregator.assert_histogram_bucket(
'test.rest_client_request_latency_seconds',
Expand All @@ -367,7 +367,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
0.001,
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.001', 'lower_bound:0'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.001', 'lower_bound:0'],
)
aggregator.assert_histogram_bucket(
'test.rest_client_request_latency_seconds',
Expand All @@ -376,7 +376,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
0.002,
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.002', 'lower_bound:0.001'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.002', 'lower_bound:0.001'],
)
aggregator.assert_histogram_bucket(
'test.rest_client_request_latency_seconds',
Expand All @@ -385,7 +385,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
0.008,
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.008', 'lower_bound:0.004'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.008', 'lower_bound:0.004'],
)
aggregator.assert_histogram_bucket(
'test.rest_client_request_latency_seconds',
Expand All @@ -394,7 +394,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
0.016,
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.016', 'lower_bound:0.008'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.016', 'lower_bound:0.008'],
)
aggregator.assert_histogram_bucket(
'test.rest_client_request_latency_seconds',
Expand All @@ -403,7 +403,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
0.032,
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.032', 'lower_bound:0.016'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.032', 'lower_bound:0.016'],
)
aggregator.assert_histogram_bucket(
'test.rest_client_request_latency_seconds',
Expand All @@ -412,7 +412,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
0.064,
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.064', 'lower_bound:0.032'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.064', 'lower_bound:0.032'],
)
aggregator.assert_histogram_bucket(
'test.rest_client_request_latency_seconds',
Expand All @@ -421,7 +421,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
0.128,
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.128', 'lower_bound:0.064'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.128', 'lower_bound:0.064'],
)
aggregator.assert_histogram_bucket(
'test.rest_client_request_latency_seconds',
Expand All @@ -430,7 +430,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
0.256,
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.256', 'lower_bound:0.128'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.256', 'lower_bound:0.128'],
)
aggregator.assert_histogram_bucket(
'test.rest_client_request_latency_seconds',
Expand All @@ -439,7 +439,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
0.512,
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:0.512', 'lower_bound:0.256'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:0.512', 'lower_bound:0.256'],
)
aggregator.assert_histogram_bucket(
'test.rest_client_request_latency_seconds',
Expand All @@ -448,7 +448,7 @@ def test_histogram_buckets_as_distributions(aggregator, dd_run_check, mock_http_
float('Inf'),
True,
check.hostname,
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'le:inf', 'lower_bound:0.512'],
['endpoint:test', 'url:http://127.0.0.1:8080/api', 'verb:GET', 'upper_bound:inf', 'lower_bound:0.512'],
)

aggregator.assert_all_metrics_covered()

0 comments on commit 0f8add2

Please sign in to comment.