Skip to content

Commit cdfb92e

Browse files
authored
chore(dogstatsd): enforce a minimum sample rate on metrics (#1021)
1 parent b78e3ab commit cdfb92e

File tree

3 files changed

+91
-1
lines changed

3 files changed

+91
-1
lines changed

lib/saluki-core/src/data_model/event/metric/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,11 @@ impl SampleRate {
190190
Self(1.0)
191191
}
192192

193+
/// Returns the sample rate.
194+
pub const fn rate(&self) -> f64 {
195+
self.0
196+
}
197+
193198
/// Returns the weight of the sample rate.
194199
pub fn weight(&self) -> u64 {
195200
(1.0 / self.0) as u64

lib/saluki-io/src/deser/codec/dogstatsd/metric.rs

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use nom::{
99
};
1010
use saluki_context::{origin::OriginTagCardinality, tags::RawTags};
1111
use saluki_core::data_model::event::metric::*;
12+
use tracing::warn;
1213

1314
use super::{helpers::*, DogstatsdCodecConfiguration, NomParserError};
1415

@@ -72,7 +73,9 @@ pub fn parse_dogstatsd_metric<'a>(
7273
// point downstream to calculate the true metric value.
7374
b'@' => {
7475
let (_, sample_rate) =
75-
all_consuming(preceded(tag("@"), map_res(double, SampleRate::try_from))).parse(chunk)?;
76+
all_consuming(preceded(tag("@"), map_res(double, sample_rate(metric_name, config))))
77+
.parse(chunk)?;
78+
7679
maybe_sample_rate = Some(sample_rate);
7780
}
7881
// Tags: additional tags to be added to the metric.
@@ -156,6 +159,24 @@ fn permissive_metric_name(input: &[u8]) -> IResult<&[u8], &str> {
156159
.parse(input)
157160
}
158161

162+
#[inline]
163+
fn sample_rate<'a>(
164+
metric_name: &'a str, config: &'a DogstatsdCodecConfiguration,
165+
) -> impl Fn(f64) -> Result<SampleRate, &'static str> + 'a {
166+
let minimum_sample_rate = config.minimum_sample_rate;
167+
168+
move |mut raw_sample_rate| {
169+
if raw_sample_rate < minimum_sample_rate {
170+
raw_sample_rate = minimum_sample_rate;
171+
warn!(
172+
"Sample rate for metric '{}' is below minimum of {}. Clamping to minimum.",
173+
metric_name, minimum_sample_rate
174+
);
175+
}
176+
SampleRate::try_from(raw_sample_rate)
177+
}
178+
}
179+
159180
#[inline]
160181
fn raw_metric_values(input: &[u8]) -> IResult<&[u8], (MetricType, &[u8])> {
161182
let (remaining, raw_values) = terminated(take_while1(|b| b != b'|'), tag("|")).parse(input)?;
@@ -568,6 +589,47 @@ mod tests {
568589
}
569590
}
570591

592+
#[test]
593+
fn minimum_sample_rate() {
594+
// Sample rate of 0.01 should lead to a count of 100 when handling a single value.
595+
let minimum_sample_rate = SampleRate::try_from(0.01).unwrap();
596+
let config = DogstatsdCodecConfiguration::default().with_minimum_sample_rate(minimum_sample_rate.rate());
597+
598+
let cases = [
599+
// Worst case scenario: sample rate of zero, or "infinitely sampled".
600+
"test:1|d|@0".to_string(),
601+
// Bunch of values with different sample rates all below the minimum sample rate.
602+
"test:1|d|@0.001".to_string(),
603+
"test:1|d|@0.0005".to_string(),
604+
"test:1|d|@0.00001".to_string(),
605+
// Control: use the minimum sample rate.
606+
format!("test:1|d|@{}", minimum_sample_rate.rate()),
607+
// Bunch of values with _greater_ sampling rates than the minimum.
608+
"test:1|d|@0.1".to_string(),
609+
"test:1|d|@0.5".to_string(),
610+
"test:1|d".to_string(),
611+
];
612+
613+
for input in cases {
614+
let metric = parse_dsd_metric_with_conf(input.as_bytes(), &config)
615+
.expect("Should not fail to parse metric.")
616+
.expect("Metric should be present.");
617+
618+
let sketch = match metric.values() {
619+
MetricValues::Distribution(points) => {
620+
points
621+
.into_iter()
622+
.next()
623+
.expect("Should have at least one sketch point.")
624+
.1
625+
}
626+
_ => panic!("Unexpected metric type."),
627+
};
628+
629+
assert!(sketch.count() as u64 <= minimum_sample_rate.weight());
630+
}
631+
}
632+
571633
proptest! {
572634
#![proptest_config(ProptestConfig::with_cases(1000))]
573635
#[test]

lib/saluki-io/src/deser/codec/dogstatsd/mod.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,16 @@ pub use self::service_check::ServiceCheckPacket;
1414

1515
type NomParserError<'a> = nom::Err<nom::error::Error<&'a [u8]>>;
1616

17+
// This is the lowest sample rate that we consider to be "safe" with typical DogStatsD default settings.
18+
//
19+
// Our logic here is:
20+
// - DogStatsD payloads are limited to 8KiB by default
21+
// - a valid distribution metric could have a multi-value payload with ~4093 values (value of `1`, when factoring for protocol overhead)
22+
// - to avoid overflow in resulting sketch, total count of all values must be less than or equal to 2^32
23+
// - 2^32 / 4093 = 1,049,344... or 1,000,000 to be safe
24+
// - 1 / 1,000,000 = 0.000001
25+
const MINIMUM_SAFE_DEFAULT_SAMPLE_RATE: f64 = 0.000001;
26+
1727
/// Parser error.
1828
#[derive(Debug)]
1929
pub struct ParseError {
@@ -64,6 +74,7 @@ pub struct DogstatsdCodecConfiguration {
6474
maximum_tag_length: usize,
6575
maximum_tag_count: usize,
6676
timestamps: bool,
77+
minimum_sample_rate: f64,
6778
}
6879

6980
impl DogstatsdCodecConfiguration {
@@ -115,6 +126,17 @@ impl DogstatsdCodecConfiguration {
115126
self.timestamps = timestamps;
116127
self
117128
}
129+
130+
/// Sets the minimum sample rate.
131+
///
132+
/// This is the minimum sample rate that is allowed for a metric payload. If the sample rate is less than this limit,
133+
/// the sample rate is clamped to this value and a log message is emitted.
134+
///
135+
/// Defaults to `0.000001`.
136+
pub fn with_minimum_sample_rate(mut self, minimum_sample_rate: f64) -> Self {
137+
self.minimum_sample_rate = minimum_sample_rate;
138+
self
139+
}
118140
}
119141

120142
impl Default for DogstatsdCodecConfiguration {
@@ -124,6 +146,7 @@ impl Default for DogstatsdCodecConfiguration {
124146
maximum_tag_count: usize::MAX,
125147
timestamps: true,
126148
permissive: false,
149+
minimum_sample_rate: MINIMUM_SAFE_DEFAULT_SAMPLE_RATE,
127150
}
128151
}
129152
}

0 commit comments

Comments
 (0)