write_prometheus plugin: Improve metric family name generation.

diff --git a/src/write_prometheus.c b/src/write_prometheus.c
index 5a77a3066fa6fc6fbbb26f50d70b2ddd7165bf24..3a17696c287f8788ae4bb6ddc46e88c707b0e789 100644 (file)
--- a/src/write_prometheus.c
+++ b/src/write_prometheus.c
@@ -107,14 +107,17 @@ static int format_label_set(strbuf_t *buf, label_set_t const *labels,
   return status;
 }
 
-static int format_metric(strbuf_t *buf, metric_t const *m) {
+static int format_metric(strbuf_t *buf, metric_t const *m,
+                         char const *metric_family_name) {
   if ((buf == NULL) || (m == NULL) || (m->family == 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, m->family->name, VALID_NAME_CHARS, '_');
+      strbuf_print_restricted(buf, metric_family_name, VALID_NAME_CHARS, '_');
   if (resource.num == 0 && m->label.num == 0) {
     return status;
   }
@@ -133,6 +136,46 @@ static int format_metric(strbuf_t *buf, metric_t const *m) {
   return status || strbuf_print(buf, "}");
 }
 
+/* format_metric_family_name creates a Prometheus compatible metric name by
+ * replacing all characters that are invalid in Prometheus with underscores,
+ * drop any leading and trailing underscores, and collapses a sequence of
+ * multiple underscores into one underscore.
+ *
+ * Visible for testing */
+void format_metric_family_name(strbuf_t *buf, metric_family_t const *fam) {
+  size_t name_len = strlen(fam->name);
+  char name[name_len + 1];
+  memset(name, 0, sizeof(name));
+
+  strbuf_t namebuf = STRBUF_CREATE_FIXED(name, sizeof(name));
+  strbuf_print_restricted(&namebuf, fam->name, VALID_NAME_CHARS, '_');
+  STRBUF_DESTROY(namebuf);
+
+  bool skip_underscore = true;
+  size_t out = 0;
+  for (size_t in = 0; in < name_len; in++) {
+    if (skip_underscore && name[in] == '_') {
+      continue;
+    }
+    skip_underscore = (name[in] == '_');
+    name[out] = name[in];
+    out++;
+  }
+  name_len = out;
+  name[name_len] = 0;
+
+  while (name_len > 0 && name[name_len - 1] == '_') {
+    name_len--;
+    name[name_len] = 0;
+  }
+
+  strbuf_print(buf, name);
+
+  if (fam->type == METRIC_TYPE_COUNTER) {
+    strbuf_print(buf, "_total");
+  }
+}
+
 /* visible for testing */
 void format_metric_family(strbuf_t *buf, metric_family_t const *prom_fam) {
   if (prom_fam->metric.num == 0)
@@ -155,7 +198,7 @@ void format_metric_family(strbuf_t *buf, metric_family_t const *prom_fam) {
   }
 
   strbuf_t family_name = STRBUF_CREATE;
-  strbuf_print_restricted(&family_name, prom_fam->name, VALID_NAME_CHARS, '_');
+  format_metric_family_name(&family_name, prom_fam);
 
   if (prom_fam->help == NULL)
     strbuf_printf(buf, "# HELP %s\n", family_name.ptr);
@@ -163,12 +206,10 @@ 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);
 
-  STRBUF_DESTROY(family_name);
-
   for (size_t i = 0; i < prom_fam->metric.num; i++) {
     metric_t *m = &prom_fam->metric.ptr[i];
 
-    format_metric(buf, m);
+    format_metric(buf, m, family_name.ptr);
 
     if (prom_fam->type == METRIC_TYPE_COUNTER)
       strbuf_printf(buf, " %" PRIu64, m->value.counter);
@@ -181,6 +222,8 @@ void format_metric_family(strbuf_t *buf, metric_family_t const *prom_fam) {
       strbuf_printf(buf, "\n");
     }
   }
+
+  STRBUF_DESTROY(family_name);
 }
 
 /* target_info prints a special "info" metric that contains all the "target

diff --git a/src/write_prometheus_test.c b/src/write_prometheus_test.c
index 83aab7ee8ddfae1a3b46efa5877ffcee2f42b531..722f526725643f11555c2c8e0dd1883edcfc9fee 100644 (file)
--- a/src/write_prometheus_test.c
+++ b/src/write_prometheus_test.c
@@ -30,6 +30,40 @@
 #include "testing.h"
 #include "utils/common/common.h"
 
+void format_metric_family_name(strbuf_t *buf, metric_family_t const *fam);
+
+DEF_TEST(format_metric_family_name) {
+  // Test cases are based on:
+  // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/translator/prometheus/README.md
+  struct {
+    char *name;
+    metric_type_t type;
+    char *want;
+  } cases[] = {
+      {"(lambda).function.executions(#)", METRIC_TYPE_UNTYPED,
+       "lambda_function_executions"},
+      {"system.processes.created", METRIC_TYPE_COUNTER,
+       "system_processes_created_total"},
+  };
+
+  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;
+
+    metric_family_t fam = {
+        .name = cases[i].name,
+        .type = cases[i].type,
+    };
+
+    format_metric_family_name(&got, &fam);
+    EXPECT_EQ_STR(cases[i].want, got.ptr);
+
+    STRBUF_DESTROY(got);
+  }
+
+  return 0;
+}
+
 void format_metric_family(strbuf_t *buf, metric_family_t const *prom_fam);
 
 DEF_TEST(format_metric_family) {
@@ -64,16 +98,16 @@ DEF_TEST(format_metric_family) {
                           .num = 1,
                       },
               },
-          .want = "# HELP unit_test\n"
-                  "# TYPE unit_test counter\n"
-                  "unit_test 42\n",
+          .want = "# HELP unit_test_total\n"
+                  "# TYPE unit_test_total counter\n"
+                  "unit_test_total 42\n",
       },
       {
           .name = "metric with one label",
           .fam =
               {
                   .name = "unittest",
-                  .type = METRIC_TYPE_COUNTER,
+                  .type = METRIC_TYPE_GAUGE,
                   .metric =
                       {
                           .ptr =
@@ -89,14 +123,14 @@ DEF_TEST(format_metric_family) {
                                       },
                                   .value =
                                       (value_t){
-                                          .counter = 42,
+                                          .gauge = 42,
                                       },
                               },
                           .num = 1,
                       },
               },
           .want = "# HELP unittest\n"
-                  "# TYPE unittest counter\n"
+                  "# TYPE unittest gauge\n"
                   "unittest{foo=\"bar\"} 42\n",
       },
       {
@@ -104,7 +138,7 @@ DEF_TEST(format_metric_family) {
           .fam =
               {
                   .name = "unit.test",
-                  .type = METRIC_TYPE_COUNTER,
+                  .type = METRIC_TYPE_UNTYPED,
                   .metric =
                       {
                           .ptr =
@@ -120,14 +154,14 @@ DEF_TEST(format_metric_family) {
                                       },
                                   .value =
                                       (value_t){
-                                          .counter = 42,
+                                          .gauge = 42,
                                       },
                               },
                           .num = 1,
                       },
               },
           .want = "# HELP unit_test\n"
-                  "# TYPE unit_test counter\n"
+                  "# TYPE unit_test untyped\n"
                   "unit_test{metric_name=\"unit.test\"} 42\n",
       },
   };
@@ -207,6 +241,7 @@ DEF_TEST(target_info) {
 }
 
 int main(void) {
+  RUN_TEST(format_metric_family_name);
   RUN_TEST(format_metric_family);
   RUN_TEST(target_info);