several fixes:

- remove unnecessary validation logic - fix url in one of the tests, - fix style - aggregated endpoint is enabled by default
DataDog · Jan 19, 2023 · 482a456 · 482a456
1 parent ec20f69
commit 482a456
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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(