From 482a4563ed6c4e4dc453d499cdbcc805ad7618c6 Mon Sep 17 00:00:00 2001 From: Ilia Kurenkov Date: Thu, 19 Jan 2023 10:14:17 +0100 Subject: [PATCH] several fixes: - remove unnecessary validation logic - fix url in one of the tests, - fix style - aggregated endpoint is enabled by default --- .../rabbitmq/config_models/validators.py | 2 -- .../rabbitmq/openmetrics/check.py | 2 +- rabbitmq/tests/common.py | 2 +- rabbitmq/tests/test_openmetrics.py | 23 ++++++++++++------- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/rabbitmq/datadog_checks/rabbitmq/config_models/validators.py b/rabbitmq/datadog_checks/rabbitmq/config_models/validators.py index 981836df26d0a1..95df3f9ce78931 100644 --- a/rabbitmq/datadog_checks/rabbitmq/config_models/validators.py +++ b/rabbitmq/datadog_checks/rabbitmq/config_models/validators.py @@ -24,8 +24,6 @@ def initialize_instance(values, **kwargs): raise ValueError("prometheus_plugin.url field must be an HTTP or HTTPS URL.") if 'unaggregated_endpoint' in plugin_settings: unagg_ep = plugin_settings['unaggregated_endpoint'] - if not isinstance(unagg_ep, str): - raise TypeError("prometheus_plugin.unaggregated_endpoint must be a string.") if not re.match(r'(per-object|detailed(\?.+)?)$', unagg_ep): raise ValueError( "prometheus_plugin.unaggregated_endpoint must be either 'per-object' or 'detailed' " diff --git a/rabbitmq/datadog_checks/rabbitmq/openmetrics/check.py b/rabbitmq/datadog_checks/rabbitmq/openmetrics/check.py index f07e270cf05288..7b2df3efa67f44 100644 --- a/rabbitmq/datadog_checks/rabbitmq/openmetrics/check.py +++ b/rabbitmq/datadog_checks/rabbitmq/openmetrics/check.py @@ -15,7 +15,7 @@ def configure_scrapers(self): if unagg_ep: renames, exclude_from_agg = metrics.unaggregated_renames_and_exclusions(unagg_ep) endpoints.append((f"/metrics/{unagg_ep}", {"metrics": [renames]})) - if self.instance['prometheus_plugin'].get('include_aggregated_endpoint', False): + if self.instance['prometheus_plugin'].get('include_aggregated_endpoint', True): endpoints.append(("/metrics", {'metrics': [metrics.aggregated_renames(exclude_from_agg)]})) for ep, ep_config in endpoints: self.scraper_configs.append({**self.instance, 'openmetrics_endpoint': base_url + ep, **ep_config}) diff --git a/rabbitmq/tests/common.py b/rabbitmq/tests/common.py index 9ed6c0ceec2770..049e0544ec9771 100644 --- a/rabbitmq/tests/common.py +++ b/rabbitmq/tests/common.py @@ -109,4 +109,4 @@ } OPENMETRICS_CONFIG = {"prometheus_plugin": {"url": OPENMETRICS_URL, "include_aggregated_endpoint": True}} -DEFAULT_OM_TAGS = ["endpoint:localhost:15692/metrics"] +DEFAULT_OM_TAGS = ["endpoint:http://localhost:15692/metrics"] diff --git a/rabbitmq/tests/test_openmetrics.py b/rabbitmq/tests/test_openmetrics.py index 148702e5866734..29dfc8179d5753 100644 --- a/rabbitmq/tests/test_openmetrics.py +++ b/rabbitmq/tests/test_openmetrics.py @@ -1,11 +1,11 @@ from itertools import product from pathlib import Path -from datadog_checks.dev.utils import get_metadata_metrics import pytest from datadog_checks.base.errors import ConfigurationError from datadog_checks.dev.http import MockResponse +from datadog_checks.dev.utils import get_metadata_metrics from datadog_checks.rabbitmq import RabbitMQ from .common import DEFAULT_OM_TAGS, HERE @@ -15,17 +15,21 @@ TEST_URL = "http://localhost:15692" -def test_aggregated_endpoint(aggregator, dd_run_check, mock_http_response): +@pytest.mark.parametrize( + 'aggregated_setting', + [ + pytest.param({"include_aggregated_endpoint": True}, id="explicitly enable"), + pytest.param({}, id="implicitly enable by default"), + ], +) +def test_aggregated_endpoint(aggregated_setting, aggregator, dd_run_check, mock_http_response): """User only enables aggregated endpoint. We expect in this case all the metrics from the '/metrics' endpoint. """ mock_http_response(file_path=OPENMETRICS_RESPONSE_FIXTURES / "metrics.txt") - check = RabbitMQ( - "rabbitmq", - {}, - [{'prometheus_plugin': {'url': TEST_URL, "include_aggregated_endpoint": True}, "metrics": [".+"]}], - ) + prometheus_settings = {'url': TEST_URL, **aggregated_setting} + check = RabbitMQ("rabbitmq", {}, [{'prometheus_plugin': prometheus_settings, "metrics": [".+"]}]) dd_run_check(check) build_info_tags = [ @@ -66,6 +70,7 @@ def test_bare_detailed_endpoint(aggregator, dd_run_check, mock_http_response): 'prometheus_plugin': { 'url': TEST_URL, 'unaggregated_endpoint': 'detailed', + "include_aggregated_endpoint": False, }, "metrics": [".+"], } @@ -111,6 +116,7 @@ def test_detailed_endpoint_queue_coarse_metrics(aggregator, dd_run_check, mock_h 'prometheus_plugin': { 'url': TEST_URL, 'unaggregated_endpoint': 'detailed?family=queue_coarse_metrics', + "include_aggregated_endpoint": False, }, "metrics": [".+"], } @@ -192,6 +198,7 @@ def test_per_object(aggregator, dd_run_check, mock_http_response): 'prometheus_plugin': { 'url': TEST_URL, 'unaggregated_endpoint': 'per-object', + "include_aggregated_endpoint": False, }, "metrics": [".+"], } @@ -365,7 +372,7 @@ def test_aggregated_and_detailed_endpoints(query, metrics, aggregator, dd_run_ch ), pytest.param( {'url': "http://localhost", "unaggregated_endpoint": []}, - r"prometheus_plugin\.unaggregated_endpoint must be a string\.", + "expected string or bytes-like object", id="Unaggregated_endpoint value must be a string.", ), pytest.param(