From 4551377f70b790585c9d224a6549c0472a18e0a1 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Sun, 17 Dec 2023 09:45:11 +0100 Subject: [PATCH] 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. --- src/utils/resource_metrics/resource_metrics.c | 32 ++++++++++++------- src/utils/resource_metrics/resource_metrics.h | 10 ++++-- .../resource_metrics/resource_metrics_test.c | 22 ++++++------- 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/src/utils/resource_metrics/resource_metrics.c b/src/utils/resource_metrics/resource_metrics.c index b5eca7f83..8e49bcc9d 100644 --- 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 ffda8e14c..f52468d44 100644 --- 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 859b660f7..8507b198d 100644 --- 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); -- 2.47.2