opentelemetry-cpp icon indicating copy to clipboard operation
opentelemetry-cpp copied to clipboard

Implementing configurable aggregation cardinality limit and collector emitting all views instead of last view

Open PradSenn opened this issue 9 months ago • 4 comments

Fixes #3292 Cardinality of the Aggregation limit (2000) is hardcoded and no ability to configure as part of the meter/aggregation.

Meter stores meter-name to storage in storage_registry_. When multiple views are added, the last view overrides the previous view, and collector can only emit the last view's metrics that are added.

Changes

Please provide a brief description of the changes here.

Introducing cardinality as configurable parameter in AggregationConfig, and implementing the limit from AggregationConfig in Storage.

Changing the storage_registry_ to keep track of view_name to storage, instead of metric_name to storage. So when multiple views are added, different view-names will be collected by collector. Please provide a brief description of the changes here.

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

PradSenn avatar Mar 12 '25 00:03 PradSenn

Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name Link
Latest commit 3af667c24b206ba0e390ac5980384409aed1cc03
Latest deploy log https://app.netlify.com/sites/opentelemetry-cpp-api-docs/deploys/67eb5383541f1100080e4345

netlify[bot] avatar Mar 12 '25 00:03 netlify[bot]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 89.57%. Comparing base (4ecafb6) to head (3af667c). :warning: Report is 181 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3299      +/-   ##
==========================================
+ Coverage   89.52%   89.57%   +0.06%     
==========================================
  Files         210      210              
  Lines        6526     6529       +3     
==========================================
+ Hits         5842     5848       +6     
+ Misses        684      681       -3     
Files with missing lines Coverage Δ
...metry/sdk/metrics/aggregation/aggregation_config.h 100.00% <100.00%> (ø)
sdk/include/opentelemetry/sdk/metrics/meter.h 57.15% <ø> (ø)
...telemetry/sdk/metrics/state/async_metric_storage.h 94.60% <100.00%> (ø)
...entelemetry/sdk/metrics/state/attributes_hashmap.h 97.02% <100.00%> (+4.71%) :arrow_up:
...ntelemetry/sdk/metrics/state/sync_metric_storage.h 86.37% <100.00%> (ø)
sdk/src/metrics/meter.cc 82.50% <100.00%> (ø)
sdk/src/metrics/state/sync_metric_storage.cc 100.00% <100.00%> (ø)
sdk/src/metrics/state/temporal_metric_storage.cc 97.37% <100.00%> (ø)
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Mar 12 '25 01:03 codecov[bot]

@PradSenn could you please resolve the conflicts and fix the CI?

ThomsonTan avatar Apr 29 '25 17:04 ThomsonTan

@PradSenn could you please resolve the conflicts and fix the CI?

@PradSenn Thanks for the PR. Please take a look.

marcalff avatar May 26 '25 14:05 marcalff

Updated and merged in https://github.com/open-telemetry/opentelemetry-cpp/pull/3624.

ThomsonTan avatar Sep 11 '25 16:09 ThomsonTan