From df12b22cfa56e140053571712e81c9df792887e3 Mon Sep 17 00:00:00 2001
From: Toby Lawrence <toby@nuclearfurnace.com>
Date: Thu, 23 Jan 2025 12:18:12 -0500
Subject: [PATCH] chore: remove ability to make tags on a context

---
 lib/saluki-context/src/context.rs  | 49 ++----------------------------
 lib/saluki-context/src/resolver.rs | 41 -------------------------
 2 files changed, 2 insertions(+), 88 deletions(-)

diff --git a/lib/saluki-context/src/context.rs b/lib/saluki-context/src/context.rs
index a740e122..77f761a6 100644
--- a/lib/saluki-context/src/context.rs
+++ b/lib/saluki-context/src/context.rs
@@ -1,7 +1,4 @@
-use std::{
-    fmt, hash,
-    sync::{Arc, OnceLock},
-};
+use std::{fmt, hash, sync::Arc};
 
 use metrics::Gauge;
 use stringtheory::MetaString;
@@ -11,8 +8,6 @@ use crate::{
     tags::TagSet,
 };
 
-static DIRTY_CONTEXT_KEY: OnceLock<ContextKey> = OnceLock::new();
-
 /// A metric context.
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub struct Context {
@@ -92,15 +87,6 @@ impl Context {
         Arc::ptr_eq(&self.inner, &other.inner)
     }
 
-    fn inner_mut(&mut self) -> &mut ContextInner {
-        Arc::make_mut(&mut self.inner)
-    }
-
-    fn mark_dirty(&mut self) {
-        let inner = self.inner_mut();
-        inner.key = get_dirty_context_key_value();
-    }
-
     /// Gets the name of this context.
     pub fn name(&self) -> &MetaString {
         &self.inner.name
@@ -110,20 +96,6 @@ impl Context {
     pub fn tags(&self) -> &TagSet {
         &self.inner.tags
     }
-
-    /// Gets a mutable reference to the tags of this context.
-    pub fn tags_mut(&mut self) -> &mut TagSet {
-        // Mark the context as dirty. We have to do this before giving back a mutable reference to the tags, which means
-        // we are _potentially_ marking the context dirty even if nothing is changed about the tags.
-        //
-        // If this somehow became a problem, we could always move part of the hash to `TagSet` itself where we had
-        // granular control and could mark ourselves dirty only when the tags were actually changed. Shouldn't matter
-        // right now, though.
-        self.mark_dirty();
-
-        let inner = self.inner_mut();
-        &mut inner.tags
-    }
 }
 
 impl From<&'static str> for Context {
@@ -201,15 +173,7 @@ impl Eq for ContextInner {}
 
 impl hash::Hash for ContextInner {
     fn hash<H: hash::Hasher>(&self, state: &mut H) {
-        // If the context is dirty -- has changed since it was originally resolved -- then our cached key is now
-        // invalid, so we need to re-hash the context. Otherwise, we can just use the cached key.
-        let key = if is_context_dirty(self.key) {
-            hash_context(&self.name, &self.tags, None)
-        } else {
-            self.key
-        };
-
-        key.hash(state);
+        self.key.hash(state);
     }
 }
 
@@ -275,12 +239,3 @@ impl<'a> Tagged for &'a TagSet {
         }
     }
 }
-
-fn get_dirty_context_key_value() -> ContextKey {
-    const EMPTY_TAGS: &[&str] = &[];
-    *DIRTY_CONTEXT_KEY.get_or_init(|| hash_context("", EMPTY_TAGS, None))
-}
-
-fn is_context_dirty(key: ContextKey) -> bool {
-    key == get_dirty_context_key_value()
-}
diff --git a/lib/saluki-context/src/resolver.rs b/lib/saluki-context/src/resolver.rs
index 381aa0c5..1861fcf9 100644
--- a/lib/saluki-context/src/resolver.rs
+++ b/lib/saluki-context/src/resolver.rs
@@ -483,7 +483,6 @@ mod tests {
     };
 
     use super::*;
-    use crate::tags::Tag;
 
     fn get_gauge_value(metrics: &[(CompositeKey, Option<Unit>, Option<SharedString>, DebugValue)], key: &str) -> f64 {
         metrics
@@ -583,46 +582,6 @@ mod tests {
         assert_eq!(active_contexts, 0.0);
     }
 
-    #[test]
-    fn mutate_tags() {
-        let mut resolver: ContextResolver = ContextResolverBuilder::for_tests();
-
-        // Create a basic context.
-        //
-        // We create two identical references so that we can later try and resolve the original context again to make
-        // sure things are still working as expected:
-        let name = "metric_name";
-        let tags = ["tag1"];
-
-        let context1 = resolver
-            .resolve(name, &tags[..], None)
-            .expect("should not fail to resolve");
-        let mut context2 = context1.clone();
-
-        // Mutate the tags of `context2`, which should end up cloning the inner state and becoming its own instance:
-        let context2_tags = context2.tags_mut();
-        context2_tags.insert_tag("tag2");
-
-        // The contexts should no longer be equal to each other, and should have distinct underlying pointers to the
-        // shared context state:
-        assert_ne!(context1, context2);
-        assert!(!context1.ptr_eq(&context2));
-
-        let expected_tags_context1 = TagSet::from_iter(vec![Tag::from("tag1")]);
-        assert_eq!(context1.tags(), &expected_tags_context1);
-
-        let expected_tags_context2 = TagSet::from_iter(vec![Tag::from("tag1"), Tag::from("tag2")]);
-        assert_eq!(context2.tags(), &expected_tags_context2);
-
-        // And just for good measure, check that we can still resolve the original context reference and get back a
-        // context that is equal to `context1`:
-        let context1_redo = resolver
-            .resolve(name, &tags[..], None)
-            .expect("should not fail to resolve");
-        assert_eq!(context1, context1_redo);
-        assert!(context1.ptr_eq(&context1_redo));
-    }
-
     #[test]
     fn duplicate_tags() {
         let mut resolver: ContextResolver = ContextResolverBuilder::for_tests();