Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix clippy lints #510

Merged
merged 5 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
too-many-lines-threshold = 150
ignore-interior-mutability = ["metrics::key::Key"]
joshka marked this conversation as resolved.
Show resolved Hide resolved
19 changes: 14 additions & 5 deletions metrics-exporter-prometheus/src/distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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));
}

Expand All @@ -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));
Expand Down
5 changes: 2 additions & 3 deletions metrics-exporter-prometheus/src/exporter/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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}");
joshka marked this conversation as resolved.
Show resolved Hide resolved

assert_eq!(rendered, expected_histogram);
}
Expand Down
8 changes: 4 additions & 4 deletions metrics-exporter-prometheus/src/exporter/http_listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
joshka marked this conversation as resolved.
Show resolved Hide resolved
let is_allowed = self.check_tcp_allowed(&stream);
let handle = self.handle.clone();
let service = service_fn(move |req: Request<body::Incoming>| {
Expand Down Expand Up @@ -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) {
joshka marked this conversation as resolved.
Show resolved Hide resolved
let handle = self.handle.clone();
let service = service_fn(move |req: Request<body::Incoming>| {
let handle = handle.clone();
Expand Down
2 changes: 1 addition & 1 deletion metrics-exporter-prometheus/src/exporter/push_gateway.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
78 changes: 40 additions & 38 deletions metrics-exporter-prometheus/src/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,35 +110,37 @@ pub fn write_metric_line<T, T2>(
/// [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].
///
/// [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].
Expand Down Expand Up @@ -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::*;

Expand Down Expand Up @@ -321,11 +323,11 @@ mod tests {
let as_chars = result.chars().collect::<Vec<_>>();

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");
}

Expand All @@ -335,7 +337,7 @@ mod tests {
let as_chars = result.chars().collect::<Vec<_>>();

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");
}

Expand All @@ -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");
}

Expand All @@ -369,7 +371,7 @@ mod tests {
let as_chars = delayered_backslashes.chars().collect::<Vec<_>>();

// 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
joshka marked this conversation as resolved.
Show resolved Hide resolved

runtime.spawn(exporter); //async { exporter.await});
tokio::time::sleep(Duration::from_millis(200)).await;
Expand All @@ -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"));
});
}

Expand Down
20 changes: 10 additions & 10 deletions metrics-tracing-context/benches/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
joshka marked this conversation as resolved.
Show resolved Hide resolved
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);
joshka marked this conversation as resolved.
Show resolved Hide resolved
static METADATA: metrics::Metadata =
metrics::Metadata::new(module_path!(), metrics::Level::INFO, Some(module_path!()));

Expand All @@ -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!()));

Expand All @@ -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!()));

Expand All @@ -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!()));

Expand All @@ -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!()));

Expand Down
2 changes: 1 addition & 1 deletion metrics-tracing-context/benches/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}

Expand Down
1 change: 1 addition & 0 deletions metrics-tracing-context/src/tracing_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ impl AsRef<Map> for Labels {
/// fields and allows them to be later on used as metrics labels.
#[derive(Default)]
pub struct MetricsLayer {
#[allow(clippy::type_complexity)]
joshka marked this conversation as resolved.
Show resolved Hide resolved
with_labels:
Option<fn(&Dispatch, &Id, f: &mut dyn FnMut(&Labels) -> Option<Key>) -> Option<Key>>,
}
Expand Down
16 changes: 8 additions & 8 deletions metrics-tracing-context/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
];
Expand Down Expand Up @@ -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!(
Expand Down
Loading
Loading