From: Florian Forster <octo@collectd.org>
Date: Wed, 31 Jan 2024 07:42:16 +0000 (+0100)
Subject: Daemon: Add a new metric type for floating point counters.
X-Git-Tag: collectd-6.0.0.rc2~6^2~19
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=04236a055045748f6bec1b871bc1046458b1de03;p=thirdparty%2Fcollectd.git

Daemon: Add a new metric type for floating point counters.
---

diff --git a/src/daemon/metric.c b/src/daemon/metric.c
index daf735b1b..2247220ea 100644
--- a/src/daemon/metric.c
+++ b/src/daemon/metric.c
@@ -52,7 +52,7 @@
 int value_marshal_text(strbuf_t *buf, value_t v, metric_type_t type) {
   switch (type) {
   case METRIC_TYPE_GAUGE:
-  case METRIC_TYPE_UNTYPED:
+  case METRIC_TYPE_FPCOUNTER:
     return strbuf_printf(buf, GAUGE_FORMAT, v.gauge);
   case METRIC_TYPE_COUNTER:
     return strbuf_printf(buf, "%" PRIu64, v.counter);
diff --git a/src/daemon/metric.h b/src/daemon/metric.h
index e2e72b5a9..c2e9d0f77 100644
--- a/src/daemon/metric.h
+++ b/src/daemon/metric.h
@@ -32,21 +32,24 @@
 #include "utils/strbuf/strbuf.h"
 #include "utils_time.h"
 
-#define VALUE_TYPE_GAUGE 1
-#define VALUE_TYPE_DERIVE 2
+#define METRIC_ATTR_DOUBLE 0x01
+#define METRIC_ATTR_CUMULATIVE 0x02
 
 typedef enum {
-  METRIC_TYPE_COUNTER = 0,
-  METRIC_TYPE_GAUGE = 1,
-  METRIC_TYPE_UNTYPED = 2,
+  METRIC_TYPE_UNTYPED = 0,
+  METRIC_TYPE_COUNTER = METRIC_ATTR_CUMULATIVE,
+  METRIC_TYPE_FPCOUNTER = METRIC_ATTR_DOUBLE | METRIC_ATTR_CUMULATIVE,
+  METRIC_TYPE_GAUGE = METRIC_ATTR_DOUBLE,
 } metric_type_t;
 
 typedef uint64_t counter_t;
+typedef double fpcounter_t;
 typedef double gauge_t;
 typedef int64_t derive_t;
 
 union value_u {
   counter_t counter;
+  fpcounter_t fpcounter;
   gauge_t gauge;
   derive_t derive;
 };
diff --git a/src/daemon/plugin.h b/src/daemon/plugin.h
index e1c54627a..ceca96d08 100644
--- a/src/daemon/plugin.h
+++ b/src/daemon/plugin.h
@@ -40,10 +40,6 @@
 #include <inttypes.h>
 #include <pthread.h>
 
-#define DS_TYPE_COUNTER 0
-#define DS_TYPE_GAUGE VALUE_TYPE_GAUGE
-#define DS_TYPE_DERIVE VALUE_TYPE_DERIVE
-
 #define DS_TYPE_TO_STRING(t)                                                   \
   (t == DS_TYPE_COUNTER)  ? "counter"                                          \
   : (t == DS_TYPE_GAUGE)  ? "gauge"                                            \
diff --git a/src/daemon/utils_cache.c b/src/daemon/utils_cache.c
index 057531e15..df0a552ad 100644
--- a/src/daemon/utils_cache.c
+++ b/src/daemon/utils_cache.c
@@ -154,14 +154,14 @@ static int uc_insert(metric_t const *m, char const *key) {
   sstrncpy(ce->name, key, sizeof(ce->name));
 
   switch (m->family->type) {
-  case DS_TYPE_COUNTER:
-  case DS_TYPE_DERIVE:
+  case METRIC_TYPE_COUNTER:
+  case METRIC_TYPE_FPCOUNTER:
     // Non-gauge types will store the rate in values_gauge when the second data
     // point is available. Initially, NAN signifies "not enough data".
     ce->values_gauge = NAN;
     break;
 
-  case DS_TYPE_GAUGE:
+  case METRIC_TYPE_GAUGE:
     ce->values_gauge = m->value.gauge;
     break;
 
@@ -349,22 +349,28 @@ static int uc_update_metric(metric_t const *m) {
     break;
   }
 
-  case METRIC_TYPE_UNTYPED:
-  case METRIC_TYPE_GAUGE: {
-    ce->values_gauge = m->value.gauge;
+  case METRIC_TYPE_FPCOUNTER: {
+    // For floating point counters, the logic is slightly different from
+    // integer counters. Floating point counters don't have a (meaningful)
+    // overflow, and we will always assume a counter reset.
+    if (ce->last_value.fpcounter > m->value.fpcounter) {
+      // counter reset
+      ce->first_time = m->time;
+      ce->first_value = m->value;
+      ce->values_gauge = NAN;
+      break;
+    }
+    gauge_t diff = m->value.fpcounter - ce->last_value.fpcounter;
+    ce->values_gauge = diff / CDTIME_T_TO_DOUBLE(m->time - ce->last_time);
     break;
   }
 
-#if 0
-  case DS_TYPE_DERIVE: { /* TODO(octo): add support for DERIVE */
-    derive_t diff = m->value.derive - ce->values_raw.derive;
-    ce->values_gauge =
-        ((double)diff) / (CDTIME_T_TO_DOUBLE(m->time - ce->last_time));
-    ce->values_raw.derive = m->value.derive;
+  case METRIC_TYPE_GAUGE: {
+    ce->values_gauge = m->value.gauge;
     break;
   }
-#endif
 
+  case METRIC_TYPE_UNTYPED:
   default: {
     /* This shouldn't happen. */
     pthread_mutex_unlock(&cache_lock);
diff --git a/src/daemon/utils_cache_test.c b/src/daemon/utils_cache_test.c
index 29d640b8d..ef2c66fd4 100644
--- a/src/daemon/utils_cache_test.c
+++ b/src/daemon/utils_cache_test.c
@@ -86,6 +86,24 @@ DEF_TEST(uc_get_rate) {
           .type = METRIC_TYPE_COUNTER,
           .want = (23. + 18. + 1.) / (110. - 100.),
       },
+      {
+          .name = "fpcounter",
+          .first_value = (value_t){.fpcounter = 4.2},
+          .second_value = (value_t){.fpcounter = 10.2},
+          .first_time = TIME_T_TO_CDTIME_T(100),
+          .second_time = TIME_T_TO_CDTIME_T(110),
+          .type = METRIC_TYPE_FPCOUNTER,
+          .want = (10.2 - 4.2) / (110 - 100),
+      },
+      {
+          .name = "fpcounter with reset",
+          .first_value = (value_t){.fpcounter = 100000.0},
+          .second_value = (value_t){.fpcounter = 0.2},
+          .first_time = TIME_T_TO_CDTIME_T(100),
+          .second_time = TIME_T_TO_CDTIME_T(110),
+          .type = METRIC_TYPE_FPCOUNTER,
+          .want = NAN,
+      },
   };
 
   for (size_t i = 0; i < STATIC_ARRAY_SIZE(cases); i++) {
diff --git a/src/utils/value_list/value_list.h b/src/utils/value_list/value_list.h
index 2b95fb819..9ba6429b6 100644
--- a/src/utils/value_list/value_list.h
+++ b/src/utils/value_list/value_list.h
@@ -30,7 +30,11 @@
 #define UTILS_VALUE_LIST_H 1
 
 #include "daemon/data_set.h"
-#include "daemon/plugin.h"
+#include "daemon/metric.h" // for value_t
+
+#define DS_TYPE_COUNTER METRIC_TYPE_COUNTER
+#define DS_TYPE_GAUGE METRIC_TYPE_GAUGE
+#define DS_TYPE_DERIVE (65536 + METRIC_ATTR_CUMULATIVE + 1)
 
 struct value_list_s {
   value_t *values;