Call metrics API for units related stats by YapingLi04 · Pull Request #158 · cooperlees/monitord

YapingLi04 · 2026-02-24T01:21:15Z

This implements support for the new metrics API in monitord.

The metrics API will be available in systemd v260. The zlink support for ANY type is in zlink release 0.4.0.

…constant file

Copilot

Pull request overview

Adds support for collecting systemd unit-related statistics via the new varlink metrics API (systemd v260+), alongside a refactor that centralizes unit state constants.

Changes:

Introduces a varlink metrics client (src/varlink/metrics.rs) and a new varlink-based unit stats collector (src/varlink_units.rs).
Refactors unit state enums / is_unit_unhealthy into src/unit_constants.rs and re-exports them from src/units.rs.
Adds a [varlink] config section and switches collectors to choose varlink vs D-Bus based on config.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`src/varlink_units.rs`	New varlink-based unit stats collection and parsing logic.
`src/varlink/mod.rs`	Adds varlink module namespace.
`src/varlink/metrics.rs`	Varlink proxy + output types for Metrics interface.
`src/units.rs`	Removes duplicated enums; re-exports from `unit_constants`.
`src/unit_constants.rs`	New shared enums + `is_unit_unhealthy` implementation.
`src/machines.rs`	Switches container unit stats collection to varlink when enabled.
`src/lib.rs`	Switches host unit stats collection to varlink when enabled; exposes modules.
`src/config.rs`	Adds `VarlinkConfig` and embeds into main config struct.
`monitord.conf`	Documents `[varlink] enabled = true`.
`Cargo.toml`	Adds futures-util and zlink-related dependencies.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/varlink_units.rs

src/lib.rs

monitord.conf

src/varlink/metrics.rs

src/varlink_units.rs

src/config.rs

Cargo.toml

src/varlink_units.rs

src/varlink/metrics.rs

src/units.rs

cooperlees

Looks a good start.

Let's add a section on varlink usage and flow in README.md please
Let's remove all .unwrap()s and see if we can throw an Error OR at least move to .expect("Helpful message") for now
Consider other suggestions.

Lots of other comments + let's see what copilot says.

src/config.rs

src/units.rs

cooperlees · 2026-02-24T17:21:56Z

Cargo.toml

 anyhow = "1.0"
 clap = { version = "4.5", features = ["derive", "std", "help"], default-features = false }
 configparser = { version = "3.1.0", features = ["indexmap"] }
+futures-util = "0.3"


I have been trying to avoid bringing in futures, but at least here we only bring in utils ...

monitord.conf

src/varlink_units.rs

cooperlees · 2026-02-24T17:29:04Z

Would love @ikruglov take a look if you have time please ...

cooperlees · 2026-02-24T18:11:32Z

Also, @YapingLi04 - If you don't have time to do the follow ups say so, and I can merge and chip away at them

YapingLi04 · 2026-02-25T05:22:53Z

@cooperlees thanks for the review! I addressed all of your comments. Please let know if anything else is needed.

cooperlees

Awesome. Thanks for the sweet docs too!

Just so you know this will auto sync to https://monitord.xyz/ when we release.

ikruglov · 2026-02-25T15:39:03Z

src/varlink_units.rs

+    let object_name = metric.object_name();
+
+    match metric_name_suffix {
+        "unit_active_state" => {


@YapingLi04, I recall you rename these metrics to make them camelCase in systemd

oops, seems I'm a bit late :-)

We can forward fix things if you find stuff - It's not released to crates.io etc. - Only merged to main

ikruglov · 2026-02-25T15:41:04Z

src/lib.rs

+            if config.varlink.enabled {
+                join_set.spawn(crate::varlink_units::update_unit_stats(
+                    Arc::clone(&config),
+                    sdc.clone(),


does varlink implementation needs dbus?

ikruglov · 2026-02-25T15:43:20Z

src/varlink_units.rs

+    match get_unit_stats(&config, &socket_path).await {
+        Ok(units_stats) => {
+            let mut machine_stats = locked_machine_stats.write().await;
+            machine_stats.units = units_stats;
+        }
+        Err(err) => {
+            warn!(
+                "Varlink units stats failed, falling back to D-Bus: {:?}",
+                err
+            );
+            crate::units::update_unit_stats(Arc::clone(&config), connection, locked_machine_stats)
+                .await?;
+        }
+    }
+    Ok(())
+}


I'd personally prefer the fallback happen in an outer layer to drop any dependency of varlink on dbus. Not sure if it's possible in rust. Not an expert here.

ikruglov · 2026-02-25T15:50:36Z

src/varlink_units.rs

+            if should_skip_unit(&object_name, config) {
+                return Ok(());
+            }
+            let value = metric.value_as_string("" /* default_value */);


@YapingLi04, why value has a default_value? I recall that value is a mandatory field in systemd. Plus, any of its value types (int, bool, enum, etc) can be converted to string. So, I'd prefer to fail and go to next metric in case of a error rather then silently ignore the failure.

ikruglov · 2026-02-25T15:52:13Z

src/varlink/metrics.rs

+    }
+
+    /// Returns the int value as u64 or default_value if not present
+    pub fn value_as_int(&self, default_value: u64) -> u64 {


negative number are completely legit value. Please don't mix gauge vs counter here. Counter is always positive, but gauge can be negative.

also, I'm not sure I'm happy with silenty ignoring the "wrong" value and converting them into default. I think we should warn/error about it and move forward. But not silently produce wrong value.

ikruglov · 2026-02-25T15:54:38Z

src/varlink_units.rs

+                .service_stats
+                .entry(object_name.to_string())
+                .or_default()
+                .nrestarts = metric.value_as_int(0 /* default_value */) as u32;


here in all other similar case. I'm not happy about "default_value". We should warn/error and move forward. But not silently produce 0.

YapingLi04 · 2026-02-25T19:17:40Z

@ikruglov thanks for the review! I put up PR #159 to address all of your comments. Let's continue there.

[refactor] Move SystemdUnitActiveState and SystemdUnitLoadState to a …

905eb8f

…constant file

YapingLi04 mentioned this pull request Feb 24, 2026

Call metrics API to get stats wherever available in monitord YapingLi04/monitord#1

Closed

cooperlees requested a review from Copilot February 24, 2026 17:17

Copilot started reviewing on behalf of cooperlees February 24, 2026 17:18 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

cooperlees requested changes Feb 24, 2026

View reviewed changes

Call metrics API for units related stats

7f6fe5a

YapingLi04 force-pushed the varlink branch from 53527f3 to 7f6fe5a Compare February 25, 2026 04:56

YapingLi04 requested a review from cooperlees February 25, 2026 05:23

cooperlees approved these changes Feb 25, 2026

View reviewed changes

cooperlees mentioned this pull request Feb 25, 2026

Move to varlink if/when APIs are available #37

Open

cooperlees merged commit 6e7b187 into cooperlees:main Feb 25, 2026
3 checks passed

ikruglov reviewed Feb 25, 2026

View reviewed changes

YapingLi04 added a commit to YapingLi04/monitord that referenced this pull request Feb 25, 2026

Followup fixes for cooperlees#158

1dc13ec

YapingLi04 mentioned this pull request Feb 25, 2026

Followup fixes for #158 #159

Open

Conversation

YapingLi04 commented Feb 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cooperlees left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cooperlees commented Feb 24, 2026

Uh oh!

cooperlees commented Feb 24, 2026

Uh oh!

YapingLi04 commented Feb 25, 2026

Uh oh!

cooperlees left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YapingLi04 commented Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cooperlees left a comment •

edited

Loading