Skip to content

Commit 708c1ec

Browse files
committed
feat(metrics/family): add Family::get_or_create_clone()
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 <[email protected]>
1 parent 9a74e99 commit 708c1ec

File tree

3 files changed

+67
-2
lines changed

3 files changed

+67
-2
lines changed

CHANGELOG.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,15 @@ All notable changes to this project will be documented in this file.
44
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
55
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

7-
## [0.23.1] - unreleased
7+
## [0.23.2]
8+
9+
### Added
10+
11+
- `Family::get_or_create_clone` can access a metric in a labeled family. This
12+
method avoids the risk of runtime deadlocks at the expense of incrementing
13+
a reference count.
14+
15+
## [0.23.1]
816

917
### Changed
1018

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "prometheus-client"
3-
version = "0.23.1"
3+
version = "0.23.2"
44
authors = ["Max Inden <[email protected]>"]
55
edition = "2021"
66
description = "Open Metrics client library allowing users to natively instrument applications."

src/metrics/family.rs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,40 @@ impl<S: Clone + std::hash::Hash + Eq, M, C> Family<S, M, C> {
207207
}
208208
}
209209

210+
impl<S: Clone + std::hash::Hash + Eq, M: Clone, C: MetricConstructor<M>> Family<S, M, C>
211+
where
212+
S: Clone + std::hash::Hash + Eq,
213+
M: Clone,
214+
C: MetricConstructor<M>,
215+
{
216+
/// Access a metric with the given label set, creating it if one does not yet exist.
217+
///
218+
/// ```
219+
/// # use prometheus_client::metrics::counter::{Atomic, Counter};
220+
/// # use prometheus_client::metrics::family::Family;
221+
/// #
222+
/// let family = Family::<Vec<(String, String)>, Counter>::default();
223+
///
224+
/// // Will create and return the metric with label `method="GET"` when first called.
225+
/// family.get_or_create_clone(&vec![("method".to_owned(), "GET".to_owned())]).inc();
226+
///
227+
/// // Will return a clone of the existing metric on all subsequent calls.
228+
/// family.get_or_create_clone(&vec![("method".to_owned(), "GET".to_owned())]).inc();
229+
/// ```
230+
///
231+
/// Callers wishing to avoid a clone of the metric `M` can call [`Family::get_or_create()`] to
232+
/// return a reference to the metric instead.
233+
pub fn get_or_create_clone(&self, label_set: &S) -> M {
234+
use std::ops::Deref;
235+
236+
let guard = self.get_or_create(label_set);
237+
let metric = guard.deref().to_owned();
238+
drop(guard);
239+
240+
metric
241+
}
242+
}
243+
210244
impl<S: Clone + std::hash::Hash + Eq, M, C: MetricConstructor<M>> Family<S, M, C> {
211245
/// Access a metric with the given label set, creating it if one does not
212246
/// yet exist.
@@ -225,6 +259,10 @@ impl<S: Clone + std::hash::Hash + Eq, M, C: MetricConstructor<M>> Family<S, M, C
225259
/// // calls.
226260
/// family.get_or_create(&vec![("method".to_owned(), "GET".to_owned())]).inc();
227261
/// ```
262+
///
263+
/// NB: This method can cause deadlocks if multiple metrics within this family are read at
264+
/// once. Use [`Family::get_or_create()`] if you would like to avoid this by cloning the
265+
/// metric `M`.
228266
pub fn get_or_create(&self, label_set: &S) -> MappedRwLockReadGuard<M> {
229267
if let Some(metric) = self.get(label_set) {
230268
return metric;
@@ -510,4 +548,23 @@ mod tests {
510548
let non_existent_string = string_family.get(&"non_existent".to_string());
511549
assert!(non_existent_string.is_none());
512550
}
551+
552+
/// Tests that [`Family::get_or_create_clone()`] does not cause deadlocks.
553+
#[test]
554+
fn counter_family_does_not_deadlock() {
555+
/// A structure we'll place two counters into, within a single expression.
556+
struct S {
557+
apples: Counter,
558+
oranges: Counter,
559+
}
560+
561+
let family = Family::<(&str, &str), Counter>::default();
562+
let s = S {
563+
apples: family.get_or_create_clone(&("kind", "apple")),
564+
oranges: family.get_or_create_clone(&("kind", "orange")),
565+
};
566+
567+
s.apples.inc();
568+
s.oranges.inc_by(2);
569+
}
513570
}

0 commit comments

Comments
 (0)