Skip to content

Commit 2581c4d

Browse files
committed
Fix EvictedQueue bug with zero capacity
The EvictedQueue was checking for the length _before_ inserting, and popping extra items, then doing the insertion. In the case where the capacity is set to zero, it caused the pop operation to be a no-op on the first insert, and then insert an item anyway. This commit fixes the issue by moving the length check after the insert and popping any extra items. Fixes open-telemetry#1148
1 parent 1f1a4fe commit 2581c4d

File tree

2 files changed

+23
-2
lines changed

2 files changed

+23
-2
lines changed

opentelemetry-sdk/CHANGELOG.md

+5
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@
99
- Propagate shutdown calls from `PeriodicReader` to metrics exporter
1010
[#1138](https://github.com/open-telemetry/opentelemetry-rust/pull/1138).
1111

12+
### Changed
13+
14+
- Fix EvictedQueue bug when capacity is set to 0
15+
[#1151](https://github.com/open-telemetry/opentelemetry-rust/pull/1151).
16+
1217
### Removed
1318

1419
- Samplers no longer has access to `InstrumentationLibrary` as one of parameters

opentelemetry-sdk/src/trace/evicted_queue.rs

+18-2
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,12 @@ impl<T> EvictedQueue<T> {
2626
/// recording dropped count if over capacity.
2727
pub(crate) fn push_back(&mut self, value: T) {
2828
let queue = self.queue.get_or_insert_with(Default::default);
29-
if queue.len() as u32 == self.max_len {
29+
queue.push_back(value);
30+
31+
if queue.len() as u32 > self.max_len {
3032
queue.pop_front();
3133
self.dropped_count += 1;
3234
}
33-
queue.push_back(value);
3435
}
3536

3637
/// Moves all the elements of other into self, leaving other empty.
@@ -119,4 +120,19 @@ mod tests {
119120
(1..=capacity).collect::<VecDeque<_>>()
120121
);
121122
}
123+
124+
#[test]
125+
fn zero_capacity_test() {
126+
let capacity = 0;
127+
let mut queue = EvictedQueue::new(capacity);
128+
129+
queue.push_back(1);
130+
131+
assert_eq!(queue.dropped_count, 1);
132+
assert_eq!(queue.len(), capacity as usize);
133+
assert_eq!(
134+
queue.queue.unwrap(),
135+
(1..=capacity).collect::<VecDeque<_>>()
136+
);
137+
}
122138
}

0 commit comments

Comments
 (0)