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 3a17696c287f8788ae4bb6ddc46e88c707b0e789..b6538b6c996f0947238020c4ab22a377c33f1cb7 100644 (file)
--- 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 722f526725643f11555c2c8e0dd1883edcfc9fee..3ea85b415e6ace549635719ae6832d101e013c16 100644 (file)
--- 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);