Skip to content
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

[sdk-metrics] Promote cardinality limit view API from experimental to stable #5926

Merged
merged 6 commits into from
Oct 28, 2024
Merged

[sdk-metrics] Promote cardinality limit view API from experimental to stable #5926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9c2691a
Promote cardinality limit view API from experimental to stable
xiang17 Oct 25, 2024
6b9d182
update changelog with PR link
xiang17 Oct 25, 2024
fcb7ac2
Revert `EnablePackageValidation` flag in Common.prod.props
xiang17 Oct 25, 2024
8f110e5
Update src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensio…
xiang17 Oct 25, 2024
6d771c0
Had to manually remove them from experimental PublicAPI for VS to rec…
xiang17 Oct 25, 2024
53ed5af
Use permenant link for OTEL1003.md
xiang17 Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion OpenTelemetry.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "experimental-apis", "experi
ProjectSection(SolutionItems) = preProject
docs\diagnostics\experimental-apis\OTEL1000.md = docs\diagnostics\experimental-apis\OTEL1000.md
docs\diagnostics\experimental-apis\OTEL1001.md = docs\diagnostics\experimental-apis\OTEL1001.md
docs\diagnostics\experimental-apis\OTEL1003.md = docs\diagnostics\experimental-apis\OTEL1003.md
docs\diagnostics\experimental-apis\OTEL1004.md = docs\diagnostics\experimental-apis\OTEL1004.md
docs\diagnostics\experimental-apis\README.md = docs\diagnostics\experimental-apis\README.md
EndProjectSection
Expand Down
2 changes: 1 addition & 1 deletion build/Common.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<NuGetAuditMode>all</NuGetAuditMode>
<NuGetAuditLevel>low</NuGetAuditLevel>
<!-- Suppress warnings for repo code using experimental features -->
<NoWarn>$(NoWarn);OTEL1000;OTEL1001;OTEL1002;OTEL1003;OTEL1004</NoWarn>
<NoWarn>$(NoWarn);OTEL1000;OTEL1001;OTEL1002;OTEL1004</NoWarn>
<!--temporarily disable. See 3958-->
<!--<AnalysisLevel>latest-All</AnalysisLevel>-->
</PropertyGroup>
Expand Down
47 changes: 0 additions & 47 deletions docs/diagnostics/experimental-apis/OTEL1003.md
View file
Open in desktop

This file was deleted.

14 changes: 8 additions & 6 deletions docs/diagnostics/experimental-apis/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ Description: Logs Bridge API

Details: [OTEL1001](./OTEL1001.md)

### OTEL1003

Description: MetricStreamConfiguration CardinalityLimit Support

Details: [OTEL1003](./OTEL1003.md)

### OTEL1004

Description: ExemplarReservoir Support
Expand All @@ -58,3 +52,11 @@ Description: Metrics Exemplar Support
Details: [OTEL1002](https://github.com/open-telemetry/opentelemetry-dotnet/blob/b8ea807bae1a5d9b0f3d6d23b1e1e10f5e096a25/docs/diagnostics/experimental-apis/OTEL1002.md)

Released stable: `1.9.0`

### OTEL1003
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be moved?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was simply following the convention of the OTEL1002 above which was released and moved from "Active" to "Inactive". Personally I think it'd also be helpful to have a record of what happened to previous experimental features.


Description: MetricStreamConfiguration CardinalityLimit Support

Details: [OTEL1003](https://github.com/open-telemetry/opentelemetry-dotnet/blob/9f41eadf03f3dcc5e76c686b61fb39849f046312/docs/diagnostics/experimental-apis/OTEL1003.md)

Released stable: `1.10.0`
10 changes: 2 additions & 8 deletions docs/metrics/customizing-the-sdk/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,9 +403,8 @@ metrics managed by a given `MeterProvider`, use the

> [!CAUTION]
> `MeterProviderBuilder.SetMaxMetricPointsPerMetricStream` is marked `Obsolete`
in pre-release builds and has been replaced by
`MetricStreamConfiguration.CardinalityLimit`. For details see:
[OTEL1003](../../diagnostics/experimental-apis/OTEL1003.md).
in stable builds since 1.10.0 and has been replaced by
`MetricStreamConfiguration.CardinalityLimit`.

```csharp
using var meterProvider = Sdk.CreateMeterProviderBuilder()
Expand All @@ -421,11 +420,6 @@ To set the [cardinality limit](../README.md#cardinality-limits) for an
individual metric, use the `MetricStreamConfiguration.CardinalityLimit` property
on the View API:

> [!NOTE]
> `MetricStreamConfiguration.CardinalityLimit` is an experimental API only
available in pre-release builds. For details see:
[OTEL1003](../../diagnostics/experimental-apis/OTEL1003.md).

```csharp
var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddMeter("MyCompany.MyProduct.MyLibrary")
Expand Down
2 changes: 0 additions & 2 deletions src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ OpenTelemetry.Metrics.FixedSizeExemplarReservoir.Capacity.get -> int
OpenTelemetry.Metrics.FixedSizeExemplarReservoir.FixedSizeExemplarReservoir(int capacity) -> void
OpenTelemetry.Metrics.FixedSizeExemplarReservoir.UpdateExemplar(int exemplarIndex, in OpenTelemetry.Metrics.ExemplarMeasurement<double> measurement) -> void
OpenTelemetry.Metrics.FixedSizeExemplarReservoir.UpdateExemplar(int exemplarIndex, in OpenTelemetry.Metrics.ExemplarMeasurement<long> measurement) -> void
OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.get -> int?
OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.set -> void
OpenTelemetry.Metrics.MetricStreamConfiguration.ExemplarReservoirFactory.get -> System.Func<OpenTelemetry.Metrics.ExemplarReservoir?>?
OpenTelemetry.Metrics.MetricStreamConfiguration.ExemplarReservoirFactory.set -> void
override sealed OpenTelemetry.Metrics.FixedSizeExemplarReservoir.Collect() -> OpenTelemetry.Metrics.ReadOnlyExemplarCollection
Expand Down
2 changes: 2 additions & 0 deletions src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.get -> int?
OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.set -> void
OpenTelemetry.OpenTelemetrySdk
OpenTelemetry.OpenTelemetrySdk.Dispose() -> void
OpenTelemetry.OpenTelemetrySdk.LoggerProvider.get -> OpenTelemetry.Logs.LoggerProvider!
Expand Down
5 changes: 5 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ Notes](../../RELEASENOTES.md).

## Unreleased

* The experimental APIs previously covered by `OTEL1003`
(`MetricStreamConfiguration.CardinalityLimit`) will now be part of the public
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will be -> is?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how folks think about https://www.theregister.com/2024/10/08/linus_torvalds_grammar_complaint/?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wording is also used in other experimental APIs in this CHANGELOG.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "is" is better. Would be good to follow up and make changes everywhere. Not a blocker for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a PR #5940 for this.

API and supported in stable builds.
([#5926](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5926))

* Promoted overflow attribute from experimental to stable and removed the
`OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE` environment variable.

Expand Down
4 changes: 1 addition & 3 deletions src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,7 @@ public static MeterProviderBuilder SetMaxMetricStreams(this MeterProviderBuilder
/// <param name="meterProviderBuilder"><see cref="MeterProviderBuilder"/>.</param>
/// <param name="maxMetricPointsPerMetricStream">Maximum number of metric points allowed per metric stream.</param>
/// <returns>The supplied <see cref="MeterProviderBuilder"/> for chaining.</returns>
#if EXPOSE_EXPERIMENTAL_FEATURES
[Obsolete("Use MetricStreamConfiguration.CardinalityLimit via the AddView API instead. This method will be removed in a future version.")]
#endif
[Obsolete("Use MetricStreamConfiguration.CardinalityLimit via the AddView API instead. This method is marked as obsolete in version 1.10.0 and will be removed in a future version.")]
public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterProviderBuilder meterProviderBuilder, int maxMetricPointsPerMetricStream)
{
Guard.ThrowIfOutOfRange(maxMetricPointsPerMetricStream, min: 1);
Expand Down
12 changes: 1 addition & 11 deletions src/OpenTelemetry/Metrics/View/MetricStreamConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,11 @@ public string[]? TagKeys
}
}

#if EXPOSE_EXPERIMENTAL_FEATURES
/// <summary>
/// Gets or sets a positive integer value defining the maximum number of
/// data points allowed for the metric managed by the view.
/// </summary>
/// <remarks>
/// <para><b>WARNING</b>: This is an experimental API which might change or
/// be removed in the future. Use at your own risk.</para>
/// <para>Spec reference: <see
/// href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#cardinality-limits">Cardinality
/// limits</see>.</para>
Expand All @@ -116,14 +113,7 @@ public string[]? TagKeys
/// If not set the default
/// MeterProvider cardinality limit of 2000 will apply.
/// </remarks>
#if NET
[Experimental(DiagnosticDefinitions.CardinalityLimitExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
#endif
public
#else
internal
#endif
int? CardinalityLimit
public int? CardinalityLimit
{
get => this.cardinalityLimit;
set
Expand Down
2 changes: 1 addition & 1 deletion src/Shared/DiagnosticDefinitions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ internal static class DiagnosticDefinitions

public const string LoggerProviderExperimentalApi = "OTEL1000";
public const string LogsBridgeExperimentalApi = "OTEL1001";
public const string CardinalityLimitExperimentalApi = "OTEL1003";
public const string ExemplarReservoirExperimentalApi = "OTEL1004";

/* Definitions which have been released stable:
public const string ExemplarExperimentalApi = "OTEL1002";
public const string CardinalityLimitExperimentalApi = "OTEL1003";
*/
}
2 changes: 1 addition & 1 deletion test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ public void ViewConflict_OneInstrument_DifferentDescription()
[Theory]
[InlineData(true)]
[InlineData(false)]
public void CardinalityLimitofMatchingViewTakesPrecedenceOverMeterProvider(bool setDefault)
public void CardinalityLimitOfMatchingViewTakesPrecedenceOverMeterProvider(bool setDefault)
{
using var meter = new Meter(Utils.GetCurrentMethodName());
var exportedItems = new List<Metric>();
Expand Down
Loading