Add connect state to OMv2 and shadow prefix tag to legacy (#16453)

* add connect_state metric to OMv2

* add more metrics

* add changelog

* lint

* remove from integrationt est

envoy/changelog.d/16453.added

@@ -0,0 +1 @@
+Add connect state metric for OpenmetricsV2 and add way to collect shadow prefixes in legacy check for rbac metrics

envoy/datadog_checks/envoy/metrics.py

@@ -376,6 +376,7 @@
     'envoy_http_local_rate_limit_enforced': 'http.local_rate_limit_enforced',
     'envoy_http_local_rate_limit_rate_limited': 'http.local_rate_limit_rate_limited',
     'envoy_http_local_rate_limit_ok': 'http.local_rate_limit_ok',
+    'envoy_control_plane_connected_state': 'control_plane.connected_state',
 }
 
 # fmt: off
@@ -3888,7 +3889,7 @@
     'http.rbac.shadow_denied': {
         'tags': (
             ('stat_prefix',),
-            (),
+            ('shadow_rule_prefix',),
             (),
         ),
         'method': 'monotonic_count',

envoy/tests/common.py

@@ -381,6 +381,8 @@
     "http.local_rate_limit_ok.count",
 ]
 
+CONNECT_STATE_METRIC = ['control_plane.connected_state']
+
 RATE_LIMIT_STAT_PREFIX_TAG = 'stat_prefix:http_local_rate_limiter'
 
 FLAKY_METRICS = [

envoy/tests/fixtures/legacy/rbac_metric.txt

@@ -1,4 +1,8 @@
 http.foo_buz_112.rbac.allowed: 0
-http.foo_buz_112.rbac.denied: 0
-http.foo_buz_112.rbac.shadow_allowed: 0
-http.foo_buz_112.rbac.shadow_denied: 0
+http.foo_buz_112.rbac.denied: 1
+http.foo_buz_112.rbac.shadow_allowed: 2
+http.foo_buz_112.rbac.shadow_denied: 3
+http.foo_buz_112.rbac.shadow_rule_prefix.allowed: 4
+http.foo_buz_112.rbac.shadow_rule_prefix.denied: 5
+http.foo_buz_112.rbac.shadow_rule_prefix.shadow_allowed: 6
+http.foo_buz_112.rbac.shadow_rule_prefix.shadow_denied: 7

envoy/tests/fixtures/openmetrics/openmetrics.txt

@@ -1307,3 +1307,5 @@ envoy_tcp_on_demand_cluster_missing{} 0
 envoy_tcp_on_demand_cluster_success{} 0
 # TYPE envoy_tcp_on_demand_cluster_timeout counter
 envoy_tcp_on_demand_cluster_timeout{} 0
+# TYPE envoy_control_plane_connected_state gauge
+envoy_control_plane_connected_state{} 1

envoy/tests/legacy/test_integration.py

@@ -7,9 +7,10 @@
 from datadog_checks.dev.utils import get_metadata_metrics
 from datadog_checks.envoy.metrics import METRIC_PREFIX, METRICS
 
-from .common import ENVOY_VERSION, EXT_METRICS, INSTANCES
+from .common import ENVOY_VERSION, EXT_METRICS, INSTANCES, RBAC_METRICS
 
 CHECK_NAME = 'envoy'
+UNIQUE_METRICS = EXT_METRICS + RBAC_METRICS
 
 pytestmark = [pytest.mark.integration, pytest.mark.usefixtures('dd_environment')]
 
@@ -22,9 +23,9 @@ def test_success(aggregator, check, dd_run_check):
     metrics_collected = 0
     for metric in METRICS:
         collected_metrics = aggregator.metrics(METRIC_PREFIX + metric)
-        # The ext_auth metrics are excluded because the stats_prefix is not always present.
+        # The ext_auth and rbac metrics are excluded because the stats_prefix is not always present.
         # They're tested in a different test.
-        if collected_metrics and collected_metrics[0].name not in EXT_METRICS:
+        if collected_metrics and collected_metrics[0].name not in UNIQUE_METRICS:
             expected_tags = [t for t in METRICS[metric]['tags'] if t]
             for tag_set in expected_tags:
                 assert all(

envoy/tests/legacy/test_unit.py

@@ -253,21 +253,47 @@ def test_metadata_not_collected(datadog_agent, check):
     check.log.assert_not_called()
 
 
-def test_stats_prefix_ext_auth(aggregator, fixture_path, mock_http_response, check, dd_run_check):
+@pytest.mark.parametrize(
+    ('fixture_file', 'metrics', 'standard_tags', 'additional_tags'),
+    [
+        ('./legacy/stat_prefix', EXT_METRICS, ['cluster_name:foo', 'envoy_cluster:foo'], ['stat_prefix:bar']),
+        (
+            './legacy/rbac_metric.txt',
+            RBAC_METRICS,
+            ['stat_prefix:foo_buz_112'],
+            ['shadow_rule_prefix:shadow_rule_prefix'],
+        ),
+    ],
+    ids=[
+        "stats_prefix_ext_auth",
+        "rbac_prefix_shadow",
+    ],
+)
+def test_stats_prefix_ext_auth(
+    aggregator,
+    fixture_path,
+    mock_http_response,
+    check,
+    dd_run_check,
+    fixture_file,
+    metrics,
+    standard_tags,
+    additional_tags,
+):
     instance = INSTANCES['main']
-    tags = ['cluster_name:foo', 'envoy_cluster:foo']
-    tags_prefix = tags + ['stat_prefix:bar']
+    tags = standard_tags
+    tags_prefix = tags + additional_tags
     c = check(instance)
-    mock_http_response(file_path=fixture_path('./legacy/stat_prefix')).return_value
+    mock_http_response(file_path=fixture_path(fixture_file)).return_value
     dd_run_check(c)
 
     # To ensure that this change didn't break the old behavior, both the value and the tags are asserted.
     # The fixture is created with a specific value and the EXT_METRICS list is done in alphabetical order
     # allowing for value to also be asserted
-    for index, metric in enumerate(EXT_METRICS):
+    for index, metric in enumerate(metrics):
         aggregator.assert_metric(
             metric,
-            value=index + 5,
+            value=index + len(metrics),
             tags=tags_prefix,
         )
         aggregator.assert_metric(metric, value=index, tags=tags)
@@ -286,17 +312,3 @@ def test_local_rate_limit_metrics(aggregator, fixture_path, mock_http_response,
             aggregator.assert_metric_has_tag(metric, tag, count=1)
 
     aggregator.assert_metrics_using_metadata(get_metadata_metrics())
-
-
-def test_rbac_metrics(aggregator, fixture_path, mock_http_response, check, dd_run_check):
-    instance = INSTANCES['main']
-    c = check(instance)
-
-    mock_http_response(file_path=fixture_path('./legacy/rbac_metric.txt'))
-    dd_run_check(c)
-
-    for metric in RBAC_METRICS:
-        aggregator.assert_metric(metric)
-        aggregator.assert_metric_has_tag(metric, STAT_PREFIX_TAG[1], count=1)
-
-    aggregator.assert_metrics_using_metadata(get_metadata_metrics())

envoy/tests/test_unit.py

@@ -10,6 +10,7 @@
 from datadog_checks.envoy.metrics import PROMETHEUS_METRICS_MAP
 
 from .common import (
+    CONNECT_STATE_METRIC,
     DEFAULT_INSTANCE,
     LOCAL_RATE_LIMIT_METRICS,
     MOCKED_PROMETHEUS_METRICS,
@@ -47,6 +48,9 @@ def test_check(aggregator, dd_run_check, check, mock_http_response):
     for metric in MOCKED_PROMETHEUS_METRICS + LOCAL_RATE_LIMIT_METRICS:
         aggregator.assert_metric("envoy.{}".format(metric))
 
+    for metric in CONNECT_STATE_METRIC:
+        aggregator.assert_metric('envoy.{}'.format(metric))
+
     aggregator.assert_service_check(
         "envoy.openmetrics.health", status=AgentCheck.OK, tags=['endpoint:http://localhost:8001/stats/prometheus']
     )