From 377ca2d86308d868576e961c778bcbad8b62ec0e Mon Sep 17 00:00:00 2001
From: katelyn martin <me+cratelyn@katelyn.world>
Date: Sun, 9 Feb 2025 15:51:38 -0500
Subject: [PATCH] =?UTF-8?q?feat(metrics/family):=20=F0=9F=8D=AB=20add=20`F?=
 =?UTF-8?q?amily::get=5For=5Fcreate=5Fowned()`=20(#244)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 CHANGELOG.md          |  8 ++++++
 src/metrics/family.rs | 57 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a0e3205..c798c59 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 
diff --git a/src/metrics/family.rs b/src/metrics/family.rs
index 1a76cf8..d3d55c6 100644
--- a/src/metrics/family.rs
+++ b/src/metrics/family.rs
@@ -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.
@@ -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;
@@ -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);
+    }
 }