Metrics: Refactor to drop usage of strv

This addresses Daan's feedback on #39202

diff --git a/src/core/varlink-metrics.c b/src/core/varlink-metrics.c
index c64d850367b1fbc799f5cc76473ebfa75b686ac9..3929f3a621e55c1daa35054ef37f72d5721d92f9 100644 (file)
--- a/src/core/varlink-metrics.c
+++ b/src/core/varlink-metrics.c
@@ -25,7 +25,7 @@ static int unit_active_state_build_json(MetricFamilyContext *context, void *user
                                 context,
                                 unit->id,
                                 unit_active_state_to_string(unit_active_state(unit)),
-                                /* field_pairs= */ NULL);
+                                /* fields= */ NULL);
                 if (r < 0)
                         return r;
         }
@@ -50,7 +50,7 @@ static int unit_load_state_build_json(MetricFamilyContext *context, void *userda
                                 context,
                                 unit->id,
                                 unit_load_state_to_string(unit->load_state),
-                                /* field_pairs= */ NULL);
+                                /* fields= */ NULL);
                 if (r < 0)
                         return r;
         }
@@ -66,7 +66,7 @@ static int nrestarts_build_json(MetricFamilyContext *context, void *userdata) {
 
         LIST_FOREACH(units_by_type, unit, manager->units_by_type[UNIT_SERVICE]) {
                 r = metric_build_send_unsigned(
-                                context, unit->id, SERVICE(unit)->n_restarts, /* field_pairs= */ NULL);
+                                context, unit->id, SERVICE(unit)->n_restarts, /* fields= */ NULL);
                 if (r < 0)
                         return r;
         }
@@ -81,16 +81,21 @@ static int units_by_type_total_build_json(MetricFamilyContext *context, void *us
         assert(context);
 
         for (UnitType type = 0; type < _UNIT_TYPE_MAX; type++) {
+                _cleanup_(sd_json_variant_unrefp) sd_json_variant *fields = NULL;
                 uint64_t counter = 0;
 
                 LIST_FOREACH(units_by_type, _u, manager->units_by_type[type])
                         counter++;
 
+                r = sd_json_buildo(&fields, SD_JSON_BUILD_PAIR_STRING("type", unit_type_to_string(type)));
+                if (r < 0)
+                        return r;
+
                 r = metric_build_send_unsigned(
                                 context,
                                 /* object= */ NULL,
                                 counter,
-                                STRV_MAKE("type", unit_type_to_string(type)));
+                                fields);
                 if (r < 0)
                         return r;
         }
@@ -117,11 +122,17 @@ static int units_by_state_total_build_json(MetricFamilyContext *context, void *u
         }
 
         for (UnitActiveState state = 0; state < _UNIT_ACTIVE_STATE_MAX; state++) {
+                _cleanup_(sd_json_variant_unrefp) sd_json_variant *fields = NULL;
+
+                r = sd_json_buildo(&fields, SD_JSON_BUILD_PAIR_STRING("state", unit_active_state_to_string(state)));
+                if (r < 0)
+                        return r;
+
                 r = metric_build_send_unsigned(
                                 context,
                                 /* object= */ NULL,
                                 counters[state],
-                                STRV_MAKE("state", unit_active_state_to_string(state)));
+                                fields);
                 if (r < 0)
                         return r;
         }
@@ -135,31 +146,31 @@ const MetricFamily metric_family_table[] = {
          .name = METRIC_IO_SYSTEMD_MANAGER_PREFIX "nrestarts",
          .description = "Per unit metric: number of restarts",
          .type = METRIC_FAMILY_TYPE_COUNTER,
-         .generate_cb = nrestarts_build_json,
+         .generate = nrestarts_build_json,
         },
         {
          .name = METRIC_IO_SYSTEMD_MANAGER_PREFIX "unit_active_state",
          .description = "Per unit metric: active state",
          .type = METRIC_FAMILY_TYPE_STRING,
-         .generate_cb = unit_active_state_build_json,
+         .generate = unit_active_state_build_json,
         },
         {
          .name = METRIC_IO_SYSTEMD_MANAGER_PREFIX "unit_load_state",
          .description = "Per unit metric: load state",
          .type = METRIC_FAMILY_TYPE_STRING,
-         .generate_cb = unit_load_state_build_json,
+         .generate = unit_load_state_build_json,
         },
         {
          .name = METRIC_IO_SYSTEMD_MANAGER_PREFIX "units_by_state_total",
          .description = "Total number of units of different state",
          .type = METRIC_FAMILY_TYPE_GAUGE,
-         .generate_cb = units_by_state_total_build_json,
+         .generate = units_by_state_total_build_json,
         },
         {
          .name = METRIC_IO_SYSTEMD_MANAGER_PREFIX "units_by_type_total",
          .description = "Total number of units of different types",
          .type = METRIC_FAMILY_TYPE_GAUGE,
-         .generate_cb = units_by_type_total_build_json,
+         .generate = units_by_type_total_build_json,
         },
         {}
 };

diff --git a/src/libsystemd/sd-common/sd-forward.h b/src/libsystemd/sd-common/sd-forward.h
index 0e9e16d91f9a221743c95bd6db0af4184b9e23b2..8abe655209dec9cc7d0ebb08daf39b75613e5ad2 100644 (file)
--- a/src/libsystemd/sd-common/sd-forward.h
+++ b/src/libsystemd/sd-common/sd-forward.h
@@ -119,6 +119,8 @@ typedef struct sd_varlink_field sd_varlink_field;
 typedef struct sd_varlink_symbol sd_varlink_symbol;
 typedef struct sd_varlink_interface sd_varlink_interface;
 
+typedef int (*sd_varlink_method_t)(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata);
+
 typedef struct sd_journal sd_journal;
 
 typedef struct sd_resolve sd_resolve;

diff --git a/src/shared/metrics.c b/src/shared/metrics.c
index 3cda00f92625fc14ef963d8819bbba13afd54756..0ce406b7f5c4b1fa353bf771f12882886a4b7bf3 100644 (file)
--- a/src/shared/metrics.c
+++ b/src/shared/metrics.c
@@ -4,7 +4,6 @@
 #include "log.h"
 #include "metrics.h"
 #include "string-table.h"
-#include "strv.h"
 #include "varlink-io.systemd.Metrics.h"
 #include "varlink-util.h"
 
@@ -54,7 +53,7 @@ int metrics_setup_varlink_server(
 
         r = sd_varlink_server_attach_event(s, event, SD_EVENT_PRIORITY_NORMAL);
         if (r < 0)
-                return log_debug_errno(r, "Failed to attach varlink metrics connection to event loop: %m");
+                return log_debug_errno(r, "Failed to attach varlink metrics server to event loop: %m");
 
         *server = TAKE_PTR(s);
 
@@ -154,10 +153,10 @@ int metrics_method_list(
 
         _cleanup_(metric_family_context_done) MetricFamilyContext ctx = { .link = link };
         for (const MetricFamily *mf = metric_family_table; mf && mf->name; mf++) {
-                assert(mf->generate_cb);
+                assert(mf->generate);
 
                 ctx.metric_family = mf;
-                r = mf->generate_cb(&ctx, userdata);
+                r = mf->generate(&ctx, userdata);
                 if (r < 0)
                         return log_debug_errno(
                                         r, "Failed to list metrics for metric family '%s': %m", mf->name);
@@ -170,45 +169,7 @@ int metrics_method_list(
         return sd_varlink_reply(link, ctx.previous);
 }
 
-static int metric_set_fields(sd_json_variant **v, char **field_pairs) {
-        _cleanup_(sd_json_variant_unrefp) sd_json_variant *w = NULL;
-        size_t n;
-        int r;
-
-        assert(v);
-
-        n = strv_length(field_pairs);
-        if (n == 0)
-                return 0;
-
-        if (n % 2 != 0)
-                return log_debug_errno(SYNTHETIC_ERRNO(ERANGE), "Odd number of field pairs: %zu", n);
-
-        sd_json_variant **array = new0(sd_json_variant *, n);
-        if (!array)
-                return log_oom();
-
-        CLEANUP_ARRAY(array, n, sd_json_variant_unref_many);
-
-        int i = 0;
-        STRV_FOREACH_PAIR(key, value, field_pairs) {
-                r = sd_json_variant_new_string(&array[i++], *key);
-                if (r < 0)
-                        return log_debug_errno(r, "Failed to create key variant: %m");
-
-                r = sd_json_variant_new_string(&array[i++], *value);
-                if (r < 0)
-                        return log_debug_errno(r, "Failed to create value variant: %m");
-        }
-
-        r = sd_json_variant_new_object(&w, array, n);
-        if (r < 0)
-                return log_debug_errno(r, "Failed to allocate JSON object: %m");
-
-        return sd_json_variant_set_field(v, "fields", w);
-}
-
-static int metric_build_send(MetricFamilyContext *context, const char *object, sd_json_variant *value, char **field_pairs) {
+static int metric_build_send(MetricFamilyContext *context, const char *object, sd_json_variant *value, sd_json_variant *fields) {
         _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL;
         int r;
 
@@ -217,21 +178,24 @@ static int metric_build_send(MetricFamilyContext *context, const char *object, s
         assert(context->link);
         assert(context->metric_family);
 
+        if (fields) {
+                assert(sd_json_variant_is_object(fields));
+
+                _unused_ const char *k;
+                _unused_ sd_json_variant *e;
+                JSON_VARIANT_OBJECT_FOREACH(k, e, fields)
+                        assert(sd_json_variant_is_string(e));
+        }
+
         r = sd_json_buildo(
                         &v,
                         SD_JSON_BUILD_PAIR_STRING("name", context->metric_family->name),
                         JSON_BUILD_PAIR_STRING_NON_EMPTY("object", object),
-                        SD_JSON_BUILD_PAIR("value", SD_JSON_BUILD_VARIANT(value)));
-        /* TODO JSON_BUILD_PAIR_OBJECT_STRV_NOT_NULL */
+                        SD_JSON_BUILD_PAIR("value", SD_JSON_BUILD_VARIANT(value)),
+                        JSON_BUILD_PAIR_VARIANT_NON_NULL("fields", fields));
         if (r < 0)
                 return r;
 
-        if (field_pairs) { /* NULL => no fields object, empty strv => fields:{} */
-                r = metric_set_fields(&v, field_pairs);
-                if (r < 0)
-                        return r;
-        }
-
         if (context->previous) {
                 r = sd_varlink_notify(context->link, context->previous);
                 if (r < 0)
@@ -245,7 +209,7 @@ static int metric_build_send(MetricFamilyContext *context, const char *object, s
         return 0;
 }
 
-int metric_build_send_string(MetricFamilyContext *context, const char *object, const char *value, char **field_pairs) {
+int metric_build_send_string(MetricFamilyContext *context, const char *object, const char *value, sd_json_variant *fields) {
         _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL;
         int r;
 
@@ -255,10 +219,10 @@ int metric_build_send_string(MetricFamilyContext *context, const char *object, c
         if (r < 0)
                 return log_debug_errno(r, "Failed to allocate JSON string: %m");
 
-        return metric_build_send(context, object, v, field_pairs);
+        return metric_build_send(context, object, v, fields);
 }
 
-int metric_build_send_unsigned(MetricFamilyContext *context, const char *object, uint64_t value, char **field_pairs) {
+int metric_build_send_unsigned(MetricFamilyContext *context, const char *object, uint64_t value, sd_json_variant *fields) {
         _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL;
         int r;
 
@@ -266,5 +230,5 @@ int metric_build_send_unsigned(MetricFamilyContext *context, const char *object,
         if (r < 0)
                 return log_debug_errno(r, "Failed to allocate JSON unsigned: %m");
 
-        return metric_build_send(context, object, v, field_pairs);
+        return metric_build_send(context, object, v, fields);
 }

diff --git a/src/shared/metrics.h b/src/shared/metrics.h
index a953ae5779469e184ec33f032009a5ddcd4f3f50..18eb5cdad42088c7788a980bdd3325ce38307447 100644 (file)
--- a/src/shared/metrics.h
+++ b/src/shared/metrics.h
@@ -1,9 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
-#include "sd-varlink.h"
-
-#include "macro-fundamental.h"
+#include "shared-forward.h"
 
 typedef enum MetricFamilyType {
         METRIC_FAMILY_TYPE_COUNTER,
@@ -21,13 +19,13 @@ typedef struct MetricFamilyContext {
         sd_json_variant *previous;
 } MetricFamilyContext;
 
-typedef int (*metric_family_generate_cb_t) (MetricFamilyContext *mfc, void *userdata);
+typedef int (*metric_family_generate_func_t) (MetricFamilyContext *mfc, void *userdata);
 
 typedef struct MetricFamily {
         const char *name;
         const char *description;
         MetricFamilyType type;
-        metric_family_generate_cb_t generate_cb;
+        metric_family_generate_func_t generate;
 } MetricFamily;
 
 int metrics_setup_varlink_server(
@@ -38,9 +36,10 @@ int metrics_setup_varlink_server(
                 sd_varlink_method_t vl_method_describe_cb,
                 void *userdata);
 
-const char* metric_family_type_to_string(MetricFamilyType i) _const_;
+DECLARE_STRING_TABLE_LOOKUP_TO_STRING(metric_family_type, MetricFamilyType);
+
 int metrics_method_describe(const MetricFamily metric_family_table[], sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata);
 int metrics_method_list(const MetricFamily metric_family_table[], sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata);
 
-int metric_build_send_string(MetricFamilyContext* context, const char *object, const char *value, char **field_pairs);
-int metric_build_send_unsigned(MetricFamilyContext* context, const char *object, uint64_t value, char **field_pairs);
+int metric_build_send_string(MetricFamilyContext* context, const char *object, const char *value, sd_json_variant *fields);
+int metric_build_send_unsigned(MetricFamilyContext* context, const char *object, uint64_t value, sd_json_variant *fields);