Merge pull request collectd#4287 from octo/6/up_down_counter

[collectd 6] Implement "UpDownCounter" metric types.

src/cpu.c

@@ -139,7 +139,7 @@ typedef struct {
 
   /* count is a scaled counter, so that all states in sum increase by 1000000
    * per second. */
-  fpcounter_t count;
+  double count;
   bool has_count;
   rate_to_value_state_t to_count;
 } usage_state_t;
@@ -488,9 +488,9 @@ static void usage_finalize(usage_t *u) {
       gauge_t ratio = us->rate / cpu_rate;
       value_t v = {0};
       int status = rate_to_value(&v, ratio, &us->to_count,
-                                 METRIC_TYPE_FPCOUNTER, u->time);
+                                 METRIC_TYPE_COUNTER_FP, u->time);
       if (status == 0) {
-        us->count = v.fpcounter;
+        us->count = v.counter_fp;
         us->has_count = true;
       }
 
@@ -504,16 +504,16 @@ static void usage_finalize(usage_t *u) {
     us->count = NAN;
     if (!us->has_rate) {
       /* Ensure that us->to_count is initialized. */
-      rate_to_value(&(value_t){0}, 0.0, &us->to_count, METRIC_TYPE_FPCOUNTER,
+      rate_to_value(&(value_t){0}, 0.0, &us->to_count, METRIC_TYPE_COUNTER_FP,
                     u->time);
       continue;
     }
 
     value_t v = {0};
     int status = rate_to_value(&v, state_ratio[s], &us->to_count,
-                               METRIC_TYPE_FPCOUNTER, u->time);
+                               METRIC_TYPE_COUNTER_FP, u->time);
     if (status == 0) {
-      us->count = v.fpcounter;
+      us->count = v.counter_fp;
       us->has_count = true;
     }
   }
@@ -554,7 +554,7 @@ static gauge_t usage_ratio(usage_t *u, size_t cpu, state_t state) {
   return usage_rate(u, cpu, state) / global_rate;
 }
 
-static fpcounter_t usage_count(usage_t *u, size_t cpu, state_t state) {
+static double usage_count(usage_t *u, size_t cpu, state_t state) {
   usage_finalize(u);
 
   usage_state_t us;
@@ -598,7 +598,7 @@ static void commit_cpu_usage(usage_t *u, size_t cpu_num) {
       .name = "system.cpu.time",
       .help = "Microseconds each logical CPU spent in each state",
       .unit = "s",
-      .type = METRIC_TYPE_FPCOUNTER,
+      .type = METRIC_TYPE_COUNTER_FP,
   };
 
   metric_t m = {0};
@@ -610,18 +610,18 @@ static void commit_cpu_usage(usage_t *u, size_t cpu_num) {
 
   if (report_by_state) {
     for (state_t state = 0; state < STATE_ACTIVE; state++) {
-      fpcounter_t usage = usage_count(u, cpu_num, state);
+      double usage = usage_count(u, cpu_num, state);
       if (isnan(usage)) {
         continue;
       }
       metric_family_append(&fam, label_state, cpu_state_names[state],
-                           (value_t){.fpcounter = usage}, &m);
+                           (value_t){.counter_fp = usage}, &m);
     }
   } else {
-    fpcounter_t usage = usage_count(u, cpu_num, STATE_ACTIVE);
+    double usage = usage_count(u, cpu_num, STATE_ACTIVE);
     if (!isnan(usage)) {
       metric_family_append(&fam, label_state, cpu_state_names[STATE_ACTIVE],
-                           (value_t){.fpcounter = usage}, &m);
+                           (value_t){.counter_fp = usage}, &m);
     }
   }
 

src/cpu_test.c

@@ -124,7 +124,7 @@ DEF_TEST(usage_ratio) {
   return 0;
 }
 
-static bool expect_usage_count(fpcounter_t want, fpcounter_t got, size_t cpu,
+static bool expect_usage_count(double want, double got, size_t cpu,
                                state_t state) {
   char cpu_str[64] = "CPU_ALL";
   if (cpu != SIZE_MAX) {
@@ -174,8 +174,8 @@ DEF_TEST(usage_count) {
     }
   }
 
-  fpcounter_t state_time[STATE_MAX] = {0};
-  fpcounter_t sum_time = 0;
+  double state_time[STATE_MAX] = {0};
+  double sum_time = 0;
   for (size_t cpu = 0; cpu < CPU_NUM; cpu++) {
     derive_t active_increment = 0;
     for (state_t s = 0; s < STATE_ACTIVE; s++) {
@@ -184,9 +184,8 @@ DEF_TEST(usage_count) {
         active_increment += increment;
       }
 
-      fpcounter_t want_time = CDTIME_T_TO_DOUBLE(interval) *
-                              ((fpcounter_t)increment) /
-                              ((fpcounter_t)cpu_increment[cpu]);
+      double want_time = CDTIME_T_TO_DOUBLE(interval) * ((double)increment) /
+                         ((double)cpu_increment[cpu]);
       state_time[s] += want_time;
       sum_time += want_time;
 
@@ -195,9 +194,9 @@ DEF_TEST(usage_count) {
       ret = ret || !ok;
     }
 
-    fpcounter_t want_active_time = CDTIME_T_TO_DOUBLE(interval) *
-                                   ((fpcounter_t)active_increment) /
-                                   ((fpcounter_t)cpu_increment[cpu]);
+    double want_active_time = CDTIME_T_TO_DOUBLE(interval) *
+                              ((double)active_increment) /
+                              ((double)cpu_increment[cpu]);
     state_time[STATE_ACTIVE] += want_active_time;
     bool ok = expect_usage_count(want_active_time,
                                  usage_count(&usage, cpu, STATE_ACTIVE), cpu,

src/daemon/metric.c

@@ -52,14 +52,26 @@
 int value_marshal_text(strbuf_t *buf, value_t v, metric_type_t type) {
   switch (type) {
   case METRIC_TYPE_GAUGE:
-  case METRIC_TYPE_FPCOUNTER:
+    if (isnan(v.gauge)) {
+      return strbuf_print(buf, "nan");
+    }
     return strbuf_printf(buf, GAUGE_FORMAT, v.gauge);
   case METRIC_TYPE_COUNTER:
     return strbuf_printf(buf, "%" PRIu64, v.counter);
-  default:
-    ERROR("Unknown metric value type: %d", (int)type);
-    return EINVAL;
+  case METRIC_TYPE_COUNTER_FP:
+    return strbuf_printf(buf, GAUGE_FORMAT, v.counter_fp);
+  case METRIC_TYPE_UP_DOWN:
+    return strbuf_printf(buf, "%" PRId64, v.up_down);
+  case METRIC_TYPE_UP_DOWN_FP:
+    if (isnan(v.up_down_fp)) {
+      return strbuf_print(buf, "nan");
+    }
+    return strbuf_printf(buf, GAUGE_FORMAT, v.up_down_fp);
+  case METRIC_TYPE_UNTYPED:
+    break;
   }
+  ERROR("Unknown metric value type: %d", (int)type);
+  return EINVAL;
 }
 
 static int label_name_compare(void const *a, void const *b) {

src/daemon/metric.h

@@ -34,25 +34,51 @@
 
 #define METRIC_ATTR_DOUBLE 0x01
 #define METRIC_ATTR_CUMULATIVE 0x02
+#define METRIC_ATTR_MONOTONIC 0x04
 
 typedef enum {
   METRIC_TYPE_UNTYPED = 0,
+  // METRIC_TYPE_GAUGE are absolute metrics that cannot (meaningfully) be summed
+  // up. Examples are temperatures and utilization ratios.
   METRIC_TYPE_GAUGE = METRIC_ATTR_DOUBLE,
-  METRIC_TYPE_COUNTER = METRIC_ATTR_CUMULATIVE,
-  METRIC_TYPE_FPCOUNTER = METRIC_ATTR_DOUBLE | METRIC_ATTR_CUMULATIVE,
+  // METRIC_TYPE_COUNTER are monotonically increasing integer counts. The rate
+  // of change is meaningful, the absolute value is not.
+  METRIC_TYPE_COUNTER = METRIC_ATTR_CUMULATIVE | METRIC_ATTR_MONOTONIC,
+  // METRIC_TYPE_COUNTER_FP are monotonically increasing floating point counts.
+  // The rate of change is meaningful, the absolute value is not.
+  METRIC_TYPE_COUNTER_FP =
+      METRIC_ATTR_DOUBLE | METRIC_ATTR_CUMULATIVE | METRIC_ATTR_MONOTONIC,
+  // METRIC_TYPE_UP_DOWN are absolute integer metrics that can
+  // (meaningfully) be summed up. Examples are filesystem space used and
+  // physical memory.
+  METRIC_TYPE_UP_DOWN = METRIC_ATTR_CUMULATIVE,
+  // METRIC_TYPE_UP_DOWN_FP are absolute floating point metrics that can
+  // (meaningfully) be summed up.
+  METRIC_TYPE_UP_DOWN_FP = METRIC_ATTR_DOUBLE | METRIC_ATTR_CUMULATIVE,
 } metric_type_t;
 
-#define IS_CUMULATIVE(t) ((t)&METRIC_ATTR_CUMULATIVE)
+#define METRIC_TYPE_TO_STRING(t)                                               \
+  (t == METRIC_TYPE_GAUGE)        ? "gauge"                                    \
+  : (t == METRIC_TYPE_COUNTER)    ? "counter"                                  \
+  : (t == METRIC_TYPE_COUNTER_FP) ? "counter_fp"                               \
+  : (t == METRIC_TYPE_UP_DOWN)    ? "up_down"                                  \
+  : (t == METRIC_TYPE_UP_DOWN_FP) ? "up_down_fp"                               \
+                                  : "unknown"
+
+#define IS_DOUBLE(t) ((t)&METRIC_ATTR_DOUBLE)
+#define IS_MONOTONIC(t) ((t)&METRIC_ATTR_MONOTONIC)
 
-typedef uint64_t counter_t;
-typedef double fpcounter_t;
 typedef double gauge_t;
+typedef uint64_t counter_t;
 typedef int64_t derive_t;
 
 union value_u {
-  counter_t counter;
-  fpcounter_t fpcounter;
   gauge_t gauge;
+  counter_t counter;
+  double counter_fp;
+  int64_t up_down;
+  double up_down_fp;
+  // For collectd 5 compatiblity. Treated the same as up_down.
   derive_t derive;
 };
 typedef union value_u value_t;

src/daemon/utils_cache.c

@@ -281,6 +281,11 @@ int uc_check_timeout(void) {
 
 static int uc_update_rate(metric_t const *m, cache_entry_t *ce) {
   switch (m->family->type) {
+  case METRIC_TYPE_GAUGE: {
+    ce->values_gauge = m->value.gauge;
+    return 0;
+  }
+
   case METRIC_TYPE_COUNTER: {
     // Counter overflows and counter resets are signaled to plugins by resetting
     // "first_time". Since we can't distinguish between an overflow and a
@@ -297,24 +302,29 @@ static int uc_update_rate(metric_t const *m, cache_entry_t *ce) {
     return 0;
   }
 
-  case METRIC_TYPE_FPCOUNTER: {
+  case METRIC_TYPE_COUNTER_FP: {
     // 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) {
+    if (ce->last_value.counter_fp > m->value.counter_fp) {
       // counter reset
       ce->first_time = m->time;
       ce->first_value = m->value;
       ce->values_gauge = NAN;
       return 0;
     }
-    gauge_t diff = m->value.fpcounter - ce->last_value.fpcounter;
+    gauge_t diff = m->value.counter_fp - ce->last_value.counter_fp;
     ce->values_gauge = diff / CDTIME_T_TO_DOUBLE(m->time - ce->last_time);
     return 0;
   }
 
-  case METRIC_TYPE_GAUGE: {
-    ce->values_gauge = m->value.gauge;
+  case METRIC_TYPE_UP_DOWN: {
+    ce->values_gauge = (gauge_t)m->value.up_down;
+    return 0;
+  }
+
+  case METRIC_TYPE_UP_DOWN_FP: {
+    ce->values_gauge = (gauge_t)m->value.up_down_fp;
     return 0;
   }
 
@@ -465,9 +475,21 @@ int uc_get_rate(metric_t const *m, gauge_t *ret) {
   if (m == NULL || m->family == NULL || ret == NULL) {
     return EINVAL;
   }
-  if (m->family->type == METRIC_TYPE_GAUGE) {
+  switch (m->family->type) {
+  case METRIC_TYPE_GAUGE:
     *ret = m->value.gauge;
     return 0;
+  case METRIC_TYPE_COUNTER:
+  case METRIC_TYPE_COUNTER_FP:
+    break;
+  case METRIC_TYPE_UP_DOWN:
+    *ret = (gauge_t)m->value.up_down;
+    return 0;
+  case METRIC_TYPE_UP_DOWN_FP:
+    *ret = (gauge_t)m->value.up_down_fp;
+    return 0;
+  case METRIC_TYPE_UNTYPED:
+    return EINVAL;
   }
 
   strbuf_t buf = STRBUF_CREATE;

src/daemon/utils_cache.h

@@ -77,7 +77,7 @@ typedef struct {
 } uc_first_metric_result_t;
 
 /* uc_first_metric returns the first observed metric value and time.
- * For cumulative metrics (METRIC_TYPE_COUNTER and METRIC_TYPE_FPCOUNTER),
+ * For cumulative metrics (METRIC_TYPE_COUNTER and METRIC_TYPE_COUNTER_FP),
  * counter resets and counter overflows will reset the value. */
 uc_first_metric_result_t uc_first_metric(metric_t const *m);
 

src/daemon/utils_cache_test.c

@@ -87,23 +87,59 @@ DEF_TEST(uc_get_rate) {
           .want = (23. + 18. + 1.) / (110. - 100.),
       },
       {
-          .name = "fpcounter",
-          .first_value = (value_t){.fpcounter = 4.2},
-          .second_value = (value_t){.fpcounter = 10.2},
+          .name = "counter_fp",
+          .first_value = (value_t){.counter_fp = 4.2},
+          .second_value = (value_t){.counter_fp = 10.2},
           .first_time = TIME_T_TO_CDTIME_T(100),
           .second_time = TIME_T_TO_CDTIME_T(110),
-          .type = METRIC_TYPE_FPCOUNTER,
+          .type = METRIC_TYPE_COUNTER_FP,
           .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},
+          .name = "counter_fp with reset",
+          .first_value = (value_t){.counter_fp = 100000.0},
+          .second_value = (value_t){.counter_fp = 0.2},
           .first_time = TIME_T_TO_CDTIME_T(100),
           .second_time = TIME_T_TO_CDTIME_T(110),
-          .type = METRIC_TYPE_FPCOUNTER,
+          .type = METRIC_TYPE_COUNTER_FP,
           .want = NAN,
       },
+      {
+          .name = "up_down",
+          .first_value = (value_t){.up_down = 10},
+          .second_value = (value_t){.up_down = 20},
+          .first_time = TIME_T_TO_CDTIME_T(100),
+          .second_time = TIME_T_TO_CDTIME_T(110),
+          .type = METRIC_TYPE_UP_DOWN,
+          .want = 20,
+      },
+      {
+          .name = "decreasing up_down",
+          .first_value = (value_t){.up_down = 1000},
+          .second_value = (value_t){.up_down = 215},
+          .first_time = TIME_T_TO_CDTIME_T(100),
+          .second_time = TIME_T_TO_CDTIME_T(110),
+          .type = METRIC_TYPE_UP_DOWN,
+          .want = 215,
+      },
+      {
+          .name = "up_down_fp",
+          .first_value = (value_t){.up_down_fp = 1.0},
+          .second_value = (value_t){.up_down_fp = 2.0},
+          .first_time = TIME_T_TO_CDTIME_T(100),
+          .second_time = TIME_T_TO_CDTIME_T(110),
+          .type = METRIC_TYPE_UP_DOWN_FP,
+          .want = 2.0,
+      },
+      {
+          .name = "decreasing up_down_fp",
+          .first_value = (value_t){.up_down_fp = 100.0},
+          .second_value = (value_t){.up_down_fp = 21.5},
+          .first_time = TIME_T_TO_CDTIME_T(100),
+          .second_time = TIME_T_TO_CDTIME_T(110),
+          .type = METRIC_TYPE_UP_DOWN_FP,
+          .want = 21.5,
+      },
   };
 
   for (size_t i = 0; i < STATIC_ARRAY_SIZE(cases); i++) {
@@ -130,9 +166,11 @@ DEF_TEST(uc_get_rate) {
     EXPECT_EQ_INT(0, uc_update(&fam));
     gauge_t got = 0;
     EXPECT_EQ_INT(0, uc_get_rate(&m, &got));
-    gauge_t want = NAN;
-    if (fam.type == METRIC_TYPE_GAUGE) {
-      want = cases[i].first_value.gauge;
+    gauge_t want = cases[i].first_value.gauge;
+    if (IS_MONOTONIC(fam.type)) {
+      want = NAN;
+    } else if (fam.type == METRIC_TYPE_UP_DOWN) {
+      want = (gauge_t)cases[i].first_value.up_down;
     }
     EXPECT_EQ_DOUBLE(want, got);