From: Florian Forster <octo@collectd.org>
Date: Wed, 13 Dec 2023 20:00:14 +0000 (+0100)
Subject: Move resource attributes into `metric_family_t`.
X-Git-Tag: 6.0.0-rc0~37^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=47e1528fb28d2259870687e80d21c6797f645705;p=thirdparty%2Fcollectd.git

Move resource attributes into `metric_family_t`.
---

diff --git a/src/daemon/metric.c b/src/daemon/metric.c
index d273863d7..9bc0c6745 100644
--- a/src/daemon/metric.c
+++ b/src/daemon/metric.c
@@ -249,7 +249,6 @@ int metric_reset(metric_t *m) {
   }
 
   label_set_reset(&m->label);
-  label_set_reset(&m->resource);
   meta_data_destroy(m->meta);
 
   memset(m, 0, sizeof(*m));
@@ -280,18 +279,19 @@ int metric_identity(strbuf_t *buf, metric_t const *m) {
   if ((buf == NULL) || (m == NULL) || (m->family == NULL)) {
     return EINVAL;
   }
+  label_set_t const *resource = &m->family->resource;
 
   int status = strbuf_print(buf, m->family->name);
-  if (m->resource.num == 0 && m->label.num == 0) {
+  if (resource->num == 0 && m->label.num == 0) {
     return status;
   }
 
   status = status || strbuf_print(buf, "{");
 
   bool first_label = true;
-  if (m->resource.num == 0) {
-    status = status || format_label_set(buf, &m->resource,
-                                        RESOURCE_LABEL_PREFIX, first_label);
+  if (resource->num != 0) {
+    status = status || format_label_set(buf, resource, RESOURCE_LABEL_PREFIX,
+                                        first_label);
     first_label = false;
   }
   status = status || format_label_set(buf, &m->label, "", first_label);
@@ -307,15 +307,6 @@ int metric_label_set(metric_t *m, char const *name, char const *value) {
   return label_set_update(&m->label, name, value);
 }
 
-int metric_resource_attribute_update(metric_t *m, char const *name,
-                                     char const *value) {
-  if (m == NULL) {
-    return EINVAL;
-  }
-
-  return label_set_update(&m->resource, name, value);
-}
-
 char const *metric_label_get(metric_t const *m, char const *name) {
   if ((m == NULL) || (name == NULL)) {
     errno = EINVAL;
@@ -398,13 +389,7 @@ static int metric_list_clone(metric_list_t *dest, metric_list_t src,
         .interval = src.ptr[i].interval,
     };
 
-    int status = label_set_clone(&ret.ptr[i].resource, src.ptr[i].resource);
-    if (status != 0) {
-      metric_list_reset(&ret);
-      return status;
-    }
-
-    status = label_set_clone(&ret.ptr[i].label, src.ptr[i].label);
+    int status = label_set_clone(&ret.ptr[i].label, src.ptr[i].label);
     if (status != 0) {
       metric_list_reset(&ret);
       return status;
@@ -473,6 +458,7 @@ void metric_family_free(metric_family_t *fam) {
 
   free(fam->name);
   free(fam->help);
+  label_set_reset(&fam->resource);
   metric_list_reset(&fam->metric);
   free(fam);
 }
@@ -494,7 +480,14 @@ metric_family_t *metric_family_clone(metric_family_t const *fam) {
   }
   ret->type = fam->type;
 
-  int status = metric_list_clone(&ret->metric, fam->metric, ret);
+  int status = label_set_clone(&ret->resource, fam->resource);
+  if (status != 0) {
+    metric_family_free(ret);
+    errno = status;
+    return NULL;
+  }
+
+  status = metric_list_clone(&ret->metric, fam->metric, ret);
   if (status != 0) {
     metric_family_free(ret);
     errno = status;
@@ -553,6 +546,16 @@ static int parse_label_value(strbuf_t *buf, char const **inout) {
   return 0;
 }
 
+int metric_family_resource_attribute_update(metric_family_t *fam,
+                                            char const *name,
+                                            char const *value) {
+  if (fam == NULL) {
+    return EINVAL;
+  }
+
+  return label_set_update(&fam->resource, name, value);
+}
+
 /* metric_family_unmarshal_identity parses the metric identity and updates
  * "inout" to point to the first character following the identity. With valid
  * input, this means that "inout" will then point either to a '\0' (null byte)
@@ -628,7 +631,7 @@ static int metric_family_unmarshal_identity(metric_family_t *fam,
     assert(fam->metric.num >= 1);
 
     if (is_resource_label) {
-      status = label_set_add(&m->resource, key, value.ptr);
+      status = metric_family_resource_attribute_update(fam, key, value.ptr);
     } else {
       status = metric_label_set(m, key, value.ptr);
     }
diff --git a/src/daemon/metric.h b/src/daemon/metric.h
index d6e1e4546..8ea3b09cd 100644
--- a/src/daemon/metric.h
+++ b/src/daemon/metric.h
@@ -96,7 +96,6 @@ typedef struct {
   metric_family_t *family; /* backreference for family->name and family->type */
 
   label_set_t label;
-  label_set_t resource;
 
   value_t value;
   cdtime_t time; /* TODO(octo): use ms or µs instead? */
@@ -122,12 +121,6 @@ metric_t *metric_parse_identity(char const *s);
  * label that does not exist is *not* an error. */
 int metric_label_set(metric_t *m, char const *name, char const *value);
 
-/* metric_resource_attribute_update adds, updates, or deletes a resource
- * attribute. If "value" is NULL or an empty string, the attribute is removed.
- * Removing an attribute that does not exist is *not* an error. */
-int metric_resource_attribute_update(metric_t *m, char const *name,
-                                     char const *value);
-
 /* metric_label_get efficiently looks up and returns the value of the "name"
  * label. If a label does not exist, NULL is returned and errno is set to
  * ENOENT. The returned pointer may not be valid after a subsequent call to
@@ -153,6 +146,7 @@ struct metric_family_s {
   char *help;
   metric_type_t type;
 
+  label_set_t resource;
   metric_list_t metric;
 };
 
@@ -160,6 +154,14 @@ struct metric_family_s {
  * allocates memory which must be freed using metric_family_metric_reset. */
 int metric_family_metric_append(metric_family_t *fam, metric_t m);
 
+/* metric_family_resource_attribute_update adds, updates, or deletes a
+ * resource attribute. If "value" is NULL or an empty string, the attribute
+ * is removed.
+ * Removing an attribute that does not exist is *not* an error. */
+int metric_family_resource_attribute_update(metric_family_t *fam,
+                                            char const *name,
+                                            char const *value);
+
 /* metric_family_append constructs a new metric_t and appends it to fam. It is
  * a convenience function that is funcitonally approximately equivalent to the
  * following code, but without modifying templ:
diff --git a/src/daemon/metric_test.c b/src/daemon/metric_test.c
index e1362b859..fbf692cb5 100644
--- a/src/daemon/metric_test.c
+++ b/src/daemon/metric_test.c
@@ -175,8 +175,8 @@ DEF_TEST(metric_identity) {
                                   cases[i].labels[j].value));
     }
     for (size_t j = 0; j < cases[i].rattr_num; j++) {
-      CHECK_ZERO(metric_resource_attribute_update(&m, cases[i].rattr[j].name,
-                                                  cases[i].rattr[j].value));
+      CHECK_ZERO(metric_family_resource_attribute_update(
+          &fam, cases[i].rattr[j].name, cases[i].rattr[j].value));
     }
 
     strbuf_t buf = STRBUF_CREATE;
@@ -186,6 +186,7 @@ DEF_TEST(metric_identity) {
 
     STRBUF_DESTROY(buf);
     metric_family_metric_reset(&fam);
+    label_set_reset(&fam.resource);
     metric_reset(&m);
   }
 
diff --git a/src/daemon/plugin.c b/src/daemon/plugin.c
index 436a8aa3c..90905b950 100644
--- a/src/daemon/plugin.c
+++ b/src/daemon/plugin.c
@@ -2198,12 +2198,12 @@ static int plugin_dispatch_metric_internal(metric_family_t const *fam) {
   return 0;
 } /* int plugin_dispatch_values_internal */
 
-static void set_default_resource_attributes(metric_t *m) {
-  if (m->resource.num > 0) {
+static void set_default_resource_attributes(metric_family_t *fam) {
+  if (fam->resource.num > 0) {
     return;
   }
 
-  label_set_clone(&m->resource, default_resource_attributes());
+  label_set_clone(&fam->resource, default_resource_attributes());
 }
 
 EXPORT int plugin_dispatch_metric_family(metric_family_t const *fam) {
@@ -2222,6 +2222,8 @@ EXPORT int plugin_dispatch_metric_family(metric_family_t const *fam) {
     return status;
   }
 
+  set_default_resource_attributes(fam_copy);
+
   cdtime_t time = cdtime();
   cdtime_t interval = plugin_get_interval();
 
@@ -2233,8 +2235,6 @@ EXPORT int plugin_dispatch_metric_family(metric_family_t const *fam) {
     if (m->interval == 0) {
       m->interval = interval;
     }
-
-    set_default_resource_attributes(m);
   }
 
   int status = plugin_dispatch_metric_internal(fam_copy);
diff --git a/src/utils/format_graphite/format_graphite.c b/src/utils/format_graphite/format_graphite.c
index eb9c0ccc8..7aad69e40 100644
--- a/src/utils/format_graphite/format_graphite.c
+++ b/src/utils/format_graphite/format_graphite.c
@@ -96,7 +96,8 @@ static int graphite_print_escaped(strbuf_t *buf, char const *s,
   return 0;
 }
 
-static void gr_format_label_set(strbuf_t *buf, label_set_t const *labels, char const escape_char, unsigned int flags) {
+static void gr_format_label_set(strbuf_t *buf, label_set_t const *labels,
+                                char const escape_char, unsigned int flags) {
   for (size_t i = 0; i < labels->num; i++) {
     label_pair_t *l = labels->ptr + i;
     strbuf_print(buf, ".");
@@ -107,8 +108,8 @@ static void gr_format_label_set(strbuf_t *buf, label_set_t const *labels, char c
 }
 
 static void gr_format_name(strbuf_t *buf, metric_t const *m, char const *prefix,
-                          char const *suffix, char const escape_char,
-                          unsigned int flags) {
+                           char const *suffix, char const escape_char,
+                           unsigned int flags) {
   if (prefix != NULL) {
     strbuf_print(buf, prefix);
   }
@@ -117,7 +118,7 @@ static void gr_format_name(strbuf_t *buf, metric_t const *m, char const *prefix,
     strbuf_print(buf, suffix);
   }
 
-  gr_format_label_set(buf, &m->resource, escape_char, flags);
+  gr_format_label_set(buf, &m->family->resource, escape_char, flags);
   gr_format_label_set(buf, &m->label, escape_char, flags);
 }