Skip to content

fix(metrics): use short-lived local registry#1575

Merged
aawsome merged 1 commit intorustic-rs:mainfrom
tilpner:prometheus-local-registry
Oct 8, 2025
Merged

fix(metrics): use short-lived local registry#1575
aawsome merged 1 commit intorustic-rs:mainfrom
tilpner:prometheus-local-registry

Conversation

@tilpner
Copy link
Copy Markdown
Contributor

@tilpner tilpner commented Oct 7, 2025

Previously, this was using the global registry (via register_gauge!), which doesn't support re-registering metrics in this way, so only the first call to PrometheusExporter::push_metrics would succeed, while any later calls would return a (nested) prometheus::Error::AlreadyReg.

This resulted in only the metrics for the first snapshot being pushed, while the metrics for subsequent snapshots errored with a somewhat vague error (the interior messages are not shown):

[WARN] error pushing metrics: pushing prometheus metrics

This PR more closely matches the behavior of the OpenTelemetry backend, which similarly only pushes the immediately-passed (as an argument) metrics, rather than including other metrics previously registered globally, as was the behavior with prometheus::gather.

There are likely less allocation-happy ways to solve this, but at the push frequency of rustic it doesn't matter.

cc @Ekleog as the original author of this module

Previously, this was using the global registry (via register_gauge!),
which doesn't support re-registering metrics in this way, so only the
first call to `PrometheusExporter::push_metrics` would succeed, while
any later calls would return a (nested) `prometheus::Error::AlreadyReg`.
Copy link
Copy Markdown
Contributor

@Ekleog Ekleog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix! And sorry about that, it's definitely my mistake 😅

Copy link
Copy Markdown
Member

@aawsome aawsome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the fix @tilpner

@aawsome aawsome enabled auto-merge October 8, 2025 07:24
@aawsome aawsome added this pull request to the merge queue Oct 8, 2025
Merged via the queue into rustic-rs:main with commit 7d7304f Oct 8, 2025
29 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Nov 8, 2025
## 🤖 New release

* `rustic-rs`: 0.10.0 -> 0.10.1 (✓ API compatible changes)

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

##
[0.10.1](v0.10.0...v0.10.1)
- 2025-11-08

### Added

- Add rewrite command
([#1583](#1583))
- *(interactive)* Add --interactive option to ls
([#1564](#1564))
- Add filter-last option
([#1574](#1574))
- Add environment variable substitution in config files
([#1577](#1577))

### Fixed

- Fix typos using the typos tool
([#1590](#1590))
- *(metrics)* use short-lived local registry
([#1575](#1575))
- fix clippy lints
([#1570](#1570))
- wrong env var for grouping option
([#1566](#1566))

### Other

- update dependencies
([#1594](#1594))
- (security) Update fuser
([#1569](#1569))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

---------

Co-authored-by: rustic-release-plz[bot] <182542030+rustic-release-plz[bot]@users.noreply.github.com>
Co-authored-by: Alexander Weiss <alex@weissfam.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants