-
Notifications
You must be signed in to change notification settings - Fork 161
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
Add support for Arc<T>
to metrics::Cow<'a, T>
.
#402
Merged
Merged
Conversation
This file contains 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
tobz
force-pushed
the
add-arc-support-cow
branch
3 times, most recently
from
October 21, 2023 15:03
a78efbb
to
25b5836
Compare
tobz
force-pushed
the
add-arc-support-cow
branch
from
October 21, 2023 15:04
25b5836
to
5d6809f
Compare
tobz
added
C-core
Component: core functionality such as traits, etc.
E-complex
Effort: complex.
T-enhancement
Type: enhancement.
S-awaiting-release
Status: awaiting a release to be considered fixed/implemented.
T-ergonomics
Type: ergonomics.
labels
Nov 29, 2023
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]>
tobz
removed
the
S-awaiting-release
Status: awaiting a release to be considered fixed/implemented.
label
Dec 24, 2023
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-core
Component: core functionality such as traits, etc.
E-complex
Effort: complex.
T-enhancement
Type: enhancement.
T-ergonomics
Type: ergonomics.
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.
This PR adds support for creating
metrics::Cow<'a, T>
fromArc<T>
.Context
Generally, users specify a metric name and optional labels for the metric, each of which has a key and value pair. All three of these -- metric name, label key, and label value -- are wrapped in
metrics::Cow<'static, str>
(which is exported from the crate asSharedString
for simplicity). Additionally, in some cases, we construct ametrics::Cow<'static, [Label]>
when labels are entirely static.All of this works fine and well for both static strings (string literals) and owned strings, but did not allow users to use strings shared via a smart pointer such as
Arc<T>
. It is common practice to amortize allocation costs for long-lived strings by sharing them via something likeArc<str>
. You still pay the cost of the atomic operations here and there, but it can end up much cheaper in the long-term than constantly allocating small strings all over the place, and that's to say nothing of the potential heap fragmentation caused by doing so.This is important to users as if you're not able to use static strings for a metric key (name, labels) then you're stuck with owned strings, which can involve many of these small allocations every single time a metric is emitted.
Solution
We've added support for passing
Arc<T>
when constructing ametrics::Cow<'a, T>
. This involved a little bit of work to properly track the variant of our pointer (borrowed vs owned vs shared) but ultimately functions a lot like if we were simply cloningArc<T>
and holding it internally. Of course, we only deal with pointers and pointer metadata (in the context of our code, not pointer metadata as the stdlib might talk about) and so we've had to resort to the manual functions for interacting withArc<T>
, such as incrementing and decrementing the strong count by hand, and so on.We've additionally simplified a lot of the
From<T>
implementations to avoid having one for each pointer variant, as well at a higher-level in the types that are based onmetrics::Cow<'a, T>
itself.