Skip to content

Commit

Permalink
chore: remove ability to make tags on a context
Browse files Browse the repository at this point in the history
  • Loading branch information
tobz committed Jan 23, 2025
1 parent 4b91260 commit df12b22
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 88 deletions.
49 changes: 2 additions & 47 deletions lib/saluki-context/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use std::{
fmt, hash,
sync::{Arc, OnceLock},
};
use std::{fmt, hash, sync::Arc};

use metrics::Gauge;
use stringtheory::MetaString;
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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()
}
41 changes: 0 additions & 41 deletions lib/saluki-context/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit df12b22

Please sign in to comment.