[METRICS SDK] Drop aggregation behaves as a no-op - #4515
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4515 +/- ##
==========================================
- Coverage 83.46% 83.45% -0.00%
==========================================
Files 521 521
Lines 20412 20467 +55
==========================================
+ Hits 17034 17078 +44
- Misses 3378 3389 +11
🚀 New features to boost your workflow:
|
…antic error warning for view configurations that duplicate metrics. Update meter tests to verify drop aggregation and multiple views.
e66422a to
8be812a
Compare
| // ENABLE_METRICS_EXEMPLAR_PREVIEW = ON | ||
| // OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW = OFF | ||
| // ------------------------------------------------------------------------------------------------------- | ||
| // ~/build/sdk/test/metrics/sync_instruments_benchmark |
There was a problem hiding this comment.
Drop aggregation cases with ABIv1 (bound instruments preview OFF). Instruments with drop aggregation are now within ~2ns of the fully disabled meter case and latency no longer scales with the number of threads recording with the instrument.
| Benchmark | Threads | main (ns) | this PR (ns) | Speedup |
|---|---|---|---|---|
| BM_Record_Counter_Drop_ByThreads | 1 | 181 | 2.29 | ~79x |
| BM_Record_Counter_Drop_ByThreads | 2 | 336 | 2.29 | ~147x |
| BM_Record_Counter_Drop_ByThreads | 4 | 548 | 2.35 | ~233x |
| BM_Record_Histogram_Drop_ByThreads | 1 | 180 | 2.50 | ~72x |
| BM_Record_Histogram_Drop_ByThreads | 2 | 198 | 2.52 | ~79x |
| BM_Record_Histogram_Drop_ByThreads | 4 | 556 | 2.56 | ~217x |
| // ------------------------------------------------------------------------------------------------------- | ||
| // ~/build/sdk/test/metrics/sync_instruments_benchmark | ||
| // 2026-08-22T19:58:12+00:00 | ||
| // 2026-09-02T04:38:03+00:00 |
There was a problem hiding this comment.
ABIv2 (bound instruments preview ON)
| Benchmark | Threads | main (ns) | this PR (ns) | Speedup |
|---|---|---|---|---|
| BM_Record_Counter_Drop_ByThreads | 1 | 291 | 1.75 | ~166x |
| BM_Record_Counter_Drop_ByThreads | 2 | 545 | 1.77 | ~308x |
| BM_Record_Counter_Drop_ByThreads | 4 | 1222 | 1.80 | ~679x |
| BM_Record_Histogram_Drop_ByThreads | 1 | 288 | 2.12 | ~136x |
| BM_Record_Histogram_Drop_ByThreads | 2 | 540 | 2.16 | ~250x |
| BM_Record_Histogram_Drop_ByThreads | 4 | 1423 | 2.16 | ~659x |
| BM_Record_Gauge_Drop_ByThreads | 1 | 286 | 1.77 | ~162x |
| BM_Record_Gauge_Drop_ByThreads | 2 | 546 | 1.76 | ~310x |
| BM_Record_Gauge_Drop_ByThreads | 4 | 1200 | 1.88 | ~638x |
| BM_Record_BoundCounter_Drop_ByThreads | 1 | 5.81 | 1.14 | ~5.1x |
| BM_Record_BoundCounter_Drop_ByThreads | 2 | 34.8 | 1.16 | ~30x |
| BM_Record_BoundCounter_Drop_ByThreads | 4 | 123 | 1.20 | ~103x |
| BM_Record_BoundHistogram_Drop_ByThreads | 1 | 6.19 | 1.76 | ~3.5x |
| BM_Record_BoundHistogram_Drop_ByThreads | 2 | 32.6 | 1.76 | ~19x |
| BM_Record_BoundHistogram_Drop_ByThreads | 4 | 155 | 1.79 | ~87x |
|
|
||
| auto &point_data1 = point_data_attr[0].point_data; | ||
| auto &point_data2 = point_data_attr[1].point_data; | ||
| TEST_F(MeterCreateInstrumentTest, SyncInstrumentWithConflictingAggregation) |
There was a problem hiding this comment.
Before this PR, this case would result in duplicate counts due to duplicate storages added to the multi storage object of the instrument. When this happened, each value passed to counter->Add would be added N times, where N is the number of non-drop views corresponding to a single metric. This bug was fixed by checking if the storage configured by a view already exists in the Multi{Sync,Async}Storage before adding it again.
Fixes #4514
When a view configures drop aggregation for an instrument (sync or async) the meter should not create a storage for that view.
When multiple views are configured for an instrument (a catch-all view and an instrument specific view), the SDK should detect semantic errors and prevent conflicting metrics.
https://opentelemetry.io/docs/specs/otel/metrics/data-model/#opentelemetry-protocol-data-model-producer-recommendations
Changes
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes