Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(util): new helper type for recovering recorder after installing it #362

Merged
merged 4 commits into from
May 12, 2023

Conversation

tobz
Copy link
Member

@tobz tobz commented Apr 18, 2023

Context

In cases like #308, there can be a desire to ensure that a recorder flushes itself on process exit. As we install the recorder globally, such that it is leaked to provide the ability to acquire a static reference to it, recorders cannot normally participate in the typical Drop logic as objects are cleaned up before process exit.

Solution

This PR introduces a generic helper -- RecoverableRecorder<R> -- that allows for installing a wrapped version of the recorder, and acts as a drop guard to allow either manually "recovering" the recorder, or recovering it on drop such that the recorder's drop logic can also be called.

We wrap the recorder in an Arc<T>, and install a custom recorder wrapper that holds a Weak<T> reference. This allows the global recorder to provide access to the inner recorder without taking ownership of it.

When the caller manually seeks to recover the recorder, they call RecoverableRecorder::into_inner which attempts to unwrap the Arc<T> until there are no active references other than from the drop guard, which gives back the original recorder. If the drop guard is simply dropped, the original recorder too is dropped as soon as all active references to it have dropped, following the normal behavior of Arc<T>.

This does come with a small performance overhead, as every call through the weakly-held Recorder implementation must clone the Arc<T> before proxying the given method call, but it should be fairly minimal in practice. If performance is a huge concern, a recorder should implement its own drop guard for controlling flushing on process exit, but this is simply meant to be a generic solution when the application has no control over the recorder implementation.

Additionally, I've stripped out all of the atomic_cas and set_recorder_racy bits, as all illusions of metrics being no_std/embedded-compatible have sort of faded at this point, and it was just an opportune time to strip out the last vestiges.

@tobz tobz added C-util Component: utility classes and helpers. E-intermediate Effort: intermediate. T-enhancement Type: enhancement. T-request Type: request. labels Apr 18, 2023
@tobz tobz merged commit c15c0ed into main May 12, 2023
@tobz tobz deleted the tobz/controlled-recorder-shutdown branch May 12, 2023 12:21
@tobz tobz added the S-awaiting-release Status: awaiting a release to be considered fixed/implemented. label May 12, 2023
@tobz tobz removed the S-awaiting-release Status: awaiting a release to be considered fixed/implemented. label Jul 2, 2023
@tobz
Copy link
Member Author

tobz commented Jul 2, 2023

Released in [email protected].

jvimal-eg added a commit to edgeguard-dev/metrics that referenced this pull request Dec 24, 2023
* fix prometheus metric name and label key sanitizer (metrics-rs#296)

Co-authored-by: Toby Lawrence <[email protected]>

* metrics-util: add ability to collect metrics on a per-thread basis via DebuggingRecorder (metrics-rs#299)

Signed-off-by: Toby Lawrence <[email protected]>

* (cargo-release) version 0.12.1

* Improve handling of the global recorder instance (metrics-rs#302)

This gets rid of the dangerous `static mut`, adds more comments
about the code, relaxes the orderings and documents the unsoundness
of the `clear` function, in addition to marking it unsafe.

It implements a small once_cell-like abstraction.

Co-authored-by: Toby Lawrence <[email protected]>

* Fix `metrics::Cow` provenance issue (metrics-rs#303)

* Quantile Remapping Fix (metrics-rs#304)

* update changelogs prior to release

* (cargo-release) version 0.19.0

* (cargo-release) version 0.13.0

* (cargo-release) version 0.11.0

* (cargo-release) version 0.10.0

* Update CHANGELOG.md

* Update README.md

* Update ci.yml

* Update ci.yml

* Add RollingSummary to prevent summary saturation (metrics-rs#306)

* Update quanta requirement from 0.9.3 to 0.10.0 (metrics-rs#301)

Updates the requirements on [quanta](https://github.com/metrics-rs/quanta) to permit the latest version.
- [Release notes](https://github.com/metrics-rs/quanta/releases)
- [Changelog](https://github.com/metrics-rs/quanta/blob/main/CHANGELOG.md)
- [Commits](metrics-rs/quanta@v0.9.3...v0.10.0)

---
updated-dependencies:
- dependency-name: quanta
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Document CI enforced msrv in readme and rust-version fields (metrics-rs#311)

* Change description to be SharedString (metrics-rs#312)

* Update hashbrown requirement from 0.11 to 0.12 (metrics-rs#266)

Updates the requirements on [hashbrown](https://github.com/rust-lang/hashbrown) to permit the latest version.
- [Release notes](https://github.com/rust-lang/hashbrown/releases)
- [Changelog](https://github.com/rust-lang/hashbrown/blob/master/CHANGELOG.md)
- [Commits](rust-lang/hashbrown@v0.11.0...v0.12.0)

---
updated-dependencies:
- dependency-name: hashbrown
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Toby Lawrence <[email protected]>

* Shims remaining AtomicU64 usage (metrics-rs#313)

* Update parking_lot requirement from 0.11 to 0.12 (metrics-rs#268)

Updates the requirements on [parking_lot](https://github.com/Amanieu/parking_lot) to permit the latest version.
- [Release notes](https://github.com/Amanieu/parking_lot/releases)
- [Changelog](https://github.com/Amanieu/parking_lot/blob/master/CHANGELOG.md)
- [Commits](Amanieu/parking_lot@0.11.0...0.12.0)

---
updated-dependencies:
- dependency-name: parking_lot
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Toby Lawrence <[email protected]>

* update changelogs

* (cargo-release) version 0.13.1

* add std atomics handle support back + changelogs

* (cargo-release) version 0.20.0

* (cargo-release) version 0.14.0

* (cargo-release) version 0.11.0

* changelog

* (cargo-release) version 0.12.0

* (cargo-release) version 0.7.0

* update changelog

* (cargo-release) version 0.6.0

* update changelog

* (cargo-release) version 0.20.1

* Remove incorrect return info (metrics-rs#316)

* Add a `KeyName` argument to `LabelFilter::should_include_label` (metrics-rs#342)

* rewind

* (cargo-release) version 0.13.0

* Use std sync primitives instead of parking_lot. (metrics-rs#344)

* Use std::sync::Mutex instead of parking_lot::Mutex.
* Bump MSRV to 1.60.0 for CI.

Co-authored-by: Toby Lawrence <[email protected]>

* clean up github CI workflow + rust-toolchain.toml to match quanta

* update portable-atomic to 1.0

* bump to prost 0.11 + fix spelling issue in metrics-tracing-context

* bump MSRV to 1.61 + update hashbrown/ahash deps

* update termion/ordered-float and pin predicates-* to avoid stupid 1.64 MSRV

* update syn

* update quanta + clean up semver notation in cargo.toml

* cleanup README wording for MSRV policy

* Add white background to splash image (metrics-rs#348)

* Install protoc in CI.

* fix spelling error in CI workflow

* Update ci.yml

* no need to run CI against macOS/Windows specifically

* Bring the metrics-observer protobufs up to date (metrics-rs#345)

* Use global paths for macros (metrics-rs#358)

* fix changes to fully qualified metrics crate ref change in macros

* update changelog

* update tui to 0.19

* bump numpy dep

* impl std::error::Error for metrics_exporter_tcp::Error

* changelog

* tweak test to avoid unused code

* rework 32 vs 64-bit arch atomics support + a lot of import consolidation/cleanup

* const-ify some stuff + rewrite some comments for the global recorder init code

* clean up clippy lints

* bump MSRV to 1.61.0

* (cargo-release) version 0.7.0

* (cargo-release) version 0.21.0

* (cargo-release) version 0.15.0

* (cargo-release) version 0.14.0

* (cargo-release) version 0.8.0

* (cargo-release) version 0.12.0

* allow publishing

* (cargo-release) version 0.2.0

* make it publishable pt 2

* push-gateway support authentication (metrics-rs#366)

* update changelog + fix failing feature check test

* (cargo-release) version 0.12.1

* feat(util): new helper type for recovering recorder after installing it (metrics-rs#362)

* Update aho-corasick to 1.0.

* Impl From<std::borrow::Cow> for KeyName (metrics-rs#378)

* changelog

* Add `Borrow` impl to `KeyName` (metrics-rs#381)

* bump deps + clippy stuff

* changelog

* pin hashbrown to avoid MSRV bump

* (cargo-release) version 0.15.1

* (cargo-release) version 0.21.1

* Add metadata to metrics (metrics-rs#380)

* add https support in Prometheus gateway (metrics-rs#392)

* migrate from procedural to declarative macros (metrics-rs#386)

* Make `Unit` methods return `&'static str` where possible (metrics-rs#393)

* simplify macros (metrics-rs#394)

* Add support for `Arc<T>` to `metrics::Cow<'a, T>`. (metrics-rs#402)

* Add support for `tracing::Span::record`ed fields in `metrics-tracing-context` (metrics-rs#408)

* fix(prom): `RollingSummary` overflow panic (metrics-rs#423)

* update CHANGELOG

* (cargo-release) version 0.12.2

* Remove unneeded unsafe from test (metrics-rs#427)

* Fix feature check in CI (metrics-rs#428)

* update changelogs/release notes

* fix unsafe/incorrect crossbeam-epoch usage in Block<T>

* Add a clippy CI check (metrics-rs#416) (metrics-rs#417)

* Try other resolved addresses if the first one fails (metrics-rs#429)

* update changelog

* Update quanta requirement from 0.11 to 0.12 (metrics-rs#396)

Updates the requirements on [quanta](https://github.com/metrics-rs/quanta) to permit the latest version.
- [Changelog](https://github.com/metrics-rs/quanta/blob/v0.12.0/CHANGELOG.md)
- [Commits](metrics-rs/quanta@v0.11.0...v0.12.0)

---
updated-dependencies:
- dependency-name: quanta
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Toby Lawrence <[email protected]>

* Update ordered-float requirement from 3.4 to 4.2 (metrics-rs#421)

Updates the requirements on [ordered-float](https://github.com/reem/rust-ordered-float) to permit the latest version.
- [Release notes](https://github.com/reem/rust-ordered-float/releases)
- [Commits](reem/rust-ordered-float@v3.4.0...v4.2.0)

---
updated-dependencies:
- dependency-name: ordered-float
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix quantile api

* missing clear

* reintroduce old way to avoid big refactor

---------

Signed-off-by: Toby Lawrence <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Shaoyuan CHEN <[email protected]>
Co-authored-by: Toby Lawrence <[email protected]>
Co-authored-by: Toby Lawrence <[email protected]>
Co-authored-by: nils <[email protected]>
Co-authored-by: Dan Wilbanks <[email protected]>
Co-authored-by: Daniel Nelson <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Lucas Kent <[email protected]>
Co-authored-by: Fredrik Enestad <[email protected]>
Co-authored-by: Christopher Hunt <[email protected]>
Co-authored-by: zohnannor <[email protected]>
Co-authored-by: Sinotov Aleksandr <[email protected]>
Co-authored-by: Jacob Kiesel <[email protected]>
Co-authored-by: C J Silverio <[email protected]>
Co-authored-by: CinchBlue <[email protected]>
Co-authored-by: JasonLi <[email protected]>
Co-authored-by: Mostafa Elhemali <[email protected]>
Co-authored-by: Harry Barber <[email protected]>
Co-authored-by: Qingwen Zhao <[email protected]>
Co-authored-by: david-perez <[email protected]>
Co-authored-by: Lucio Franco <[email protected]>
Co-authored-by: Nicolas Stinus <[email protected]>
Co-authored-by: Valeriy V. Vorotyntsev <[email protected]>
mnpw pushed a commit to mnpw/metrics that referenced this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-util Component: utility classes and helpers. E-intermediate Effort: intermediate. T-enhancement Type: enhancement. T-request Type: request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant