-
Notifications
You must be signed in to change notification settings - Fork 861
[sdk-metrics] Obsolete SetMaxMetricPointsPerMetricStream + standarize on "Cardinality Limit" name #5328
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.
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
[sdk-metrics] Obsolete SetMaxMetricPointsPerMetricStream + standarize on "Cardinality Limit" name #5328
Conversation
src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
Do we want to expose this at all or we want to point folks to View level limit? Asking this because the spec doesn't ask for it https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#configuration-1 |
…ns.cs Co-authored-by: Yun-Ting Lin <yunl@microsoft.com>
My thinking is we already exposed it with If you are thinking we should just do something like... #if EXPOSE_EXPERIMENTAL_FEATURES
[Obsolete("Use MetricStreamConfiguration.CardinalityLimit instead. This method will be removed in a future version.")]
#endif
public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterProviderBuilder meterProviderBuilder, int maxMetricPointsPerMetricStream)I'm also good with that. |
src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
|
I had an offline discussion with @reyang. We decided to obsolete |
|
I feel
|
|
CardinalityLimitofMatchingViewTakesPrecedenceOverMetricProviderWhenBothWereSet can be removed. |
Follow-up to #5312
Changes
CardinalityLimitallowed for the metric managed by the view. #5312 where a view specifyingCardinalityLimitactually changes the value stored on theMetricReaderwhich will then apply to any metrics recorded after the one which had the view apply: f1b62b0MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStreamin favor of the View API added in [sdk] Added support for settingCardinalityLimitallowed for the metric managed by the view. #5312.MeterProvidertoMetricReader.Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changes