From: Florian Forster Date: Sat, 23 Dec 2023 19:27:44 +0000 (+0100) Subject: write_prometheus plugin: Improve label sanitation. X-Git-Tag: 6.0.0-rc0~23^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ab8639d3f31dab81de5da085e3b77fff27cea43f;p=thirdparty%2Fcollectd.git write_prometheus plugin: Improve label sanitation. This copies the behavior of the OpenTelemetry Prometheus translator: 1. Replace all invalid characters with underscores. 2. If the name starts with a single underscore, add the prefix `key`. 3. If the name stars with a digit, add the prefix `key_`. --- diff --git a/src/write_prometheus.c b/src/write_prometheus.c index 3a17696c2..b6538b6c9 100644 --- a/src/write_prometheus.c +++ b/src/write_prometheus.c @@ -76,64 +76,86 @@ static struct MHD_Daemon *httpd; static cdtime_t staleness_delta = PROMETHEUS_DEFAULT_STALENESS_DELTA; -static int format_label_set(strbuf_t *buf, label_set_t const *labels, - char const *prefix, bool translate, - bool first_label) { +/* Visible for testing */ +int format_label_name(strbuf_t *buf, char const *name) { int status = 0; - for (size_t i = 0; i < labels->num; i++) { - if (!first_label) { - status = status || strbuf_print(buf, ","); - } - char const *name = labels->ptr[i].name; - if (translate) { - if (strcmp("service.name", name) == 0) { - name = "job"; - } else if (strcmp("service.instance.id", name) == 0) { - name = "instance"; - } - } + strbuf_t namebuf = STRBUF_CREATE; + status = + status || strbuf_print_restricted(&namebuf, name, VALID_LABEL_CHARS, '_'); + if (strncmp("__", namebuf.ptr, 2) == 0) { + /* no prefix */ + } else if (namebuf.ptr[0] == '_') { + status = status || strbuf_print(buf, "key"); + } else if (isdigit(namebuf.ptr[0])) { + status = status || strbuf_print(buf, "key_"); + } + + status = status || strbuf_print(buf, namebuf.ptr); + + STRBUF_DESTROY(namebuf); + return status; +} + +static int format_label_pair(strbuf_t *buf, label_pair_t l, bool *first_label) { + int status = 0; + + if (!*first_label) { + status = status || strbuf_print(buf, ","); + } + status = status || format_label_name(buf, l.name); + status = status || strbuf_print(buf, "=\""); + status = status || strbuf_print_escaped(buf, l.value, "\\\"\n\r\t", '\\'); + status = status || strbuf_print(buf, "\""); + *first_label = false; + + return status; +} + +static int format_label_set(strbuf_t *buf, label_set_t labels, char const *job, + char const *instance) { + bool first_label = true; + int status = 0; + + if (job != NULL) { status = - status || strbuf_print_restricted(buf, prefix, VALID_LABEL_CHARS, '_'); - status = - status || strbuf_print_restricted(buf, name, VALID_LABEL_CHARS, '_'); - status = status || strbuf_print(buf, "=\""); - status = status || strbuf_print_escaped(buf, labels->ptr[i].value, - "\\\"\n\r\t", '\\'); - status = status || strbuf_print(buf, "\""); - first_label = false; + status || format_label_pair(buf, (label_pair_t){"job", (char *)job}, + &first_label); } + if (instance != NULL) { + status = status || format_label_pair( + buf, (label_pair_t){"instance", (char *)instance}, + &first_label); + } + + for (size_t i = 0; i < labels.num; i++) { + status = status || format_label_pair(buf, labels.ptr[i], &first_label); + } + return status; } static int format_metric(strbuf_t *buf, metric_t const *m, - char const *metric_family_name) { - if ((buf == NULL) || (m == NULL) || (m->family == NULL)) { + char const *metric_family_name, char const *job, + char const *instance) { + if ((buf == NULL) || (m == NULL)) { return EINVAL; } - label_set_t resource = m->family->resource; /* metric_family_name is already escaped, so strbuf_print_restricted should * not replace any characters. */ int status = strbuf_print_restricted(buf, metric_family_name, VALID_NAME_CHARS, '_'); - if (resource.num == 0 && m->label.num == 0) { + if (m->label.num == 0) { return status; } status = status || strbuf_print(buf, "{"); + status = status || format_label_set(buf, m->label, job, instance); + status = status || strbuf_print(buf, "}"); - bool first_label = true; - if (resource.num != 0 && - label_set_compare(resource, default_resource_attributes()) != 0) { - status = status || format_label_set(buf, &resource, RESOURCE_LABEL_PREFIX, - true, first_label); - first_label = false; - } - status = status || format_label_set(buf, &m->label, "", false, first_label); - - return status || strbuf_print(buf, "}"); + return status; } /* format_metric_family_name creates a Prometheus compatible metric name by @@ -206,10 +228,14 @@ void format_metric_family(strbuf_t *buf, metric_family_t const *prom_fam) { strbuf_printf(buf, "# HELP %s %s\n", family_name.ptr, prom_fam->help); strbuf_printf(buf, "# TYPE %s %s\n", family_name.ptr, type); + char const *job = label_set_get(prom_fam->resource, "service.name"); + char const *instance = + label_set_get(prom_fam->resource, "service.instance.id"); + for (size_t i = 0; i < prom_fam->metric.num; i++) { metric_t *m = &prom_fam->metric.ptr[i]; - format_metric(buf, m, family_name.ptr); + format_metric(buf, m, family_name.ptr, job, instance); if (prom_fam->type == METRIC_TYPE_COUNTER) strbuf_printf(buf, " %" PRIu64, m->value.counter); @@ -237,12 +263,22 @@ void target_info(strbuf_t *buf, label_set_t resource) { return; } + char const *job = label_set_get(resource, "service.name"); + char const *instance = label_set_get(resource, "service.instance.id"); + + label_set_t rattr = {0}; + label_set_clone(&rattr, resource); + label_set_update(&rattr, "service.name", NULL); + label_set_update(&rattr, "service.instance.id", NULL); + strbuf_print(buf, "# TYPE target info\n"); strbuf_print(buf, "# HELP target Target metadata\n"); strbuf_print(buf, "target_info{"); - format_label_set(buf, &resource, "", true, true); + format_label_set(buf, rattr, job, instance); strbuf_print(buf, "} 1\n"); + + label_set_reset(&rattr); } static void format_text(strbuf_t *buf) { diff --git a/src/write_prometheus_test.c b/src/write_prometheus_test.c index 722f52672..3ea85b415 100644 --- a/src/write_prometheus_test.c +++ b/src/write_prometheus_test.c @@ -30,6 +30,33 @@ #include "testing.h" #include "utils/common/common.h" +int format_label_name(strbuf_t *buf, char const *name); + +DEF_TEST(format_label_name) { + // Test cases are based on: + // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/translator/prometheus/README.md + struct { + char *name; + char *want; + } cases[] = { + {"name", "name"}, {"host.name", "host_name"}, + {"host_name", "host_name"}, {"name (of the host)", "name__of_the_host_"}, + {"2 cents", "key_2_cents"}, {"__name", "__name"}, + {"_name", "key_name"}, {"(name)", "key_name_"}, + }; + + for (size_t i = 0; i < STATIC_ARRAY_SIZE(cases); i++) { + printf("# Case %zu: %s\n", i, cases[i].name); + strbuf_t got = STRBUF_CREATE; + + EXPECT_EQ_INT(0, format_label_name(&got, cases[i].name)); + EXPECT_EQ_STR(cases[i].want, got.ptr); + + STRBUF_DESTROY(got); + } + + return 0; +} void format_metric_family_name(strbuf_t *buf, metric_family_t const *fam); DEF_TEST(format_metric_family_name) { @@ -164,6 +191,48 @@ DEF_TEST(format_metric_family) { "# TYPE unit_test untyped\n" "unit_test{metric_name=\"unit.test\"} 42\n", }, + { + .name = "most resource attributes are ignored", + .fam = + { + .name = "unit.test", + .type = METRIC_TYPE_UNTYPED, + .resource = + { + .ptr = + (label_pair_t[]){ + {"service.instance.id", + "service instance id"}, + {"service.name", "service name"}, + {"zzz.all.other.attributes", "are ignored"}, + }, + .num = 3, + }, + .metric = + { + .ptr = + &(metric_t){ + .label = + { + .ptr = + (label_pair_t[]){ + {"metric.name", "unit.test"}, + }, + .num = 1, + }, + .value = + (value_t){ + .gauge = 42, + }, + }, + .num = 1, + }, + }, + .want = "# HELP unit_test\n" + "# TYPE unit_test untyped\n" + "unit_test{job=\"service name\",instance=\"service instance " + "id\",metric_name=\"unit.test\"} 42\n", + }, }; for (size_t i = 0; i < STATIC_ARRAY_SIZE(cases); i++) { @@ -241,6 +310,7 @@ DEF_TEST(target_info) { } int main(void) { + RUN_TEST(format_label_name); RUN_TEST(format_metric_family_name); RUN_TEST(format_metric_family); RUN_TEST(target_info);