[Asterisk-code-review] res_prometheus: Do not generate broken metrics (asterisk[20])

[Holger Hans Peter Freyther]

Holger Hans Peter Freyther has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/20037 )


Change subject: res_prometheus: Do not generate broken metrics
......................................................................

res_prometheus: Do not generate broken metrics

In I58ef9f44036feded5966b5fc70ae754f8182883d invisible bridges were
skipped but that lead to producing metrics with no name and no help.

Keep track of the number of metrics configured and then only emit these.
Add a basic testcase that verifies that there is no '(NULL)' in the
output.

ASTERISK-30474

Change-Id: I06798d6085e3b36fdc63ab8554912f2b5bcc480b
---
M res/prometheus/bridges.c
M tests/test_res_prometheus.c
2 files changed, 73 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/37/20037/1

diff --git a/res/prometheus/bridges.c b/res/prometheus/bridges.c
index 505dab8..a0ca9f7 100644
--- a/res/prometheus/bridges.c
+++ b/res/prometheus/bridges.c
@@ -83,7 +83,7 @@
 	struct ast_bridge *bridge;
 	struct prometheus_metric *bridge_metrics;
 	char eid_str[32];
-	int i, j, num_bridges;
+	int i, j, num_bridges, num_outputs = 0;
 	struct prometheus_metric bridge_count = PROMETHEUS_METRIC_STATIC_INITIALIZATION(
 		PROMETHEUS_METRIC_GAUGE,
 		"asterisk_bridges_count",
@@ -138,7 +138,7 @@
 		}
 
 		for (j = 0; j < ARRAY_LEN(bridge_metric_defs); j++) {
-			int index = i * ARRAY_LEN(bridge_metric_defs) + j;
+			int index = num_outputs++;
 
 			bridge_metrics[index].type = PROMETHEUS_METRIC_GAUGE;
 			ast_copy_string(bridge_metrics[index].name, bridge_metric_defs[j].name, sizeof(bridge_metrics[index].name));
@@ -159,7 +159,7 @@
 	}
 	ao2_iterator_destroy(&it_bridges);
 
-	for (j = 0; j < ARRAY_LEN(bridge_metric_defs); j++) {
+	for (j = 0; j < num_outputs; j++) {
 		prometheus_metric_to_string(&bridge_metrics[j], response);
 	}
 
diff --git a/tests/test_res_prometheus.c b/tests/test_res_prometheus.c
index 4df4cb9..2e0f501 100644
--- a/tests/test_res_prometheus.c
+++ b/tests/test_res_prometheus.c
@@ -29,8 +29,11 @@
 
 #include "asterisk/test.h"
 #include "asterisk/module.h"
+#include "asterisk/bridge.h"
+#include "asterisk/bridge_basic.h"
 #include "asterisk/config.h"
 #include "asterisk/res_prometheus.h"
+#include "../res/prometheus/prometheus_internal.h"
 
 #define CATEGORY "/res/prometheus/"
 
@@ -699,6 +702,51 @@
 	return AST_TEST_PASS;
 }
 
+static void safe_bridge_destroy(struct ast_bridge *bridge)
+{
+	if (!bridge) {
+		return;
+	}
+	ast_bridge_destroy(bridge, 0);
+}
+
+AST_TEST_DEFINE(bridge_to_string)
+{
+	RAII_VAR(struct ast_bridge *, bridge1, NULL, safe_bridge_destroy);
+	RAII_VAR(struct ast_bridge *, bridge2, NULL, safe_bridge_destroy);
+	struct ast_str *response;
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = __func__;
+		info->category = CATEGORY;
+		info->summary = "Test producing bridge metrics";
+		info->description =
+			"This test covers checking the metrics produced by the\n"
+			"bridge support of the basic Promtheus module.";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	bridge1 = ast_bridge_basic_new();
+	ast_test_validate(test, bridge1 != NULL);
+
+	bridge2 = ast_bridge_base_new(AST_BRIDGE_CAPABILITY_HOLDING,
+		AST_BRIDGE_FLAG_INVISIBLE,
+		"test_res_prometheus", "test_bridge_invisible", NULL);
+
+	response = prometheus_scrape_to_string();
+	if (!response) {
+		return AST_TEST_FAIL;
+	}
+
+	ast_test_status_update(test, " -> Retrieved: %s\n", ast_str_buffer(response));
+	ast_test_validate(test, strstr(ast_str_buffer(response), "(null)") == NULL);
+	ast_free(response);
+	return AST_TEST_PASS;
+}
+
 static int process_config(int reload)
 {
 	struct ast_config *config;
@@ -791,6 +839,8 @@
 	AST_TEST_UNREGISTER(config_general_basic_auth);
 	AST_TEST_UNREGISTER(config_general_core_metrics);
 
+	AST_TEST_UNREGISTER(bridge_to_string);
+
 	return 0;
 }
 
@@ -813,6 +863,8 @@
 	AST_TEST_REGISTER(config_general_basic_auth);
 	AST_TEST_REGISTER(config_general_core_metrics);
 
+	AST_TEST_REGISTER(bridge_to_string);
+
 	ast_test_register_init(CATEGORY, &test_init_cb);
 	ast_test_register_cleanup(CATEGORY, &test_cleanup_cb);
 

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/20037
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 20
Gerrit-Change-Id: I06798d6085e3b36fdc63ab8554912f2b5bcc480b
Gerrit-Change-Number: 20037
Gerrit-PatchSet: 1
Gerrit-Owner: Holger Hans Peter Freyther <automatic at freyther.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20230407/90f621b5/attachment.html>