Skip to content

Commit c61c51c

Browse files
authored
fix(otlp): preserve cumulative explicit histogram state across payload generations (#1919)
* feat(otlp): enforce count strictly across amongst other invariants * feat(otlp): add changelog and replace falsely removed comments
1 parent 6577087 commit c61c51c

5 files changed

Lines changed: 52 additions & 23 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
metric kind to simplify intake debugging.
1313
- OpenTelemetry cumulative metric generation now updates cumulative sums using
1414
aggregation temporality, preserves cumulative histogram min/max bounds, and
15-
supports configurable histogram count limits.
15+
supports configurable histogram count limits. Cumulative explicit histograms
16+
now retain their state across payload generations.
1617
- HTTP blackhole now supports an `openmetrics` body variant for generated
1718
Prometheus/OpenMetrics scrape responses.
1819
- Updated to rand 0.10.x
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
otel_metrics_grpc: dfd6a48683a2731748fcb9c40a636ac93b7fffc679f73aa6e660f11b3060d4aa entropy=6.7678
1+
otel_metrics_grpc: 3cd72bf5b323d0482a620db2383d4f81e257a91eeb58090888bc5b2a27502b57 entropy=6.8202

lading_payload/src/opentelemetry/common/templates.rs

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -58,38 +58,56 @@ where
5858
G: crate::SizedGenerator<'a, Output = T>,
5959
G::Error: 'a,
6060
{
61-
// If we are at context cap, search by_size for templates <= budget and
62-
// return a random choice. If we are not at context cap, call
63-
// generator with the budget and then store the result
64-
// for future use in `by_size`.
61+
self.fetch_mut(rng, budget).map(|template| &*template)
62+
}
63+
64+
/// Return a mutable reference to an item from the pool.
65+
///
66+
/// Callers that model cumulative protocol state update this stored template
67+
/// so later selections of the same template retain their prior values.
68+
pub(crate) fn fetch_mut<'a, R>(
69+
&'a mut self,
70+
rng: &mut R,
71+
budget: &mut usize,
72+
) -> Result<&'a mut T, PoolError<G::Error>>
73+
where
74+
R: Rng + ?Sized,
75+
G: crate::SizedGenerator<'a, Output = T>,
76+
G::Error: 'a,
77+
{
78+
// If we are at context cap, search by_size for templates within the
79+
// budget. Otherwise, generate and store one additional template before
80+
// selecting a random eligible template.
6581
//
6682
// Size search is in the interval (0, budget].
67-
6883
let upper = *budget;
6984

7085
// Generate new instances until either context_cap is hit or the
71-
// remaining space drops below our lookup interval.
86+
// remaining storage budget is exhausted.
7287
if self.len < self.context_cap && self.consumed_bytes < self.max_available_bytes {
7388
let mut limit = *budget;
7489
if let Ok(item) = self.generator.generate(rng, &mut limit) {
75-
let sz = item.encoded_len();
76-
self.by_size.entry(sz).or_default().push(item);
90+
let size = item.encoded_len();
91+
self.by_size.entry(size).or_default().push(item);
7792
self.len += 1;
78-
self.consumed_bytes = self.consumed_bytes.saturating_add(sz);
79-
} else {
80-
// Generation failed. It's possible there's an existing
81-
// template that fits the budget.
93+
self.consumed_bytes = self.consumed_bytes.saturating_add(size);
8294
}
8395
}
96+
// A generation failure does not prevent selecting an existing template
97+
// that fits the requested budget.
8498

85-
let (choice_sz, choices) = self
99+
let choice_size = *self
86100
.by_size
87101
.range(..=upper)
102+
.map(|(size, _)| size)
88103
.choose(rng)
89104
.ok_or(PoolError::EmptyChoice)?;
90-
91-
let choice = choices.choose(rng).ok_or(PoolError::EmptyChoice)?;
92-
*budget = budget.saturating_sub(*choice_sz);
105+
let choices = self
106+
.by_size
107+
.get_mut(&choice_size)
108+
.ok_or(PoolError::EmptyChoice)?;
109+
let choice = choices.choose_mut(rng).ok_or(PoolError::EmptyChoice)?;
110+
*budget = budget.saturating_sub(choice_size);
93111

94112
Ok(choice)
95113
}

lading_payload/src/opentelemetry/metric.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -618,8 +618,8 @@ impl<'a> SizedGenerator<'a> for OpentelemetryMetrics {
618618
self.incr_f += rng.random_range(1.0..=100.0);
619619
self.incr_i += rng.random_range(1_i64..=100_i64);
620620

621-
let mut tpl: ResourceMetrics = match self.pool.fetch(rng, budget) {
622-
Ok(t) => t.to_owned(),
621+
let tpl: &mut ResourceMetrics = match self.pool.fetch_mut(rng, budget) {
622+
Ok(template) => template,
623623
Err(PoolError::EmptyChoice) => {
624624
debug!("Pool was unable to satify request for {budget} size");
625625
Err(PoolError::EmptyChoice)?
@@ -882,7 +882,7 @@ impl<'a> SizedGenerator<'a> for OpentelemetryMetrics {
882882
*budget = original_budget - required_bytes;
883883
self.data_points_per_resource = data_points_count;
884884

885-
Ok(tpl)
885+
Ok(tpl.clone())
886886
}
887887
}
888888

@@ -1814,7 +1814,10 @@ mod test {
18141814
let previous = first_histogram_point(&first);
18151815
let current = first_histogram_point(&second);
18161816
assert_non_decreasing_buckets(&previous.bucket_counts, &current.bucket_counts);
1817+
assert_eq!(current.count, current.bucket_counts.iter().sum::<u64>());
18171818
assert!(current.count >= previous.count);
1819+
assert!(current.time_unix_nano > previous.time_unix_nano);
1820+
assert_eq!(current.start_time_unix_nano, previous.start_time_unix_nano);
18181821
assert!(current.sum >= previous.sum);
18191822
assert!(current.min <= previous.min);
18201823
assert!(current.max >= previous.max);

lading_payload/src/opentelemetry/metric/templates.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,15 @@ impl<'a> crate::SizedGenerator<'a> for MetricTemplateGenerator {
194194
let prefix = kind.name_prefix();
195195
let name = format!("{prefix}_{name_suffix}");
196196

197-
// Use weighted distribution: heavily favors small numbers (1-2) but can go up to 60
198-
let total_data_points = exponential_weighted_range(rng, 1, 60);
197+
// Cumulative histograms keep one state per metric identity. Other
198+
// metric kinds use a weighted distribution, heavily favoring one or
199+
// two points while allowing up to 60 for payload variety.
200+
let total_data_points = match kind {
201+
Kind::Histogram {
202+
aggregation_temporality: 2,
203+
} => 1,
204+
_ => exponential_weighted_range(rng, 1, 60),
205+
};
199206
let data = match kind {
200207
Kind::Gauge => {
201208
let data_points = (0..total_data_points)

0 commit comments

Comments
 (0)