From 0e5c7495d7aaf364647e9cec8603d07028f87ac4 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 13:10:51 -0800 Subject: [PATCH 01/26] Standarize on "Cardinality Limit" name. --- .../diagnostics/experimental-apis/OTEL1003.md | 34 +++++++++++--- .../Experimental/PublicAPI.Unshipped.txt | 1 + src/OpenTelemetry/CHANGELOG.md | 6 +++ src/OpenTelemetry/Metrics/AggregatorStore.cs | 32 ++++++------- .../Builder/MeterProviderBuilderExtensions.cs | 45 ++++++++++++++++--- .../Builder/MeterProviderBuilderSdk.cs | 8 ++-- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 9 ++-- src/OpenTelemetry/Metrics/Metric.cs | 4 +- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 26 +++++------ .../Metrics/MetricStreamConfiguration.cs | 2 +- .../Metrics/AggregatorTestsBase.cs | 6 +-- .../Metrics/MetricApiTestsBase.cs | 12 ++--- .../MetricOverflowAttributeTestsBase.cs | 4 +- .../Metrics/MetricPointReclaimTestsBase.cs | 2 +- 14 files changed, 127 insertions(+), 64 deletions(-) diff --git a/docs/diagnostics/experimental-apis/OTEL1003.md b/docs/diagnostics/experimental-apis/OTEL1003.md index d6ed954b2bc..bf533cb1490 100644 --- a/docs/diagnostics/experimental-apis/OTEL1003.md +++ b/docs/diagnostics/experimental-apis/OTEL1003.md @@ -4,6 +4,7 @@ This is an Experimental API diagnostic covering the following API: +* `MeterProviderBuilderExtensions.SetCardinalityLimit` * `MetricStreamConfiguration.CardinalityLimit.get` * `MetricStreamConfiguration.CardinalityLimit.set` @@ -11,14 +12,10 @@ Experimental APIs may be changed or removed in the future. ## Details -The OpenTelemetry Specification defines the -[cardinality limit](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#cardinality-limits) -of a metric can be set by the matching view. - From the specification: -> The cardinality limit for an aggregation is defined in one of three ways: -> A view with criteria matching the instrument an aggregation is created for has +> The cardinality limit for an aggregation is defined in one of three ways: A +> view with criteria matching the instrument an aggregation is created for has > an aggregation_cardinality_limit value defined for the stream, that value > SHOULD be used. If there is no matching view, but the MetricReader defines a > default cardinality limit value based on the instrument an aggregation is @@ -27,3 +24,28 @@ From the specification: We are exposing these APIs experimentally until the specification declares them stable. + +### Setting the default cardinality limit for the MeterProvider: + +There is nothing in the OpenTelemetry Specification currently for defining the +default cardinality limit for a MeterProvider. + +```csharp +using var meterProvider = Sdk.CreateMeterProviderBuilder() + .SetCardinalityLimit(5000) + .Build(); +``` + +### Setting cardinality limit for a specific Metric via the View API: + +The OpenTelemetry Specification defines the [cardinality +limit](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#cardinality-limits) +of a metric can be set by the matching view. + +```csharp +using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddView( + instrumentName: "MyFruitCounter", + new MetricStreamConfiguration { CardinalityLimit = 10 }) + .Build(); +``` diff --git a/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt index 16832495101..51e824567d7 100644 --- a/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -31,6 +31,7 @@ static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.SetResourceBuilder(thi static OpenTelemetry.Logs.LoggerProviderExtensions.AddProcessor(this OpenTelemetry.Logs.LoggerProvider! provider, OpenTelemetry.BaseProcessor! processor) -> OpenTelemetry.Logs.LoggerProvider! static OpenTelemetry.Logs.LoggerProviderExtensions.ForceFlush(this OpenTelemetry.Logs.LoggerProvider! provider, int timeoutMilliseconds = -1) -> bool static OpenTelemetry.Logs.LoggerProviderExtensions.Shutdown(this OpenTelemetry.Logs.LoggerProvider! provider, int timeoutMilliseconds = -1) -> bool +static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetCardinalityLimit(this OpenTelemetry.Metrics.MeterProviderBuilder! meterProviderBuilder, int cardinalityLimit) -> OpenTelemetry.Metrics.MeterProviderBuilder! static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetExemplarFilter(this OpenTelemetry.Metrics.MeterProviderBuilder! meterProviderBuilder, OpenTelemetry.Metrics.ExemplarFilter! exemplarFilter) -> OpenTelemetry.Metrics.MeterProviderBuilder! static OpenTelemetry.OpenTelemetryBuilderSdkExtensions.WithLogging(this OpenTelemetry.IOpenTelemetryBuilder! builder) -> OpenTelemetry.IOpenTelemetryBuilder! static OpenTelemetry.OpenTelemetryBuilderSdkExtensions.WithLogging(this OpenTelemetry.IOpenTelemetryBuilder! builder, System.Action! configure) -> OpenTelemetry.IOpenTelemetryBuilder! diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index b2636f35de3..d87ce9d0ac3 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -38,6 +38,12 @@ [IMetricsListener](https://learn.microsoft.com/dotNet/api/microsoft.extensions.diagnostics.metrics.imetricslistener). ([#5265](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5265)) +* **Experimental (pre-release builds only):** Obsoleted + `MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream` and added + `MeterProviderBuilderExtensions.SetCardinalityLimit`. The new method does the + same thing but has a more spec-compliant name. + ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ## 1.7.0 Released 2023-Dec-08 diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index e181a81fb82..716d1eabcf5 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -42,7 +42,7 @@ internal sealed class AggregatorStore private readonly int exponentialHistogramMaxScale; private readonly UpdateLongDelegate updateLongCallback; private readonly UpdateDoubleDelegate updateDoubleCallback; - private readonly int maxMetricPoints; + private readonly int cardinalityLimit; private readonly bool emitOverflowAttribute; private readonly ExemplarFilter exemplarFilter; private readonly Func[], int, int> lookupAggregatorStore; @@ -57,17 +57,17 @@ internal AggregatorStore( MetricStreamIdentity metricStreamIdentity, AggregationType aggType, AggregationTemporality temporality, - int maxMetricPoints, + int cardinalityLimit, bool emitOverflowAttribute, bool shouldReclaimUnusedMetricPoints, ExemplarFilter? exemplarFilter = null) { this.name = metricStreamIdentity.InstrumentName; - this.maxMetricPoints = maxMetricPoints; + this.cardinalityLimit = cardinalityLimit; - this.metricPointCapHitMessage = $"Maximum MetricPoints limit reached for this Metric stream. Configured limit: {this.maxMetricPoints}"; - this.metricPoints = new MetricPoint[maxMetricPoints]; - this.currentMetricPointBatch = new int[maxMetricPoints]; + this.metricPointCapHitMessage = $"Maximum MetricPoints limit reached for this Metric stream. Configured limit: {this.cardinalityLimit}"; + this.metricPoints = new MetricPoint[cardinalityLimit]; + this.currentMetricPointBatch = new int[cardinalityLimit]; this.aggType = aggType; this.OutputDelta = temporality == AggregationTemporality.Delta; this.histogramBounds = metricStreamIdentity.HistogramBucketBounds ?? FindDefaultHistogramBounds(in metricStreamIdentity); @@ -105,17 +105,17 @@ internal AggregatorStore( if (this.OutputDeltaWithUnusedMetricPointReclaimEnabled) { - this.availableMetricPoints = new Queue(maxMetricPoints - reservedMetricPointsCount); + this.availableMetricPoints = new Queue(cardinalityLimit - reservedMetricPointsCount); // There is no overload which only takes capacity as the parameter // Using the DefaultConcurrencyLevel defined in the ConcurrentDictionary class: https://github.com/dotnet/runtime/blob/v7.0.5/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs#L2020 // We expect at the most (maxMetricPoints - reservedMetricPointsCount) * 2 entries- one for sorted and one for unsorted input this.tagsToMetricPointIndexDictionaryDelta = - new ConcurrentDictionary(concurrencyLevel: Environment.ProcessorCount, capacity: (maxMetricPoints - reservedMetricPointsCount) * 2); + new ConcurrentDictionary(concurrencyLevel: Environment.ProcessorCount, capacity: (cardinalityLimit - reservedMetricPointsCount) * 2); // Add all the indices except for the reserved ones to the queue so that threads have // readily available access to these MetricPoints for their use. - for (int i = reservedMetricPointsCount; i < this.maxMetricPoints; i++) + for (int i = reservedMetricPointsCount; i < this.cardinalityLimit; i++) { this.availableMetricPoints.Enqueue(i); } @@ -164,12 +164,12 @@ internal int Snapshot() } else if (this.OutputDelta) { - var indexSnapshot = Math.Min(this.metricPointIndex, this.maxMetricPoints - 1); + var indexSnapshot = Math.Min(this.metricPointIndex, this.cardinalityLimit - 1); this.SnapshotDelta(indexSnapshot); } else { - var indexSnapshot = Math.Min(this.metricPointIndex, this.maxMetricPoints - 1); + var indexSnapshot = Math.Min(this.metricPointIndex, this.cardinalityLimit - 1); this.SnapshotCumulative(indexSnapshot); } @@ -249,7 +249,7 @@ internal void SnapshotDeltaWithMetricPointReclaim() } } - for (int i = startIndexForReclaimableMetricPoints; i < this.maxMetricPoints; i++) + for (int i = startIndexForReclaimableMetricPoints; i < this.cardinalityLimit; i++) { ref var metricPoint = ref this.metricPoints[i]; @@ -440,7 +440,7 @@ private int LookupAggregatorStore(KeyValuePair[] tagKeysAndValu if (!this.tagsToMetricPointIndexDictionary.TryGetValue(sortedTags, out aggregatorIndex)) { aggregatorIndex = this.metricPointIndex; - if (aggregatorIndex >= this.maxMetricPoints) + if (aggregatorIndex >= this.cardinalityLimit) { // sorry! out of data points. // TODO: Once we support cleanup of @@ -469,7 +469,7 @@ private int LookupAggregatorStore(KeyValuePair[] tagKeysAndValu if (!this.tagsToMetricPointIndexDictionary.TryGetValue(sortedTags, out aggregatorIndex)) { aggregatorIndex = ++this.metricPointIndex; - if (aggregatorIndex >= this.maxMetricPoints) + if (aggregatorIndex >= this.cardinalityLimit) { // sorry! out of data points. // TODO: Once we support cleanup of @@ -496,7 +496,7 @@ private int LookupAggregatorStore(KeyValuePair[] tagKeysAndValu { // This else block is for tag length = 1 aggregatorIndex = this.metricPointIndex; - if (aggregatorIndex >= this.maxMetricPoints) + if (aggregatorIndex >= this.cardinalityLimit) { // sorry! out of data points. // TODO: Once we support cleanup of @@ -518,7 +518,7 @@ private int LookupAggregatorStore(KeyValuePair[] tagKeysAndValu if (!this.tagsToMetricPointIndexDictionary.TryGetValue(givenTags, out aggregatorIndex)) { aggregatorIndex = ++this.metricPointIndex; - if (aggregatorIndex >= this.maxMetricPoints) + if (aggregatorIndex >= this.cardinalityLimit) { // sorry! out of data points. // TODO: Once we support cleanup of diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs index 4e33de18a66..d99e4074392 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs @@ -4,6 +4,7 @@ #if NET6_0_OR_GREATER using System.Diagnostics.CodeAnalysis; #endif +using System.ComponentModel; using System.Diagnostics.Metrics; using System.Text.RegularExpressions; using Microsoft.Extensions.DependencyInjection; @@ -230,14 +231,14 @@ public static MeterProviderBuilder SetMaxMetricStreams(this MeterProviderBuilder /// This limits the number of unique combinations of key/value pairs used /// for reporting measurements. /// - /// - /// If a particular key/value pair combination is used at least once, - /// it will contribute to the limit for the life of the process. - /// This may change in the future. See: https://github.com/open-telemetry/opentelemetry-dotnet/issues/2360. - /// + /// /// . /// Maximum number of metric points allowed per metric stream. /// The supplied for chaining. +#if EXPOSE_EXPERIMENTAL_FEATURES + [Obsolete("Call SetCardinalityLimit instead this method will be removed in a future version.")] + [EditorBrowsable(EditorBrowsableState.Never)] +#endif public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterProviderBuilder meterProviderBuilder, int maxMetricPointsPerMetricStream) { Guard.ThrowIfOutOfRange(maxMetricPointsPerMetricStream, min: 1); @@ -253,6 +254,40 @@ public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterP return meterProviderBuilder; } +#if EXPOSE_EXPERIMENTAL_FEATURES + /// + /// Sets a positive integer value defining the maximum number of + /// data points allowed for metrics managed by the MeterProvider. + /// + /// + /// WARNING: This is an experimental API which might change or + /// be removed in the future. Use at your own risk. + /// + /// . + /// Cardinality limit. + /// The supplied for chaining. +#if NET8_0_OR_GREATER + [Experimental(DiagnosticDefinitions.CardinalityLimitExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] +#endif + public +#else + internal +#endif + static MeterProviderBuilder SetCardinalityLimit(this MeterProviderBuilder meterProviderBuilder, int cardinalityLimit) + { + Guard.ThrowIfOutOfRange(cardinalityLimit, min: 1, max: int.MaxValue); + + meterProviderBuilder.ConfigureBuilder((sp, builder) => + { + if (builder is MeterProviderBuilderSdk meterProviderBuilderSdk) + { + meterProviderBuilderSdk.SetCardinalityLimit(cardinalityLimit); + } + }); + + return meterProviderBuilder; + } + /// /// Sets the from which the Resource associated with /// this provider is built from. Overwrites currently set ResourceBuilder. diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs index 8f09a896254..79ec9a1eb24 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs @@ -16,7 +16,7 @@ namespace OpenTelemetry.Metrics; internal sealed class MeterProviderBuilderSdk : MeterProviderBuilder, IMeterProviderBuilder { public const int MaxMetricsDefault = 1000; - public const int MaxMetricPointsPerMetricDefault = 2000; + public const int CardinalityLimitDefault = 2000; private const string DefaultInstrumentationVersion = "1.0.0.0"; private readonly IServiceProvider serviceProvider; @@ -51,7 +51,7 @@ public MeterProviderBuilderSdk(IServiceProvider serviceProvider) public int MaxMetricStreams { get; private set; } = MaxMetricsDefault; - public int MaxMetricPointsPerMetricStream { get; private set; } = MaxMetricPointsPerMetricDefault; + public int CardinalityLimit { get; private set; } = CardinalityLimitDefault; /// /// Returns whether the given instrument name is valid according to the specification. @@ -193,9 +193,9 @@ public MeterProviderBuilder SetMaxMetricStreams(int maxMetricStreams) return this; } - public MeterProviderBuilder SetMaxMetricPointsPerMetricStream(int maxMetricPointsPerMetricStream) + public MeterProviderBuilder SetCardinalityLimit(int cardinalityLimit) { - this.MaxMetricPointsPerMetricStream = maxMetricPointsPerMetricStream; + this.CardinalityLimit = cardinalityLimit; return this; } diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 5dc529cfbd9..7a71712dd31 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -76,9 +76,12 @@ internal MeterProviderSdk( Guard.ThrowIfNull(reader); reader.SetParentProvider(this); - reader.SetMaxMetricStreams(state.MaxMetricStreams); - reader.SetMaxMetricPointsPerMetricStream(state.MaxMetricPointsPerMetricStream, isEmitOverflowAttributeKeySet); - reader.SetExemplarFilter(state.ExemplarFilter); + + reader.ApplyParentProviderSettings( + state.MaxMetricStreams, + state.CardinalityLimit, + state.ExemplarFilter, + isEmitOverflowAttributeKeySet); if (this.reader == null) { diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 60abe6051cc..d234ae0bd70 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -46,7 +46,7 @@ public sealed class Metric internal Metric( MetricStreamIdentity instrumentIdentity, AggregationTemporality temporality, - int maxMetricPointsPerMetricStream, + int cardinalityLimit, bool emitOverflowAttribute, bool shouldReclaimUnusedMetricPoints, ExemplarFilter? exemplarFilter = null) @@ -155,7 +155,7 @@ internal Metric( throw new NotSupportedException($"Unsupported Instrument Type: {instrumentIdentity.InstrumentType.FullName}"); } - this.aggStore = new AggregatorStore(instrumentIdentity, aggType, temporality, maxMetricPointsPerMetricStream, emitOverflowAttribute, shouldReclaimUnusedMetricPoints, exemplarFilter); + this.aggStore = new AggregatorStore(instrumentIdentity, aggType, temporality, cardinalityLimit, emitOverflowAttribute, shouldReclaimUnusedMetricPoints, exemplarFilter); this.Temporality = temporality; } diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index c8a5fa9696e..f67e8d34fad 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -18,7 +18,7 @@ public abstract partial class MetricReader private readonly ConcurrentDictionary instrumentIdentityToMetric = new(); private readonly object instrumentCreationLock = new(); private int maxMetricStreams; - private int maxMetricPointsPerMetricStream; + private int cardinalityLimit; private Metric?[]? metrics; private Metric[]? metricsCurrentBatch; private int metricIndex = -1; @@ -55,7 +55,7 @@ internal AggregationTemporality GetAggregationTemporality(Type instrumentType) try { bool shouldReclaimUnusedMetricPoints = this.parentProvider is MeterProviderSdk meterProviderSdk && meterProviderSdk.ShouldReclaimUnusedMetricPoints; - metric = new Metric(metricStreamIdentity, this.GetAggregationTemporality(metricStreamIdentity.InstrumentType), this.maxMetricPointsPerMetricStream, this.emitOverflowAttribute, shouldReclaimUnusedMetricPoints, this.exemplarFilter); + metric = new Metric(metricStreamIdentity, this.GetAggregationTemporality(metricStreamIdentity.InstrumentType), this.cardinalityLimit, this.emitOverflowAttribute, shouldReclaimUnusedMetricPoints, this.exemplarFilter); } catch (NotSupportedException nse) { @@ -139,10 +139,10 @@ internal List AddMetricsListWithViews(Instrument instrument, List metrics) } } - internal void SetMaxMetricStreams(int maxMetricStreams) + internal void ApplyParentProviderSettings( + int maxMetricStreams, + int cardinalityLimit, + ExemplarFilter? exemplarFilter, + bool isEmitOverflowAttributeKeySet) { this.maxMetricStreams = maxMetricStreams; this.metrics = new Metric[maxMetricStreams]; this.metricsCurrentBatch = new Metric[maxMetricStreams]; - } - - internal void SetExemplarFilter(ExemplarFilter? exemplarFilter) - { + this.cardinalityLimit = cardinalityLimit; this.exemplarFilter = exemplarFilter; - } - - internal void SetMaxMetricPointsPerMetricStream(int maxMetricPointsPerMetricStream, bool isEmitOverflowAttributeKeySet) - { - this.maxMetricPointsPerMetricStream = maxMetricPointsPerMetricStream; if (isEmitOverflowAttributeKeySet) { // We need at least two metric points. One is reserved for zero tags and the other one for overflow attribute - if (maxMetricPointsPerMetricStream > 1) + if (cardinalityLimit > 1) { this.emitOverflowAttribute = true; } diff --git a/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs b/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs index 077f332ea68..5552471bdc6 100644 --- a/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs +++ b/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs @@ -111,7 +111,7 @@ public string[]? TagKeys /// limits. /// Note: If not set, the MeterProvider cardinality limit value will be /// used, which defaults to 2000. Call + /// cref="MeterProviderBuilderExtensions.SetCardinalityLimit"/> /// to configure the MeterProvider default. /// #if NET8_0_OR_GREATER diff --git a/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs index bb9f1489b9f..61d739d6a18 100644 --- a/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs @@ -255,7 +255,7 @@ public void HistogramBucketsDefaultUpdatesForSecondsTest(string meterName, strin metricStreamIdentity, AggregationType.Histogram, AggregationTemporality.Cumulative, - maxMetricPoints: 1024, + cardinalityLimit: 1024, this.emitOverflowAttribute, this.shouldReclaimUnusedMetricPoints); @@ -332,7 +332,7 @@ internal void ExponentialHistogramTests(AggregationType aggregationType, Aggrega metricStreamIdentity, aggregationType, aggregationTemporality, - maxMetricPoints: 1024, + cardinalityLimit: 1024, this.emitOverflowAttribute, this.shouldReclaimUnusedMetricPoints, exemplarsEnabled ? new AlwaysOnExemplarFilter() : null); @@ -442,7 +442,7 @@ internal void ExponentialMaxScaleConfigWorks(int? maxScale) metricStreamIdentity, AggregationType.Base2ExponentialHistogram, AggregationTemporality.Cumulative, - maxMetricPoints: 1024, + cardinalityLimit: 1024, this.emitOverflowAttribute, this.shouldReclaimUnusedMetricPoints); diff --git a/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs index 1e0c8a59dd1..5fae199e20c 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs @@ -1423,26 +1423,26 @@ int MetricPointCount() // for no tag point! // This may be changed later. counterLong.Add(10); - for (int i = 0; i < MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault + 1; i++) + for (int i = 0; i < MeterProviderBuilderSdk.CardinalityLimitDefault + 1; i++) { counterLong.Add(10, new KeyValuePair("key", "value" + i)); } meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault, MetricPointCount()); + Assert.Equal(MeterProviderBuilderSdk.CardinalityLimitDefault, MetricPointCount()); exportedItems.Clear(); counterLong.Add(10); - for (int i = 0; i < MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault + 1; i++) + for (int i = 0; i < MeterProviderBuilderSdk.CardinalityLimitDefault + 1; i++) { counterLong.Add(10, new KeyValuePair("key", "value" + i)); } meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault, MetricPointCount()); + Assert.Equal(MeterProviderBuilderSdk.CardinalityLimitDefault, MetricPointCount()); counterLong.Add(10); - for (int i = 0; i < MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault + 1; i++) + for (int i = 0; i < MeterProviderBuilderSdk.CardinalityLimitDefault + 1; i++) { counterLong.Add(10, new KeyValuePair("key", "value" + i)); } @@ -1453,7 +1453,7 @@ int MetricPointCount() counterLong.Add(10, new KeyValuePair("key", "valueC")); exportedItems.Clear(); meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault, MetricPointCount()); + Assert.Equal(MeterProviderBuilderSdk.CardinalityLimitDefault, MetricPointCount()); } [Fact] diff --git a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTestsBase.cs index 4726b38043f..ec98132b19c 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTestsBase.cs @@ -174,7 +174,7 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForCounter(MetricReaderTem counter.Add(10); // Record measurement for zero tags // Max number for MetricPoints available for use when emitted with tags - int maxMetricPointsForUse = MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault - 2; + int maxMetricPointsForUse = MeterProviderBuilderSdk.CardinalityLimitDefault - 2; for (int i = 0; i < maxMetricPointsForUse; i++) { @@ -325,7 +325,7 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForHistogram(MetricReaderT histogram.Record(10); // Record measurement for zero tags // Max number for MetricPoints available for use when emitted with tags - int maxMetricPointsForUse = MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault - 2; + int maxMetricPointsForUse = MeterProviderBuilderSdk.CardinalityLimitDefault - 2; for (int i = 0; i < maxMetricPointsForUse; i++) { diff --git a/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs index 6b8505a611a..7bf0a92b5c0 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs @@ -316,7 +316,7 @@ public override ExportResult Export(in Batch batch) } // This is to ensure that the lookup dictionary does not have unbounded growth - Assert.True(metricPointLookupDictionary.Count <= (MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault * 2)); + Assert.True(metricPointLookupDictionary.Count <= (MeterProviderBuilderSdk.CardinalityLimitDefault * 2)); foreach (ref readonly var metricPoint in metric.GetMetricPoints()) { From ce0afe20662b36f00e736259d1347674ff07bae8 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 13:16:17 -0800 Subject: [PATCH 02/26] Warning fixup. --- .../Metrics/Builder/MeterProviderBuilderExtensions.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs index d99e4074392..0cdec4786e5 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs @@ -1,10 +1,12 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#if EXPOSE_EXPERIMENTAL_FEATURES +using System.ComponentModel; #if NET6_0_OR_GREATER using System.Diagnostics.CodeAnalysis; #endif -using System.ComponentModel; +#endif using System.Diagnostics.Metrics; using System.Text.RegularExpressions; using Microsoft.Extensions.DependencyInjection; From 5730d14dbe9101ad2be5ce76a0e51f9c00240eab Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 13:53:53 -0800 Subject: [PATCH 03/26] Update src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs Co-authored-by: Yun-Ting Lin --- .../Metrics/Builder/MeterProviderBuilderExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs index 0cdec4786e5..ed15ad660de 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs @@ -238,7 +238,7 @@ public static MeterProviderBuilder SetMaxMetricStreams(this MeterProviderBuilder /// Maximum number of metric points allowed per metric stream. /// The supplied for chaining. #if EXPOSE_EXPERIMENTAL_FEATURES - [Obsolete("Call SetCardinalityLimit instead this method will be removed in a future version.")] + [Obsolete("Call SetCardinalityLimit instead. This method will be removed in a future version.")] [EditorBrowsable(EditorBrowsableState.Never)] #endif public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterProviderBuilder meterProviderBuilder, int maxMetricPointsPerMetricStream) From 9bfe6cc16938c2f55e1dead9168e762e79e8e423 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 13:54:47 -0800 Subject: [PATCH 04/26] CHANGELOG patch. --- src/OpenTelemetry/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index d87ce9d0ac3..2a638e090b3 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -42,7 +42,7 @@ `MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream` and added `MeterProviderBuilderExtensions.SetCardinalityLimit`. The new method does the same thing but has a more spec-compliant name. - ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ([#5328](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5328)) ## 1.7.0 From ae6d742a92cc05301c5ddff8ea7413d1925039ab Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 14:01:07 -0800 Subject: [PATCH 05/26] Lint. --- docs/diagnostics/experimental-apis/OTEL1003.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/diagnostics/experimental-apis/OTEL1003.md b/docs/diagnostics/experimental-apis/OTEL1003.md index bf533cb1490..5109149ab34 100644 --- a/docs/diagnostics/experimental-apis/OTEL1003.md +++ b/docs/diagnostics/experimental-apis/OTEL1003.md @@ -25,7 +25,7 @@ From the specification: We are exposing these APIs experimentally until the specification declares them stable. -### Setting the default cardinality limit for the MeterProvider: +### Setting the default cardinality limit for the MeterProvider There is nothing in the OpenTelemetry Specification currently for defining the default cardinality limit for a MeterProvider. @@ -36,7 +36,7 @@ using var meterProvider = Sdk.CreateMeterProviderBuilder() .Build(); ``` -### Setting cardinality limit for a specific Metric via the View API: +### Setting cardinality limit for a specific Metric via the View API The OpenTelemetry Specification defines the [cardinality limit](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#cardinality-limits) From ea6aaceba4d3e43fce688e907418bd81bf209b22 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 14:20:52 -0800 Subject: [PATCH 06/26] Warning fixes. --- .../Builder/MeterProviderBuilderExtensions.cs | 70 +++++++++---------- .../Metrics/MetricViewTests.cs | 2 +- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs index ed15ad660de..84d5bdc4308 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs @@ -3,10 +3,10 @@ #if EXPOSE_EXPERIMENTAL_FEATURES using System.ComponentModel; +#endif #if NET6_0_OR_GREATER using System.Diagnostics.CodeAnalysis; #endif -#endif using System.Diagnostics.Metrics; using System.Text.RegularExpressions; using Microsoft.Extensions.DependencyInjection; @@ -256,40 +256,6 @@ public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterP return meterProviderBuilder; } -#if EXPOSE_EXPERIMENTAL_FEATURES - /// - /// Sets a positive integer value defining the maximum number of - /// data points allowed for metrics managed by the MeterProvider. - /// - /// - /// WARNING: This is an experimental API which might change or - /// be removed in the future. Use at your own risk. - /// - /// . - /// Cardinality limit. - /// The supplied for chaining. -#if NET8_0_OR_GREATER - [Experimental(DiagnosticDefinitions.CardinalityLimitExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] -#endif - public -#else - internal -#endif - static MeterProviderBuilder SetCardinalityLimit(this MeterProviderBuilder meterProviderBuilder, int cardinalityLimit) - { - Guard.ThrowIfOutOfRange(cardinalityLimit, min: 1, max: int.MaxValue); - - meterProviderBuilder.ConfigureBuilder((sp, builder) => - { - if (builder is MeterProviderBuilderSdk meterProviderBuilderSdk) - { - meterProviderBuilderSdk.SetCardinalityLimit(cardinalityLimit); - } - }); - - return meterProviderBuilder; - } - /// /// Sets the from which the Resource associated with /// this provider is built from. Overwrites currently set ResourceBuilder. @@ -351,6 +317,40 @@ public static MeterProvider Build(this MeterProviderBuilder meterProviderBuilder throw new NotSupportedException($"Build is not supported on '{meterProviderBuilder?.GetType().FullName ?? "null"}' instances."); } +#if EXPOSE_EXPERIMENTAL_FEATURES + /// + /// Sets a positive integer value defining the maximum number of + /// data points allowed for metrics managed by the MeterProvider. + /// + /// + /// WARNING: This is an experimental API which might change or + /// be removed in the future. Use at your own risk. + /// + /// . + /// Cardinality limit. + /// The supplied for chaining. +#if NET8_0_OR_GREATER + [Experimental(DiagnosticDefinitions.CardinalityLimitExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] +#endif + public +#else + internal +#endif + static MeterProviderBuilder SetCardinalityLimit(this MeterProviderBuilder meterProviderBuilder, int cardinalityLimit) + { + Guard.ThrowIfOutOfRange(cardinalityLimit, min: 1, max: int.MaxValue); + + meterProviderBuilder.ConfigureBuilder((sp, builder) => + { + if (builder is MeterProviderBuilderSdk meterProviderBuilderSdk) + { + meterProviderBuilderSdk.SetCardinalityLimit(cardinalityLimit); + } + }); + + return meterProviderBuilder; + } + #if EXPOSE_EXPERIMENTAL_FEATURES /// /// Sets the to be used for this provider. diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index ca8bbf3163b..1348bab04cd 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -928,7 +928,7 @@ public void CardinalityLimitofMatchingViewTakesPrecedenceOverMetricProviderWhenB using var container = this.BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) - .SetMaxMetricPointsPerMetricStream(3) + .SetCardinalityLimit(3) .AddView((instrument) => { return new MetricStreamConfiguration() { Name = "MetricStreamA", CardinalityLimit = 10000 }; From d79ca7c79d1460b6a8c59a21dbd4bd83abaa4fe4 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 15:20:30 -0800 Subject: [PATCH 07/26] Code review. --- .../diagnostics/experimental-apis/OTEL1003.md | 12 ----- .../Experimental/PublicAPI.Unshipped.txt | 1 - .../Builder/MeterProviderBuilderExtensions.cs | 48 +++---------------- .../Builder/MeterProviderBuilderSdk.cs | 10 ++-- .../Metrics/MetricStreamConfiguration.cs | 6 +-- .../Metrics/MetricApiTestsBase.cs | 12 ++--- .../MetricOverflowAttributeTestsBase.cs | 4 +- .../Metrics/MetricPointReclaimTestsBase.cs | 2 +- .../Metrics/MetricViewTests.cs | 2 +- 9 files changed, 24 insertions(+), 73 deletions(-) diff --git a/docs/diagnostics/experimental-apis/OTEL1003.md b/docs/diagnostics/experimental-apis/OTEL1003.md index 5109149ab34..19913edc916 100644 --- a/docs/diagnostics/experimental-apis/OTEL1003.md +++ b/docs/diagnostics/experimental-apis/OTEL1003.md @@ -4,7 +4,6 @@ This is an Experimental API diagnostic covering the following API: -* `MeterProviderBuilderExtensions.SetCardinalityLimit` * `MetricStreamConfiguration.CardinalityLimit.get` * `MetricStreamConfiguration.CardinalityLimit.set` @@ -25,17 +24,6 @@ From the specification: We are exposing these APIs experimentally until the specification declares them stable. -### Setting the default cardinality limit for the MeterProvider - -There is nothing in the OpenTelemetry Specification currently for defining the -default cardinality limit for a MeterProvider. - -```csharp -using var meterProvider = Sdk.CreateMeterProviderBuilder() - .SetCardinalityLimit(5000) - .Build(); -``` - ### Setting cardinality limit for a specific Metric via the View API The OpenTelemetry Specification defines the [cardinality diff --git a/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt index 51e824567d7..16832495101 100644 --- a/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -31,7 +31,6 @@ static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.SetResourceBuilder(thi static OpenTelemetry.Logs.LoggerProviderExtensions.AddProcessor(this OpenTelemetry.Logs.LoggerProvider! provider, OpenTelemetry.BaseProcessor! processor) -> OpenTelemetry.Logs.LoggerProvider! static OpenTelemetry.Logs.LoggerProviderExtensions.ForceFlush(this OpenTelemetry.Logs.LoggerProvider! provider, int timeoutMilliseconds = -1) -> bool static OpenTelemetry.Logs.LoggerProviderExtensions.Shutdown(this OpenTelemetry.Logs.LoggerProvider! provider, int timeoutMilliseconds = -1) -> bool -static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetCardinalityLimit(this OpenTelemetry.Metrics.MeterProviderBuilder! meterProviderBuilder, int cardinalityLimit) -> OpenTelemetry.Metrics.MeterProviderBuilder! static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetExemplarFilter(this OpenTelemetry.Metrics.MeterProviderBuilder! meterProviderBuilder, OpenTelemetry.Metrics.ExemplarFilter! exemplarFilter) -> OpenTelemetry.Metrics.MeterProviderBuilder! static OpenTelemetry.OpenTelemetryBuilderSdkExtensions.WithLogging(this OpenTelemetry.IOpenTelemetryBuilder! builder) -> OpenTelemetry.IOpenTelemetryBuilder! static OpenTelemetry.OpenTelemetryBuilderSdkExtensions.WithLogging(this OpenTelemetry.IOpenTelemetryBuilder! builder, System.Action! configure) -> OpenTelemetry.IOpenTelemetryBuilder! diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs index 84d5bdc4308..19b400ebe5e 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs @@ -1,9 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#if EXPOSE_EXPERIMENTAL_FEATURES -using System.ComponentModel; -#endif #if NET6_0_OR_GREATER using System.Diagnostics.CodeAnalysis; #endif @@ -233,13 +230,16 @@ public static MeterProviderBuilder SetMaxMetricStreams(this MeterProviderBuilder /// This limits the number of unique combinations of key/value pairs used /// for reporting measurements. /// - /// + /// + /// If a particular key/value pair combination is used at least once, + /// it will contribute to the limit for the life of the process. + /// This may change in the future. See: https://github.com/open-telemetry/opentelemetry-dotnet/issues/2360. + /// /// . /// Maximum number of metric points allowed per metric stream. /// The supplied for chaining. #if EXPOSE_EXPERIMENTAL_FEATURES - [Obsolete("Call SetCardinalityLimit instead. This method will be removed in a future version.")] - [EditorBrowsable(EditorBrowsableState.Never)] + [Obsolete("Use MetricStreamConfiguration.CardinalityLimit via the AddView API instead. This method will be removed in a future version.")] #endif public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterProviderBuilder meterProviderBuilder, int maxMetricPointsPerMetricStream) { @@ -249,7 +249,7 @@ public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterP { if (builder is MeterProviderBuilderSdk meterProviderBuilderSdk) { - meterProviderBuilderSdk.SetMaxMetricPointsPerMetricStream(maxMetricPointsPerMetricStream); + meterProviderBuilderSdk.SetDefaultCardinalityLimit(maxMetricPointsPerMetricStream); } }); @@ -317,40 +317,6 @@ public static MeterProvider Build(this MeterProviderBuilder meterProviderBuilder throw new NotSupportedException($"Build is not supported on '{meterProviderBuilder?.GetType().FullName ?? "null"}' instances."); } -#if EXPOSE_EXPERIMENTAL_FEATURES - /// - /// Sets a positive integer value defining the maximum number of - /// data points allowed for metrics managed by the MeterProvider. - /// - /// - /// WARNING: This is an experimental API which might change or - /// be removed in the future. Use at your own risk. - /// - /// . - /// Cardinality limit. - /// The supplied for chaining. -#if NET8_0_OR_GREATER - [Experimental(DiagnosticDefinitions.CardinalityLimitExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] -#endif - public -#else - internal -#endif - static MeterProviderBuilder SetCardinalityLimit(this MeterProviderBuilder meterProviderBuilder, int cardinalityLimit) - { - Guard.ThrowIfOutOfRange(cardinalityLimit, min: 1, max: int.MaxValue); - - meterProviderBuilder.ConfigureBuilder((sp, builder) => - { - if (builder is MeterProviderBuilderSdk meterProviderBuilderSdk) - { - meterProviderBuilderSdk.SetCardinalityLimit(cardinalityLimit); - } - }); - - return meterProviderBuilder; - } - #if EXPOSE_EXPERIMENTAL_FEATURES /// /// Sets the to be used for this provider. diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs index 79ec9a1eb24..4d77e9075e5 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs @@ -15,8 +15,8 @@ namespace OpenTelemetry.Metrics; /// internal sealed class MeterProviderBuilderSdk : MeterProviderBuilder, IMeterProviderBuilder { - public const int MaxMetricsDefault = 1000; - public const int CardinalityLimitDefault = 2000; + public const int DefaultMaxMetricStreams = 1000; + public const int DefaultCardinalityLimit = 2000; private const string DefaultInstrumentationVersion = "1.0.0.0"; private readonly IServiceProvider serviceProvider; @@ -49,9 +49,9 @@ public MeterProviderBuilderSdk(IServiceProvider serviceProvider) public List> ViewConfigs { get; } = new(); - public int MaxMetricStreams { get; private set; } = MaxMetricsDefault; + public int MaxMetricStreams { get; private set; } = DefaultMaxMetricStreams; - public int CardinalityLimit { get; private set; } = CardinalityLimitDefault; + public int CardinalityLimit { get; private set; } = DefaultCardinalityLimit; /// /// Returns whether the given instrument name is valid according to the specification. @@ -193,7 +193,7 @@ public MeterProviderBuilder SetMaxMetricStreams(int maxMetricStreams) return this; } - public MeterProviderBuilder SetCardinalityLimit(int cardinalityLimit) + public MeterProviderBuilder SetDefaultCardinalityLimit(int cardinalityLimit) { this.CardinalityLimit = cardinalityLimit; diff --git a/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs b/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs index 5552471bdc6..c38379bb718 100644 --- a/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs +++ b/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs @@ -109,10 +109,8 @@ public string[]? TagKeys /// Spec reference: Cardinality /// limits. - /// Note: If not set, the MeterProvider cardinality limit value will be - /// used, which defaults to 2000. Call - /// to configure the MeterProvider default. + /// Note: If not set the default MeterProvider cardinality limit of 2000 + /// will apply. /// #if NET8_0_OR_GREATER [Experimental(DiagnosticDefinitions.CardinalityLimitExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] diff --git a/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs index 5fae199e20c..ad18233a016 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs @@ -1423,26 +1423,26 @@ int MetricPointCount() // for no tag point! // This may be changed later. counterLong.Add(10); - for (int i = 0; i < MeterProviderBuilderSdk.CardinalityLimitDefault + 1; i++) + for (int i = 0; i < MeterProviderBuilderSdk.DefaultCardinalityLimit + 1; i++) { counterLong.Add(10, new KeyValuePair("key", "value" + i)); } meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(MeterProviderBuilderSdk.CardinalityLimitDefault, MetricPointCount()); + Assert.Equal(MeterProviderBuilderSdk.DefaultCardinalityLimit, MetricPointCount()); exportedItems.Clear(); counterLong.Add(10); - for (int i = 0; i < MeterProviderBuilderSdk.CardinalityLimitDefault + 1; i++) + for (int i = 0; i < MeterProviderBuilderSdk.DefaultCardinalityLimit + 1; i++) { counterLong.Add(10, new KeyValuePair("key", "value" + i)); } meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(MeterProviderBuilderSdk.CardinalityLimitDefault, MetricPointCount()); + Assert.Equal(MeterProviderBuilderSdk.DefaultCardinalityLimit, MetricPointCount()); counterLong.Add(10); - for (int i = 0; i < MeterProviderBuilderSdk.CardinalityLimitDefault + 1; i++) + for (int i = 0; i < MeterProviderBuilderSdk.DefaultCardinalityLimit + 1; i++) { counterLong.Add(10, new KeyValuePair("key", "value" + i)); } @@ -1453,7 +1453,7 @@ int MetricPointCount() counterLong.Add(10, new KeyValuePair("key", "valueC")); exportedItems.Clear(); meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(MeterProviderBuilderSdk.CardinalityLimitDefault, MetricPointCount()); + Assert.Equal(MeterProviderBuilderSdk.DefaultCardinalityLimit, MetricPointCount()); } [Fact] diff --git a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTestsBase.cs index ec98132b19c..35831fc4e88 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTestsBase.cs @@ -174,7 +174,7 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForCounter(MetricReaderTem counter.Add(10); // Record measurement for zero tags // Max number for MetricPoints available for use when emitted with tags - int maxMetricPointsForUse = MeterProviderBuilderSdk.CardinalityLimitDefault - 2; + int maxMetricPointsForUse = MeterProviderBuilderSdk.DefaultCardinalityLimit - 2; for (int i = 0; i < maxMetricPointsForUse; i++) { @@ -325,7 +325,7 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForHistogram(MetricReaderT histogram.Record(10); // Record measurement for zero tags // Max number for MetricPoints available for use when emitted with tags - int maxMetricPointsForUse = MeterProviderBuilderSdk.CardinalityLimitDefault - 2; + int maxMetricPointsForUse = MeterProviderBuilderSdk.DefaultCardinalityLimit - 2; for (int i = 0; i < maxMetricPointsForUse; i++) { diff --git a/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs index 7bf0a92b5c0..d0339d69ba9 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs @@ -316,7 +316,7 @@ public override ExportResult Export(in Batch batch) } // This is to ensure that the lookup dictionary does not have unbounded growth - Assert.True(metricPointLookupDictionary.Count <= (MeterProviderBuilderSdk.CardinalityLimitDefault * 2)); + Assert.True(metricPointLookupDictionary.Count <= (MeterProviderBuilderSdk.DefaultCardinalityLimit * 2)); foreach (ref readonly var metricPoint in metric.GetMetricPoints()) { diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 1348bab04cd..ca8bbf3163b 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -928,7 +928,7 @@ public void CardinalityLimitofMatchingViewTakesPrecedenceOverMetricProviderWhenB using var container = this.BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) - .SetCardinalityLimit(3) + .SetMaxMetricPointsPerMetricStream(3) .AddView((instrument) => { return new MetricStreamConfiguration() { Name = "MetricStreamA", CardinalityLimit = 10000 }; From 51674e6a365c47f26f76f7bd3dd6213c733ef4e5 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 15:26:06 -0800 Subject: [PATCH 08/26] Fixes. --- src/OpenTelemetry/Metrics/AggregatorStore.cs | 22 +++++++++--------- src/OpenTelemetry/Metrics/Metric.cs | 12 +++++----- .../Metrics/MetricViewTests.cs | 23 +++++-------------- 3 files changed, 23 insertions(+), 34 deletions(-) diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index 716d1eabcf5..8428517ffd3 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -13,6 +13,7 @@ internal sealed class AggregatorStore { internal readonly bool OutputDelta; internal readonly bool OutputDeltaWithUnusedMetricPointReclaimEnabled; + internal readonly int CardinalityLimit; internal long DroppedMeasurements = 0; private static readonly string MetricPointCapHitFixMessage = "Consider opting in for the experimental SDK feature to emit all the throttled metrics under the overflow attribute by setting env variable OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE = true. You could also modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit."; @@ -42,7 +43,6 @@ internal sealed class AggregatorStore private readonly int exponentialHistogramMaxScale; private readonly UpdateLongDelegate updateLongCallback; private readonly UpdateDoubleDelegate updateDoubleCallback; - private readonly int cardinalityLimit; private readonly bool emitOverflowAttribute; private readonly ExemplarFilter exemplarFilter; private readonly Func[], int, int> lookupAggregatorStore; @@ -63,9 +63,9 @@ internal AggregatorStore( ExemplarFilter? exemplarFilter = null) { this.name = metricStreamIdentity.InstrumentName; - this.cardinalityLimit = cardinalityLimit; + this.CardinalityLimit = cardinalityLimit; - this.metricPointCapHitMessage = $"Maximum MetricPoints limit reached for this Metric stream. Configured limit: {this.cardinalityLimit}"; + this.metricPointCapHitMessage = $"Maximum MetricPoints limit reached for this Metric stream. Configured limit: {this.CardinalityLimit}"; this.metricPoints = new MetricPoint[cardinalityLimit]; this.currentMetricPointBatch = new int[cardinalityLimit]; this.aggType = aggType; @@ -115,7 +115,7 @@ internal AggregatorStore( // Add all the indices except for the reserved ones to the queue so that threads have // readily available access to these MetricPoints for their use. - for (int i = reservedMetricPointsCount; i < this.cardinalityLimit; i++) + for (int i = reservedMetricPointsCount; i < this.CardinalityLimit; i++) { this.availableMetricPoints.Enqueue(i); } @@ -164,12 +164,12 @@ internal int Snapshot() } else if (this.OutputDelta) { - var indexSnapshot = Math.Min(this.metricPointIndex, this.cardinalityLimit - 1); + var indexSnapshot = Math.Min(this.metricPointIndex, this.CardinalityLimit - 1); this.SnapshotDelta(indexSnapshot); } else { - var indexSnapshot = Math.Min(this.metricPointIndex, this.cardinalityLimit - 1); + var indexSnapshot = Math.Min(this.metricPointIndex, this.CardinalityLimit - 1); this.SnapshotCumulative(indexSnapshot); } @@ -249,7 +249,7 @@ internal void SnapshotDeltaWithMetricPointReclaim() } } - for (int i = startIndexForReclaimableMetricPoints; i < this.cardinalityLimit; i++) + for (int i = startIndexForReclaimableMetricPoints; i < this.CardinalityLimit; i++) { ref var metricPoint = ref this.metricPoints[i]; @@ -440,7 +440,7 @@ private int LookupAggregatorStore(KeyValuePair[] tagKeysAndValu if (!this.tagsToMetricPointIndexDictionary.TryGetValue(sortedTags, out aggregatorIndex)) { aggregatorIndex = this.metricPointIndex; - if (aggregatorIndex >= this.cardinalityLimit) + if (aggregatorIndex >= this.CardinalityLimit) { // sorry! out of data points. // TODO: Once we support cleanup of @@ -469,7 +469,7 @@ private int LookupAggregatorStore(KeyValuePair[] tagKeysAndValu if (!this.tagsToMetricPointIndexDictionary.TryGetValue(sortedTags, out aggregatorIndex)) { aggregatorIndex = ++this.metricPointIndex; - if (aggregatorIndex >= this.cardinalityLimit) + if (aggregatorIndex >= this.CardinalityLimit) { // sorry! out of data points. // TODO: Once we support cleanup of @@ -496,7 +496,7 @@ private int LookupAggregatorStore(KeyValuePair[] tagKeysAndValu { // This else block is for tag length = 1 aggregatorIndex = this.metricPointIndex; - if (aggregatorIndex >= this.cardinalityLimit) + if (aggregatorIndex >= this.CardinalityLimit) { // sorry! out of data points. // TODO: Once we support cleanup of @@ -518,7 +518,7 @@ private int LookupAggregatorStore(KeyValuePair[] tagKeysAndValu if (!this.tagsToMetricPointIndexDictionary.TryGetValue(givenTags, out aggregatorIndex)) { aggregatorIndex = ++this.metricPointIndex; - if (aggregatorIndex >= this.cardinalityLimit) + if (aggregatorIndex >= this.CardinalityLimit) { // sorry! out of data points. // TODO: Once we support cleanup of diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index d234ae0bd70..cfd6b4e3463 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -41,7 +41,7 @@ public sealed class Metric ("System.Net.Http", "http.client.connection.duration"), }; - private readonly AggregatorStore aggStore; + internal readonly AggregatorStore AggregatorStore; internal Metric( MetricStreamIdentity instrumentIdentity, @@ -155,7 +155,7 @@ internal Metric( throw new NotSupportedException($"Unsupported Instrument Type: {instrumentIdentity.InstrumentType.FullName}"); } - this.aggStore = new AggregatorStore(instrumentIdentity, aggType, temporality, cardinalityLimit, emitOverflowAttribute, shouldReclaimUnusedMetricPoints, exemplarFilter); + this.AggregatorStore = new AggregatorStore(instrumentIdentity, aggType, temporality, cardinalityLimit, emitOverflowAttribute, shouldReclaimUnusedMetricPoints, exemplarFilter); this.Temporality = temporality; } @@ -211,14 +211,14 @@ internal Metric( /// /// . public MetricPointsAccessor GetMetricPoints() - => this.aggStore.GetMetricPoints(); + => this.AggregatorStore.GetMetricPoints(); internal void UpdateLong(long value, ReadOnlySpan> tags) - => this.aggStore.Update(value, tags); + => this.AggregatorStore.Update(value, tags); internal void UpdateDouble(double value, ReadOnlySpan> tags) - => this.aggStore.Update(value, tags); + => this.AggregatorStore.Update(value, tags); internal int Snapshot() - => this.aggStore.Snapshot(); + => this.AggregatorStore.Snapshot(); } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index ca8bbf3163b..a45ac67cb04 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics.Metrics; -using System.Reflection; using OpenTelemetry.Internal; using OpenTelemetry.Tests; using Xunit; @@ -926,6 +925,7 @@ public void CardinalityLimitofMatchingViewTakesPrecedenceOverMetricProviderWhenB using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); +#pragma warning disable CS0618 // Type or member is obsolete using var container = this.BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .SetMaxMetricPointsPerMetricStream(3) @@ -934,6 +934,7 @@ public void CardinalityLimitofMatchingViewTakesPrecedenceOverMetricProviderWhenB return new MetricStreamConfiguration() { Name = "MetricStreamA", CardinalityLimit = 10000 }; }) .AddInMemoryExporter(exportedItems)); +#pragma warning restore CS0618 // Type or member is obsolete var counter = meter.CreateCounter("counter"); counter.Add(100); @@ -942,10 +943,7 @@ public void CardinalityLimitofMatchingViewTakesPrecedenceOverMetricProviderWhenB var metric = exportedItems[0]; - var aggregatorStore = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metric) as AggregatorStore; - var maxMetricPointsAttribute = (int)typeof(AggregatorStore).GetField("maxMetricPoints", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStore); - - Assert.Equal(10000, maxMetricPointsAttribute); + Assert.Equal(10000, metric.AggregatorStore.CardinalityLimit); } [Fact] @@ -987,24 +985,15 @@ public void ViewConflict_TwoDistinctInstruments_ThreeStreams() var metricB = exportedItems[1]; var metricC = exportedItems[2]; - var aggregatorStoreA = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metricA) as AggregatorStore; - var maxMetricPointsAttributeA = (int)typeof(AggregatorStore).GetField("maxMetricPoints", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStoreA); - - Assert.Equal(256, maxMetricPointsAttributeA); + Assert.Equal(256, metricA.AggregatorStore.CardinalityLimit); Assert.Equal("MetricStreamA", metricA.Name); Assert.Equal(20, GetAggregatedValue(metricA)); - var aggregatorStoreB = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metricB) as AggregatorStore; - var maxMetricPointsAttributeB = (int)typeof(AggregatorStore).GetField("maxMetricPoints", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStoreB); - - Assert.Equal(3, maxMetricPointsAttributeB); + Assert.Equal(3, metricB.AggregatorStore.CardinalityLimit); Assert.Equal("MetricStreamB", metricB.Name); Assert.Equal(10, GetAggregatedValue(metricB)); - var aggregatorStoreC = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metricC) as AggregatorStore; - var maxMetricPointsAttributeC = (int)typeof(AggregatorStore).GetField("maxMetricPoints", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStoreC); - - Assert.Equal(200000, maxMetricPointsAttributeC); + Assert.Equal(200000, metricC.AggregatorStore.CardinalityLimit); Assert.Equal("MetricStreamC", metricC.Name); Assert.Equal(10, GetAggregatedValue(metricC)); From 8b91bf30a4aab0e1ad4fe5984ea36340d2168a3c Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 15:38:02 -0800 Subject: [PATCH 09/26] Doc updates. --- docs/metrics/README.md | 9 +-- docs/metrics/customizing-the-sdk/README.md | 91 +++------------------- 2 files changed, 16 insertions(+), 84 deletions(-) diff --git a/docs/metrics/README.md b/docs/metrics/README.md index 412a824ea11..df59b636309 100644 --- a/docs/metrics/README.md +++ b/docs/metrics/README.md @@ -356,11 +356,10 @@ predictable and reliable behavior when excessive cardinality happens, whether it was due to a malicious attack or developer making mistakes while writing code. OpenTelemetry has a default cardinality limit of `2000` per metric. This limit -can be configured at `MeterProvider` level using the -`SetMaxMetricPointsPerMetricStream` method, or at individual -[view](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#view) -level using `MetricStreamConfiguration.CardinalityLimit`. Refer to this -[doc](../../docs/metrics/customizing-the-sdk/README.md#changing-maximum-metricpoints-per-metricstream) +can be configured for individual metrics by using +[views](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#view) +and the `MetricStreamConfiguration.CardinalityLimit` setting. Refer to this +[doc](../../docs/metrics/customizing-the-sdk/README.md#changing-the-cardinality-limit-for-a-metric) for more information. Given a metric, once the cardinality limit is reached, any new measurement which diff --git a/docs/metrics/customizing-the-sdk/README.md b/docs/metrics/customizing-the-sdk/README.md index d7cf59961a9..2a16242e02a 100644 --- a/docs/metrics/customizing-the-sdk/README.md +++ b/docs/metrics/customizing-the-sdk/README.md @@ -367,85 +367,18 @@ MyFruitCounter.Add(1, new("name", "apple"), new("color", "red")); AnotherFruitCounter.Add(1, new("name", "apple"), new("color", "red")); ``` -### Changing maximum MetricPoints per MetricStream - -A Metric stream can contain as many Metric points as the number of unique -combination of keys and values. To protect the SDK from unbounded memory usage, -SDK limits the maximum number of metric points per metric stream, to a default -of 2000. Once the limit is hit, any new key/value combination for that metric is -ignored. The SDK chooses the key/value combinations in the order in which they -are emitted. `SetMaxMetricPointsPerMetricStream` can be used to override the -default. - -> [!NOTE] -> One `MetricPoint` is reserved for every `MetricStream` for the -special case where there is no key/value pair associated with the metric. The -maximum number of `MetricPoint`s has to accommodate for this special case. - -Consider the below example. Here we set the maximum number of `MetricPoint`s -allowed to be `3`. This means that for every `MetricStream`, the SDK will export -measurements for up to `3` distinct key/value combinations of the metric. There -are two instruments published here: `MyFruitCounter` and `AnotherFruitCounter`. -There are two total `MetricStream`s created one for each of these instruments. -SDK will limit the maximum number of distinct key/value combinations for each of -these `MetricStream`s to `3`. - -```csharp -using System.Collections.Generic; -using System.Diagnostics.Metrics; -using OpenTelemetry; -using OpenTelemetry.Metrics; - -Counter MyFruitCounter = MyMeter.CreateCounter("MyFruitCounter"); -Counter AnotherFruitCounter = MyMeter.CreateCounter("AnotherFruitCounter"); - -using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter("*") - .AddConsoleExporter() - .SetMaxMetricPointsPerMetricStream(3) // The default value is 2000 - .Build(); - -// There are four distinct key/value combinations emitted for `MyFruitCounter`: -// 1. No key/value pair -// 2. (name:apple, color:red) -// 3. (name:lemon, color:yellow) -// 4. (name:apple, color:green) - -// Since the maximum number of `MetricPoint`s allowed is `3`, the SDK will only export measurements for the following three combinations: -// 1. No key/value pair -// 2. (name:apple, color:red) -// 3. (name:lemon, color:yellow) - -MyFruitCounter.Add(1); // Exported (No key/value pair) -MyFruitCounter.Add(1, new("name", "apple"), new("color", "red")); // Exported -MyFruitCounter.Add(2, new("name", "lemon"), new("color", "yellow")); // Exported -MyFruitCounter.Add(1, new("name", "lemon"), new("color", "yellow")); // Exported -MyFruitCounter.Add(2, new("name", "apple"), new("color", "green")); // Not exported -MyFruitCounter.Add(5, new("name", "apple"), new("color", "red")); // Exported -MyFruitCounter.Add(4, new("name", "lemon"), new("color", "yellow")); // Exported - -// There are four distinct key/value combinations emitted for `AnotherFruitCounter`: -// 1. (name:kiwi) -// 2. (name:banana, color:yellow) -// 3. (name:mango, color:yellow) -// 4. (name:banana, color:green) - -// Since the maximum number of `MetricPoint`s allowed is `3`, the SDK will only export measurements for the following three combinations: -// 1. No key/value pair (This is a special case. The SDK reserves a `MetricPoint` for it even if it's not explicitly emitted.) -// 2. (name:kiwi) -// 3. (name:banana, color:yellow) - -AnotherFruitCounter.Add(4, new KeyValuePair("name", "kiwi")); // Exported -AnotherFruitCounter.Add(1, new("name", "banana"), new("color", "yellow")); // Exported -AnotherFruitCounter.Add(2, new("name", "mango"), new("color", "yellow")); // Not exported -AnotherFruitCounter.Add(1, new("name", "mango"), new("color", "yellow")); // Not exported -AnotherFruitCounter.Add(2, new("name", "banana"), new("color", "green")); // Not exported -AnotherFruitCounter.Add(5, new("name", "banana"), new("color", "yellow")); // Exported -AnotherFruitCounter.Add(4, new("name", "mango"), new("color", "yellow")); // Not exported -``` - -To set the [cardinality limit](../README.md#cardinality-limits) at individual -metric level, use `MetricStreamConfiguration.CardinalityLimit`: +### Changing the cardinality limit for a Metric + +A Metric contains many Metric points which are the number of unique combinations +of key/values recorded by an instrument. To protect the SDK from unbounded +memory usage, there is a default limit of 2000 metric points per metric which +will be maintained at any given time. Once the limit is hit, any new key/value +combination for a metric will be ignored. The SDK chooses the key/value +combinations in the order in which they are emitted. + +To set the [cardinality limit](../README.md#cardinality-limits) for an +individual metric, use `MetricStreamConfiguration.CardinalityLimit` setting on +the View API: ```csharp var meterProvider = Sdk.CreateMeterProviderBuilder() From e3eb7862d214484b6733715832fd51601f1a0cac Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 15:43:45 -0800 Subject: [PATCH 10/26] CHANGELOG update. --- src/OpenTelemetry/CHANGELOG.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 2a638e090b3..7a9240ee513 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -39,9 +39,8 @@ ([#5265](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5265)) * **Experimental (pre-release builds only):** Obsoleted - `MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream` and added - `MeterProviderBuilderExtensions.SetCardinalityLimit`. The new method does the - same thing but has a more spec-compliant name. + `MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream` in favor of + the new View-based `CardinalityLimit` API. ([#5328](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5328)) ## 1.7.0 From c2f8f88d86409c3e0183a90a874ccd6cf7ce18ec Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 7 Feb 2024 16:00:22 -0800 Subject: [PATCH 11/26] Remove more reflection from tests. --- src/OpenTelemetry/Metrics/AggregatorStore.cs | 14 ++++++------ .../MetricOverflowAttributeTestsBase.cs | 22 +++---------------- 2 files changed, 10 insertions(+), 26 deletions(-) diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index 8428517ffd3..bccdbbf42c7 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -14,6 +14,7 @@ internal sealed class AggregatorStore internal readonly bool OutputDelta; internal readonly bool OutputDeltaWithUnusedMetricPointReclaimEnabled; internal readonly int CardinalityLimit; + internal readonly bool EmitOverflowAttribute; internal long DroppedMeasurements = 0; private static readonly string MetricPointCapHitFixMessage = "Consider opting in for the experimental SDK feature to emit all the throttled metrics under the overflow attribute by setting env variable OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE = true. You could also modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit."; @@ -43,7 +44,6 @@ internal sealed class AggregatorStore private readonly int exponentialHistogramMaxScale; private readonly UpdateLongDelegate updateLongCallback; private readonly UpdateDoubleDelegate updateDoubleCallback; - private readonly bool emitOverflowAttribute; private readonly ExemplarFilter exemplarFilter; private readonly Func[], int, int> lookupAggregatorStore; @@ -89,7 +89,7 @@ internal AggregatorStore( this.tagsKeysInterestingCount = hs.Count; } - this.emitOverflowAttribute = emitOverflowAttribute; + this.EmitOverflowAttribute = emitOverflowAttribute; var reservedMetricPointsCount = 1; @@ -227,7 +227,7 @@ internal void SnapshotDeltaWithMetricPointReclaim() int startIndexForReclaimableMetricPoints = 1; - if (this.emitOverflowAttribute) + if (this.EmitOverflowAttribute) { startIndexForReclaimableMetricPoints = 2; // Index 0 and 1 are reserved for no tags and overflow @@ -929,7 +929,7 @@ private void UpdateLong(long value, ReadOnlySpan> { Interlocked.Increment(ref this.DroppedMeasurements); - if (this.emitOverflowAttribute) + if (this.EmitOverflowAttribute) { this.InitializeOverflowTagPointIfNotInitialized(); this.metricPoints[1].Update(value); @@ -973,7 +973,7 @@ private void UpdateLongCustomTags(long value, ReadOnlySpan Date: Wed, 7 Feb 2024 16:11:08 -0800 Subject: [PATCH 12/26] Remove more test reflection. --- .../Metrics/MetricPointReclaimTestsBase.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs index d0339d69ba9..de5eb88a0af 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs @@ -286,17 +286,12 @@ private sealed class CustomExporter : BaseExporter private readonly bool assertNoDroppedMeasurements; - private readonly FieldInfo aggStoreFieldInfo; - private readonly FieldInfo metricPointLookupDictionaryFieldInfo; public CustomExporter(bool assertNoDroppedMeasurements) { this.assertNoDroppedMeasurements = assertNoDroppedMeasurements; - var metricFields = typeof(Metric).GetFields(BindingFlags.NonPublic | BindingFlags.Instance); - this.aggStoreFieldInfo = metricFields!.FirstOrDefault(field => field.Name == "aggStore"); - var aggregatorStoreFields = typeof(AggregatorStore).GetFields(BindingFlags.NonPublic | BindingFlags.Instance); this.metricPointLookupDictionaryFieldInfo = aggregatorStoreFields!.FirstOrDefault(field => field.Name == "tagsToMetricPointIndexDictionaryDelta"); } @@ -305,7 +300,7 @@ public override ExportResult Export(in Batch batch) { foreach (var metric in batch) { - var aggStore = this.aggStoreFieldInfo.GetValue(metric) as AggregatorStore; + var aggStore = metric.AggregatorStore; var metricPointLookupDictionary = this.metricPointLookupDictionaryFieldInfo.GetValue(aggStore) as ConcurrentDictionary; var droppedMeasurements = aggStore.DroppedMeasurements; From bab53c997e549abecf82a0d28924b43a66e3b5a3 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 8 Feb 2024 13:25:13 -0800 Subject: [PATCH 13/26] Rename MaxMetricStreams -> MetricLimit. --- .../Builder/MeterProviderBuilderExtensions.cs | 2 +- .../Metrics/Builder/MeterProviderBuilderSdk.cs | 8 ++++---- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 2 +- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 16 ++++++++-------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs index 19b400ebe5e..4a6d67a5082 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs @@ -218,7 +218,7 @@ public static MeterProviderBuilder SetMaxMetricStreams(this MeterProviderBuilder { if (builder is MeterProviderBuilderSdk meterProviderBuilderSdk) { - meterProviderBuilderSdk.SetMaxMetricStreams(maxMetricStreams); + meterProviderBuilderSdk.SetMetricLimit(maxMetricStreams); } }); diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs index 4d77e9075e5..a3ca2a15e56 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs @@ -15,7 +15,7 @@ namespace OpenTelemetry.Metrics; /// internal sealed class MeterProviderBuilderSdk : MeterProviderBuilder, IMeterProviderBuilder { - public const int DefaultMaxMetricStreams = 1000; + public const int DefaultMetricLimit = 1000; public const int DefaultCardinalityLimit = 2000; private const string DefaultInstrumentationVersion = "1.0.0.0"; @@ -49,7 +49,7 @@ public MeterProviderBuilderSdk(IServiceProvider serviceProvider) public List> ViewConfigs { get; } = new(); - public int MaxMetricStreams { get; private set; } = DefaultMaxMetricStreams; + public int MetricLimit { get; private set; } = DefaultMetricLimit; public int CardinalityLimit { get; private set; } = DefaultCardinalityLimit; @@ -186,9 +186,9 @@ public MeterProviderBuilder AddView(Func return this; } - public MeterProviderBuilder SetMaxMetricStreams(int maxMetricStreams) + public MeterProviderBuilder SetMetricLimit(int metricLimit) { - this.MaxMetricStreams = maxMetricStreams; + this.MetricLimit = metricLimit; return this; } diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 7a71712dd31..37be71cf77c 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -78,7 +78,7 @@ internal MeterProviderSdk( reader.SetParentProvider(this); reader.ApplyParentProviderSettings( - state.MaxMetricStreams, + state.MetricLimit, state.CardinalityLimit, state.ExemplarFilter, isEmitOverflowAttributeKeySet); diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index f67e8d34fad..c89fb1810bd 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -17,7 +17,7 @@ public abstract partial class MetricReader private readonly HashSet metricStreamNames = new(StringComparer.OrdinalIgnoreCase); private readonly ConcurrentDictionary instrumentIdentityToMetric = new(); private readonly object instrumentCreationLock = new(); - private int maxMetricStreams; + private int metricLimit; private int cardinalityLimit; private Metric?[]? metrics; private Metric[]? metricsCurrentBatch; @@ -44,7 +44,7 @@ internal AggregationTemporality GetAggregationTemporality(Type instrumentType) } var index = ++this.metricIndex; - if (index >= this.maxMetricStreams) + if (index >= this.metricLimit) { OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricStreamIdentity.InstrumentName, metricStreamIdentity.MeterName, "Maximum allowed Metric streams for the provider exceeded.", "Use MeterProviderBuilder.AddView to drop unused instruments. Or use MeterProviderBuilder.SetMaxMetricStreams to configure MeterProvider to allow higher limit."); return null; @@ -129,7 +129,7 @@ internal List AddMetricsListWithViews(Instrument instrument, List= this.maxMetricStreams) + if (index >= this.metricLimit) { OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricStreamIdentity.InstrumentName, metricStreamIdentity.MeterName, "Maximum allowed Metric streams for the provider exceeded.", "Use MeterProviderBuilder.AddView to drop unused instruments. Or use MeterProviderBuilder.SetMaxMetricStreams to configure MeterProvider to allow higher limit."); } @@ -206,14 +206,14 @@ internal void CompleteMeasurement(List metrics) } internal void ApplyParentProviderSettings( - int maxMetricStreams, + int metricLimit, int cardinalityLimit, ExemplarFilter? exemplarFilter, bool isEmitOverflowAttributeKeySet) { - this.maxMetricStreams = maxMetricStreams; - this.metrics = new Metric[maxMetricStreams]; - this.metricsCurrentBatch = new Metric[maxMetricStreams]; + this.metricLimit = metricLimit; + this.metrics = new Metric[metricLimit]; + this.metricsCurrentBatch = new Metric[metricLimit]; this.cardinalityLimit = cardinalityLimit; this.exemplarFilter = exemplarFilter; @@ -269,7 +269,7 @@ private Batch GetMetricsBatch() try { - var indexSnapshot = Math.Min(this.metricIndex, this.maxMetricStreams - 1); + var indexSnapshot = Math.Min(this.metricIndex, this.metricLimit - 1); var target = indexSnapshot + 1; int metricCountCurrentBatch = 0; for (int i = 0; i < target; i++) From 856446fd4f39e18786b6a96f5eee0dbf642941ad Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 8 Feb 2024 13:28:49 -0800 Subject: [PATCH 14/26] Code review. --- .../diagnostics/experimental-apis/OTEL1003.md | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/diagnostics/experimental-apis/OTEL1003.md b/docs/diagnostics/experimental-apis/OTEL1003.md index 19913edc916..169ea4ebadf 100644 --- a/docs/diagnostics/experimental-apis/OTEL1003.md +++ b/docs/diagnostics/experimental-apis/OTEL1003.md @@ -13,13 +13,16 @@ Experimental APIs may be changed or removed in the future. From the specification: -> The cardinality limit for an aggregation is defined in one of three ways: A -> view with criteria matching the instrument an aggregation is created for has -> an aggregation_cardinality_limit value defined for the stream, that value -> SHOULD be used. If there is no matching view, but the MetricReader defines a -> default cardinality limit value based on the instrument an aggregation is -> created for, that value SHOULD be used. If none of the previous values are -> defined, the default value of 2000 SHOULD be used. +> The cardinality limit for an aggregation is defined in one of three ways: +> +> 1. A [view](#view) with criteria matching the instrument an aggregation is +> created for has an `aggregation_cardinality_limit` value defined for the +> stream, that value SHOULD be used. +> 2. If there is no matching view, but the `MetricReader` defines a default +> cardinality limit value based on the instrument an aggregation is created +> for, that value SHOULD be used. +> 3. If none of the previous values are defined, the default value of 2000 SHOULD +> be used. We are exposing these APIs experimentally until the specification declares them stable. @@ -37,3 +40,7 @@ using var meterProvider = Sdk.CreateMeterProviderBuilder() new MetricStreamConfiguration { CardinalityLimit = 10 }) .Build(); ``` + +### Setting cardinality limit for a specific MetricReader + +This is NOT currently supported by OpenTelemetry .NET. From 6d05f7bd4cd09f077aa124a5afdf2ffb7f91959c Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 8 Feb 2024 13:30:02 -0800 Subject: [PATCH 15/26] Lint. --- docs/diagnostics/experimental-apis/OTEL1003.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/diagnostics/experimental-apis/OTEL1003.md b/docs/diagnostics/experimental-apis/OTEL1003.md index 169ea4ebadf..8d66004da01 100644 --- a/docs/diagnostics/experimental-apis/OTEL1003.md +++ b/docs/diagnostics/experimental-apis/OTEL1003.md @@ -14,7 +14,7 @@ Experimental APIs may be changed or removed in the future. From the specification: > The cardinality limit for an aggregation is defined in one of three ways: -> +> > 1. A [view](#view) with criteria matching the instrument an aggregation is > created for has an `aggregation_cardinality_limit` value defined for the > stream, that value SHOULD be used. From f3218a52f87704ea1c2ca5f9f0e8790a8b8da8cb Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 8 Feb 2024 13:34:55 -0800 Subject: [PATCH 16/26] Lint. --- docs/diagnostics/experimental-apis/OTEL1003.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/diagnostics/experimental-apis/OTEL1003.md b/docs/diagnostics/experimental-apis/OTEL1003.md index 8d66004da01..143aeea46b6 100644 --- a/docs/diagnostics/experimental-apis/OTEL1003.md +++ b/docs/diagnostics/experimental-apis/OTEL1003.md @@ -15,14 +15,14 @@ From the specification: > The cardinality limit for an aggregation is defined in one of three ways: > -> 1. A [view](#view) with criteria matching the instrument an aggregation is -> created for has an `aggregation_cardinality_limit` value defined for the -> stream, that value SHOULD be used. +> 1. A view with criteria matching the instrument an aggregation is created for +> has an `aggregation_cardinality_limit` value defined for the stream, that +> value SHOULD be used. > 2. If there is no matching view, but the `MetricReader` defines a default > cardinality limit value based on the instrument an aggregation is created > for, that value SHOULD be used. -> 3. If none of the previous values are defined, the default value of 2000 SHOULD -> be used. +> 3. If none of the previous values are defined, the default value of 2000 +> SHOULD be used. We are exposing these APIs experimentally until the specification declares them stable. From 1b6c2a4025368babbb0f7e176ad8e13d5d864d57 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 8 Feb 2024 13:41:36 -0800 Subject: [PATCH 17/26] Code review. --- docs/metrics/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/metrics/README.md b/docs/metrics/README.md index df59b636309..a55463e49b2 100644 --- a/docs/metrics/README.md +++ b/docs/metrics/README.md @@ -356,8 +356,8 @@ predictable and reliable behavior when excessive cardinality happens, whether it was due to a malicious attack or developer making mistakes while writing code. OpenTelemetry has a default cardinality limit of `2000` per metric. This limit -can be configured for individual metrics by using -[views](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#view) +can be configured at the individual metric level using the [View +API](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#view) and the `MetricStreamConfiguration.CardinalityLimit` setting. Refer to this [doc](../../docs/metrics/customizing-the-sdk/README.md#changing-the-cardinality-limit-for-a-metric) for more information. From ae5c68dfc5ec17e9f076ce7da9d6a860cdf379f0 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 8 Feb 2024 13:42:48 -0800 Subject: [PATCH 18/26] Code review. --- docs/metrics/customizing-the-sdk/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/metrics/customizing-the-sdk/README.md b/docs/metrics/customizing-the-sdk/README.md index 2a16242e02a..bf1df09ccb2 100644 --- a/docs/metrics/customizing-the-sdk/README.md +++ b/docs/metrics/customizing-the-sdk/README.md @@ -372,7 +372,7 @@ AnotherFruitCounter.Add(1, new("name", "apple"), new("color", "red")); A Metric contains many Metric points which are the number of unique combinations of key/values recorded by an instrument. To protect the SDK from unbounded memory usage, there is a default limit of 2000 metric points per metric which -will be maintained at any given time. Once the limit is hit, any new key/value +will be maintained by the SDK. Once the limit is hit, any new key/value combination for a metric will be ignored. The SDK chooses the key/value combinations in the order in which they are emitted. From 73df3ecd2ff6caae33690fed0c00f93b3fb4210f Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 8 Feb 2024 13:48:44 -0800 Subject: [PATCH 19/26] Code review. --- docs/diagnostics/experimental-apis/OTEL1003.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/diagnostics/experimental-apis/OTEL1003.md b/docs/diagnostics/experimental-apis/OTEL1003.md index 143aeea46b6..5f62f03575f 100644 --- a/docs/diagnostics/experimental-apis/OTEL1003.md +++ b/docs/diagnostics/experimental-apis/OTEL1003.md @@ -43,4 +43,5 @@ using var meterProvider = Sdk.CreateMeterProviderBuilder() ### Setting cardinality limit for a specific MetricReader -This is NOT currently supported by OpenTelemetry .NET. +[This is not currently supported by OpenTelemetry +.NET.](https://github.com/open-telemetry/opentelemetry-dotnet/issues/5331) From 695eb661c1c33ca7f22426276ac724f64a8a9fcb Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 8 Feb 2024 13:53:20 -0800 Subject: [PATCH 20/26] Code review. --- docs/metrics/customizing-the-sdk/README.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/metrics/customizing-the-sdk/README.md b/docs/metrics/customizing-the-sdk/README.md index bf1df09ccb2..6ea9f48f484 100644 --- a/docs/metrics/customizing-the-sdk/README.md +++ b/docs/metrics/customizing-the-sdk/README.md @@ -369,12 +369,13 @@ AnotherFruitCounter.Add(1, new("name", "apple"), new("color", "red")); ### Changing the cardinality limit for a Metric -A Metric contains many Metric points which are the number of unique combinations -of key/values recorded by an instrument. To protect the SDK from unbounded -memory usage, there is a default limit of 2000 metric points per metric which -will be maintained by the SDK. Once the limit is hit, any new key/value -combination for a metric will be ignored. The SDK chooses the key/value -combinations in the order in which they are emitted. +A Metric contains many Metric points which are the storage buckets for +aggregation of measurements for the unique combinations of key/values recorded +by an instrument. To protect the SDK from unbounded memory usage, there is a +default limit of 2000 metric points per metric which will be maintained by the +SDK. Once the limit is hit, any new key/value combination for a metric will be +ignored. The SDK chooses the key/value combinations in the order in which they +are emitted. To set the [cardinality limit](../README.md#cardinality-limits) for an individual metric, use `MetricStreamConfiguration.CardinalityLimit` setting on From f1b62b0763253ae978b6e57ce999044d509810f4 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 8 Feb 2024 14:06:03 -0800 Subject: [PATCH 21/26] Bug fix. --- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 6 +- .../Metrics/MetricViewTests.cs | 58 ++++++++++++++----- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index c89fb1810bd..46c3b464557 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -137,12 +137,14 @@ internal List AddMetricsListWithViews(Instrument instrument, List(); -#pragma warning disable CS0618 // Type or member is obsolete - using var container = this.BuildMeterProvider(out var meterProvider, builder => builder - .AddMeter(meter.Name) - .SetMaxMetricPointsPerMetricStream(3) - .AddView((instrument) => + using var container = this.BuildMeterProvider(out var meterProvider, builder => + { + if (setDefault) { - return new MetricStreamConfiguration() { Name = "MetricStreamA", CardinalityLimit = 10000 }; - }) - .AddInMemoryExporter(exportedItems)); +#pragma warning disable CS0618 // Type or member is obsolete + builder.SetMaxMetricPointsPerMetricStream(3); #pragma warning restore CS0618 // Type or member is obsolete + } + + builder + .AddMeter(meter.Name) + .AddView((instrument) => + { + if (instrument.Name == "counter2") + { + return new MetricStreamConfiguration() { Name = "MetricStreamA", CardinalityLimit = 10000 }; + } + + return null; + }) + .AddInMemoryExporter(exportedItems); + }); - var counter = meter.CreateCounter("counter"); - counter.Add(100); + var counter1 = meter.CreateCounter("counter1"); + counter1.Add(100); + + var counter2 = meter.CreateCounter("counter2"); + counter2.Add(100); + + var counter3 = meter.CreateCounter("counter3"); + counter3.Add(100); meterProvider.ForceFlush(MaxTimeToAllowForFlush); - var metric = exportedItems[0]; + Assert.Equal(3, exportedItems.Count); - Assert.Equal(10000, metric.AggregatorStore.CardinalityLimit); + Assert.Equal(10000, exportedItems[1].AggregatorStore.CardinalityLimit); + if (setDefault) + { + Assert.Equal(3, exportedItems[0].AggregatorStore.CardinalityLimit); + Assert.Equal(3, exportedItems[2].AggregatorStore.CardinalityLimit); + } + else + { + Assert.Equal(2000, exportedItems[0].AggregatorStore.CardinalityLimit); + Assert.Equal(2000, exportedItems[2].AggregatorStore.CardinalityLimit); + } } [Fact] From 9064b7ac959947a7880360b624daca64881fe93a Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 8 Feb 2024 14:36:48 -0800 Subject: [PATCH 22/26] Code review. --- docs/metrics/README.md | 7 +++++-- docs/metrics/customizing-the-sdk/README.md | 13 ++++--------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/docs/metrics/README.md b/docs/metrics/README.md index a55463e49b2..18888d0e1fe 100644 --- a/docs/metrics/README.md +++ b/docs/metrics/README.md @@ -363,9 +363,12 @@ and the `MetricStreamConfiguration.CardinalityLimit` setting. Refer to this for more information. Given a metric, once the cardinality limit is reached, any new measurement which -cannot be independently aggregated because of the limit will be aggregated using +cannot be independently aggregated because of the limit will be dropped (a +warning is written to the [self-diagnostic +log](../../src/OpenTelemetry/README.md#self-diagnostics)) or aggregated using the [overflow -attribute](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#overflow-attribute). +attribute](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#overflow-attribute) +(if enabled). > [!NOTE] > Overflow attribute was introduced in OpenTelemetry .NET diff --git a/docs/metrics/customizing-the-sdk/README.md b/docs/metrics/customizing-the-sdk/README.md index 6ea9f48f484..48193382f7e 100644 --- a/docs/metrics/customizing-the-sdk/README.md +++ b/docs/metrics/customizing-the-sdk/README.md @@ -369,14 +369,6 @@ AnotherFruitCounter.Add(1, new("name", "apple"), new("color", "red")); ### Changing the cardinality limit for a Metric -A Metric contains many Metric points which are the storage buckets for -aggregation of measurements for the unique combinations of key/values recorded -by an instrument. To protect the SDK from unbounded memory usage, there is a -default limit of 2000 metric points per metric which will be maintained by the -SDK. Once the limit is hit, any new key/value combination for a metric will be -ignored. The SDK chooses the key/value combinations in the order in which they -are emitted. - To set the [cardinality limit](../README.md#cardinality-limits) for an individual metric, use `MetricStreamConfiguration.CardinalityLimit` setting on the View API: @@ -384,7 +376,10 @@ the View API: ```csharp var meterProvider = Sdk.CreateMeterProviderBuilder() .AddMeter("MyCompany.MyProduct.MyLibrary") - .AddView(instrumentName: "MyFruitCounter", new MetricStreamConfiguration { CardinalityLimit = 10 }) + // Set a custom CardinalityLimit (10) for "MyFruitCounter" + .AddView( + instrumentName: "MyFruitCounter", + new MetricStreamConfiguration { CardinalityLimit = 10 }) .AddConsoleExporter() .Build(); ``` From 454bd1795f61ab07ccca478d5d8980c33a895a15 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 9 Feb 2024 10:27:04 -0800 Subject: [PATCH 23/26] Code review. --- docs/metrics/customizing-the-sdk/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/metrics/customizing-the-sdk/README.md b/docs/metrics/customizing-the-sdk/README.md index 48193382f7e..653dd4979f3 100644 --- a/docs/metrics/customizing-the-sdk/README.md +++ b/docs/metrics/customizing-the-sdk/README.md @@ -373,6 +373,11 @@ To set the [cardinality limit](../README.md#cardinality-limits) for an individual metric, use `MetricStreamConfiguration.CardinalityLimit` setting 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") From 87d5875f491fa0e06fe614bdba09ef267fd23e7d Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 9 Feb 2024 10:35:28 -0800 Subject: [PATCH 24/26] Code review. --- src/OpenTelemetry/CHANGELOG.md | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 7a9240ee513..2858ee22245 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -21,8 +21,11 @@ * **Experimental (pre-release builds only):** Added support for setting `CardinalityLimit` (the maximum number of data points allowed for a metric) - when configuring a view. - ([#5312](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5312)) + when configuring a view and obsoleted + `MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream`. The + default cardinality limit for metrics remains at `2000`. + ([#5312](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5312), + [#5328](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5328)) * Updated `LogRecord` to keep `CategoryName` and `Logger` in sync when using the experimental Log Bridge API. @@ -38,11 +41,6 @@ [IMetricsListener](https://learn.microsoft.com/dotNet/api/microsoft.extensions.diagnostics.metrics.imetricslistener). ([#5265](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5265)) -* **Experimental (pre-release builds only):** Obsoleted - `MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream` in favor of - the new View-based `CardinalityLimit` API. - ([#5328](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5328)) - ## 1.7.0 Released 2023-Dec-08 From c24e42c62f442c4c5270719db8110d48774f9173 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 9 Feb 2024 10:39:44 -0800 Subject: [PATCH 25/26] CHANGELOG tweak. --- src/OpenTelemetry/CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 2858ee22245..6260596c78c 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -21,9 +21,10 @@ * **Experimental (pre-release builds only):** Added support for setting `CardinalityLimit` (the maximum number of data points allowed for a metric) - when configuring a view and obsoleted - `MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream`. The - default cardinality limit for metrics remains at `2000`. + when configuring a view (applies to individual metrics) and obsoleted + `MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream` (previously + applied to all metrics). The default cardinality limit for metrics remains at + `2000`. ([#5312](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5312), [#5328](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5328)) From 8a6d2c2f4c73e5fc0b273f2dfc203d4b31902bc5 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 9 Feb 2024 10:50:19 -0800 Subject: [PATCH 26/26] Code review. --- docs/metrics/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/metrics/README.md b/docs/metrics/README.md index 24b99a1f18b..2d50ebb7bc4 100644 --- a/docs/metrics/README.md +++ b/docs/metrics/README.md @@ -386,12 +386,12 @@ and the `MetricStreamConfiguration.CardinalityLimit` setting. Refer to this for more information. Given a metric, once the cardinality limit is reached, any new measurement which -cannot be independently aggregated because of the limit will be dropped (a -warning is written to the [self-diagnostic -log](../../src/OpenTelemetry/README.md#self-diagnostics)) or aggregated using -the [overflow +cannot be independently aggregated because of the limit will be dropped or +aggregated using the [overflow attribute](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#overflow-attribute) -(if enabled). +(if enabled). When NOT using the overflow attribute feature a warning is written +to the [self-diagnostic log](../../src/OpenTelemetry/README.md#self-diagnostics) +the first time an overflow is detected for a given metric. > [!NOTE] > Overflow attribute was introduced in OpenTelemetry .NET