chore: make Tagged relevant to materialized tags, not raw tags (#438)

lib/saluki-context/src/context.rs

@@ -5,7 +5,7 @@ use stringtheory::MetaString;
 
 use crate::{
     hash::{hash_context, ContextKey},
-    tags::TagSet,
+    tags::{Tag, TagSet, Tagged},
 };
 
 /// A metric context.
@@ -134,6 +134,15 @@ impl fmt::Display for Context {
     }
 }
 
+impl Tagged for Context {
+    fn visit_tags<F>(&self, mut visitor: F)
+    where
+        F: FnMut(&Tag),
+    {
+        self.tags().visit_tags(&mut visitor);
+    }
+}
+
 pub struct ContextInner {
     pub key: ContextKey,
     pub name: MetaString,
@@ -186,56 +195,3 @@ impl fmt::Debug for ContextInner {
             .finish()
     }
 }
-
-/// A value containing tags that can be visited.
-pub trait Tagged {
-    /// Visits the tags in this value.
-    fn visit_tags<F>(&self, visitor: F)
-    where
-        F: FnMut(&str);
-}
-
-impl<'a, T> Tagged for &'a T
-where
-    T: Tagged,
-{
-    fn visit_tags<F>(&self, visitor: F)
-    where
-        F: FnMut(&str),
-    {
-        (*self).visit_tags(visitor)
-    }
-}
-
-impl<'a> Tagged for &'a [&'static str] {
-    fn visit_tags<F>(&self, mut visitor: F)
-    where
-        F: FnMut(&str),
-    {
-        for tag in self.iter() {
-            visitor(tag);
-        }
-    }
-}
-
-impl<'a> Tagged for &'a [MetaString] {
-    fn visit_tags<F>(&self, mut visitor: F)
-    where
-        F: FnMut(&str),
-    {
-        for tag in self.iter() {
-            visitor(tag);
-        }
-    }
-}
-
-impl<'a> Tagged for &'a TagSet {
-    fn visit_tags<F>(&self, mut visitor: F)
-    where
-        F: FnMut(&str),
-    {
-        for tag in self.into_iter() {
-            visitor(tag.as_str());
-        }
-    }
-}

lib/saluki-context/src/hash.rs

@@ -3,7 +3,7 @@ use std::{
     hash::{Hash as _, Hasher as _},
 };
 
-use crate::{context::Tagged, origin::OriginKey};
+use crate::origin::OriginKey;
 
 pub type FastHashSet<T> = HashSet<T, ahash::RandomState>;
 
@@ -30,9 +30,10 @@ fn hash_string(s: &str) -> u64 {
 /// set.
 ///
 /// Returns a hash that uniquely identifies the combination of name, tags, and origin of the value.
-pub fn hash_context<T>(name: &str, tags: T, origin_key: Option<OriginKey>) -> ContextKey
+pub fn hash_context<I, T>(name: &str, tags: I, origin_key: Option<OriginKey>) -> ContextKey
 where
-    T: Tagged,
+    I: IntoIterator<Item = T>,
+    T: AsRef<str>,
 {
     let mut seen = new_fast_hashset();
     hash_context_with_seen(name, tags, origin_key, &mut seen)
@@ -49,11 +50,12 @@ where
 /// allocates a new hash set each time.
 ///
 /// Returns a hash that uniquely identifies the combination of name, tags, and origin of the value.
-pub(super) fn hash_context_with_seen<T>(
-    name: &str, tags: T, origin_key: Option<OriginKey>, seen: &mut FastHashSet<u64>,
+pub(super) fn hash_context_with_seen<I, T>(
+    name: &str, tags: I, origin_key: Option<OriginKey>, seen: &mut FastHashSet<u64>,
 ) -> ContextKey
 where
-    T: Tagged,
+    I: IntoIterator<Item = T>,
+    T: AsRef<str>,
 {
     seen.clear();
 
@@ -63,16 +65,16 @@ where
     // Hash the tags individually and XOR their hashes together, which allows us to be order-oblivious:
     let mut combined_tags_hash = 0;
 
-    tags.visit_tags(|tag| {
-        let tag_hash = hash_string(tag);
+    for tag in tags {
+        let tag_hash = hash_string(tag.as_ref());
 
         // If we've already seen this tag before, skip combining it again.
         if !seen.insert(tag_hash) {
-            return;
+            continue;
         }
 
         combined_tags_hash ^= tag_hash;
-    });
+    }
 
     hasher.write_u64(combined_tags_hash);
 

lib/saluki-context/src/lib.rs

@@ -3,7 +3,7 @@
 #![deny(missing_docs)]
 
 mod context;
-pub use self::context::{Context, Tagged};
+pub use self::context::Context;
 
 mod expiry;
 

lib/saluki-context/src/resolver.rs

@@ -8,7 +8,7 @@ use tokio::time::sleep;
 use tracing::debug;
 
 use crate::{
-    context::{Context, ContextInner, Tagged},
+    context::{Context, ContextInner},
     expiry::{Expiration, ExpirationBuilder, ExpiryCapableLifecycle},
     hash::{hash_context_with_seen, new_fast_hashset, ContextKey, FastHashSet},
     origin::{OriginEnricher, OriginInfo},
@@ -323,11 +323,10 @@ impl ContextResolver {
             })
     }
 
-    fn create_context_key_from_resolvable<T>(
-        &mut self, name: &str, tags: T, origin_info: Option<OriginInfo<'_>>,
-    ) -> ContextKey
+    fn create_context_key<I, T>(&mut self, name: &str, tags: I, origin_info: Option<OriginInfo<'_>>) -> ContextKey
     where
-        T: Tagged,
+        I: IntoIterator<Item = T>,
+        T: AsRef<str>,
     {
         // If we have an origin enricher configured, and the resolvable value has origin information defined, attempt to
         // look up the origin key for it. We'll pass that along to the hasher to include as part of the context key.
@@ -339,29 +338,18 @@ impl ContextResolver {
         hash_context_with_seen(name, tags, origin_key, &mut self.hash_seen_buffer)
     }
 
-    fn create_context_from_resolvable<T>(&self, key: ContextKey, name: &str, tags: T) -> Option<Context>
+    fn create_context<I, T>(&self, key: ContextKey, name: &str, tags: I) -> Option<Context>
     where
-        T: Tagged,
+        I: IntoIterator<Item = T>,
+        T: AsRef<str>,
     {
         // Intern the name and tags of the context.
         let context_name = self.intern(name)?;
 
         let mut context_tags = TagSet::default();
-
-        // TODO: This is clunky.
-        let mut failed = false;
-        tags.visit_tags(|tag| {
-            // Don't keep trying to intern the tags if we failed to intern the last one.
-            if !failed {
-                match self.intern(tag) {
-                    Some(tag) => context_tags.insert_tag(tag),
-                    None => failed = true,
-                }
-            }
-        });
-
-        if failed {
-            return None;
+        for tag in tags {
+            let tag = self.intern(tag.as_ref())?;
+            context_tags.insert_tag(tag);
         }
 
         // Collect any enriched tags based on the origin key of the context, if any.
@@ -389,18 +377,19 @@ impl ContextResolver {
     ///
     /// `None` may be returned if the interner is full and outside allocations are disallowed. See
     /// `allow_heap_allocations` for more information.
-    pub fn resolve<T>(&mut self, name: &str, tags: T, origin_info: Option<OriginInfo<'_>>) -> Option<Context>
+    pub fn resolve<I, T>(&mut self, name: &str, tags: I, origin_info: Option<OriginInfo<'_>>) -> Option<Context>
     where
-        T: Tagged,
+        I: IntoIterator<Item = T> + Clone,
+        T: AsRef<str>,
     {
-        let context_key = self.create_context_key_from_resolvable(name, &tags, origin_info);
+        let context_key = self.create_context_key(name, tags.clone(), origin_info);
         match self.context_cache.get(&context_key) {
             Some(context) => {
                 self.stats.resolved_existing_context_total().increment(1);
                 self.expiration.mark_entry_accessed(context_key);
                 Some(context)
             }
-            None => match self.create_context_from_resolvable(context_key, name, tags) {
+            None => match self.create_context(context_key, name, tags) {
                 Some(context) => {
                     debug!(?context_key, ?context, "Resolved new context.");
 

lib/saluki-context/src/tags/mod.rs

@@ -7,6 +7,14 @@ use stringtheory::MetaString;
 mod raw;
 pub use self::raw::RawTags;
 
+/// A value containing tags that can be visited.
+pub trait Tagged {
+    /// Visits the tags in this value.
+    fn visit_tags<F>(&self, visitor: F)
+    where
+        F: FnMut(&Tag);
+}
+
 /// A metric tag.
 #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
 pub struct Tag(MetaString);
@@ -349,6 +357,17 @@ impl From<Tag> for TagSet {
     }
 }
 
+impl Tagged for TagSet {
+    fn visit_tags<F>(&self, mut visitor: F)
+    where
+        F: FnMut(&Tag),
+    {
+        for tag in &self.0 {
+            visitor(tag);
+        }
+    }
+}
+
 /// A shared, read-only set of tags.
 #[derive(Clone, Debug)]
 pub struct SharedTagSet(Arc<TagSet>);
@@ -431,6 +450,15 @@ impl<'a> IntoIterator for &'a SharedTagSet {
     }
 }
 
+impl Tagged for SharedTagSet {
+    fn visit_tags<F>(&self, visitor: F)
+    where
+        F: FnMut(&Tag),
+    {
+        self.0.visit_tags(visitor);
+    }
+}
+
 fn tag_has_name(tag: &Tag, tag_name: &str) -> bool {
     // Try matching it as a key-value pair (e.g., `env:production`) first, and then just try matching it as a bare tag
     // (e.g., `production`).

lib/saluki-context/src/tags/raw.rs

@@ -1,5 +1,3 @@
-use crate::Tagged;
-
 /// A wrapper over raw tags in their unprocessed form.
 ///
 /// This type is meant to handle raw tags that have been extracted from network payloads, such as DogStatsD, where the
@@ -65,17 +63,6 @@ impl<'a> IntoIterator for RawTags<'a> {
     }
 }
 
-impl<'a> Tagged for RawTags<'a> {
-    fn visit_tags<F>(&self, mut visitor: F)
-    where
-        F: FnMut(&str),
-    {
-        for tag in self.tags_iter() {
-            visitor(tag);
-        }
-    }
-}
-
 pub struct RawTagsIter<'a> {
     raw_tags: &'a str,
     parsed_tags: usize,