Skip to content
Open
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
132 changes: 128 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ http = "1.1.0"
http-body-util = "0.1.2"
libsqlite3-sys = "0.30.1"
metrics = "0.24.0"
metrics-exporter-dogstatsd = "0.9.6"
metrics-exporter-statsd = "0.9.0"
prost = "0.13"
prost-types = "0.13.3"
Expand Down
13 changes: 11 additions & 2 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ pub struct Config {
/// The statsd address to report metrics to.
pub statsd_addr: String,

/// The dogstatsd metrics host.
pub metrics_host: String,

/// Default tags to add to all metrics.
pub default_metrics_tags: BTreeMap<String, String>,

Expand Down Expand Up @@ -246,12 +249,15 @@ impl Default for Config {
sentry_dsn: None,
sentry_env: None,
traces_sample_rate: Some(0.0),
log_filter: "info,librdkafka=warn,h2=off".to_owned(),
log_filter:
"info,librdkafka=warn,h2=off,metrics_exporter_dogstatsd::forwarder::sync=off"
.to_owned(),
log_format: LogFormat::Text,
grpc_addr: "0.0.0.0".to_owned(),
grpc_port: 50051,
grpc_shared_secret: vec![],
statsd_addr: "127.0.0.1:8126".parse().unwrap(),
metrics_host: "".to_owned(),
default_metrics_tags: Default::default(),
kafka_cluster: "127.0.0.1:9092".to_owned(),
kafka_consumer_group: "taskworker".to_owned(),
Expand Down Expand Up @@ -432,7 +438,10 @@ mod tests {
};
assert_eq!(config.sentry_dsn, None);
assert_eq!(config.sentry_env, None);
assert_eq!(config.log_filter, "info,librdkafka=warn,h2=off");
assert_eq!(
config.log_filter,
"info,librdkafka=warn,h2=off,metrics_exporter_dogstatsd::forwarder::sync=off"
);
assert_eq!(config.log_format, LogFormat::Text);
assert_eq!(config.grpc_port, 50051);
assert_eq!(config.kafka_topic, "taskworker");
Expand Down
50 changes: 38 additions & 12 deletions src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use crate::config::Config;
use metrics_exporter_dogstatsd::DogStatsDBuilder;
use metrics_exporter_statsd::StatsdBuilder;
use std::{
collections::BTreeMap,
net::{SocketAddr, ToSocketAddrs},
};

pub struct MetricsConfig {
pub metrics_host: String,
pub statsd_addr: SocketAddr,
pub default_tags: BTreeMap<String, String>,
}
Expand All @@ -21,25 +23,49 @@ impl MetricsConfig {
};
MetricsConfig {
statsd_addr: *statsd_addr,
metrics_host: config.metrics_host.clone(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unconditional statsd resolution panics when using dogstatsd

Low Severity

MetricsConfig::from_config unconditionally resolves statsd_addr via to_socket_addrs() with .expect(), even when metrics_host is configured and statsd won't be used. If a deployment only intends to use dogstatsd and statsd_addr is set to an invalid or unresolvable value, the application will panic at startup despite statsd not being needed. The resolution of statsd_addr could be deferred or made conditional on metrics_host being empty.

Fix in Cursor Fix in Web

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's ok. If metrics_host works, I'll remove all the statsd_addr logic.

default_tags: config.default_metrics_tags.clone(),
}
}
}

pub fn init(metrics_config: MetricsConfig) {
let address = metrics_config.statsd_addr;
if metrics_config.metrics_host.is_empty() {
// Fallback to statsd forwarder
let address = metrics_config.statsd_addr;

let builder = StatsdBuilder::from(address.ip().to_string(), address.port());
let builder = StatsdBuilder::from(address.ip().to_string(), address.port());

let recorder = metrics_config
.default_tags
.into_iter()
.fold(
builder.with_queue_size(5000).with_buffer_size(1024),
|builder, (key, value)| builder.with_default_tag(key, value),
)
.build(Some("taskbroker"))
.expect("Could not create StatsdRecorder");
let recorder = metrics_config
.default_tags
.into_iter()
.fold(
builder.with_queue_size(5000).with_buffer_size(1024),
|builder, (key, value)| builder.with_default_tag(key, value),
)
.build(Some("taskbroker"))
.expect("Could not create StatsdRecorder");

metrics::set_global_recorder(recorder).expect("Could not set global metrics recorder")
metrics::set_global_recorder(recorder).expect("Could not set global metrics recorder")
} else {
let default_tags = metrics_config
.default_tags
.into_iter()
.map(|(key, value)| metrics::Label::new(key, value))
.collect();

// Use dogstatsd exporter if enabled.
let builder = DogStatsDBuilder::default()
.with_remote_address(metrics_config.metrics_host)
.expect("Could not set metrics host address")
.with_telemetry(true)
.send_histograms_as_distributions(true)
.with_histogram_sampling(true)
.set_global_prefix("taskbroker")
.with_global_labels(default_tags);

builder
.install()
.expect("Could not create DogStatsDBuilder");
}
}
Loading