Skip to content

feat(metrics-sdk)!: Metrics cardinality limit #1909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

feat(metrics-sdk)!: Metrics cardinality limit #1909

wants to merge 13 commits into from

Conversation

xuan-cao-swi
Copy link
Contributor

Description

This PR introduces cardinality limits for metric attributes, as defined in the metrics SDK specification.

Users can configure the cardinality limit in two ways:

  1. Through a view:
OpenTelemetry.meter_provider.add_view(
  'test_histogram',
  aggregation: OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new,
  aggregation_cardinality_limit: 1
)
  1. Through an exporter:
exporter_with_limit = ConsoleMetricPullExporter.new(aggregation_cardinality_limit: 100)

If no limit is provided, the default value of 2000 is used (also negative value will turn to 0 for the limit).

The cardinality limit is enforced during the aggregation update process. When the number of unique attribute combinations exceeds the limit, the extra data points are grouped into a separate data point with the attribute:

{ 'otel.metric.overflow' => true }

This overflow data point is treated like any other data point.

Future work: There is currently duplicated logic across different aggregation implementations. A shared base aggregation class should be introduced to reduce repetition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbogsany fbogsany Awaiting requested review from fbogsany fbogsany will be requested when the pull request is marked ready for review fbogsany is a code owner

@mwear mwear Awaiting requested review from mwear mwear will be requested when the pull request is marked ready for review mwear is a code owner

@robertlaurin robertlaurin Awaiting requested review from robertlaurin robertlaurin will be requested when the pull request is marked ready for review robertlaurin is a code owner

@dazuma dazuma Awaiting requested review from dazuma dazuma will be requested when the pull request is marked ready for review dazuma is a code owner

@ericmustin ericmustin Awaiting requested review from ericmustin ericmustin will be requested when the pull request is marked ready for review ericmustin is a code owner

@arielvalentin arielvalentin Awaiting requested review from arielvalentin arielvalentin will be requested when the pull request is marked ready for review arielvalentin is a code owner

@ahayworth ahayworth Awaiting requested review from ahayworth ahayworth will be requested when the pull request is marked ready for review ahayworth is a code owner

@plantfansam plantfansam Awaiting requested review from plantfansam plantfansam will be requested when the pull request is marked ready for review plantfansam is a code owner

@robbkidd robbkidd Awaiting requested review from robbkidd robbkidd will be requested when the pull request is marked ready for review robbkidd is a code owner

@kaylareopelle kaylareopelle Awaiting requested review from kaylareopelle kaylareopelle will be requested when the pull request is marked ready for review kaylareopelle is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@xuan-cao-swi