From 47c48f3830dc3962697f450fac0ef1e03636909b Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Thu, 23 Jan 2025 12:26:35 -0500 Subject: [PATCH] chore: make Tagged relevant to materialized tags, not raw tags --- lib/saluki-context/src/context.rs | 64 +++++------------------------- lib/saluki-context/src/hash.rs | 22 +++++----- lib/saluki-context/src/lib.rs | 2 +- lib/saluki-context/src/resolver.rs | 41 +++++++------------ lib/saluki-context/src/tags/mod.rs | 28 +++++++++++++ lib/saluki-context/src/tags/raw.rs | 13 ------ 6 files changed, 66 insertions(+), 104 deletions(-) diff --git a/lib/saluki-context/src/context.rs b/lib/saluki-context/src/context.rs index 77f761a6..8c67d3f6 100644 --- a/lib/saluki-context/src/context.rs +++ b/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(&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(&self, visitor: F) - where - F: FnMut(&str); -} - -impl<'a, T> Tagged for &'a T -where - T: Tagged, -{ - fn visit_tags(&self, visitor: F) - where - F: FnMut(&str), - { - (*self).visit_tags(visitor) - } -} - -impl<'a> Tagged for &'a [&'static str] { - fn visit_tags(&self, mut visitor: F) - where - F: FnMut(&str), - { - for tag in self.iter() { - visitor(tag); - } - } -} - -impl<'a> Tagged for &'a [MetaString] { - fn visit_tags(&self, mut visitor: F) - where - F: FnMut(&str), - { - for tag in self.iter() { - visitor(tag); - } - } -} - -impl<'a> Tagged for &'a TagSet { - fn visit_tags(&self, mut visitor: F) - where - F: FnMut(&str), - { - for tag in self.into_iter() { - visitor(tag.as_str()); - } - } -} diff --git a/lib/saluki-context/src/hash.rs b/lib/saluki-context/src/hash.rs index 868d24b1..327a3019 100644 --- a/lib/saluki-context/src/hash.rs +++ b/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 = HashSet; @@ -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(name: &str, tags: T, origin_key: Option) -> ContextKey +pub fn hash_context(name: &str, tags: I, origin_key: Option) -> ContextKey where - T: Tagged, + I: IntoIterator, + T: AsRef, { 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( - name: &str, tags: T, origin_key: Option, seen: &mut FastHashSet, +pub(super) fn hash_context_with_seen( + name: &str, tags: I, origin_key: Option, seen: &mut FastHashSet, ) -> ContextKey where - T: Tagged, + I: IntoIterator, + T: AsRef, { 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); diff --git a/lib/saluki-context/src/lib.rs b/lib/saluki-context/src/lib.rs index cd8ae0d7..8f8bcf6e 100644 --- a/lib/saluki-context/src/lib.rs +++ b/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; diff --git a/lib/saluki-context/src/resolver.rs b/lib/saluki-context/src/resolver.rs index 1861fcf9..233cadc1 100644 --- a/lib/saluki-context/src/resolver.rs +++ b/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( - &mut self, name: &str, tags: T, origin_info: Option>, - ) -> ContextKey + fn create_context_key(&mut self, name: &str, tags: I, origin_info: Option>) -> ContextKey where - T: Tagged, + I: IntoIterator, + T: AsRef, { // 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(&self, key: ContextKey, name: &str, tags: T) -> Option + fn create_context(&self, key: ContextKey, name: &str, tags: I) -> Option where - T: Tagged, + I: IntoIterator, + T: AsRef, { // 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(&mut self, name: &str, tags: T, origin_info: Option>) -> Option + pub fn resolve(&mut self, name: &str, tags: I, origin_info: Option>) -> Option where - T: Tagged, + I: IntoIterator + Clone, + T: AsRef, { - 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."); diff --git a/lib/saluki-context/src/tags/mod.rs b/lib/saluki-context/src/tags/mod.rs index 2498539a..e5ff13e9 100644 --- a/lib/saluki-context/src/tags/mod.rs +++ b/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(&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 for TagSet { } } +impl Tagged for TagSet { + fn visit_tags(&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); @@ -431,6 +450,15 @@ impl<'a> IntoIterator for &'a SharedTagSet { } } +impl Tagged for SharedTagSet { + fn visit_tags(&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`). diff --git a/lib/saluki-context/src/tags/raw.rs b/lib/saluki-context/src/tags/raw.rs index 216ae815..75dabfe9 100644 --- a/lib/saluki-context/src/tags/raw.rs +++ b/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(&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,