From f335da6b74aca89a3e4b44a59ba9827b9b0b5b3c Mon Sep 17 00:00:00 2001 From: Holger Hans Peter Freyther Date: Fri, 7 Apr 2023 17:44:44 +0800 Subject: [PATCH] res_prometheus: Do not generate broken metrics In 8d6fdf9c3adede201f0ef026dab201b3a37b26b6 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 --- res/prometheus/bridges.c | 6 ++--- tests/test_res_prometheus.c | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/res/prometheus/bridges.c b/res/prometheus/bridges.c index 505dab8397..a0ca9f7ee6 100644 --- a/res/prometheus/bridges.c +++ b/res/prometheus/bridges.c @@ -83,7 +83,7 @@ static void bridges_scrape_cb(struct ast_str **response) 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 @@ static void bridges_scrape_cb(struct ast_str **response) } 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 @@ static void bridges_scrape_cb(struct ast_str **response) } 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 4df4cb9561..b7ff6d8417 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,52 @@ AST_TEST_DEFINE(config_general_core_metrics) 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_test_validate(test, strstr(ast_str_buffer(response), "asterisk_bridges_channels_count{") != NULL); + ast_free(response); + return AST_TEST_PASS; +} + static int process_config(int reload) { struct ast_config *config; @@ -791,6 +840,8 @@ static int unload_module(void) AST_TEST_UNREGISTER(config_general_basic_auth); AST_TEST_UNREGISTER(config_general_core_metrics); + AST_TEST_UNREGISTER(bridge_to_string); + return 0; } @@ -813,6 +864,8 @@ static int load_module(void) 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);