resource_metrics: Skip duplicate metrics instead of erroring out.

The semantics have been changed to simply ignore metrics that are already
in the set. The previous semantic was optimized for a "add and if that
fails flush and try again" plugin behavior. With the support for multiple
instances of the same metric (at different times), there no longer is a
need to ensure metrics in the set are unique.

diff --git a/src/utils/resource_metrics/resource_metrics.c b/src/utils/resource_metrics/resource_metrics.c
index b5eca7f831291c532953f015bf138a9bf29e9bc5..8e49bcc9dd2cf352ae03fa72dd5c5c6fea7f42cb 100644 (file)
--- a/src/utils/resource_metrics/resource_metrics.c
+++ b/src/utils/resource_metrics/resource_metrics.c
@@ -167,32 +167,42 @@ static bool metric_exists(metric_family_t const *fam, metric_t const *m) {
 }
 
 static int insert_metrics(metric_family_t *fam, metric_list_t metrics) {
+  int skipped = 0;
   for (size_t i = 0; i < metrics.num; i++) {
-    if (metric_exists(fam, metrics.ptr + i)) {
-      return EEXIST;
+    metric_t const *m = metrics.ptr + i;
+
+    if (metric_exists(fam, m)) {
+#if COLLECT_DEBUG
+      strbuf_t buf = STRBUF_CREATE;
+      metric_identity(&buf, m);
+      DEBUG("resource_metrics: Skipping duplicate of metric %s", buf.ptr);
+      STRBUF_DESTROY(buf);
+#endif
+      skipped++;
+      continue;
     }
-  }
 
-  int last_err = 0;
-  for (size_t i = 0; i < metrics.num; i++) {
-    int status = metric_family_metric_append(fam, metrics.ptr[i]);
+    int status = metric_family_metric_append(fam, *m);
     if (status != 0) {
       ERROR("resource_metrics: metric_family_metric_append failed: %s",
             STRERROR(status));
       /* DO NOT RETURN: the metric list may be unsorted */
-      last_err = status;
+      skipped++;
     }
   }
 
-  qsort(fam->metric.ptr, fam->metric.num, sizeof(*fam->metric.ptr),
-        (void *)compare_metrics);
-  return last_err;
+  if (((size_t)skipped) != metrics.num) {
+    qsort(fam->metric.ptr, fam->metric.num, sizeof(*fam->metric.ptr),
+          (void *)compare_metrics);
+  }
+
+  return skipped;
 }
 
 int resource_metrics_add(resource_metrics_set_t *set,
                          metric_family_t const *fam) {
   if (set == NULL || fam == NULL) {
-    return EINVAL;
+    return -1;
   }
 
   resource_metrics_t *rm = lookup_or_insert_resource(set, fam->resource);

diff --git a/src/utils/resource_metrics/resource_metrics.h b/src/utils/resource_metrics/resource_metrics.h
index ffda8e14c5164c6a4b5085ba2019b67b790f29b7..f52468d44ded70144bebbf6bec6590df8beb8973 100644 (file)
--- a/src/utils/resource_metrics/resource_metrics.h
+++ b/src/utils/resource_metrics/resource_metrics.h
@@ -47,8 +47,14 @@ typedef struct {
 } resource_metrics_set_t;
 
 /* resource_metrics_add copies a metric family to the resource metrics set.
- * If any metric within the metric family is already part of the resource
- * metrics set, the function will return EEXIST and rm remains unmodified. */
+ * Identical metrics are skipped and not added to the set. Metrics are
+ * identical, if their resource attributes, metric family name, metric labels,
+ * and time stamp are equal.
+ * Returns the number of metrics that were skipped or -1 on error. That means
+ * that zero indicates complete success, a positive number indicates partial
+ * success, and a negative number indicates an error condition. The number of
+ * skipped entries may be equal to the total number of metrics provided; this is
+ * not indicated as an error. */
 int resource_metrics_add(resource_metrics_set_t *rm,
                          metric_family_t const *fam);
 

diff --git a/src/utils/resource_metrics/resource_metrics_test.c b/src/utils/resource_metrics/resource_metrics_test.c
index 859b660f7cf7ad52fc38d691054d13243adbee1d..8507b198d3559016ab72f68b86045f5759ebb1ee 100644 (file)
--- a/src/utils/resource_metrics/resource_metrics_test.c
+++ b/src/utils/resource_metrics/resource_metrics_test.c
@@ -65,20 +65,20 @@ DEF_TEST(resource_metrics_add) {
   CHECK_ZERO(resource_metrics_add(&set, fam));
   EXPECT_EQ_INT(1, set.num);
   EXPECT_EQ_INT(1, count_metrics(set));
-  /* adding the same familiy twice should return EEXIST. */
-  EXPECT_EQ_INT(EEXIST, resource_metrics_add(&set, fam));
+  /* adding the same metric twice should return one skipped metric. */
+  EXPECT_EQ_INT(1, resource_metrics_add(&set, fam));
   EXPECT_EQ_INT(1, set.num);
   EXPECT_EQ_INT(1, count_metrics(set));
   metric_family_free(fam);
 
-  /* adding the same metric (but with a different resource attribute) should
+  /* adding the same metric family with different resource attributes should
    * succeed. */
   CHECK_NOT_NULL(fam = make_metric_family(2, 1, 1, 1));
   CHECK_ZERO(resource_metrics_add(&set, fam));
   EXPECT_EQ_INT(2, set.num);
   EXPECT_EQ_INT(2, count_metrics(set));
-  /* adding the same familiy twice should return EEXIST. */
-  EXPECT_EQ_INT(EEXIST, resource_metrics_add(&set, fam));
+  /* adding the same metric twice should return one skipped metric. */
+  EXPECT_EQ_INT(1, resource_metrics_add(&set, fam));
   EXPECT_EQ_INT(2, set.num);
   EXPECT_EQ_INT(2, count_metrics(set));
   metric_family_free(fam);
@@ -89,8 +89,8 @@ DEF_TEST(resource_metrics_add) {
   /* reuses existing resource */
   EXPECT_EQ_INT(2, set.num);
   EXPECT_EQ_INT(3, count_metrics(set));
-  /* adding the same familiy twice should return EEXIST. */
-  EXPECT_EQ_INT(EEXIST, resource_metrics_add(&set, fam));
+  /* adding the same metric twice should return one skipped metric. */
+  EXPECT_EQ_INT(1, resource_metrics_add(&set, fam));
   EXPECT_EQ_INT(2, set.num);
   EXPECT_EQ_INT(3, count_metrics(set));
   metric_family_free(fam);
@@ -101,8 +101,8 @@ DEF_TEST(resource_metrics_add) {
   /* reuses existing resource */
   EXPECT_EQ_INT(2, set.num);
   EXPECT_EQ_INT(4, count_metrics(set));
-  /* adding the same familiy twice should return EEXIST. */
-  EXPECT_EQ_INT(EEXIST, resource_metrics_add(&set, fam));
+  /* adding the same metric twice should return one skipped metric. */
+  EXPECT_EQ_INT(1, resource_metrics_add(&set, fam));
   EXPECT_EQ_INT(2, set.num);
   EXPECT_EQ_INT(4, count_metrics(set));
   metric_family_free(fam);
@@ -113,8 +113,8 @@ DEF_TEST(resource_metrics_add) {
   /* reuses existing resource */
   EXPECT_EQ_INT(2, set.num);
   EXPECT_EQ_INT(5, count_metrics(set));
-  /* adding the same metric twice should return EEXIST. */
-  EXPECT_EQ_INT(EEXIST, resource_metrics_add(&set, fam));
+  /* adding the same metric twice should return one skipped metric. */
+  EXPECT_EQ_INT(1, resource_metrics_add(&set, fam));
   EXPECT_EQ_INT(2, set.num);
   EXPECT_EQ_INT(5, count_metrics(set));
   metric_family_free(fam);