From 04236a055045748f6bec1b871bc1046458b1de03 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 08:42:16 +0100 Subject: [PATCH 01/20] Daemon: Add a new metric type for floating point counters. --- src/daemon/metric.c | 2 +- src/daemon/metric.h | 13 ++++++++----- src/daemon/plugin.h | 4 ---- src/daemon/utils_cache.c | 32 ++++++++++++++++++------------- src/daemon/utils_cache_test.c | 18 +++++++++++++++++ src/utils/value_list/value_list.h | 6 +++++- 6 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/daemon/metric.c b/src/daemon/metric.c index daf735b1bd..2247220ea7 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 e2e72b5a94..c2e9d0f779 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 e1c54627a6..ceca96d08d 100644 --- a/src/daemon/plugin.h +++ b/src/daemon/plugin.h @@ -40,10 +40,6 @@ #include #include -#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 057531e150..df0a552ad9 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 29d640b8de..ef2c66fd4d 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 2b95fb8197..9ba6429b6e 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; From cf613d1b3b1ae323537e4382d4d908b9b513cc50 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 08:46:29 +0100 Subject: [PATCH 02/20] common: Add support for `METRIC_TYPE_FPCOUNTER`. --- src/utils/common/common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/utils/common/common.c b/src/utils/common/common.c index afa49d4db3..6fe7427658 100644 --- a/src/utils/common/common.c +++ b/src/utils/common/common.c @@ -957,6 +957,8 @@ int format_values(strbuf_t *buf, metric_t const *m, bool store_rates) { } } else if (m->family->type == METRIC_TYPE_COUNTER) { strbuf_printf(buf, ":%" PRIu64, m->value.counter); + } else if (m->family->type == METRIC_TYPE_FPCOUNTER) { + strbuf_printf(buf, ":" GAUGE_FORMAT, m->value.fpcounter); } else if (m->family->type == DS_TYPE_DERIVE) { strbuf_printf(buf, ":%" PRIi64, m->value.derive); } else { From 32a9042170ce7a98f726c8f4fc5645322000ab42 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 08:46:29 +0100 Subject: [PATCH 03/20] cmds: Add support for `METRIC_TYPE_FPCOUNTER`. --- src/utils/cmds/cmds_test.c | 19 ++++++++++--------- src/utils/cmds/putmetric.c | 17 +++++++++-------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/utils/cmds/cmds_test.c b/src/utils/cmds/cmds_test.c index 9c061f5756..30ce3832a5 100644 --- a/src/utils/cmds/cmds_test.c +++ b/src/utils/cmds/cmds_test.c @@ -314,8 +314,8 @@ static struct { { "PUTMETRIC untyped type=untyped 42", NULL, - CMD_OK, - CMD_PUTMETRIC, + CMD_ERROR, + CMD_UNKNOWN, }, { "PUTMETRIC quoted_gauge type=\"GAUGE\" 42", @@ -428,7 +428,7 @@ DEF_TEST(format_putmetric) { }, .value.gauge = 42, }, - .want = "PUTMETRIC test 42", + .want_err = EINVAL, }, { .m = @@ -460,12 +460,12 @@ DEF_TEST(format_putmetric) { .family = &(metric_family_t){ .name = "test", - .type = METRIC_TYPE_UNTYPED, + .type = METRIC_TYPE_GAUGE, }, .value.gauge = 42, .time = TIME_T_TO_CDTIME_T(1594809888), }, - .want = "PUTMETRIC test time=1594809888.000 42", + .want = "PUTMETRIC test type=GAUGE time=1594809888.000 42", }, { .m = @@ -473,12 +473,12 @@ DEF_TEST(format_putmetric) { .family = &(metric_family_t){ .name = "test", - .type = METRIC_TYPE_UNTYPED, + .type = METRIC_TYPE_GAUGE, }, .value.gauge = 42, .interval = TIME_T_TO_CDTIME_T(10), }, - .want = "PUTMETRIC test interval=10.000 42", + .want = "PUTMETRIC test type=GAUGE interval=10.000 42", }, { .m = @@ -486,7 +486,7 @@ DEF_TEST(format_putmetric) { .family = &(metric_family_t){ .name = "test", - .type = METRIC_TYPE_UNTYPED, + .type = METRIC_TYPE_GAUGE, }, .value.gauge = 42, .label.ptr = @@ -496,7 +496,8 @@ DEF_TEST(format_putmetric) { }, .label.num = 1, }, - .want = "PUTMETRIC test label:foo=\"with \\\"quotes\\\"\" 42", + .want = + "PUTMETRIC test type=GAUGE label:foo=\"with \\\"quotes\\\"\" 42", }, }; diff --git a/src/utils/cmds/putmetric.c b/src/utils/cmds/putmetric.c index f2b1ac5b43..11969860e6 100644 --- a/src/utils/cmds/putmetric.c +++ b/src/utils/cmds/putmetric.c @@ -46,8 +46,8 @@ static int set_option(metric_t *m, char const *key, char const *value, m->family->type = METRIC_TYPE_GAUGE; } else if (strcasecmp("COUNTER", value) == 0) { m->family->type = METRIC_TYPE_COUNTER; - } else if (strcasecmp("UNTYPED", value) == 0) { - m->family->type = METRIC_TYPE_UNTYPED; + } else if (strcasecmp("FPCOUNTER", value) == 0) { + m->family->type = METRIC_TYPE_FPCOUNTER; } else { return CMD_ERROR; } @@ -104,7 +104,7 @@ cmd_status_t cmd_parse_putmetric(size_t argc, char **argv, cmd_error(CMD_ERROR, errhndl, "calloc failed"); return CMD_ERROR; } - fam->type = METRIC_TYPE_UNTYPED; + fam->type = METRIC_TYPE_GAUGE; int status = metric_family_metric_append(fam, (metric_t){0}); if (status != 0) { @@ -244,17 +244,18 @@ int cmd_format_putmetric(strbuf_t *buf, metric_t const *m) { /* {{{ */ strbuf_print(buf, "PUTMETRIC "); strbuf_print(buf, m->family->name); switch (m->family->type) { - case METRIC_TYPE_UNTYPED: - /* no op */ - break; case METRIC_TYPE_COUNTER: strbuf_print(buf, " type=COUNTER"); break; + case METRIC_TYPE_FPCOUNTER: + strbuf_print(buf, " type=FPCOUNTER"); + break; case METRIC_TYPE_GAUGE: strbuf_print(buf, " type=GAUGE"); break; - default: - return EINVAL; + case METRIC_TYPE_UNTYPED: + /* no op */ + break; } if (m->time != 0) { From b949fb26295eeac57f408865bdd253a5bfe8d88e Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 08:46:29 +0100 Subject: [PATCH 04/20] format_graphite: Add support for `METRIC_TYPE_FPCOUNTER`. --- src/utils/format_graphite/format_graphite.c | 29 ++++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/utils/format_graphite/format_graphite.c b/src/utils/format_graphite/format_graphite.c index 7aad69e404..eb9bf21212 100644 --- a/src/utils/format_graphite/format_graphite.c +++ b/src/utils/format_graphite/format_graphite.c @@ -35,22 +35,31 @@ /* Utils functions to format data sets in graphite format. * Largely taken from write_graphite.c as it remains the same formatting */ +static int format_double(strbuf_t *buf, double d) { + if (isnan(d)) { + return strbuf_print(buf, "nan"); + } + return strbuf_printf(buf, GAUGE_FORMAT, d); +} + static int gr_format_values(strbuf_t *buf, metric_t const *m, gauge_t rate, bool store_rate) { - if (!store_rate && ((m->family->type == METRIC_TYPE_GAUGE) || - (m->family->type == METRIC_TYPE_UNTYPED))) { + if (m->family->type == METRIC_TYPE_GAUGE) { rate = m->value.gauge; - store_rate = true; } if (store_rate) { - if (isnan(rate)) { - return strbuf_print(buf, "nan"); - } else { - return strbuf_printf(buf, GAUGE_FORMAT, m->value.gauge); - } - } else if (m->family->type == METRIC_TYPE_COUNTER) { - return strbuf_printf(buf, "%" PRIu64, (uint64_t)m->value.counter); + return format_double(buf, rate); + } + + switch (m->family->type) { + case METRIC_TYPE_COUNTER: + return strbuf_printf(buf, "%" PRIu64, m->value.counter); + case METRIC_TYPE_FPCOUNTER: + return format_double(buf, m->value.fpcounter); + case METRIC_TYPE_GAUGE: + return format_double(buf, m->value.gauge); + case METRIC_TYPE_UNTYPED: } P_ERROR("gr_format_values: Unknown data source type: %d", m->family->type); From fc94134712f74ca79bd6e291926c087fa8aecfcc Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 08:46:29 +0100 Subject: [PATCH 05/20] format_influxdb: Add support for `METRIC_TYPE_FPCOUNTER`. --- src/utils/format_influxdb/format_influxdb.c | 121 +++++++++++--------- src/utils/format_influxdb/format_influxdb.h | 2 +- src/write_http.c | 5 +- src/write_influxdb_udp.c | 4 +- 4 files changed, 70 insertions(+), 62 deletions(-) diff --git a/src/utils/format_influxdb/format_influxdb.c b/src/utils/format_influxdb/format_influxdb.c index 2e9588bbfd..bef8fa5181 100644 --- a/src/utils/format_influxdb/format_influxdb.c +++ b/src/utils/format_influxdb/format_influxdb.c @@ -31,73 +31,82 @@ #include "utils/format_influxdb/format_influxdb.h" -int format_influxdb_point(strbuf_t *sb, metric_t metric, bool store_rates) { - bool have_values = false; +#define NEED_ESCAPE "\\ ,=\"" +#define ESCAPE_CHAR '\\' -#define BUFFER_ADD_ESCAPE(...) \ +#define ERR_COMBINE(err, f) \ do { \ - int status = strbuf_print_escaped(sb, __VA_ARGS__, "\\ ,=\"", '\\'); \ - if (status != 0) \ - return status; \ + err = err ? err : f; \ } while (0) -#define BUFFER_ADD(...) \ - do { \ - int status = strbuf_printf(sb, __VA_ARGS__); \ - if (status != 0) \ - return status; \ - } while (0) +static int format_metric_identity(strbuf_t *sb, metric_t const *m) { + int err = strbuf_print_escaped(sb, m->family->name, NEED_ESCAPE, ESCAPE_CHAR); + for (size_t j = 0; j < m->label.num; j++) { + label_pair_t label = m->label.ptr[j]; + ERR_COMBINE(err, strbuf_printf(sb, ",")); + ERR_COMBINE(err, + strbuf_print_escaped(sb, label.name, NEED_ESCAPE, ESCAPE_CHAR)); + ERR_COMBINE(err, strbuf_printf(sb, "=")); + ERR_COMBINE(err, + strbuf_print_escaped(sb, label.value, "\\ ,=\"", ESCAPE_CHAR)); + } + ERR_COMBINE(err, strbuf_printf(sb, " ")); + return err; +} - have_values = false; - BUFFER_ADD_ESCAPE(metric.family->name); - for (size_t j = 0; j < metric.label.num; j++) { - label_pair_t label = metric.label.ptr[j]; - BUFFER_ADD(","); - BUFFER_ADD_ESCAPE(label.name); - BUFFER_ADD("="); - BUFFER_ADD_ESCAPE(label.value); +static int format_metric_rate(strbuf_t *sb, metric_t const *m) { + gauge_t rate = 0; + if (uc_get_rate(m, &rate) != 0) { + WARNING("write_influxdb_udp plugin: uc_get_rate failed."); + return EINVAL; + } + if (isnan(rate)) { + return EAGAIN; } - BUFFER_ADD(" "); - if (store_rates && (metric.family->type == METRIC_TYPE_COUNTER)) { - gauge_t rate; - if (uc_get_rate(&metric, &rate) != 0) { - WARNING("write_influxdb_udp plugin: " - "uc_get_rate failed."); - return EINVAL; - } - if (!isnan(rate)) { - BUFFER_ADD("value=" GAUGE_FORMAT, rate); - have_values = true; - } - } else { - switch (metric.family->type) { - case METRIC_TYPE_GAUGE: - case METRIC_TYPE_UNTYPED: - if (!isnan(metric.value.gauge)) { - BUFFER_ADD("value=" GAUGE_FORMAT, metric.value.gauge); - have_values = true; - } - break; - case METRIC_TYPE_COUNTER: - BUFFER_ADD("value=%" PRIi64 "i", metric.value.counter); - have_values = true; - break; - default: - WARNING("write_influxdb_udp plugin: " - "unknown family type."); - return EINVAL; - break; + return strbuf_printf(sb, "value=" GAUGE_FORMAT, rate); +} + +static int format_metric_value(strbuf_t *sb, metric_t const *m, + bool store_rate) { + if (store_rate) { + return format_metric_rate(sb, m); + } + + switch (m->family->type) { + case METRIC_TYPE_GAUGE: + if (isnan(m->value.gauge)) { + return EAGAIN; } + return strbuf_printf(sb, "value=" GAUGE_FORMAT, m->value.gauge); + case METRIC_TYPE_COUNTER: + return strbuf_printf(sb, "value=%" PRIu64 "i", m->value.counter); + case METRIC_TYPE_FPCOUNTER: + return strbuf_printf(sb, "value=" GAUGE_FORMAT, m->value.fpcounter); + case METRIC_TYPE_UNTYPED: } - if (!have_values) - return 0; + ERROR("format_influxdb plugin: invalid metric type: %d", m->family->type); + return EINVAL; +} - BUFFER_ADD(" %" PRIu64 "\n", CDTIME_T_TO_MS(metric.time)); +static int format_metric_time(strbuf_t *sb, metric_t const *m) { + return strbuf_printf(sb, " %" PRIu64 "\n", CDTIME_T_TO_MS(m->time)); +} + +int format_influxdb_point(strbuf_t *sb, metric_t const *m, bool store_rate) { + int err = format_metric_identity(sb, m); + if (err != 0) { + return err; + } -#undef BUFFER_ADD_ESCAPE -#undef BUFFER_ADD + err = format_metric_value(sb, m, store_rate); + if (err == EAGAIN) { + return 0; + } + if (err != 0) { + return err; + } - return 0; + return format_metric_time(sb, m); } /* int write_influxdb_point */ diff --git a/src/utils/format_influxdb/format_influxdb.h b/src/utils/format_influxdb/format_influxdb.h index bd6b9746b9..b9c5e5257f 100644 --- a/src/utils/format_influxdb/format_influxdb.h +++ b/src/utils/format_influxdb/format_influxdb.h @@ -30,6 +30,6 @@ #include "plugin.h" -int format_influxdb_point(strbuf_t *sb, metric_t metric, bool store_rates); +int format_influxdb_point(strbuf_t *sb, metric_t const *m, bool store_rate); #endif /* UTILS_FORMAT_INFLUXDB_H */ diff --git a/src/write_http.c b/src/write_http.c index 28eb3d69b0..59a79af865 100644 --- a/src/write_http.c +++ b/src/write_http.c @@ -473,9 +473,8 @@ static int wh_write_influxdb(metric_family_t const *fam, wh_callback_t *cb) { pthread_mutex_lock(&cb->send_buffer_lock); for (size_t i = 0; i < fam->metric.num; i++) { - metric_t metric = fam->metric.ptr[i]; - int status = - format_influxdb_point(&cb->send_buffer, metric, cb->store_rates); + metric_t const *m = fam->metric.ptr + i; + int status = format_influxdb_point(&cb->send_buffer, m, cb->store_rates); if (status != 0) { pthread_mutex_unlock(&cb->send_buffer_lock); ERROR("write_http plugin: format_influxdb_point failed: %s", diff --git a/src/write_influxdb_udp.c b/src/write_influxdb_udp.c index 4f9c9c4cd9..0f727c7f98 100644 --- a/src/write_influxdb_udp.c +++ b/src/write_influxdb_udp.c @@ -356,9 +356,9 @@ static int write_influxdb_udp_write(metric_family_t const *fam, strbuf_t sb = STRBUF_CREATE_FIXED(buffer, buffer_len); for (size_t i = 0; i < fam->metric.num;) { - metric_t metric = fam->metric.ptr[i]; + metric_t const *m = fam->metric.ptr + i; const size_t pos = sb.pos; - int status = format_influxdb_point(&sb, metric, wifxudp_config_store_rates); + int status = format_influxdb_point(&sb, m, wifxudp_config_store_rates); if (status == ENOSPC) { fill_send_buffer(sb.ptr, pos); strbuf_reset(&sb); From 524eb92bfb83a7a29675295df6abfd7a33167398 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 08:46:29 +0100 Subject: [PATCH 06/20] format_json: Add support for `METRIC_TYPE_FPCOUNTER`. --- src/utils/format_json/format_json.c | 6 +++--- src/utils/format_json/format_json_test.c | 10 +++++----- src/utils/format_json/open_telemetry.c | 11 +++++++---- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/utils/format_json/format_json.c b/src/utils/format_json/format_json.c index 47c66e06e5..c65ab97bfd 100644 --- a/src/utils/format_json/format_json.c +++ b/src/utils/format_json/format_json.c @@ -190,12 +190,12 @@ static int json_metric_family(yajl_gen g, metric_family_t const *fam) { case METRIC_TYPE_COUNTER: type = "COUNTER"; break; + case METRIC_TYPE_FPCOUNTER: + type = "FPCOUNTER"; + break; case METRIC_TYPE_UNTYPED: type = "UNTYPED"; break; - default: - ERROR("format_json_metric: Unknown value type: %d", fam->type); - return EINVAL; } CHECK(json_add_string(g, "type")); CHECK(json_add_string(g, type)); diff --git a/src/utils/format_json/format_json_test.c b/src/utils/format_json/format_json_test.c index bf2ecfa75d..aa4da28424 100644 --- a/src/utils/format_json/format_json_test.c +++ b/src/utils/format_json/format_json_test.c @@ -255,16 +255,16 @@ DEF_TEST(metric_family_append) { metric_family_t fam = { .name = "first", - .type = METRIC_TYPE_UNTYPED, + .type = METRIC_TYPE_COUNTER, }; metric_family_metric_append(&fam, (metric_t){ - .value.gauge = 0, + .value.counter = 0, }); metric_family_metric_append(&fam, (metric_t){ - .value.gauge = 1, + .value.counter = 1, }); CHECK_ZERO(format_json_metric_family(&buf, &fam, false)); - EXPECT_EQ_STR("[{\"name\":\"first\",\"type\":\"UNTYPED\",\"metrics\":[{" + EXPECT_EQ_STR("[{\"name\":\"first\",\"type\":\"COUNTER\",\"metrics\":[{" "\"value\":\"0\"},{\"value\":\"1\"}]}]", buf.ptr); @@ -279,7 +279,7 @@ DEF_TEST(metric_family_append) { }); CHECK_ZERO(format_json_metric_family(&buf, &fam, false)); - EXPECT_EQ_STR("[{\"name\":\"first\",\"type\":\"UNTYPED\",\"metrics\":[{" + EXPECT_EQ_STR("[{\"name\":\"first\",\"type\":\"COUNTER\",\"metrics\":[{" "\"value\":\"0\"},{\"value\":\"1\"}]},{\"name\":\"second\"," "\"type\":\"GAUGE\",\"metrics\":[{\"value\":\"2\"}]}]", buf.ptr); diff --git a/src/utils/format_json/open_telemetry.c b/src/utils/format_json/open_telemetry.c index 195e48b522..8a510d4f2f 100644 --- a/src/utils/format_json/open_telemetry.c +++ b/src/utils/format_json/open_telemetry.c @@ -96,13 +96,15 @@ static int number_data_point(yajl_gen g, metric_t const *m) { CHECK(json_add_string(g, "asInt")); CHECK(yajl_gen_integer(g, m->value.counter)); break; + case METRIC_TYPE_FPCOUNTER: case METRIC_TYPE_GAUGE: CHECK(json_add_string(g, "asDouble")); CHECK(yajl_gen_double(g, m->value.gauge)); break; case METRIC_TYPE_UNTYPED: - // TODO - assert(0); + ERROR("format_json_open_telemetry: Unexpected metric type: %d", + m->family->type); + return EINVAL; } CHECK(yajl_gen_map_close(g)); /* END NumberDataPoint */ @@ -162,6 +164,7 @@ static int metric(yajl_gen g, metric_family_t const *fam) { switch (fam->type) { case METRIC_TYPE_COUNTER: + case METRIC_TYPE_FPCOUNTER: CHECK(json_add_string(g, "sum")); CHECK(sum(g, fam)); break; @@ -170,8 +173,8 @@ static int metric(yajl_gen g, metric_family_t const *fam) { CHECK(gauge(g, fam)); break; case METRIC_TYPE_UNTYPED: - // TODO - assert(0); + ERROR("format_json_open_telemetry: Unexpected metric type: %d", fam->type); + return EINVAL; } CHECK(yajl_gen_map_close(g)); /* END Metric */ From 1860423c218385d354039bf8e36a194aa582fcf6 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 08:46:29 +0100 Subject: [PATCH 07/20] format_kairosdb: Add support for `METRIC_TYPE_FPCOUNTER`. --- src/utils/format_kairosdb/format_kairosdb.c | 48 ++++++++++++++------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/src/utils/format_kairosdb/format_kairosdb.c b/src/utils/format_kairosdb/format_kairosdb.c index 2f4e7f6789..8838cf7422 100644 --- a/src/utils/format_kairosdb/format_kairosdb.c +++ b/src/utils/format_kairosdb/format_kairosdb.c @@ -81,33 +81,49 @@ static int json_add_string(yajl_gen g, char const *str) /* {{{ */ } \ } while (0) +static int json_add_rate(yajl_gen g, metric_t const *m) { + gauge_t rate = NAN; + int err = uc_get_rate(m, &rate); + if (err) { + return err; + } + + if (isfinite(rate)) { + CHECK(yajl_gen_double(g, rate)); + } else { + CHECK(yajl_gen_null(g)); + } + + return 0; +} + static int json_add_value(yajl_gen g, metric_t const *m, format_kairosdb_opts_t const *opts) { - if ((m->family->type == METRIC_TYPE_GAUGE) || - (m->family->type == METRIC_TYPE_UNTYPED)) { + if (opts != NULL && opts->store_rates) { + return json_add_rate(g, m); + } + + switch (m->family->type) { + case METRIC_TYPE_GAUGE: { double v = m->value.gauge; if (isfinite(v)) { CHECK(yajl_gen_double(g, v)); } else { CHECK(yajl_gen_null(g)); } - } else if ((opts != NULL) && opts->store_rates) { - gauge_t rate = NAN; - uc_get_rate(m, &rate); - - if (isfinite(rate)) { - CHECK(yajl_gen_double(g, rate)); - } else { - CHECK(yajl_gen_null(g)); - } - } else if (m->family->type == METRIC_TYPE_COUNTER) { + return 0; + } + case METRIC_TYPE_FPCOUNTER: + CHECK(yajl_gen_double(g, m->value.fpcounter)); + return 0; + case METRIC_TYPE_COUNTER: CHECK(yajl_gen_integer(g, (long long int)m->value.counter)); - } else { - ERROR("format_kairosdb: Unknown data source type: %d", m->family->type); - CHECK(yajl_gen_null(g)); + return 0; + case METRIC_TYPE_UNTYPED: } - return 0; + ERROR("format_kairosdb: Invalid metric type: %d", m->family->type); + return EINVAL; } static int json_add_metric(yajl_gen g, metric_t const *m, From 04aad0dffbc52f6464aa91374a6977354ce32e7e Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 08:46:29 +0100 Subject: [PATCH 08/20] format_open_telemetry: Add support for `METRIC_TYPE_FPCOUNTER`. --- .../format_open_telemetry.cc | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/utils/format_open_telemetry/format_open_telemetry.cc b/src/utils/format_open_telemetry/format_open_telemetry.cc index 6ae823ee29..9e1daffc09 100644 --- a/src/utils/format_open_telemetry/format_open_telemetry.cc +++ b/src/utils/format_open_telemetry/format_open_telemetry.cc @@ -66,19 +66,22 @@ static void metric_to_number_data_point(NumberDataPoint *dp, // when we've seen a metric for the first time. // A valid metric type is guaranteed by add_metric(). - assert(m->family->type == METRIC_TYPE_COUNTER || - m->family->type == METRIC_TYPE_GAUGE); switch (m->family->type) { case METRIC_TYPE_COUNTER: dp->set_as_int(m->value.derive); - break; + return; case METRIC_TYPE_GAUGE: + case METRIC_TYPE_FPCOUNTER: dp->set_as_double(m->value.gauge); - break; + return; case METRIC_TYPE_UNTYPED: - // never reached, only here to make the compiler happy + // Fall through. This case signals the compiler that we're checking all + // values of the enum. We report an error outside of the switch to also + // cover other values. break; } + // never reached, only here to make the compiler happy + ERROR("format_open_telemetry: unexpected metric type: %d", m->family->type); } static void set_sum(Metric *m, metric_family_t const *fam) { @@ -107,10 +110,9 @@ static void set_gauge(Metric *m, metric_family_t const *fam) { } static void add_metric(ScopeMetrics *sm, metric_family_t const *fam) { - if (fam->type != METRIC_TYPE_COUNTER && fam->type != METRIC_TYPE_GAUGE) { - WARNING("format_open_telemetry: metric family \"%s\" with type %d is not " - "supported.", - fam->name, fam->type); + if (fam->type == METRIC_TYPE_UNTYPED) { + ERROR("format_open_telemetry: metric family \"%s\" has invalid type %d.", + fam->name, fam->type); return; } @@ -125,15 +127,19 @@ static void add_metric(ScopeMetrics *sm, metric_family_t const *fam) { switch (fam->type) { case METRIC_TYPE_COUNTER: + case METRIC_TYPE_FPCOUNTER: set_sum(m, fam); return; case METRIC_TYPE_GAUGE: set_gauge(m, fam); return; case METRIC_TYPE_UNTYPED: - // never reached, only here to make the compiler happy - return; + // Fall through. This case signals the compiler that we're checking all + // values of the enum. We report an error outside of the switch to also + // cover other values. + break; } + ERROR("format_open_telemetry: unexpected metric type: %d", fam->type); } static void set_instrumentation_scope(ScopeMetrics *sm) { From c69b288e6a2d44d26031e9f421106ae3ca9bcd11 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 08:46:29 +0100 Subject: [PATCH 09/20] format_stackdriver: Add support for `METRIC_TYPE_FPCOUNTER`. --- .../format_stackdriver/format_stackdriver.c | 45 +++++++++++-------- .../format_stackdriver_test.c | 19 ++++++-- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/src/utils/format_stackdriver/format_stackdriver.c b/src/utils/format_stackdriver/format_stackdriver.c index 9e9df4cf34..fc434ef12d 100644 --- a/src/utils/format_stackdriver/format_stackdriver.c +++ b/src/utils/format_stackdriver/format_stackdriver.c @@ -134,16 +134,24 @@ static int format_typed_value(yajl_gen gen, metric_t const *m, switch (m->family->type) { case METRIC_TYPE_GAUGE: { - int status = json_string(gen, "doubleValue"); - if (status != 0) + int status = json_string(gen, "doubleValue") || + (int)yajl_gen_double(gen, m->value.gauge); + if (status != 0) { return status; + } + break; + } + case METRIC_TYPE_FPCOUNTER: { + /* Counter resets are handled in format_time_series(). */ + assert(m->value.fpcounter >= start_value.fpcounter); - status = (int)yajl_gen_double(gen, (double)m->value.gauge); - if (status != yajl_gen_status_ok) + fpcounter_t diff = m->value.fpcounter - start_value.fpcounter; + int status = + json_string(gen, "doubleValue") || (int)yajl_gen_double(gen, diff); + if (status != 0) { return status; - - yajl_gen_map_close(gen); - return 0; + } + break; } case METRIC_TYPE_COUNTER: { /* Counter resets are handled in format_time_series(). */ @@ -160,8 +168,7 @@ static int format_typed_value(yajl_gen gen, metric_t const *m, break; } case METRIC_TYPE_UNTYPED: - default: - ERROR("format_typed_value: invalid metric type: %d", m->family->type); + ERROR("format_typed_value: invalid metric type %d.", m->family->type); return EINVAL; } @@ -179,14 +186,14 @@ static int format_typed_value(yajl_gen gen, metric_t const *m, static int format_metric_kind(yajl_gen gen, metric_t const *m) { switch (m->family->type) { case METRIC_TYPE_GAUGE: - case METRIC_TYPE_UNTYPED: return json_string(gen, "GAUGE"); case METRIC_TYPE_COUNTER: + case METRIC_TYPE_FPCOUNTER: return json_string(gen, "CUMULATIVE"); - default: - ERROR("format_metric_kind: unknown value type %d.", m->family->type); - return EINVAL; + case METRIC_TYPE_UNTYPED: } + ERROR("format_metric_kind: invalid metric type %d.", m->family->type); + return EINVAL; } /* ValueType @@ -199,14 +206,14 @@ static int format_metric_kind(yajl_gen gen, metric_t const *m) { static int format_value_type(yajl_gen gen, metric_t const *m) { switch (m->family->type) { case METRIC_TYPE_GAUGE: - case METRIC_TYPE_UNTYPED: + case METRIC_TYPE_FPCOUNTER: return json_string(gen, "DOUBLE"); case METRIC_TYPE_COUNTER: return json_string(gen, "INT64"); - default: - ERROR("format_value_type: unknown value type %d.", m->family->type); - return EINVAL; + case METRIC_TYPE_UNTYPED: } + ERROR("format_metric_kind: invalid metric type %d.", m->family->type); + return EINVAL; } static int metric_type(strbuf_t *buf, metric_t const *m) { @@ -264,7 +271,8 @@ static int format_time_interval(yajl_gen gen, metric_t const *m, if (status != 0) return status; - if (m->family->type == METRIC_TYPE_COUNTER) { + if (m->family->type == METRIC_TYPE_COUNTER || + m->family->type == METRIC_TYPE_FPCOUNTER) { int status = json_string(gen, "startTime") || json_time(gen, start_time); if (status != 0) return status; @@ -377,6 +385,7 @@ static int format_time_series(yajl_gen gen, metric_t const *m, } break; case METRIC_TYPE_COUNTER: + case METRIC_TYPE_FPCOUNTER: // for cumulative metrics the interval must not be zero. if (start.time == m->time) { return EAGAIN; diff --git a/src/utils/format_stackdriver/format_stackdriver_test.c b/src/utils/format_stackdriver/format_stackdriver_test.c index 8597bcc07f..b39913a511 100644 --- a/src/utils/format_stackdriver/format_stackdriver_test.c +++ b/src/utils/format_stackdriver/format_stackdriver_test.c @@ -32,6 +32,7 @@ DEF_TEST(sd_format_metric_descriptor) { label_pair_t *labels; size_t labels_num; char *want; + int want_err; } cases[] = { { .name = "gauge_metric", @@ -47,12 +48,17 @@ DEF_TEST(sd_format_metric_descriptor) { "counter_metric\",\"metricKind\":\"CUMULATIVE\"," "\"valueType\":\"INT64\",\"labels\":[]}", }, + { + .name = "fpcounter_metric", + .type = METRIC_TYPE_FPCOUNTER, + .want = "{\"type\":\"custom.googleapis.com/collectd/" + "fpcounter_metric\",\"metricKind\":\"CUMULATIVE\"," + "\"valueType\":\"DOUBLE\",\"labels\":[]}", + }, { .name = "untyped_metric", .type = METRIC_TYPE_UNTYPED, - .want = "{\"type\":\"custom.googleapis.com/collectd/" - "untyped_metric\",\"metricKind\":\"GAUGE\",\"valueType\":" - "\"DOUBLE\",\"labels\":[]}", + .want_err = 1, }, { .name = "metric_with_labels", @@ -78,6 +84,7 @@ DEF_TEST(sd_format_metric_descriptor) { }; for (size_t i = 0; i < (sizeof(cases) / sizeof(cases[0])); i++) { + printf("## Case %zu: %s\n", i, cases[i].name); metric_family_t fam = { .name = cases[i].name, .type = cases[i].type, @@ -91,7 +98,11 @@ DEF_TEST(sd_format_metric_descriptor) { } strbuf_t buf = STRBUF_CREATE; - EXPECT_EQ_INT(0, sd_format_metric_descriptor(&buf, &m)); + EXPECT_EQ_INT(cases[i].want_err, sd_format_metric_descriptor(&buf, &m)); + if (cases[i].want_err) { + continue; + } + EXPECT_EQ_STR(cases[i].want, buf.ptr); STRBUF_DESTROY(buf); From 0423bfff6e76147274551c2494dfc1c8cbe6247c Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 08:46:29 +0100 Subject: [PATCH 10/20] write_prometheus plugin: Add support for `METRIC_TYPE_FPCOUNTER`. --- src/write_prometheus.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/write_prometheus.c b/src/write_prometheus.c index 430cf3b726..58e526207b 100644 --- a/src/write_prometheus.c +++ b/src/write_prometheus.c @@ -267,7 +267,7 @@ void format_metric_family_name(strbuf_t *buf, metric_family_t const *fam) { strbuf_print_restricted(buf, fam->unit, VALID_NAME_CHARS, '_'); } - if (fam->type == METRIC_TYPE_COUNTER) { + if (fam->type == METRIC_TYPE_COUNTER || fam->type == METRIC_TYPE_FPCOUNTER) { strbuf_print(buf, "_total"); } } @@ -283,6 +283,7 @@ void format_metric_family(strbuf_t *buf, metric_family_t const *prom_fam) { type = "gauge"; break; case METRIC_TYPE_COUNTER: + case METRIC_TYPE_FPCOUNTER: type = "counter"; break; case METRIC_TYPE_UNTYPED: From 2009745bfa80822ba775073f5412ad8102859ebc Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 10:42:16 +0100 Subject: [PATCH 11/20] format_{graphite,influxdb,kairosdb,stackdriver}: Fix compiler warning. Some compilers don't like empty statements, e.g.: ``` src/utils/format_kairosdb/format_kairosdb.c:122:8: error: label at end of compound statement 122 | case METRIC_TYPE_UNTYPED: | ^~~~~~~~~~~~~~~~~~~ ``` --- src/utils/format_graphite/format_graphite.c | 1 + src/utils/format_influxdb/format_influxdb.c | 1 + src/utils/format_kairosdb/format_kairosdb.c | 1 + src/utils/format_stackdriver/format_stackdriver.c | 2 ++ 4 files changed, 5 insertions(+) diff --git a/src/utils/format_graphite/format_graphite.c b/src/utils/format_graphite/format_graphite.c index eb9bf21212..1f466bf7f8 100644 --- a/src/utils/format_graphite/format_graphite.c +++ b/src/utils/format_graphite/format_graphite.c @@ -60,6 +60,7 @@ static int gr_format_values(strbuf_t *buf, metric_t const *m, gauge_t rate, case METRIC_TYPE_GAUGE: return format_double(buf, m->value.gauge); case METRIC_TYPE_UNTYPED: + break; } P_ERROR("gr_format_values: Unknown data source type: %d", m->family->type); diff --git a/src/utils/format_influxdb/format_influxdb.c b/src/utils/format_influxdb/format_influxdb.c index bef8fa5181..2735352e63 100644 --- a/src/utils/format_influxdb/format_influxdb.c +++ b/src/utils/format_influxdb/format_influxdb.c @@ -84,6 +84,7 @@ static int format_metric_value(strbuf_t *sb, metric_t const *m, case METRIC_TYPE_FPCOUNTER: return strbuf_printf(sb, "value=" GAUGE_FORMAT, m->value.fpcounter); case METRIC_TYPE_UNTYPED: + break; } ERROR("format_influxdb plugin: invalid metric type: %d", m->family->type); diff --git a/src/utils/format_kairosdb/format_kairosdb.c b/src/utils/format_kairosdb/format_kairosdb.c index 8838cf7422..ad9d944071 100644 --- a/src/utils/format_kairosdb/format_kairosdb.c +++ b/src/utils/format_kairosdb/format_kairosdb.c @@ -120,6 +120,7 @@ static int json_add_value(yajl_gen g, metric_t const *m, CHECK(yajl_gen_integer(g, (long long int)m->value.counter)); return 0; case METRIC_TYPE_UNTYPED: + break; } ERROR("format_kairosdb: Invalid metric type: %d", m->family->type); diff --git a/src/utils/format_stackdriver/format_stackdriver.c b/src/utils/format_stackdriver/format_stackdriver.c index fc434ef12d..ec15bdead8 100644 --- a/src/utils/format_stackdriver/format_stackdriver.c +++ b/src/utils/format_stackdriver/format_stackdriver.c @@ -191,6 +191,7 @@ static int format_metric_kind(yajl_gen gen, metric_t const *m) { case METRIC_TYPE_FPCOUNTER: return json_string(gen, "CUMULATIVE"); case METRIC_TYPE_UNTYPED: + break; } ERROR("format_metric_kind: invalid metric type %d.", m->family->type); return EINVAL; @@ -211,6 +212,7 @@ static int format_value_type(yajl_gen gen, metric_t const *m) { case METRIC_TYPE_COUNTER: return json_string(gen, "INT64"); case METRIC_TYPE_UNTYPED: + break; } ERROR("format_metric_kind: invalid metric type %d.", m->family->type); return EINVAL; From 7fbe2a0fcc846c07ba4a8c95e5c4a6bda289313a Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 13:57:20 +0100 Subject: [PATCH 12/20] value_list: Move `DS_TYPE_TO_STRING` out of `src/daemon/plugin.h`. --- src/daemon/plugin.h | 6 ------ src/utils/value_list/value_list.h | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/daemon/plugin.h b/src/daemon/plugin.h index ceca96d08d..4818d6311d 100644 --- a/src/daemon/plugin.h +++ b/src/daemon/plugin.h @@ -40,12 +40,6 @@ #include #include -#define DS_TYPE_TO_STRING(t) \ - (t == DS_TYPE_COUNTER) ? "counter" \ - : (t == DS_TYPE_GAUGE) ? "gauge" \ - : (t == DS_TYPE_DERIVE) ? "derive" \ - : "unknown" - #ifndef LOG_ERR #define LOG_ERR 3 #endif diff --git a/src/utils/value_list/value_list.h b/src/utils/value_list/value_list.h index 9ba6429b6e..7e27e3bbcf 100644 --- a/src/utils/value_list/value_list.h +++ b/src/utils/value_list/value_list.h @@ -36,6 +36,12 @@ #define DS_TYPE_GAUGE METRIC_TYPE_GAUGE #define DS_TYPE_DERIVE (65536 + METRIC_ATTR_CUMULATIVE + 1) +#define DS_TYPE_TO_STRING(t) \ + (t == DS_TYPE_COUNTER) ? "counter" \ + : (t == DS_TYPE_GAUGE) ? "gauge" \ + : (t == DS_TYPE_DERIVE) ? "derive" \ + : "unknown" + struct value_list_s { value_t *values; size_t values_len; From 0943758dd7fa16071955210c7ad9b3bfb05888a5 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 13:58:51 +0100 Subject: [PATCH 13/20] src/daemon/metric.h: Order metric types by value. --- src/daemon/metric.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/daemon/metric.h b/src/daemon/metric.h index c2e9d0f779..947a1dfea6 100644 --- a/src/daemon/metric.h +++ b/src/daemon/metric.h @@ -37,9 +37,9 @@ typedef enum { METRIC_TYPE_UNTYPED = 0, + METRIC_TYPE_GAUGE = METRIC_ATTR_DOUBLE, 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; From b5cef7981e88acbf5e4524ed92fe22578c4d0377 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 14:49:20 +0100 Subject: [PATCH 14/20] format_json: Don't access `(value_t).gauge` when the type is `FPCOUNTER`. --- src/utils/format_json/open_telemetry.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/utils/format_json/open_telemetry.c b/src/utils/format_json/open_telemetry.c index 8a510d4f2f..3b5ac6ba3d 100644 --- a/src/utils/format_json/open_telemetry.c +++ b/src/utils/format_json/open_telemetry.c @@ -97,6 +97,9 @@ static int number_data_point(yajl_gen g, metric_t const *m) { CHECK(yajl_gen_integer(g, m->value.counter)); break; case METRIC_TYPE_FPCOUNTER: + CHECK(json_add_string(g, "asDouble")); + CHECK(yajl_gen_double(g, m->value.fpcounter)); + break; case METRIC_TYPE_GAUGE: CHECK(json_add_string(g, "asDouble")); CHECK(yajl_gen_double(g, m->value.gauge)); From d5e0a24d67027a15a3d966e3e577f60244c32b2f Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 14:51:19 +0100 Subject: [PATCH 15/20] format_kairosdb: Log an error when `uc_get_rate()` fails. --- src/utils/format_kairosdb/format_kairosdb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils/format_kairosdb/format_kairosdb.c b/src/utils/format_kairosdb/format_kairosdb.c index ad9d944071..d8f92a1950 100644 --- a/src/utils/format_kairosdb/format_kairosdb.c +++ b/src/utils/format_kairosdb/format_kairosdb.c @@ -85,6 +85,7 @@ static int json_add_rate(yajl_gen g, metric_t const *m) { gauge_t rate = NAN; int err = uc_get_rate(m, &rate); if (err) { + ERROR("format_kairosdb: uc_get_rate failed: %s", STRERROR(err)); return err; } From 1b0d86a4ad16121761a8ff3bee8b6720f98740a0 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 31 Jan 2024 15:03:24 +0100 Subject: [PATCH 16/20] src/daemon/metric.h: Add an `IS_CUMULATIVE` macro. --- src/daemon/metric.h | 2 ++ src/utils/format_stackdriver/format_stackdriver.c | 3 +-- src/write_prometheus.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/daemon/metric.h b/src/daemon/metric.h index 947a1dfea6..0cbc50882f 100644 --- a/src/daemon/metric.h +++ b/src/daemon/metric.h @@ -42,6 +42,8 @@ typedef enum { METRIC_TYPE_FPCOUNTER = METRIC_ATTR_DOUBLE | METRIC_ATTR_CUMULATIVE, } metric_type_t; +#define IS_CUMULATIVE(t) ((t)&METRIC_ATTR_CUMULATIVE) + typedef uint64_t counter_t; typedef double fpcounter_t; typedef double gauge_t; diff --git a/src/utils/format_stackdriver/format_stackdriver.c b/src/utils/format_stackdriver/format_stackdriver.c index ec15bdead8..2976a66f41 100644 --- a/src/utils/format_stackdriver/format_stackdriver.c +++ b/src/utils/format_stackdriver/format_stackdriver.c @@ -273,8 +273,7 @@ static int format_time_interval(yajl_gen gen, metric_t const *m, if (status != 0) return status; - if (m->family->type == METRIC_TYPE_COUNTER || - m->family->type == METRIC_TYPE_FPCOUNTER) { + if (IS_CUMULATIVE(m->family->type)) { int status = json_string(gen, "startTime") || json_time(gen, start_time); if (status != 0) return status; diff --git a/src/write_prometheus.c b/src/write_prometheus.c index 58e526207b..893d680a70 100644 --- a/src/write_prometheus.c +++ b/src/write_prometheus.c @@ -267,7 +267,7 @@ void format_metric_family_name(strbuf_t *buf, metric_family_t const *fam) { strbuf_print_restricted(buf, fam->unit, VALID_NAME_CHARS, '_'); } - if (fam->type == METRIC_TYPE_COUNTER || fam->type == METRIC_TYPE_FPCOUNTER) { + if (IS_CUMULATIVE(fam->type)) { strbuf_print(buf, "_total"); } } From d61b3d02f4a83c5971363bdbff57d17f7440e6b8 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Thu, 1 Feb 2024 16:19:27 +0100 Subject: [PATCH 17/20] src/daemon/utils_cache.c: Avoid `default` in the `family->type` switch statement. This way the compiler can check that we handle all values in the `metric_type_t` enum. --- src/daemon/utils_cache.c | 115 ++++++++++++++++++--------------------- 1 file changed, 53 insertions(+), 62 deletions(-) diff --git a/src/daemon/utils_cache.c b/src/daemon/utils_cache.c index df0a552ad9..69fe06d8d0 100644 --- a/src/daemon/utils_cache.c +++ b/src/daemon/utils_cache.c @@ -150,27 +150,12 @@ static int uc_insert(metric_t const *m, char const *key) { .last_update = cdtime(), .interval = m->interval, .state = STATE_UNKNOWN, + .values_gauge = NAN, }; sstrncpy(ce->name, key, sizeof(ce->name)); - switch (m->family->type) { - 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 METRIC_TYPE_GAUGE: + if (m->family->type == METRIC_TYPE_GAUGE) { ce->values_gauge = m->value.gauge; - break; - - default: - /* This shouldn't happen. */ - ERROR("uc_insert: Unexpected metric type %d.", m->family->type); - sfree(key_copy); - cache_free(ce); - return -1; } if (c_avl_insert(cache_tree, key_copy, ce) != 0) { @@ -294,6 +279,54 @@ int uc_check_timeout(void) { return 0; } /* int uc_check_timeout */ +static int uc_update_rate(metric_t const *m, cache_entry_t *ce) { + switch (m->family->type) { + 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 + // reset, we still provide a non-NAN rate value. In case of a counter + // reset, the rate value will likely be unreasonably huge. + if (ce->last_value.counter > m->value.counter) { + // counter reset + ce->first_time = m->time; + ce->first_value = m->value; + } + counter_t diff = counter_diff(ce->last_value.counter, m->value.counter); + ce->values_gauge = + ((double)diff) / CDTIME_T_TO_DOUBLE(m->time - ce->last_time); + return 0; + } + + 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; + return 0; + } + gauge_t diff = m->value.fpcounter - ce->last_value.fpcounter; + 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; + return 0; + } + + case METRIC_TYPE_UNTYPED: + break; + } /* switch (m->family->type) */ + + /* This shouldn't happen. */ + ERROR("uc_update: Invalid metric type: %d", m->family->type); + return EINVAL; +} + static int uc_update_metric(metric_t const *m) { strbuf_t buf = STRBUF_CREATE; int status = metric_identity(&buf, m); @@ -332,54 +365,12 @@ static int uc_update_metric(metric_t const *m) { return -1; } - switch (m->family->type) { - 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 - // reset, we still provide a non-NAN rate value. In case of a counter - // reset, the rate value will likely be unreasonably huge. - if (ce->last_value.counter > m->value.counter) { - // counter reset - ce->first_time = m->time; - ce->first_value = m->value; - } - counter_t diff = counter_diff(ce->last_value.counter, m->value.counter); - ce->values_gauge = - ((double)diff) / CDTIME_T_TO_DOUBLE(m->time - ce->last_time); - break; - } - - 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; - } - - case METRIC_TYPE_GAUGE: { - ce->values_gauge = m->value.gauge; - break; - } - - case METRIC_TYPE_UNTYPED: - default: { - /* This shouldn't happen. */ + int err = uc_update_rate(m, ce); + if (err) { pthread_mutex_unlock(&cache_lock); - ERROR("uc_update: Don't know how to handle data source type %i.", - m->family->type); STRBUF_DESTROY(buf); - return -1; + return err; } - } /* switch (m->family->type) */ DEBUG("uc_update: %s = %f", buf.ptr, ce->values_gauge); From 60ac462be4a2c390494821c5dd89721c425a2e71 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Thu, 1 Feb 2024 16:26:14 +0100 Subject: [PATCH 18/20] format_open_telemetry: Move the "never reached" comment to `add_metric()`. --- src/daemon/utils_cache.c | 2 +- .../format_open_telemetry/format_open_telemetry.cc | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/daemon/utils_cache.c b/src/daemon/utils_cache.c index 69fe06d8d0..13fb7e444d 100644 --- a/src/daemon/utils_cache.c +++ b/src/daemon/utils_cache.c @@ -323,7 +323,7 @@ static int uc_update_rate(metric_t const *m, cache_entry_t *ce) { } /* switch (m->family->type) */ /* This shouldn't happen. */ - ERROR("uc_update: Invalid metric type: %d", m->family->type); + ERROR("uc_update: invalid metric type: %d", m->family->type); return EINVAL; } diff --git a/src/utils/format_open_telemetry/format_open_telemetry.cc b/src/utils/format_open_telemetry/format_open_telemetry.cc index 9e1daffc09..584c2318bc 100644 --- a/src/utils/format_open_telemetry/format_open_telemetry.cc +++ b/src/utils/format_open_telemetry/format_open_telemetry.cc @@ -80,8 +80,7 @@ static void metric_to_number_data_point(NumberDataPoint *dp, // cover other values. break; } - // never reached, only here to make the compiler happy - ERROR("format_open_telemetry: unexpected metric type: %d", m->family->type); + ERROR("format_open_telemetry: invalid metric type: %d", m->family->type); } static void set_sum(Metric *m, metric_family_t const *fam) { @@ -134,12 +133,11 @@ static void add_metric(ScopeMetrics *sm, metric_family_t const *fam) { set_gauge(m, fam); return; case METRIC_TYPE_UNTYPED: - // Fall through. This case signals the compiler that we're checking all - // values of the enum. We report an error outside of the switch to also - // cover other values. + // Never reached, only here to show the compiler we're handling all possible + // `metric_type_t` values. break; } - ERROR("format_open_telemetry: unexpected metric type: %d", fam->type); + ERROR("format_open_telemetry: invalid metric type: %d", fam->type); } static void set_instrumentation_scope(ScopeMetrics *sm) { From 1a917d8d9066d9d6b81a4651f892df0e89502e6a Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Thu, 1 Feb 2024 16:33:14 +0100 Subject: [PATCH 19/20] src/utils/value_list/value_list.h: Drop outdated comment. --- src/utils/value_list/value_list.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/value_list/value_list.h b/src/utils/value_list/value_list.h index 7e27e3bbcf..bad84c275d 100644 --- a/src/utils/value_list/value_list.h +++ b/src/utils/value_list/value_list.h @@ -30,7 +30,7 @@ #define UTILS_VALUE_LIST_H 1 #include "daemon/data_set.h" -#include "daemon/metric.h" // for value_t +#include "daemon/metric.h" #define DS_TYPE_COUNTER METRIC_TYPE_COUNTER #define DS_TYPE_GAUGE METRIC_TYPE_GAUGE From 4573fe6ca47e2dc51384455a165ede6209ac4033 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Fri, 2 Feb 2024 08:42:17 +0100 Subject: [PATCH 20/20] format_stackdriver: Remove `int` cast from `yajl_gen_double()`. --- src/utils/format_stackdriver/format_stackdriver.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/utils/format_stackdriver/format_stackdriver.c b/src/utils/format_stackdriver/format_stackdriver.c index 2976a66f41..5c027e0101 100644 --- a/src/utils/format_stackdriver/format_stackdriver.c +++ b/src/utils/format_stackdriver/format_stackdriver.c @@ -134,8 +134,8 @@ static int format_typed_value(yajl_gen gen, metric_t const *m, switch (m->family->type) { case METRIC_TYPE_GAUGE: { - int status = json_string(gen, "doubleValue") || - (int)yajl_gen_double(gen, m->value.gauge); + int status = + json_string(gen, "doubleValue") || yajl_gen_double(gen, m->value.gauge); if (status != 0) { return status; } @@ -146,8 +146,7 @@ static int format_typed_value(yajl_gen gen, metric_t const *m, assert(m->value.fpcounter >= start_value.fpcounter); fpcounter_t diff = m->value.fpcounter - start_value.fpcounter; - int status = - json_string(gen, "doubleValue") || (int)yajl_gen_double(gen, diff); + int status = json_string(gen, "doubleValue") || yajl_gen_double(gen, diff); if (status != 0) { return status; }