From 5dcf29f770b30f8e37c1de0768829639e376fb57 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Fri, 19 Nov 2021 10:50:16 -0800 Subject: [PATCH] Allow ability to override max Metric streams and MetricPoints per stream (#2635) --- .../.publicApi/net461/PublicAPI.Unshipped.txt | 2 + .../netstandard2.0/PublicAPI.Unshipped.txt | 2 + src/OpenTelemetry/CHANGELOG.md | 3 ++ src/OpenTelemetry/Metrics/AggregatorStore.cs | 14 +++--- .../Metrics/MeterProviderBuilderBase.cs | 22 +++++++++ .../Metrics/MeterProviderBuilderExtensions.cs | 47 +++++++++++++++++++ src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 4 ++ src/OpenTelemetry/Metrics/Metric.cs | 3 +- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 29 ++++++++---- .../Metrics/MetricAPITest.cs | 12 ++--- 10 files changed, 117 insertions(+), 21 deletions(-) diff --git a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt index c7eaa42d3..68a085d4b 100644 --- a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt @@ -112,6 +112,8 @@ static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddView(this OpenTel static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddView(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder, string instrumentName, string name) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddView(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder, System.Func viewConfig) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.Build(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder) -> OpenTelemetry.Metrics.MeterProvider +static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder, int maxMetricPointsPerMetricStream) -> OpenTelemetry.Metrics.MeterProviderBuilder +static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetMaxMetricStreams(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder, int maxMetricStreams) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetResourceBuilder(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder, OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Metrics.MeterProviderExtensions.ForceFlush(this OpenTelemetry.Metrics.MeterProvider provider, int timeoutMilliseconds = -1) -> bool static OpenTelemetry.Metrics.MeterProviderExtensions.Shutdown(this OpenTelemetry.Metrics.MeterProvider provider, int timeoutMilliseconds = -1) -> bool diff --git a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index c7eaa42d3..68a085d4b 100644 --- a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -112,6 +112,8 @@ static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddView(this OpenTel static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddView(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder, string instrumentName, string name) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddView(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder, System.Func viewConfig) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.Build(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder) -> OpenTelemetry.Metrics.MeterProvider +static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder, int maxMetricPointsPerMetricStream) -> OpenTelemetry.Metrics.MeterProviderBuilder +static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetMaxMetricStreams(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder, int maxMetricStreams) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetResourceBuilder(this OpenTelemetry.Metrics.MeterProviderBuilder meterProviderBuilder, OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Metrics.MeterProviderExtensions.ForceFlush(this OpenTelemetry.Metrics.MeterProvider provider, int timeoutMilliseconds = -1) -> bool static OpenTelemetry.Metrics.MeterProviderExtensions.Shutdown(this OpenTelemetry.Metrics.MeterProvider provider, int timeoutMilliseconds = -1) -> bool diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 871bfc5fc..cd243fcec 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -31,6 +31,9 @@ * Add support for multiple Metric readers ([#2596](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2596)) +* Add ability to configure MaxMetricStreams, MaxMetricPointsPerMetricStream + ([#2635](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2635)) + ## 1.2.0-beta1 Released 2021-Oct-08 diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index b9a7baf21..3d9f65873 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -24,7 +24,6 @@ namespace OpenTelemetry.Metrics { internal sealed class AggregatorStore { - internal const int MaxMetricPoints = 2000; private static readonly ObjectArrayEqualityComparer ObjectArrayComparer = new ObjectArrayEqualityComparer(); private readonly object lockZeroTags = new object(); private readonly HashSet tagKeysInteresting; @@ -42,6 +41,7 @@ namespace OpenTelemetry.Metrics private readonly double[] histogramBounds; private readonly UpdateLongDelegate updateLongCallback; private readonly UpdateDoubleDelegate updateDoubleCallback; + private readonly int maxMetricPoints; private int metricPointIndex = 0; private int batchSize = 0; private bool zeroTagMetricPointInitialized; @@ -51,11 +51,13 @@ namespace OpenTelemetry.Metrics internal AggregatorStore( AggregationType aggType, AggregationTemporality temporality, + int maxMetricPoints, double[] histogramBounds, string[] tagKeysInteresting = null) { - this.metricPoints = new MetricPoint[MaxMetricPoints]; - this.currentMetricPointBatch = new int[MaxMetricPoints]; + this.maxMetricPoints = maxMetricPoints; + this.metricPoints = new MetricPoint[maxMetricPoints]; + this.currentMetricPointBatch = new int[maxMetricPoints]; this.aggType = aggType; this.temporality = temporality; this.outputDelta = temporality == AggregationTemporality.Delta ? true : false; @@ -98,7 +100,7 @@ namespace OpenTelemetry.Metrics internal int Snapshot() { this.batchSize = 0; - var indexSnapshot = Math.Min(this.metricPointIndex, MaxMetricPoints - 1); + var indexSnapshot = Math.Min(this.metricPointIndex, this.maxMetricPoints - 1); if (this.temporality == AggregationTemporality.Delta) { this.SnapshotDelta(indexSnapshot); @@ -197,7 +199,7 @@ namespace OpenTelemetry.Metrics if (!value2metrics.TryGetValue(tagValues, out aggregatorIndex)) { aggregatorIndex = this.metricPointIndex; - if (aggregatorIndex >= MaxMetricPoints) + if (aggregatorIndex >= this.maxMetricPoints) { // sorry! out of data points. // TODO: Once we support cleanup of @@ -212,7 +214,7 @@ namespace OpenTelemetry.Metrics if (!value2metrics.TryGetValue(tagValues, out aggregatorIndex)) { aggregatorIndex = Interlocked.Increment(ref this.metricPointIndex); - if (aggregatorIndex >= MaxMetricPoints) + if (aggregatorIndex >= this.maxMetricPoints) { // sorry! out of data points. // TODO: Once we support cleanup of diff --git a/src/OpenTelemetry/Metrics/MeterProviderBuilderBase.cs b/src/OpenTelemetry/Metrics/MeterProviderBuilderBase.cs index 2bc47af6a..97101c8ef 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderBuilderBase.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderBuilderBase.cs @@ -29,10 +29,14 @@ namespace OpenTelemetry.Metrics /// public abstract class MeterProviderBuilderBase : MeterProviderBuilder { + internal const int MaxMetricsDefault = 1000; + internal const int MaxMetricPointsPerMetricDefault = 2000; private readonly List instrumentationFactories = new List(); private readonly List meterSources = new List(); private readonly List> viewConfigs = new List>(); private ResourceBuilder resourceBuilder = ResourceBuilder.CreateDefault(); + private int maxMetricStreams = MaxMetricsDefault; + private int maxMetricPointsPerMetricStream = MaxMetricPointsPerMetricDefault; protected MeterProviderBuilderBase() { @@ -100,6 +104,22 @@ namespace OpenTelemetry.Metrics return this; } + internal MeterProviderBuilder SetMaxMetricStreams(int maxMetricStreams) + { + Guard.Range(maxMetricStreams, min: 1); + + this.maxMetricStreams = maxMetricStreams; + return this; + } + + internal MeterProviderBuilder SetMaxMetricPointsPerMetricStream(int maxMetricPointsPerMetricStream) + { + Guard.Range(maxMetricPointsPerMetricStream, min: 1); + + this.maxMetricPointsPerMetricStream = maxMetricPointsPerMetricStream; + return this; + } + internal MeterProviderBuilder SetResourceBuilder(ResourceBuilder resourceBuilder) { Debug.Assert(resourceBuilder != null, $"{nameof(resourceBuilder)} must not be null"); @@ -119,6 +139,8 @@ namespace OpenTelemetry.Metrics this.meterSources, this.instrumentationFactories, this.viewConfigs, + this.maxMetricStreams, + this.maxMetricPointsPerMetricStream, this.MetricReaders.ToArray()); } diff --git a/src/OpenTelemetry/Metrics/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/MeterProviderBuilderExtensions.cs index 233d0cdd3..8e5485350 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderBuilderExtensions.cs @@ -121,6 +121,53 @@ namespace OpenTelemetry.Metrics return meterProviderBuilder; } + /// + /// Sets the maximum number of Metric streams supported by the MeterProvider. + /// When no Views are configured, every instrument will result in one metric stream, + /// so this control the numbers of instruments supported. + /// When Views are configued, a single instrument can result in multiple metric streams, + /// so this control the number of streams. + /// + /// MeterProviderBuilder instance. + /// Maximum number of metric streams allowed. + /// Returns for chaining. + /// + /// If an instrument is created, but disposed later, this will still be contributing to the limit. + /// This may change in the future. + /// + public static MeterProviderBuilder SetMaxMetricStreams(this MeterProviderBuilder meterProviderBuilder, int maxMetricStreams) + { + if (meterProviderBuilder is MeterProviderBuilderBase meterProviderBuilderBase) + { + meterProviderBuilderBase.SetMaxMetricStreams(maxMetricStreams); + } + + return meterProviderBuilder; + } + + /// + /// Sets the maximum number of MetricPoints allowed per metric stream. + /// This limits the number of unique combinations of key/value pairs used + /// for reporting measurements. + /// + /// MeterProviderBuilder instance. + /// Maximum maximum number of metric points allowed per metric stream. + /// Returns for chaining. + /// + /// 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. + /// + public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterProviderBuilder meterProviderBuilder, int maxMetricPointsPerMetricStream) + { + if (meterProviderBuilder is MeterProviderBuilderBase meterProviderBuilderBase) + { + meterProviderBuilderBase.SetMaxMetricPointsPerMetricStream(maxMetricPointsPerMetricStream); + } + + 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/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 63448e2c4..4ff0fbe83 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -41,6 +41,8 @@ namespace OpenTelemetry.Metrics IEnumerable meterSources, List instrumentationFactories, List> viewConfigs, + int maxMetricStreams, + int maxMetricPointsPerMetricStream, IEnumerable readers) { this.Resource = resource; @@ -51,6 +53,8 @@ namespace OpenTelemetry.Metrics Guard.Null(reader, nameof(reader)); reader.SetParentProvider(this); + reader.SetMaxMetricStreams(maxMetricStreams); + reader.SetMaxMetricPointsPerMetricStream(maxMetricPointsPerMetricStream); if (this.reader == null) { diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index bb524b02f..9ff13a63c 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -30,6 +30,7 @@ namespace OpenTelemetry.Metrics AggregationTemporality temporality, string metricName, string metricDescription, + int maxMetricPointsPerMetricStream, double[] histogramBounds = null, string[] tagKeysInteresting = null) { @@ -104,7 +105,7 @@ namespace OpenTelemetry.Metrics // TODO: Log and assign some invalid Enum. } - this.aggStore = new AggregatorStore(aggType, temporality, histogramBounds ?? DefaultHistogramBounds, tagKeysInteresting); + this.aggStore = new AggregatorStore(aggType, temporality, maxMetricPointsPerMetricStream, histogramBounds ?? DefaultHistogramBounds, tagKeysInteresting); this.Temporality = temporality; this.InstrumentDisposed = false; } diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index 902a37e72..7cd50392a 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -26,11 +26,12 @@ namespace OpenTelemetry.Metrics /// public abstract partial class MetricReader { - internal const int MaxMetrics = 1000; - private readonly Metric[] metrics = new Metric[MaxMetrics]; - private readonly Metric[] metricsCurrentBatch = new Metric[MaxMetrics]; private readonly HashSet metricStreamNames = new HashSet(StringComparer.OrdinalIgnoreCase); private readonly object instrumentCreationLock = new object(); + private int maxMetricStreams; + private int maxMetricPointsPerMetricStream; + private Metric[] metrics; + private Metric[] metricsCurrentBatch; private int metricIndex = -1; internal Metric AddMetricWithNoViews(Instrument instrument) @@ -47,14 +48,14 @@ namespace OpenTelemetry.Metrics } var index = ++this.metricIndex; - if (index >= MaxMetrics) + if (index >= this.maxMetricStreams) { OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricName, instrument.Meter.Name, "Maximum allowed Metrics for the provider exceeded.", "Use views to drop unused instruments. Or configure Provider to allow higher limit."); return null; } else { - var metric = new Metric(instrument, this.preferredAggregationTemporality, metricName, instrument.Description); + var metric = new Metric(instrument, this.preferredAggregationTemporality, metricName, instrument.Description, this.maxMetricPointsPerMetricStream); this.metrics[index] = metric; this.metricStreamNames.Add(metricStreamName); return metric; @@ -118,7 +119,7 @@ namespace OpenTelemetry.Metrics } var index = ++this.metricIndex; - if (index >= MaxMetrics) + if (index >= this.maxMetricStreams) { // TODO: Log that instrument is ignored // as max number of Metrics have reached. @@ -130,7 +131,7 @@ namespace OpenTelemetry.Metrics string[] tagKeysInteresting = metricStreamConfig?.TagKeys; double[] histogramBucketBounds = (metricStreamConfig is ExplicitBucketHistogramConfiguration histogramConfig && histogramConfig.Boundaries != null) ? histogramConfig.Boundaries : null; - metric = new Metric(instrument, this.preferredAggregationTemporality, metricName, metricDescription, histogramBucketBounds, tagKeysInteresting); + metric = new Metric(instrument, this.preferredAggregationTemporality, metricName, metricDescription, this.maxMetricPointsPerMetricStream, histogramBucketBounds, tagKeysInteresting); this.metrics[index] = metric; metrics.Add(metric); @@ -191,11 +192,23 @@ namespace OpenTelemetry.Metrics } } + internal void SetMaxMetricStreams(int maxMetricStreams) + { + this.maxMetricStreams = maxMetricStreams; + this.metrics = new Metric[maxMetricStreams]; + this.metricsCurrentBatch = new Metric[maxMetricStreams]; + } + + internal void SetMaxMetricPointsPerMetricStream(int maxMetricPointsPerMetricStream) + { + this.maxMetricPointsPerMetricStream = maxMetricPointsPerMetricStream; + } + private Batch GetMetricsBatch() { try { - var indexSnapshot = Math.Min(this.metricIndex, MaxMetrics - 1); + var indexSnapshot = Math.Min(this.metricIndex, this.maxMetricStreams - 1); var target = indexSnapshot + 1; int metricCountCurrentBatch = 0; for (int i = 0; i < target; i++) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 3c5ba50fa..b2717baab 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -489,26 +489,26 @@ namespace OpenTelemetry.Metrics.Tests // for no tag point! // This may be changed later. counterLong.Add(10); - for (int i = 0; i < AggregatorStore.MaxMetricPoints + 1; i++) + for (int i = 0; i < MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault + 1; i++) { counterLong.Add(10, new KeyValuePair("key", "value" + i)); } metricReader.Collect(); - Assert.Equal(AggregatorStore.MaxMetricPoints, MetricPointCount()); + Assert.Equal(MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault, MetricPointCount()); metricItems.Clear(); counterLong.Add(10); - for (int i = 0; i < AggregatorStore.MaxMetricPoints + 1; i++) + for (int i = 0; i < MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault + 1; i++) { counterLong.Add(10, new KeyValuePair("key", "value" + i)); } metricReader.Collect(); - Assert.Equal(AggregatorStore.MaxMetricPoints, MetricPointCount()); + Assert.Equal(MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault, MetricPointCount()); counterLong.Add(10); - for (int i = 0; i < AggregatorStore.MaxMetricPoints + 1; i++) + for (int i = 0; i < MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault + 1; i++) { counterLong.Add(10, new KeyValuePair("key", "value" + i)); } @@ -519,7 +519,7 @@ namespace OpenTelemetry.Metrics.Tests counterLong.Add(10, new KeyValuePair("key", "valueC")); metricItems.Clear(); metricReader.Collect(); - Assert.Equal(AggregatorStore.MaxMetricPoints, MetricPointCount()); + Assert.Equal(MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault, MetricPointCount()); } [Fact]