Skip to content

[METRICS SDK] Drop aggregation behaves as a no-op - #4515

Open
dbarker wants to merge 5 commits into
open-telemetry:mainfrom
dbarker:perf_metrics_sync_storage_drop_aggregation
Open

[METRICS SDK] Drop aggregation behaves as a no-op#4515
dbarker wants to merge 5 commits into
open-telemetry:mainfrom
dbarker:perf_metrics_sync_storage_drop_aggregation

Conversation

@dbarker

@dbarker dbarker commented Sep 1, 2026

Copy link
Copy Markdown
Member

Fixes #4514

Changes

  • Check if a view configures drop aggregation before creating storage
  • Log a warning if the views will create duplicate conflicting metric storage objects.
  • Update meter test to verify drop aggregation (including catch-all view cases) and general cleanup/enhancement.
  • Updates sync instruments benchmark results

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.45455% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.44%. Comparing base (cadb1d6) to head (991aed7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...dk/include/opentelemetry/sdk/metrics/instruments.h 62.50% 6 Missing ⚠️
...telemetry/sdk/metrics/state/multi_metric_storage.h 83.34% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4515      +/-   ##
==========================================
- Coverage   83.46%   83.44%   -0.02%     
==========================================
  Files         521      521              
  Lines       20380    20435      +55     
==========================================
+ Hits        17009    17049      +40     
- Misses       3371     3386      +15     
Files with missing lines Coverage Δ
sdk/include/opentelemetry/sdk/metrics/meter.h 57.15% <ø> (ø)
sdk/src/metrics/meter.cc 82.92% <100.00%> (+1.57%) ⬆️
...telemetry/sdk/metrics/state/multi_metric_storage.h 88.68% <83.34%> (-1.56%) ⬇️
...dk/include/opentelemetry/sdk/metrics/instruments.h 73.18% <62.50%> (-2.58%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…antic error warning for view configurations that duplicate metrics. Update meter tests to verify drop aggregation and multiple views.
@dbarker
dbarker force-pushed the perf_metrics_sync_storage_drop_aggregation branch from e66422a to 8be812a Compare September 2, 2026 05:00
@dbarker dbarker changed the title [METRICS SDK] Metrics sync instrument drop aggregation behaves as a no-op [METRICS SDK] Drop aggregation behaves as a no-op Sep 2, 2026
@dbarker
dbarker marked this pull request as ready for review September 2, 2026 11:58
@dbarker
dbarker requested a review from a team as a code owner September 2, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[METRICS SDK] Synchronous instruments with drop aggregation must behave as no-op

1 participant