From: Zbigniew Jędrzejewski-Szmek Date: Tue, 28 Apr 2026 21:36:15 +0000 (+0200) Subject: report: absorb "facts" into "metrics" X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e5687f689f20b051420fe0154ea4391af697ed7c;p=thirdparty%2Fsystemd.git report: absorb "facts" into "metrics" This gets rid of the duality in the cmdline interface and various APIs. The general plan is collect both "facts" and "metrics" in a single list. We have various producers which respond on the io.systemd.Facts endpoint. Those will need to be adjusted to respond to io.systemd.Metrics. Cmdline interface: 'metrics' (unchanged) 'describe-metrics' → 'describe' 'facts' → merged into 'metrics' 'describe-facts' → merged into 'describe' --- diff --git a/man/systemd-report.xml b/man/systemd-report.xml index b53a50c2f86..f14600dfe5d 100644 --- a/man/systemd-report.xml +++ b/man/systemd-report.xml @@ -18,7 +18,7 @@ systemd-report - Generate report of system facts and metrics + Generate report of system metrics @@ -33,7 +33,7 @@ Note: this command is experimental for now. While it is likely to become a regular component of systemd, it might still change in behaviour and interface. - systemd-report requests facts and metrics from the system and writes them to + systemd-report requests metrics from the system and writes them to standard output. @@ -56,14 +56,14 @@ - describe-metrics MATCH + describe MATCH Acquire a list of metric families from all local services providing them, and write them to standard output. This returns primarily static information about metrics, their data types and human readable description, without values. - Match expressions similar to those supported by metrics are supported for - describe-metrics, too. + Match expressions supported by metrics are supported by + describe too. diff --git a/src/report/report-upload.c b/src/report/report-upload.c index c64bf86e133..486e815e8d8 100644 --- a/src/report/report-upload.c +++ b/src/report/report-upload.c @@ -59,18 +59,10 @@ static int build_json_report(Context *context, sd_json_variant **ret) { usec_t ts = now(CLOCK_REALTIME); int r; - const char *ident; - if (IN_SET(context->action, ACTION_LIST_METRICS, ACTION_DESCRIBE_METRICS)) - ident = "metrics"; - else if (IN_SET(context->action, ACTION_LIST_FACTS, ACTION_DESCRIBE_FACTS)) - ident = "facts"; - else - assert_not_reached(); - r = sd_json_buildo(ret, SD_JSON_BUILD_PAIR("timestamp", SD_JSON_BUILD_STRING(FORMAT_TIMESTAMP_STYLE(ts, TIMESTAMP_UTC))), - SD_JSON_BUILD_PAIR(ident, + SD_JSON_BUILD_PAIR("metrics", SD_JSON_BUILD_VARIANT_ARRAY(context->metrics, context->n_metrics))); if (r < 0) return log_error_errno(r, "Failed to build JSON data: %m"); diff --git a/src/report/report.c b/src/report/report.c index fef01c094ef..feedcaa43a5 100644 --- a/src/report/report.c +++ b/src/report/report.c @@ -28,8 +28,8 @@ #include "verbs.h" #include "web-util.h" -#define METRICS_OR_FACTS_MAX 4096U -#define METRICS_OR_FACTS_LINKS_MAX 128U +#define METRICS_MAX 4096U +#define METRICS_LINKS_MAX 128U #define TIMEOUT_USEC (30 * USEC_PER_SEC) /* 30 seconds */ static PagerFlags arg_pager_flags = 0; @@ -87,8 +87,6 @@ DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( static const char* const action_method_table[] = { [ACTION_LIST_METRICS] = "io.systemd.Metrics.List", [ACTION_DESCRIBE_METRICS] = "io.systemd.Metrics.Describe", - [ACTION_LIST_FACTS] = "io.systemd.Facts.List", - [ACTION_DESCRIBE_FACTS] = "io.systemd.Facts.Describe", }; DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(action_method, Action); @@ -249,7 +247,7 @@ static int on_query_reply( goto finish; } - if (context->n_metrics >= METRICS_OR_FACTS_MAX) { + if (context->n_metrics >= METRICS_MAX) { context->n_skipped_metrics++; goto finish; } @@ -436,107 +434,6 @@ static int output_collected_describe(Context *context, Table **ret) { return 0; } -static int facts_output_list(Context *context, Table **ret) { - int r; - - assert(context); - assert(ret); - - _cleanup_(table_unrefp) Table *table = table_new("family", "object", "value"); - if (!table) - return log_oom(); - - table_set_ersatz_string(table, TABLE_ERSATZ_DASH); - table_set_sort(table, (size_t) 0, (size_t) 1, (size_t) 2); - - FOREACH_ARRAY(m, context->metrics, context->n_metrics) { - struct { - const char *name; - const char *object; - sd_json_variant *value; - } d = {}; - - static const sd_json_dispatch_field dispatch_table[] = { - { "name", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, voffsetof(d, name), SD_JSON_MANDATORY }, - { "object", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, voffsetof(d, object), 0 }, - { "value", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_variant_noref, voffsetof(d, value), SD_JSON_MANDATORY }, - {} - }; - - r = sd_json_dispatch(*m, dispatch_table, SD_JSON_ALLOW_EXTENSIONS, &d); - if (r < 0) { - _cleanup_free_ char *t = NULL; - int k = sd_json_variant_format(*m, /* flags= */ 0, &t); - if (k < 0) - return log_error_errno(k, "Failed to format JSON: %m"); - - log_warning_errno(r, "Cannot parse fact, skipping: %s", t); - continue; - } - - r = table_add_many( - table, - TABLE_STRING, d.name, - TABLE_STRING, d.object, - TABLE_JSON, d.value, - TABLE_SET_WEIGHT, 50U); - if (r < 0) - return table_log_add_error(r); - } - - *ret = TAKE_PTR(table); - return 0; -} - -static int facts_output_describe(Context *context, Table **ret) { - int r; - - assert(context); - assert(ret); - - _cleanup_(table_unrefp) Table *table = table_new("family", "description"); - if (!table) - return log_oom(); - - table_set_ersatz_string(table, TABLE_ERSATZ_DASH); - table_set_sort(table, (size_t) 0, (size_t) 1); - - FOREACH_ARRAY(m, context->metrics, context->n_metrics) { - struct { - const char *name; - const char *description; - } d = {}; - - static const sd_json_dispatch_field dispatch_table[] = { - { "name", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, voffsetof(d, name), SD_JSON_MANDATORY }, - { "description", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, voffsetof(d, description), 0 }, - {} - }; - - r = sd_json_dispatch(*m, dispatch_table, SD_JSON_ALLOW_EXTENSIONS, &d); - if (r < 0) { - _cleanup_free_ char *t = NULL; - int k = sd_json_variant_format(*m, /* flags= */ 0, &t); - if (k < 0) - return log_error_errno(k, "Failed to format JSON: %m"); - - log_warning_errno(r, "Cannot parse fact description, skipping: %s", t); - continue; - } - - r = table_add_many( - table, - TABLE_STRING, d.name, - TABLE_STRING, d.description, - TABLE_SET_WEIGHT, 50U); - if (r < 0) - return table_log_add_error(r); - } - - *ret = TAKE_PTR(table); - return 0; -} - static int output_collected(Context *context) { int r; @@ -555,13 +452,8 @@ static int output_collected(Context *context) { return log_error_errno(r, "Failed to write JSON: %m"); } - if (context->n_metrics == 0 && arg_legend) { - if (IN_SET(context->action, ACTION_LIST_FACTS, ACTION_DESCRIBE_FACTS)) - log_info("No facts collected."); - else - log_info("No metrics collected."); - } - + if (context->n_metrics == 0 && arg_legend) + log_info("No metrics collected."); return 0; } @@ -577,14 +469,6 @@ static int output_collected(Context *context) { r = output_collected_describe(context, &table); break; - case ACTION_LIST_FACTS: - r = facts_output_list(context, &table); - break; - - case ACTION_DESCRIBE_FACTS: - r = facts_output_describe(context, &table); - break; - default: assert_not_reached(); } @@ -598,12 +482,10 @@ static int output_collected(Context *context) { } if (arg_legend && !sd_json_format_enabled(arg_json_format_flags)) { - const char *type = IN_SET(context->action, ACTION_LIST_FACTS, ACTION_DESCRIBE_FACTS) ? "facts" : "metrics"; - if (table_isempty(table)) - printf("No %s available.\n", type); + printf("No metrics available.\n"); else - printf("\n%zu %s listed.\n", table_get_rows(table) - 1, type); + printf("\n%zu metrics listed.\n", table_get_rows(table) - 1); } return 0; @@ -707,7 +589,7 @@ static int readdir_sources(char **ret_directory, DirectoryEntries **ret) { VERB_FULL(verb_metrics, "metrics", "[MATCH…]", VERB_ANY, VERB_ANY, 0, ACTION_LIST_METRICS, "Acquire list of metrics and their values"); -VERB_FULL(verb_metrics, "describe-metrics", "[MATCH…]", VERB_ANY, VERB_ANY, 0, ACTION_DESCRIBE_METRICS, +VERB_FULL(verb_metrics, "describe", "[MATCH…]", VERB_ANY, VERB_ANY, 0, ACTION_DESCRIBE_METRICS, "Describe available metrics"); static int verb_metrics(int argc, char *argv[], uintptr_t data, void *userdata) { Action action = data; @@ -746,7 +628,7 @@ static int verb_metrics(int argc, char *argv[], uintptr_t data, void *userdata) FOREACH_ARRAY(i, de->entries, de->n_entries) { struct dirent *d = *i; - if (set_size(context.link_infos) >= METRICS_OR_FACTS_LINKS_MAX) { + if (set_size(context.link_infos) >= METRICS_LINKS_MAX) { n_skipped_sources++; break; } @@ -792,93 +674,6 @@ static int verb_metrics(int argc, char *argv[], uintptr_t data, void *userdata) return 0; } -VERB_FULL(verb_facts, "facts", "[MATCH…]", VERB_ANY, VERB_ANY, 0, ACTION_LIST_FACTS, - "Acquire list of facts and their values"); -VERB_FULL(verb_facts, "describe-facts", "[MATCH…]", VERB_ANY, VERB_ANY, 0, ACTION_DESCRIBE_FACTS, - "Describe available facts"); -static int verb_facts(int argc, char *argv[], uintptr_t data, void *userdata) { - Action action = data; - int r; - - assert(argc >= 1); - assert(argv); - assert(IN_SET(action, ACTION_LIST_FACTS, ACTION_DESCRIBE_FACTS)); - - /* Enable JSON-SEQ mode here, since we'll dump a large series of JSON objects */ - arg_json_format_flags |= SD_JSON_FORMAT_SEQ; - - r = parse_metrics_matches(argv + 1); - if (r < 0) - return r; - - _cleanup_(context_done) Context context = { - .action = action, - }; - size_t n_skipped_sources = 0; - - _cleanup_free_ DirectoryEntries *de = NULL; - _cleanup_free_ char *sources_path = NULL; - r = readdir_sources(&sources_path, &de); - if (r < 0) - return r; - if (r > 0) { - r = sd_event_default(&context.event); - if (r < 0) - return log_error_errno(r, "Failed to get event loop: %m"); - - r = sd_event_set_signal_exit(context.event, true); - if (r < 0) - return log_error_errno(r, "Failed to enable exit on SIGINT/SIGTERM: %m"); - - FOREACH_ARRAY(i, de->entries, de->n_entries) { - struct dirent *d = *i; - - if (set_size(context.link_infos) >= METRICS_OR_FACTS_LINKS_MAX) { - n_skipped_sources++; - break; - } - - _cleanup_free_ char *p = path_join(sources_path, d->d_name); - if (!p) - return log_oom(); - - (void) call_collect(&context, d->d_name, p); - } - } - - if (set_isempty(context.link_infos)) { - if (arg_legend) - log_info("No facts sources found."); - } else { - assert(context.event); - - r = sd_event_loop(context.event); - if (r < 0) - return log_error_errno(r, "Failed to run event loop: %m"); - - if (arg_url) - r = upload_collected(&context); - else - r = output_collected(&context); - if (r < 0) - return r; - } - - if (n_skipped_sources > 0) - return log_warning_errno(SYNTHETIC_ERRNO(EUCLEAN), - "Too many facts sources, only %u sources contacted, %zu sources skipped.", - set_size(context.link_infos), n_skipped_sources); - if (context.n_invalid_metrics > 0) - return log_warning_errno(SYNTHETIC_ERRNO(EUCLEAN), - "%zu facts are not valid.", - context.n_invalid_metrics); - if (context.n_skipped_metrics > 0) - return log_warning_errno(SYNTHETIC_ERRNO(EUCLEAN), - "Too many facts, only %zu facts collected, %zu facts skipped.", - context.n_metrics, context.n_skipped_metrics); - return 0; -} - VERB_NOARG(verb_list_sources, "list-sources", "Show list of known metrics sources"); static int verb_list_sources(int argc, char *argv[], uintptr_t _data, void *userdata) { int r; @@ -955,7 +750,7 @@ static int help(void) { (void) table_sync_column_widths(0, options, verbs); printf("%s [OPTIONS...] COMMAND ...\n" - "\n%sAcquire metrics and facts from local sources.%s\n" + "\n%sAcquire metrics from local sources.%s\n" "\n%sCommands:%s\n", program_invocation_short_name, ansi_highlight(), diff --git a/src/report/report.h b/src/report/report.h index 4d7b5bdd3f0..4adb2034951 100644 --- a/src/report/report.h +++ b/src/report/report.h @@ -16,19 +16,16 @@ extern usec_t arg_network_timeout_usec; typedef enum Action { ACTION_LIST_METRICS, ACTION_DESCRIBE_METRICS, - ACTION_LIST_FACTS, - ACTION_DESCRIBE_FACTS, _ACTION_MAX, _ACTION_INVALID = -EINVAL, } Action; -/* The structure for collected "metrics" or "facts". The fields - * are prefixed with just "metrics" for brevity. */ +/* The structure for collected "metrics". */ typedef struct Context { Action action; sd_event *event; Set *link_infos; - sd_json_variant **metrics; /* Collected metrics or facts for sorting */ + sd_json_variant **metrics; /* Collected metrics for sorting */ size_t n_metrics, n_skipped_metrics, n_invalid_metrics; struct iovec_wrapper upload_answer; } Context; diff --git a/test/integration-tests/TEST-74-AUX-UTILS/TEST-74-AUX-UTILS.units/fake-report-server.py b/test/integration-tests/TEST-74-AUX-UTILS/TEST-74-AUX-UTILS.units/fake-report-server.py index 4875a00bada..6beb6383c20 100755 --- a/test/integration-tests/TEST-74-AUX-UTILS/TEST-74-AUX-UTILS.units/fake-report-server.py +++ b/test/integration-tests/TEST-74-AUX-UTILS/TEST-74-AUX-UTILS.units/fake-report-server.py @@ -35,8 +35,8 @@ class Handler(BaseHTTPRequestHandler): print(f"JSON: {s if len(s := str(data)) < 80 else s[:40] + '…' + s[-40:]}") - if "metrics" not in data and "facts" not in data: - self.send_error(400, "Missing 'metrics' or 'facts' field") + if "metrics" not in data: + self.send_error(400, "Missing 'metrics' field") return response = json.dumps({"status": "ok"}).encode() diff --git a/test/units/TEST-74-AUX-UTILS.report.sh b/test/units/TEST-74-AUX-UTILS.report.sh index 53b83c4dd94..73678fcabf1 100755 --- a/test/units/TEST-74-AUX-UTILS.report.sh +++ b/test/units/TEST-74-AUX-UTILS.report.sh @@ -16,9 +16,9 @@ REPORT=/usr/lib/systemd/systemd-report "$REPORT" metrics "$REPORT" metrics -j "$REPORT" metrics --no-legend -"$REPORT" describe-metrics -"$REPORT" describe-metrics -j -"$REPORT" describe-metrics --no-legend +"$REPORT" describe +"$REPORT" describe -j +"$REPORT" describe --no-legend "$REPORT" list-sources "$REPORT" list-sources -j "$REPORT" list-sources --no-legend @@ -26,9 +26,9 @@ REPORT=/usr/lib/systemd/systemd-report "$REPORT" metrics io "$REPORT" metrics io.systemd piff "$REPORT" metrics piff -"$REPORT" describe-metrics io -"$REPORT" describe-metrics io.systemd piff -"$REPORT" describe-metrics piff +"$REPORT" describe io +"$REPORT" describe io.systemd piff +"$REPORT" describe piff # test io.systemd.CGroup Metrics systemctl start systemd-report-cgroup.socket @@ -46,26 +46,6 @@ varlinkctl --more call /run/systemd/report/io.systemd.Network io.systemd.Metrics # Make sure the service for "system facts" is enabled systemctl start systemd-report-basic.socket -# Test facts verbs -"$REPORT" facts -"$REPORT" facts -j -"$REPORT" facts --no-legend -"$REPORT" describe-facts -"$REPORT" describe-facts -j -"$REPORT" describe-facts --no-legend - -# Test facts with match filters -"$REPORT" facts io -"$REPORT" facts io.systemd piff -"$REPORT" facts piff -"$REPORT" describe-facts io -"$REPORT" describe-facts io.systemd piff -"$REPORT" describe-facts piff - -# Test facts via direct Varlink call on existing socket -varlinkctl --more call /run/systemd/report/io.systemd.Basic io.systemd.Facts.List {} -varlinkctl --more call /run/systemd/report/io.systemd.Basic io.systemd.Facts.Describe {} - # Test HTTP upload (plain http) FAKE_SERVER=/usr/lib/systemd/tests/integration-tests/TEST-74-AUX-UTILS/TEST-74-AUX-UTILS.units/fake-report-server.py CERTDIR=$(mktemp -d) @@ -81,7 +61,6 @@ systemd-run -p Type=notify --unit=fake-report-server "$FAKE_SERVER" systemctl status fake-report-server "$REPORT" metrics --url=http://localhost:8089/ -"$REPORT" facts --url=http://localhost:8089/ # Test HTTPS upload with generated TLS certificates openssl req -x509 -newkey rsa:2048 -keyout "$CERTDIR/server.key" -out "$CERTDIR/server.crt" \ @@ -91,6 +70,5 @@ systemd-run -p Type=notify --unit=fake-report-server-tls \ "$FAKE_SERVER" --cert="$CERTDIR/server.crt" --key="$CERTDIR/server.key" --port=8090 systemctl status fake-report-server-tls -"$REPORT" metrics --url=https://localhost:8090/ --key=- --trust="$CERTDIR/server.crt" -"$REPORT" facts --url=https://localhost:8090/ --key=- --trust="$CERTDIR/server.crt" \ +"$REPORT" metrics --url=https://localhost:8090/ --key=- --trust="$CERTDIR/server.crt" \ --extra-header='Authorization: Bearer magic string'