Skip to content

Commit 28ed7eb

Browse files
committed
Bugfix - add validation for custom buckets provided for Histograms
1 parent c9388e4 commit 28ed7eb

File tree

3 files changed

+60
-0
lines changed

3 files changed

+60
-0
lines changed

opentelemetry-sdk/CHANGELOG.md

+6
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@
2626
"rust.opentelemetry.io/sdk/logger"
2727
[#2316](https://github.com/open-telemetry/opentelemetry-rust/pull/2316)
2828

29+
- **Bug Fix:** Validates the `with_boundaries` bucket boundaries used in
30+
Histograms. The boundaries provided by the user must be a non-empty vector,
31+
sorted in strictly increasing order, and contain no duplicates. Instruments
32+
will not record measurements if the boundaries are invalid.
33+
[#2351](https://github.com/open-telemetry/opentelemetry-rust/pull/2351)
34+
2935
## 0.27.0
3036

3137
Released 2024-Nov-11

opentelemetry-sdk/src/metrics/meter.rs

+33
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,20 @@ impl SdkMeter {
394394
return Histogram::new(Arc::new(NoopSyncInstrument::new()));
395395
}
396396

397+
if let Some(ref boundaries) = builder.boundaries {
398+
let validation_result = validate_buckets(boundaries);
399+
if let Err(err) = validation_result {
400+
otel_error!(
401+
name: "InstrumentCreationFailed",
402+
meter_name = self.scope.name(),
403+
instrument_name = builder.name.as_ref(),
404+
message = "Measurements from this Histogram will be ignored.",
405+
reason = format!("{}", err)
406+
);
407+
return Histogram::new(Arc::new(NoopSyncInstrument::new()));
408+
}
409+
}
410+
397411
match resolver
398412
.lookup(
399413
InstrumentKind::Histogram,
@@ -533,6 +547,25 @@ fn validate_instrument_config(name: &str, unit: &Option<Cow<'static, str>>) -> M
533547
validate_instrument_name(name).and_then(|_| validate_instrument_unit(unit))
534548
}
535549

550+
fn validate_buckets(buckets: &[f64]) -> MetricResult<()> {
551+
if buckets.is_empty() {
552+
return Err(MetricError::InvalidInstrumentConfiguration(
553+
"Buckets must not be empty",
554+
));
555+
}
556+
557+
// validate that buckets are sorted and non-duplicate
558+
for window in buckets.windows(2) {
559+
if window[0] >= window[1] {
560+
return Err(MetricError::InvalidInstrumentConfiguration(
561+
"Buckets must be sorted and non-duplicate",
562+
));
563+
}
564+
}
565+
566+
Ok(())
567+
}
568+
536569
fn validate_instrument_name(name: &str) -> MetricResult<()> {
537570
if name.is_empty() {
538571
return Err(MetricError::InvalidInstrumentConfiguration(

opentelemetry-sdk/src/metrics/mod.rs

+21
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,27 @@ mod tests {
178178
// As instrument name is invalid, no metrics should be exported
179179
test_context.check_no_metrics();
180180
}
181+
182+
let invalid_buckets = vec![
183+
vec![], // empty buckets
184+
vec![1.0, 1.0], // duplicate buckets
185+
vec![1.0, 2.0, 3.0, 2.0], // duplicate non consequent buckets
186+
vec![1.0, 2.0, 3.0, 4.0, 2.5], // unsorted buckets
187+
];
188+
for buckets in invalid_buckets {
189+
let test_context = TestContext::new(Temporality::Cumulative);
190+
let histogram = test_context
191+
.meter()
192+
.f64_histogram("test")
193+
.with_boundaries(buckets)
194+
.build();
195+
histogram.record(1.9, &[]);
196+
test_context.flush_metrics();
197+
198+
// As buckets via Advisory params are invalid, no metrics should be
199+
// exported
200+
test_context.check_no_metrics();
201+
}
181202
}
182203

183204
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]

0 commit comments

Comments
 (0)