refactor: refactor rs/ic_os/metrics: shared write helper, replace hand-rolled formatter, rename tools#9330
Open
shilingwang wants to merge 5 commits intomasterfrom
Open
refactor: refactor rs/ic_os/metrics: shared write helper, replace hand-rolled formatter, rename tools#9330shilingwang wants to merge 5 commits intomasterfrom
shilingwang wants to merge 5 commits intomasterfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#NODE-1524
Summary
This PR cleans up the five metric-writing tools under
rs/ic_os/in three ways:rs/ic_os/metrics/utils)A new
ic-os-metrics-utilscrate provides a singlewrite_registry_to_file(registry, path)function. It encodes a Prometheus registry and writes the result atomically via a temp-file rename (usingic_sys::write_string_using_tmp_file), so a partially-written.promfile is never visible tonode_exporter.Previously,
metrics_tool,nft_exporter, andguest_vm_runner all duplicated the same 6-lineBufWriter / TextEncoder / flushblock and wrote non-atomically. All four are now updated to use the shared helper.hostos_tool/src/prometheus_metric.rs(158 lines) reimplemented the Prometheus text exposition format from scratch, used f32 for metric values, and only supported writing a single metric. It is deleted and replaced with the standardprometheuscrate (GaugeVecwith a gen label), consistent with the rest of the codebase.The
_toolsuffix was used inconsistently —hostos_tool,guestos_tool,setupos_toolare all configuration/provisioning binaries, whilefstrim_toolandmetrics_toolare periodic metric exporters that live inrs/ic_os/metrics/. Both are renamed to match the folder's naming convention:metrics_tool→custom_metrics(binary, crate, systemd units)fstrim_tool→fstrim_metrics(binary, crate, library, systemd units)The public entry-point function
fstrim_tool(...)in the library crate is also simplified to run(...).4. Fix integration test metric ordering
fstrim_metricsintegration tests were asserting on a hardcoded metric ordering that did not match the alphabetical order Prometheus registries actually produce (fstrim_datadir_*<fstrim_last_*<fstrim_runs_*). All three affected expected strings are corrected.