From 55273f04cb8647f30f4e603ccfbfd081da6939cd Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Wed, 28 Aug 2024 09:19:19 -0700 Subject: [PATCH] Fix clippy lints (#510) --- clippy.toml | 1 + .../src/distribution.rs | 19 +++-- .../src/exporter/builder.rs | 5 +- .../src/exporter/http_listener.rs | 8 +- .../src/exporter/push_gateway.rs | 2 +- metrics-exporter-prometheus/src/formatting.rs | 78 ++++++++++--------- .../tests/http_listener_integration_test.rs | 4 +- metrics-tracing-context/benches/layer.rs | 20 ++--- metrics-tracing-context/benches/visit.rs | 2 +- .../src/tracing_integration.rs | 1 + metrics-tracing-context/tests/integration.rs | 16 ++-- metrics-util/benches/filter.rs | 12 +-- metrics-util/benches/prefix.rs | 8 +- metrics-util/benches/registry.rs | 20 ++--- metrics-util/src/layers/filter.rs | 4 +- metrics-util/src/layers/router.rs | 3 +- metrics-util/src/registry/recency.rs | 1 + metrics/benches/macros.rs | 20 ++--- metrics/examples/basic.rs | 4 +- metrics/src/key.rs | 36 ++++----- 20 files changed, 132 insertions(+), 132 deletions(-) diff --git a/clippy.toml b/clippy.toml index 5e4d2492..be264b66 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1,2 @@ too-many-lines-threshold = 150 +ignore-interior-mutability = ["metrics::key::Key"] diff --git a/metrics-exporter-prometheus/src/distribution.rs b/metrics-exporter-prometheus/src/distribution.rs index 5a8d0f0b..192c75f7 100644 --- a/metrics-exporter-prometheus/src/distribution.rs +++ b/metrics-exporter-prometheus/src/distribution.rs @@ -33,7 +33,10 @@ pub enum Distribution { impl Distribution { /// Creates a histogram distribution. - #[warn(clippy::missing_panics_doc)] + /// + /// # Panics + /// + /// Panics if `buckets` is empty. pub fn new_histogram(buckets: &[f64]) -> Distribution { let hist = Histogram::new(buckets).expect("buckets should never be empty"); Distribution::Histogram(hist) @@ -299,8 +302,11 @@ mod tests { let snapshot = summary.snapshot(clock.now()); assert_eq!(0, snapshot.count()); - assert_eq!(f64::INFINITY, snapshot.min()); - assert_eq!(f64::NEG_INFINITY, snapshot.max()); + #[allow(clippy::float_cmp)] + { + assert_eq!(f64::INFINITY, snapshot.min()); + assert_eq!(f64::NEG_INFINITY, snapshot.max()); + } assert_eq!(None, snapshot.quantile(0.5)); } @@ -318,8 +324,11 @@ mod tests { let snapshot = summary.snapshot(clock.now()); - assert_eq!(42.0, snapshot.min()); - assert_eq!(42.0, snapshot.max()); + #[allow(clippy::float_cmp)] + { + assert_eq!(42.0, snapshot.min()); + assert_eq!(42.0, snapshot.max()); + } // 42 +/- (42 * 0.0001) assert!(Some(41.9958) < snapshot.quantile(0.5)); assert!(Some(42.0042) > snapshot.quantile(0.5)); diff --git a/metrics-exporter-prometheus/src/exporter/builder.rs b/metrics-exporter-prometheus/src/exporter/builder.rs index 7565ba2b..909a3ecd 100644 --- a/metrics-exporter-prometheus/src/exporter/builder.rs +++ b/metrics-exporter-prometheus/src/exporter/builder.rs @@ -560,8 +560,7 @@ mod tests { gauge1.set(-3.14); let rendered = handle.render(); let expected_gauge = format!( - "{}# TYPE basic_gauge gauge\nbasic_gauge{{wutang=\"forever\"}} -3.14\n\n", - expected_counter + "{expected_counter}# TYPE basic_gauge gauge\nbasic_gauge{{wutang=\"forever\"}} -3.14\n\n", ); assert_eq!(rendered, expected_gauge); @@ -579,7 +578,7 @@ mod tests { "basic_histogram_count 1\n", "\n" ); - let expected_histogram = format!("{}{}", expected_gauge, histogram_data); + let expected_histogram = format!("{expected_gauge}{histogram_data}"); assert_eq!(rendered, expected_histogram); } diff --git a/metrics-exporter-prometheus/src/exporter/http_listener.rs b/metrics-exporter-prometheus/src/exporter/http_listener.rs index e820cec9..441826b4 100644 --- a/metrics-exporter-prometheus/src/exporter/http_listener.rs +++ b/metrics-exporter-prometheus/src/exporter/http_listener.rs @@ -59,11 +59,11 @@ impl HttpListeningExporter { continue; } }; - self.process_tcp_stream(stream).await; + self.process_tcp_stream(stream); } } - async fn process_tcp_stream(&self, stream: TcpStream) { + fn process_tcp_stream(&self, stream: TcpStream) { let is_allowed = self.check_tcp_allowed(&stream); let handle = self.handle.clone(); let service = service_fn(move |req: Request| { @@ -107,12 +107,12 @@ impl HttpListeningExporter { continue; } }; - self.process_uds_stream(stream).await; + self.process_uds_stream(stream); } } #[cfg(feature = "uds-listener")] - async fn process_uds_stream(&self, stream: UnixStream) { + fn process_uds_stream(&self, stream: UnixStream) { let handle = self.handle.clone(); let service = service_fn(move |req: Request| { let handle = handle.clone(); diff --git a/metrics-exporter-prometheus/src/exporter/push_gateway.rs b/metrics-exporter-prometheus/src/exporter/push_gateway.rs index 3c349918..a1c2a4e7 100644 --- a/metrics-exporter-prometheus/src/exporter/push_gateway.rs +++ b/metrics-exporter-prometheus/src/exporter/push_gateway.rs @@ -94,7 +94,7 @@ fn basic_auth(username: &str, password: Option<&str>) -> HeaderValue { header } -#[cfg(all(test))] +#[cfg(test)] mod tests { use super::basic_auth; diff --git a/metrics-exporter-prometheus/src/formatting.rs b/metrics-exporter-prometheus/src/formatting.rs index 75f61524..ea26219f 100644 --- a/metrics-exporter-prometheus/src/formatting.rs +++ b/metrics-exporter-prometheus/src/formatting.rs @@ -110,17 +110,18 @@ pub fn write_metric_line( /// [data model]: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels pub fn sanitize_metric_name(name: &str) -> String { // The first character must be [a-zA-Z_:], and all subsequent characters must be [a-zA-Z0-9_:]. - let mut out = String::with_capacity(name.len()); - let mut is_invalid: fn(char) -> bool = invalid_metric_name_start_character; - for c in name.chars() { - if is_invalid(c) { - out.push('_'); - } else { - out.push(c); - } - is_invalid = invalid_metric_name_character; - } - out + name.chars() + .enumerate() + .map(|(i, c)| { + if i == 0 && valid_metric_name_start_character(c) + || i != 0 && valid_metric_name_character(c) + { + c + } else { + '_' + } + }) + .collect() } /// Sanitizes a label key to be valid under the Prometheus [data model]. @@ -128,17 +129,18 @@ pub fn sanitize_metric_name(name: &str) -> String { /// [data model]: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels pub fn sanitize_label_key(key: &str) -> String { // The first character must be [a-zA-Z_], and all subsequent characters must be [a-zA-Z0-9_]. - let mut out = String::with_capacity(key.len()); - let mut is_invalid: fn(char) -> bool = invalid_label_key_start_character; - for c in key.chars() { - if is_invalid(c) { - out.push('_'); - } else { - out.push(c); - } - is_invalid = invalid_label_key_character; - } - out + key.chars() + .enumerate() + .map(|(i, c)| { + if i == 0 && valid_label_key_start_character(c) + || i != 0 && valid_label_key_character(c) + { + c + } else { + '_' + } + }) + .collect() } /// Sanitizes a label value to be valid under the Prometheus [data model]. @@ -209,35 +211,35 @@ fn sanitize_label_value_or_description(value: &str, is_desc: bool) -> String { } #[inline] -fn invalid_metric_name_start_character(c: char) -> bool { +fn valid_metric_name_start_character(c: char) -> bool { // Essentially, needs to match the regex pattern of [a-zA-Z_:]. - !(c.is_ascii_alphabetic() || c == '_' || c == ':') + c.is_ascii_alphabetic() || c == '_' || c == ':' } #[inline] -fn invalid_metric_name_character(c: char) -> bool { +fn valid_metric_name_character(c: char) -> bool { // Essentially, needs to match the regex pattern of [a-zA-Z0-9_:]. - !(c.is_ascii_alphanumeric() || c == '_' || c == ':') + c.is_ascii_alphanumeric() || c == '_' || c == ':' } #[inline] -fn invalid_label_key_start_character(c: char) -> bool { +fn valid_label_key_start_character(c: char) -> bool { // Essentially, needs to match the regex pattern of [a-zA-Z_]. - !(c.is_ascii_alphabetic() || c == '_') + c.is_ascii_alphabetic() || c == '_' } #[inline] -fn invalid_label_key_character(c: char) -> bool { +fn valid_label_key_character(c: char) -> bool { // Essentially, needs to match the regex pattern of [a-zA-Z0-9_]. - !(c.is_ascii_alphanumeric() || c == '_') + c.is_ascii_alphanumeric() || c == '_' } #[cfg(test)] mod tests { use crate::formatting::{ - invalid_label_key_character, invalid_label_key_start_character, - invalid_metric_name_character, invalid_metric_name_start_character, sanitize_description, - sanitize_label_key, sanitize_label_value, sanitize_metric_name, + sanitize_description, sanitize_label_key, sanitize_label_value, sanitize_metric_name, + valid_label_key_character, valid_label_key_start_character, valid_metric_name_character, + valid_metric_name_start_character, }; use proptest::prelude::*; @@ -321,11 +323,11 @@ mod tests { let as_chars = result.chars().collect::>(); if let Some(c) = as_chars.first() { - assert_eq!(false, invalid_metric_name_start_character(*c), + assert!(valid_metric_name_start_character(*c), "first character of metric name was not valid"); } - assert!(!as_chars.iter().any(|c| invalid_metric_name_character(*c)), + assert!(as_chars.iter().all(|c| valid_metric_name_character(*c)), "invalid character in metric name"); } @@ -335,7 +337,7 @@ mod tests { let as_chars = result.chars().collect::>(); if let Some(c) = as_chars.first() { - assert_eq!(false, invalid_label_key_start_character(*c), + assert!(valid_label_key_start_character(*c), "first character of label key was not valid"); } @@ -353,7 +355,7 @@ mod tests { } }*/ - assert!(!as_chars.iter().any(|c| invalid_label_key_character(*c)), + assert!(as_chars.iter().all(|c| valid_label_key_character(*c)), "invalid character in label key"); } @@ -369,7 +371,7 @@ mod tests { let as_chars = delayered_backslashes.chars().collect::>(); // If the first character is a double quote, then we messed up. - assert!(as_chars.first().map(|c| *c != '"').unwrap_or(true), + assert!(as_chars.first().map_or(true, |c| *c != '"'), "first character cannot be a double quote: {}", result); // Now look for unescaped characters in the rest of the string, in a windowed fashion. diff --git a/metrics-exporter-prometheus/tests/http_listener_integration_test.rs b/metrics-exporter-prometheus/tests/http_listener_integration_test.rs index 93f69f08..ffbd3efa 100644 --- a/metrics-exporter-prometheus/tests/http_listener_integration_test.rs +++ b/metrics-exporter-prometheus/tests/http_listener_integration_test.rs @@ -36,7 +36,7 @@ mod http_listener_test { let labels = vec![Label::new("wutang", "forever")]; let key = Key::from_parts("basic_gauge", labels); let gauge = recorder.register_gauge(&key, &METADATA); - gauge.set(-3.14); + gauge.set(-1.23); runtime.spawn(exporter); //async { exporter.await}); tokio::time::sleep(Duration::from_millis(200)).await; @@ -48,7 +48,7 @@ mod http_listener_test { let (status, body) = read_from(uri).await; assert_eq!(status, StatusCode::OK); - assert!(body.contains("basic_gauge{wutang=\"forever\"} -3.14")); + assert!(body.contains("basic_gauge{wutang=\"forever\"} -1.23")); }); } diff --git a/metrics-tracing-context/benches/layer.rs b/metrics-tracing-context/benches/layer.rs index 892b6668..512e0f1a 100644 --- a/metrics-tracing-context/benches/layer.rs +++ b/metrics-tracing-context/benches/layer.rs @@ -12,9 +12,9 @@ fn layer_benchmark(c: &mut Criterion) { let mut group = c.benchmark_group("layer"); group.bench_function("base case", |b| { let recorder = NoopRecorder; - static KEY_NAME: &'static str = "key"; + static KEY_NAME: &str = "key"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("foo", "bar")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); static METADATA: metrics::Metadata = metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!())); @@ -32,9 +32,9 @@ fn layer_benchmark(c: &mut Criterion) { let _guard = span.enter(); let recorder = NoopRecorder; - static KEY_NAME: &'static str = "key"; + static KEY_NAME: &str = "key"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("foo", "bar")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); static METADATA: metrics::Metadata = metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!())); @@ -53,9 +53,9 @@ fn layer_benchmark(c: &mut Criterion) { let _guard = span.enter(); let recorder = NoopRecorder; - static KEY_NAME: &'static str = "key"; + static KEY_NAME: &str = "key"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("foo", "bar")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); static METADATA: metrics::Metadata = metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!())); @@ -75,9 +75,9 @@ fn layer_benchmark(c: &mut Criterion) { let tracing_layer = TracingContextLayer::all(); let recorder = tracing_layer.layer(NoopRecorder); - static KEY_NAME: &'static str = "key"; + static KEY_NAME: &str = "key"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("foo", "bar")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); static METADATA: metrics::Metadata = metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!())); @@ -97,9 +97,9 @@ fn layer_benchmark(c: &mut Criterion) { let tracing_layer = TracingContextLayer::all(); let recorder = tracing_layer.layer(NoopRecorder); - static KEY_NAME: &'static str = "key"; + static KEY_NAME: &str = "key"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("foo", "bar")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); static METADATA: metrics::Metadata = metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!())); diff --git a/metrics-tracing-context/benches/visit.rs b/metrics-tracing-context/benches/visit.rs index 9b6db089..4e54217b 100644 --- a/metrics-tracing-context/benches/visit.rs +++ b/metrics-tracing-context/benches/visit.rs @@ -140,7 +140,7 @@ struct DebugStruct { impl DebugStruct { pub fn new() -> DebugStruct { - DebugStruct { field1: format!("yeehaw!"), field2: 324242343243 } + DebugStruct { field1: "yeehaw!".to_string(), field2: 324242343243 } } } diff --git a/metrics-tracing-context/src/tracing_integration.rs b/metrics-tracing-context/src/tracing_integration.rs index d6e235f4..70fa2810 100644 --- a/metrics-tracing-context/src/tracing_integration.rs +++ b/metrics-tracing-context/src/tracing_integration.rs @@ -97,6 +97,7 @@ impl AsRef for Labels { /// fields and allows them to be later on used as metrics labels. #[derive(Default)] pub struct MetricsLayer { + #[allow(clippy::type_complexity)] with_labels: Option Option) -> Option>, } diff --git a/metrics-tracing-context/tests/integration.rs b/metrics-tracing-context/tests/integration.rs index ccceed30..a522a816 100644 --- a/metrics-tracing-context/tests/integration.rs +++ b/metrics-tracing-context/tests/integration.rs @@ -7,13 +7,13 @@ use tracing::dispatcher::{set_default, Dispatch}; use tracing::{span, Level}; use tracing_subscriber::{layer::SubscriberExt, Registry}; -static LOGIN_ATTEMPTS: &'static str = "login_attempts"; -static LOGIN_ATTEMPTS_NONE: &'static str = "login_attempts_no_labels"; -static LOGIN_ATTEMPTS_STATIC: &'static str = "login_attempts_static_labels"; -static LOGIN_ATTEMPTS_DYNAMIC: &'static str = "login_attempts_dynamic_labels"; -static LOGIN_ATTEMPTS_BOTH: &'static str = "login_attempts_static_and_dynamic_labels"; -static MY_COUNTER: &'static str = "my_counter"; -static USER_EMAIL: &'static [Label] = &[ +static LOGIN_ATTEMPTS: &str = "login_attempts"; +static LOGIN_ATTEMPTS_NONE: &str = "login_attempts_no_labels"; +static LOGIN_ATTEMPTS_STATIC: &str = "login_attempts_static_labels"; +static LOGIN_ATTEMPTS_DYNAMIC: &str = "login_attempts_dynamic_labels"; +static LOGIN_ATTEMPTS_BOTH: &str = "login_attempts_static_and_dynamic_labels"; +static MY_COUNTER: &str = "my_counter"; +static USER_EMAIL: &[Label] = &[ Label::from_static_parts("user", "ferris"), Label::from_static_parts("user.email", "ferris@rust-lang.org"), ]; @@ -560,7 +560,7 @@ fn test_label_filtering() { #[test] fn test_label_allowlist() { - let snapshot = with_tracing_layer(TracingContextLayer::only_allow(&["env", "service"]), || { + let snapshot = with_tracing_layer(TracingContextLayer::only_allow(["env", "service"]), || { let user = "ferris"; let email = "ferris@rust-lang.org"; let span = span!( diff --git a/metrics-util/benches/filter.rs b/metrics-util/benches/filter.rs index 402b24be..40f4e5c5 100644 --- a/metrics-util/benches/filter.rs +++ b/metrics-util/benches/filter.rs @@ -15,9 +15,9 @@ fn layer_benchmark(c: &mut Criterion) { let patterns = vec!["tokio"]; let filter_layer = FilterLayer::from_patterns(patterns); let recorder = filter_layer.layer(NoopRecorder); - static KEY_NAME: &'static str = "tokio.foo"; + static KEY_NAME: &str = "tokio.foo"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("foo", "bar")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); static METADATA: metrics::Metadata = metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!())); @@ -29,9 +29,9 @@ fn layer_benchmark(c: &mut Criterion) { let patterns = vec!["tokio"]; let filter_layer = FilterLayer::from_patterns(patterns); let recorder = filter_layer.layer(NoopRecorder); - static KEY_NAME: &'static str = "hyper.foo"; + static KEY_NAME: &str = "hyper.foo"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("foo", "bar")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); static METADATA: metrics::Metadata = metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!())); @@ -41,9 +41,9 @@ fn layer_benchmark(c: &mut Criterion) { }); group.bench_function("noop recorder overhead (increment_counter)", |b| { let recorder = NoopRecorder; - static KEY_NAME: &'static str = "tokio.foo"; + static KEY_NAME: &str = "tokio.foo"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("foo", "bar")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); static METADATA: metrics::Metadata = metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!())); diff --git a/metrics-util/benches/prefix.rs b/metrics-util/benches/prefix.rs index ce2beb3a..54bf964a 100644 --- a/metrics-util/benches/prefix.rs +++ b/metrics-util/benches/prefix.rs @@ -7,9 +7,9 @@ fn layer_benchmark(c: &mut Criterion) { group.bench_function("basic", |b| { let prefix_layer = PrefixLayer::new("prefix"); let recorder = prefix_layer.layer(NoopRecorder); - static KEY_NAME: &'static str = "simple_key"; + static KEY_NAME: &str = "simple_key"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("foo", "bar")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); static METADATA: metrics::Metadata = metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!())); @@ -19,9 +19,9 @@ fn layer_benchmark(c: &mut Criterion) { }); group.bench_function("noop recorder overhead (increment_counter)", |b| { let recorder = NoopRecorder; - static KEY_NAME: &'static str = "simple_key"; + static KEY_NAME: &str = "simple_key"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("foo", "bar")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); static METADATA: metrics::Metadata = metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!())); diff --git a/metrics-util/benches/registry.rs b/metrics-util/benches/registry.rs index 916440ff..80e386c9 100644 --- a/metrics-util/benches/registry.rs +++ b/metrics-util/benches/registry.rs @@ -6,22 +6,22 @@ fn registry_benchmark(c: &mut Criterion) { let mut group = c.benchmark_group("registry"); group.bench_function("cached op (basic)", |b| { let registry = Registry::atomic(); - static KEY_NAME: &'static str = "simple_key"; - static KEY_DATA: Key = Key::from_static_name(&KEY_NAME); + static KEY_NAME: &str = "simple_key"; + static KEY_DATA: Key = Key::from_static_name(KEY_NAME); b.iter(|| registry.get_or_create_counter(&KEY_DATA, |_| ())) }); group.bench_function("cached op (labels)", |b| { let registry = Registry::atomic(); - static KEY_NAME: &'static str = "simple_key"; + static KEY_NAME: &str = "simple_key"; static KEY_LABELS: [Label; 1] = [Label::from_static_parts("type", "http")]; - static KEY_DATA: Key = Key::from_static_parts(&KEY_NAME, &KEY_LABELS); + static KEY_DATA: Key = Key::from_static_parts(KEY_NAME, &KEY_LABELS); b.iter(|| registry.get_or_create_counter(&KEY_DATA, |_| ())) }); group.bench_function("uncached op (basic)", |b| { b.iter_batched_ref( - || Registry::atomic(), + Registry::atomic, |registry| { let key = "simple_key".into(); registry.get_or_create_counter(&key, |_| ()) @@ -31,7 +31,7 @@ fn registry_benchmark(c: &mut Criterion) { }); group.bench_function("uncached op (labels)", |b| { b.iter_batched_ref( - || Registry::atomic(), + Registry::atomic, |registry| { let labels = vec![Label::new("type", "http")]; let key = ("simple_key", labels).into(); @@ -45,15 +45,15 @@ fn registry_benchmark(c: &mut Criterion) { }); group.bench_function("const key overhead (basic)", |b| { b.iter(|| { - static KEY_NAME: &'static str = "simple_key"; - Key::from_static_name(&KEY_NAME) + static KEY_NAME: &str = "simple_key"; + Key::from_static_name(KEY_NAME) }) }); group.bench_function("const key data overhead (labels)", |b| { b.iter(|| { - static KEY_NAME: &'static str = "simple_key"; + static KEY_NAME: &str = "simple_key"; static LABELS: [Label; 1] = [Label::from_static_parts("type", "http")]; - Key::from_static_parts(&KEY_NAME, &LABELS) + Key::from_static_parts(KEY_NAME, &LABELS) }) }); group.bench_function("owned key overhead (basic)", |b| b.iter(|| Key::from_name("simple_key"))); diff --git a/metrics-util/src/layers/filter.rs b/metrics-util/src/layers/filter.rs index 4d0db28b..0841ac79 100644 --- a/metrics-util/src/layers/filter.rs +++ b/metrics-util/src/layers/filter.rs @@ -223,7 +223,7 @@ mod tests { ]; let recorder = MockBasicRecorder::from_operations(expectations); - let filter = FilterLayer::from_patterns(&["tokio", "bb8"]); + let filter = FilterLayer::from_patterns(["tokio", "bb8"]); let filter = filter.layer(recorder); for operation in inputs { @@ -294,7 +294,7 @@ mod tests { ]; let recorder = MockBasicRecorder::from_operations(expectations); - let mut filter = FilterLayer::from_patterns(&["tokio", "bb8"]); + let mut filter = FilterLayer::from_patterns(["tokio", "bb8"]); let filter = filter.case_insensitive(true).layer(recorder); for operation in inputs { diff --git a/metrics-util/src/layers/router.rs b/metrics-util/src/layers/router.rs index 867049d1..9dccc0a5 100644 --- a/metrics-util/src/layers/router.rs +++ b/metrics-util/src/layers/router.rs @@ -193,9 +193,10 @@ mod tests { let _ = RouterBuilder::from_recorder(MockTestRecorder::new()).build(); let mut builder = RouterBuilder::from_recorder(MockTestRecorder::new()); + // ensure that &str, String, and Cow are all are accepted by the builder builder .add_route(MetricKindMask::COUNTER, "foo", MockTestRecorder::new()) - .add_route(MetricKindMask::GAUGE, "bar".to_owned(), MockTestRecorder::new()) + .add_route(MetricKindMask::GAUGE, String::from("bar"), MockTestRecorder::new()) .add_route(MetricKindMask::HISTOGRAM, Cow::Borrowed("baz"), MockTestRecorder::new()) .add_route(MetricKindMask::ALL, "quux", MockTestRecorder::new()); let _ = builder.build(); diff --git a/metrics-util/src/registry/recency.rs b/metrics-util/src/registry/recency.rs index 0ccf014a..805953c3 100644 --- a/metrics-util/src/registry/recency.rs +++ b/metrics-util/src/registry/recency.rs @@ -217,6 +217,7 @@ impl GenerationalAtomicStorage { /// tracking recency does not matter, despite their otherwise tight coupling. pub struct Recency { mask: MetricKindMask, + #[allow(clippy::type_complexity)] inner: Mutex<(Clock, HashMap)>, idle_timeout: Option, } diff --git a/metrics/benches/macros.rs b/metrics/benches/macros.rs index 291d143f..cd1e5201 100644 --- a/metrics/benches/macros.rs +++ b/metrics/benches/macros.rs @@ -8,8 +8,8 @@ use metrics::{ }; use rand::{thread_rng, Rng}; -#[derive(Default)] struct TestRecorder; + impl Recorder for TestRecorder { fn describe_counter(&self, _: KeyName, _: Option, _: SharedString) {} fn describe_gauge(&self, _: KeyName, _: Option, _: SharedString) {} @@ -38,19 +38,19 @@ fn macro_benchmark(c: &mut Criterion) { }) }); group.bench_function("global_initialized/no_labels", |b| { - let _ = metrics::set_global_recorder(TestRecorder::default()); + let _ = metrics::set_global_recorder(TestRecorder); b.iter(|| { counter!("counter_bench").increment(42); }); }); group.bench_function("global_initialized/with_static_labels", |b| { - let _ = metrics::set_global_recorder(TestRecorder::default()); + let _ = metrics::set_global_recorder(TestRecorder); b.iter(|| { counter!("counter_bench", "request" => "http", "svc" => "admin").increment(42); }); }); group.bench_function("global_initialized/with_dynamic_labels", |b| { - let _ = metrics::set_global_recorder(TestRecorder::default()); + let _ = metrics::set_global_recorder(TestRecorder); let label_val = thread_rng().gen::().to_string(); b.iter(move || { @@ -59,27 +59,21 @@ fn macro_benchmark(c: &mut Criterion) { }); }); group.bench_function("local_initialized/no_labels", |b| { - let recorder = TestRecorder::default(); - - metrics::with_local_recorder(&recorder, || { + metrics::with_local_recorder(&TestRecorder, || { b.iter(|| { counter!("counter_bench").increment(42); }); }); }); group.bench_function("local_initialized/with_static_labels", |b| { - let recorder = TestRecorder::default(); - - metrics::with_local_recorder(&recorder, || { + metrics::with_local_recorder(&TestRecorder, || { b.iter(|| { counter!("counter_bench", "request" => "http", "svc" => "admin").increment(42); }); }); }); group.bench_function("local_initialized/with_dynamic_labels", |b| { - let recorder = TestRecorder::default(); - - metrics::with_local_recorder(&recorder, || { + metrics::with_local_recorder(&TestRecorder, || { let label_val = thread_rng().gen::().to_string(); b.iter(move || { counter!("counter_bench", "request" => "http", "uid" => label_val.clone()) diff --git a/metrics/examples/basic.rs b/metrics/examples/basic.rs index 8d638386..d81c2759 100644 --- a/metrics/examples/basic.rs +++ b/metrics/examples/basic.rs @@ -45,7 +45,6 @@ impl HistogramFn for PrintHandle { } } -#[derive(Default)] struct PrintRecorder; impl Recorder for PrintRecorder { @@ -90,8 +89,7 @@ impl Recorder for PrintRecorder { } fn init_print_logger() { - let recorder = PrintRecorder::default(); - metrics::set_global_recorder(recorder).unwrap() + metrics::set_global_recorder(PrintRecorder).unwrap() } fn main() { diff --git a/metrics/src/key.rs b/metrics/src/key.rs index 900a7bf9..881fce29 100644 --- a/metrics/src/key.rs +++ b/metrics/src/key.rs @@ -260,25 +260,19 @@ mod tests { use crate::{KeyName, Label}; use std::{collections::HashMap, ops::Deref, sync::Arc}; - static BORROWED_NAME: &'static str = "name"; - static FOOBAR_NAME: &'static str = "foobar"; - static BORROWED_BASIC: Key = Key::from_static_name(&BORROWED_NAME); + static BORROWED_NAME: &str = "name"; + static FOOBAR_NAME: &str = "foobar"; + static BORROWED_BASIC: Key = Key::from_static_name(BORROWED_NAME); static LABELS: [Label; 1] = [Label::from_static_parts("key", "value")]; - static BORROWED_LABELS: Key = Key::from_static_parts(&BORROWED_NAME, &LABELS); + static BORROWED_LABELS: Key = Key::from_static_parts(BORROWED_NAME, &LABELS); #[test] fn test_key_ord_and_partialord() { - let keys_expected: Vec = vec![ - Key::from_name("aaaa").into(), - Key::from_name("bbbb").into(), - Key::from_name("cccc").into(), - ]; - - let keys_unsorted: Vec = vec![ - Key::from_name("bbbb").into(), - Key::from_name("cccc").into(), - Key::from_name("aaaa").into(), - ]; + let keys_expected: Vec = + vec![Key::from_name("aaaa"), Key::from_name("bbbb"), Key::from_name("cccc")]; + + let keys_unsorted: Vec = + vec![Key::from_name("bbbb"), Key::from_name("cccc"), Key::from_name("aaaa")]; let keys = { let mut keys = keys_unsorted.clone(); @@ -299,7 +293,7 @@ mod tests { fn test_key_eq_and_hash() { let mut keys = HashMap::new(); - let owned_basic: Key = Key::from_name("name").into(); + let owned_basic: Key = Key::from_name("name"); assert_eq!(&owned_basic, &BORROWED_BASIC); let previous = keys.insert(owned_basic, 42); @@ -309,7 +303,7 @@ mod tests { assert_eq!(previous, Some(&42)); let labels = LABELS.to_vec(); - let owned_labels = Key::from_parts(&BORROWED_NAME[..], labels); + let owned_labels = Key::from_parts(BORROWED_NAME, labels); assert_eq!(&owned_labels, &BORROWED_LABELS); let previous = keys.insert(owned_labels, 43); @@ -329,19 +323,19 @@ mod tests { let result1 = key1.to_string(); assert_eq!(result1, "Key(foobar)"); - let key2 = Key::from_parts(&FOOBAR_NAME[..], vec![Label::new("system", "http")]); + let key2 = Key::from_parts(FOOBAR_NAME, vec![Label::new("system", "http")]); let result2 = key2.to_string(); assert_eq!(result2, "Key(foobar, [system = http])"); let key3 = Key::from_parts( - &FOOBAR_NAME[..], + FOOBAR_NAME, vec![Label::new("system", "http"), Label::new("user", "joe")], ); let result3 = key3.to_string(); assert_eq!(result3, "Key(foobar, [system = http, user = joe])"); let key4 = Key::from_parts( - &FOOBAR_NAME[..], + FOOBAR_NAME, vec![ Label::new("black", "black"), Label::new("lives", "lives"), @@ -354,7 +348,7 @@ mod tests { #[test] fn test_key_name_equality() { - static KEY_NAME: &'static str = "key_name"; + static KEY_NAME: &str = "key_name"; let borrowed_const = KeyName::from_const_str(KEY_NAME); let borrowed_nonconst = KeyName::from(KEY_NAME);