Skip to content

Commit

Permalink
feat(metrics/family): 🍫 add Family::get_or_create_owned() (#244)
Browse files Browse the repository at this point in the history
fixes #231.

this introduces a new method to `Family<S, M, C>` to help alleviate the
risk of deadlocks when accessing multiple series within a given metrics
family.

this returns a plain `M`, rather than the `MappedRwLockReadGuard<M>`
RAII guard returned by `get_or_create()`.

a test case is introduced in this commit to demonstrate that structures
accessing multiple series within a single expression will not
accidentally create a deadlock.

Signed-off-by: katelyn martin <me+cratelyn@katelyn.world>
  • Loading branch information
cratelyn authored Feb 9, 2025
1 parent 84e2cc6 commit 377ca2d
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 0 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

- `EncodeLabelSet` is now implemented for tuples `(A: EncodeLabelSet, B: EncodeLabelSet)`.
See [PR 257].

- `Family::get_or_create_owned` can access a metric in a labeled family. This
method avoids the risk of runtime deadlocks at the expense of creating an
owned type. See [PR 244].

[PR 244]: https://github.com/prometheus/client_rust/pull/244
[PR 257]: https://github.com/prometheus/client_rust/pull/257

### Changed

Expand Down
57 changes: 57 additions & 0 deletions src/metrics/family.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,40 @@ impl<S: Clone + std::hash::Hash + Eq, M, C> Family<S, M, C> {
}
}

impl<S: Clone + std::hash::Hash + Eq, M: Clone, C: MetricConstructor<M>> Family<S, M, C>
where
S: Clone + std::hash::Hash + Eq,
M: Clone,
C: MetricConstructor<M>,
{
/// Access a metric with the given label set, creating it if one does not yet exist.
///
/// ```
/// # use prometheus_client::metrics::counter::{Atomic, Counter};
/// # use prometheus_client::metrics::family::Family;
/// #
/// let family = Family::<Vec<(String, String)>, Counter>::default();
///
/// // Will create and return the metric with label `method="GET"` when first called.
/// family.get_or_create_owned(&vec![("method".to_owned(), "GET".to_owned())]).inc();
///
/// // Will return a clone of the existing metric on all subsequent calls.
/// family.get_or_create_owned(&vec![("method".to_owned(), "GET".to_owned())]).inc();
/// ```
///
/// Callers wishing to avoid a clone of the metric `M` can call [`Family::get_or_create()`] to
/// return a reference to the metric instead.
pub fn get_or_create_owned(&self, label_set: &S) -> M {
use std::ops::Deref;

let guard = self.get_or_create(label_set);
let metric = guard.deref().to_owned();
drop(guard);

metric
}
}

impl<S: Clone + std::hash::Hash + Eq, M, C: MetricConstructor<M>> Family<S, M, C> {
/// Access a metric with the given label set, creating it if one does not
/// yet exist.
Expand All @@ -225,6 +259,10 @@ impl<S: Clone + std::hash::Hash + Eq, M, C: MetricConstructor<M>> Family<S, M, C
/// // calls.
/// family.get_or_create(&vec![("method".to_owned(), "GET".to_owned())]).inc();
/// ```
///
/// NB: This method can cause deadlocks if multiple metrics within this family are read at
/// once. Use [`Family::get_or_create_owned()`] if you would like to avoid this by cloning the
/// metric `M`.
pub fn get_or_create(&self, label_set: &S) -> MappedRwLockReadGuard<M> {
if let Some(metric) = self.get(label_set) {
return metric;
Expand Down Expand Up @@ -510,4 +548,23 @@ mod tests {
let non_existent_string = string_family.get(&"non_existent".to_string());
assert!(non_existent_string.is_none());
}

/// Tests that [`Family::get_or_create_owned()`] does not cause deadlocks.
#[test]
fn counter_family_does_not_deadlock() {
/// A structure we'll place two counters into, within a single expression.
struct S {
apples: Counter,
oranges: Counter,
}

let family = Family::<(&str, &str), Counter>::default();
let s = S {
apples: family.get_or_create_owned(&("kind", "apple")),
oranges: family.get_or_create_owned(&("kind", "orange")),
};

s.apples.inc();
s.oranges.inc_by(2);
}
}

0 comments on commit 377ca2d

Please sign in to comment.