From ac653d7bcfe7066c1c433c2bc29d8ef54b088b44 Mon Sep 17 00:00:00 2001
From: Lois Soto Lopez <lois@84codes.com>
Date: Fri, 12 Jul 2024 11:20:19 +0200
Subject: [PATCH] New metrics require a new macro on collector

Previous commits include new metrics on the `CLUTER_METRICS` group,
since those ones do not get aggregated and are only shown on under
the `/metrics/detailed` endpoint. However that group of metrics
correspond to cluster-wide metrics (that's why they don't need to
get aggregated) and therefore the new metrics we are providing do
not perfectly fit in that group. A new macro/group is provided by
this commit.
---
 ...etheus_rabbitmq_core_metrics_collector.erl | 30 +++++++++++--------
 .../test/rabbit_prometheus_http_SUITE.erl     | 12 +++++++-
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/deps/rabbitmq_prometheus/src/collectors/prometheus_rabbitmq_core_metrics_collector.erl b/deps/rabbitmq_prometheus/src/collectors/prometheus_rabbitmq_core_metrics_collector.erl
index c5836ef122c2..8015dc3fa78b 100644
--- a/deps/rabbitmq_prometheus/src/collectors/prometheus_rabbitmq_core_metrics_collector.erl
+++ b/deps/rabbitmq_prometheus/src/collectors/prometheus_rabbitmq_core_metrics_collector.erl
@@ -214,17 +214,8 @@
     ]}
 ]).
 
-%% Metrics that can be only requested through `/metrics/detailed`
--define(METRICS_CLUSTER,[
-    {vhost_status, [
-        {2, undefined, vhost_status, gauge, "Whether a given vhost is running"}
-    ]},
-    {exchange_bindings, [
-        {2, undefined, exchange_bindings, gauge, "Number of bindings for an exchange. This value is cluster-wide."}
-    ]},
-    {exchange_names, [
-        {2, undefined, exchange_name, gauge, "Enumerates exchanges without any additional info. This value is cluster-wide. A cheaper alternative to `exchange_bindings`"}
-    ]},
+%% Non aggregable detailed metrics
+-define(METRICS_RAW_NON_AGGREGABLE, [
     {queue_exchange_metrics, [
         {2, undefined, queue_exchange_messages_published_total, counter, "Total number of messages published into a queue through an exchange"}
     ]},
@@ -242,7 +233,21 @@
         {6, undefined, queue_messages_redelivered_total, counter, "Total number of messages redelivered from a queue to consumers"},
         {7, undefined, queue_messages_acked_total, counter, "Total number of messages acknowledged by consumers on a queue"},
         {8, undefined, queue_get_empty_total, counter, "Total number of times basic.get operations fetched no message on a queue"}
-    ]}]).
+    ]}
+]).
+
+%% Metrics that can be only requested through `/metrics/detailed`
+-define(METRICS_CLUSTER,[
+    {vhost_status, [
+        {2, undefined, vhost_status, gauge, "Whether a given vhost is running"}
+    ]},
+    {exchange_bindings, [
+        {2, undefined, exchange_bindings, gauge, "Number of bindings for an exchange. This value is cluster-wide."}
+    ]},
+    {exchange_names, [
+        {2, undefined, exchange_name, gauge, "Enumerates exchanges without any additional info. This value is cluster-wide. A cheaper alternative to `exchange_bindings`"}
+    ]}
+]).
 
 -define(TOTALS, [
     %% ordering differs from metrics above, refer to list comprehension
@@ -263,6 +268,7 @@ deregister_cleanup(_) -> ok.
 
 collect_mf('detailed', Callback) ->
     collect(true, ?DETAILED_METRIC_NAME_PREFIX, vhosts_filter_from_pdict(), enabled_mfs_from_pdict(?METRICS_RAW), Callback),
+    collect(true, ?DETAILED_METRIC_NAME_PREFIX, vhosts_filter_from_pdict(), enabled_mfs_from_pdict(?METRICS_RAW_NON_AGGREGABLE), Callback),
     collect(true, ?CLUSTER_METRIC_NAME_PREFIX, vhosts_filter_from_pdict(), enabled_mfs_from_pdict(?METRICS_CLUSTER), Callback),
     %% identity is here to enable filtering on a cluster name (as already happens in existing dashboards)
     emit_identity_info(Callback),
diff --git a/deps/rabbitmq_prometheus/test/rabbit_prometheus_http_SUITE.erl b/deps/rabbitmq_prometheus/test/rabbit_prometheus_http_SUITE.erl
index 50bf0b1ad62a..6ef4b8b542bf 100644
--- a/deps/rabbitmq_prometheus/test/rabbit_prometheus_http_SUITE.erl
+++ b/deps/rabbitmq_prometheus/test/rabbit_prometheus_http_SUITE.erl
@@ -61,6 +61,7 @@ groups() ->
                                      queue_exchange_metrics_per_object_test,
                                      queue_metrics_per_object_test,
                                      queue_consumer_count_and_queue_metrics_mutually_exclusive_test,
+                                     exchange_metrics_per_object_test,
                                      vhost_status_metric,
                                      exchange_bindings_metric,
                                      exchange_names_metric
@@ -304,7 +305,16 @@ end_per_group_(Config) ->
     inets:stop(),
     rabbit_ct_helpers:run_teardown_steps(Config, rabbit_ct_client_helpers:teardown_steps()
                                          ++ rabbit_ct_broker_helpers:teardown_steps()).
-
+init_per_testcase(Testcase, Config)
+    when Testcase =:= queue_counter_metrics_per_object_test;
+         Testcase =:= queue_exchange_metrics_per_object_test ->
+    case rabbit_ct_helpers:is_mixed_versions() of
+        false ->
+            rabbit_ct_helpers:testcase_started(Config, Testcase);
+        true ->
+            %% skip the test in mixed version mode
+            {skip, "Should not run in mixed version environments"}
+    end;
 init_per_testcase(Testcase, Config) ->
     rabbit_ct_helpers:testcase_started(Config, Testcase).