res_prometheus: Fix duplicate output of metric and help text
The prometheus exposition format requires each line to be unique[1]. This is handled by struct prometheus_metric having a list of children that is managed when registering a metric. In case the scrape callback is used, it is the responsibility of the implementation to handle this correctly. Originally the bridge callback didn't handle NULL snapshots, the crash fix lead to NULL metrics, and fixing that lead to duplicates. The original code assumed that snapshots are not NULL and then relied on "if (i > 0)" to establish the parent/children relationship between metrics of the same class. This is not workerable as the first bridge might be invisible/lacks a snapshot. Fix this by keeping a separate array of the first metric by class. Instead of relying on the index of the bridge, check whether the array has an entry. Use that array for the output. Add a test case that verifies that the help text is not duplicated. Resolves: #642 [1] https://prometheus.io/docs/instrumenting/exposition_formats/#grouping-and-sorting
This commit is contained in:
parent
34196f8796
commit
765a91a4ce
|
@ -81,7 +81,8 @@ static void bridges_scrape_cb(struct ast_str **response)
|
|||
struct ao2_container *bridges;
|
||||
struct ao2_iterator it_bridges;
|
||||
struct ast_bridge *bridge;
|
||||
struct prometheus_metric *bridge_metrics;
|
||||
struct prometheus_metric *metrics;
|
||||
struct prometheus_metric **bridge_metrics;
|
||||
char eid_str[32];
|
||||
int i, j, num_bridges, num_outputs = 0;
|
||||
struct prometheus_metric bridge_count = PROMETHEUS_METRIC_STATIC_INITIALIZATION(
|
||||
|
@ -116,8 +117,15 @@ static void bridges_scrape_cb(struct ast_str **response)
|
|||
return;
|
||||
}
|
||||
|
||||
bridge_metrics = ast_calloc(ARRAY_LEN(bridge_metric_defs) * num_bridges, sizeof(*bridge_metrics));
|
||||
metrics = ast_calloc(ARRAY_LEN(bridge_metric_defs) * num_bridges, sizeof(*metrics));
|
||||
if (!metrics) {
|
||||
ao2_ref(bridges, -1);
|
||||
return;
|
||||
}
|
||||
|
||||
bridge_metrics = ast_calloc(ARRAY_LEN(bridge_metric_defs), sizeof(bridge_metrics));
|
||||
if (!bridge_metrics) {
|
||||
ast_free(metrics);
|
||||
ao2_ref(bridges, -1);
|
||||
return;
|
||||
}
|
||||
|
@ -140,30 +148,35 @@ static void bridges_scrape_cb(struct ast_str **response)
|
|||
for (j = 0; j < 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));
|
||||
bridge_metrics[index].help = bridge_metric_defs[j].help;
|
||||
PROMETHEUS_METRIC_SET_LABEL(&bridge_metrics[index], 0, "eid", eid_str);
|
||||
PROMETHEUS_METRIC_SET_LABEL(&bridge_metrics[index], 1, "id", (snapshot->uniqueid));
|
||||
PROMETHEUS_METRIC_SET_LABEL(&bridge_metrics[index], 2, "tech", (snapshot->technology));
|
||||
PROMETHEUS_METRIC_SET_LABEL(&bridge_metrics[index], 3, "subclass", (snapshot->subclass));
|
||||
PROMETHEUS_METRIC_SET_LABEL(&bridge_metrics[index], 4, "creator", (snapshot->creator));
|
||||
PROMETHEUS_METRIC_SET_LABEL(&bridge_metrics[index], 5, "name", (snapshot->name));
|
||||
bridge_metric_defs[j].get_value(&bridge_metrics[index], snapshot);
|
||||
metrics[index].type = PROMETHEUS_METRIC_GAUGE;
|
||||
ast_copy_string(metrics[index].name, bridge_metric_defs[j].name, sizeof(metrics[index].name));
|
||||
metrics[index].help = bridge_metric_defs[j].help;
|
||||
PROMETHEUS_METRIC_SET_LABEL(&metrics[index], 0, "eid", eid_str);
|
||||
PROMETHEUS_METRIC_SET_LABEL(&metrics[index], 1, "id", (snapshot->uniqueid));
|
||||
PROMETHEUS_METRIC_SET_LABEL(&metrics[index], 2, "tech", (snapshot->technology));
|
||||
PROMETHEUS_METRIC_SET_LABEL(&metrics[index], 3, "subclass", (snapshot->subclass));
|
||||
PROMETHEUS_METRIC_SET_LABEL(&metrics[index], 4, "creator", (snapshot->creator));
|
||||
PROMETHEUS_METRIC_SET_LABEL(&metrics[index], 5, "name", (snapshot->name));
|
||||
bridge_metric_defs[j].get_value(&metrics[index], snapshot);
|
||||
|
||||
if (i > 0) {
|
||||
AST_LIST_INSERT_TAIL(&bridge_metrics[j].children, &bridge_metrics[index], entry);
|
||||
if (bridge_metrics[j] == NULL) {
|
||||
bridge_metrics[j] = &metrics[index];
|
||||
} else {
|
||||
AST_LIST_INSERT_TAIL(&bridge_metrics[j]->children, &metrics[index], entry);
|
||||
}
|
||||
}
|
||||
ao2_ref(snapshot, -1);
|
||||
}
|
||||
ao2_iterator_destroy(&it_bridges);
|
||||
|
||||
for (j = 0; j < num_outputs; j++) {
|
||||
prometheus_metric_to_string(&bridge_metrics[j], response);
|
||||
for (j = 0; j < ARRAY_LEN(bridge_metric_defs); j++) {
|
||||
if (bridge_metrics[j]) {
|
||||
prometheus_metric_to_string(bridge_metrics[j], response);
|
||||
}
|
||||
}
|
||||
|
||||
ast_free(bridge_metrics);
|
||||
ast_free(metrics);
|
||||
ao2_ref(bridges, -1);
|
||||
}
|
||||
|
||||
|
|
|
@ -710,10 +710,23 @@ static void safe_bridge_destroy(struct ast_bridge *bridge)
|
|||
ast_bridge_destroy(bridge, 0);
|
||||
}
|
||||
|
||||
static int match_count(const char *str, const char *needle)
|
||||
{
|
||||
int count = 0;
|
||||
|
||||
while ((str = strstr(str, needle))) {
|
||||
str += strlen(needle);
|
||||
count++;
|
||||
}
|
||||
|
||||
return count;
|
||||
}
|
||||
|
||||
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);
|
||||
RAII_VAR(struct ast_bridge *, bridge3, NULL, safe_bridge_destroy);
|
||||
struct ast_str *response;
|
||||
|
||||
switch (cmd) {
|
||||
|
@ -736,6 +749,9 @@ AST_TEST_DEFINE(bridge_to_string)
|
|||
AST_BRIDGE_FLAG_INVISIBLE,
|
||||
"test_res_prometheus", "test_bridge_invisible", NULL);
|
||||
|
||||
bridge3 = ast_bridge_basic_new();
|
||||
ast_test_validate(test, bridge3 != NULL);
|
||||
|
||||
response = prometheus_scrape_to_string();
|
||||
if (!response) {
|
||||
return AST_TEST_FAIL;
|
||||
|
@ -744,6 +760,7 @@ AST_TEST_DEFINE(bridge_to_string)
|
|||
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_test_validate(test, match_count(ast_str_buffer(response), "# HELP asterisk_bridges_channels_count Number of channels in the bridge.") == 1);
|
||||
ast_free(response);
|
||||
return AST_TEST_PASS;
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue