From f48e3b44939ed4f4e8bd7209318e3dd53b009d40 Mon Sep 17 00:00:00 2001 From: Eddy Nakamura Date: Thu, 27 Aug 2020 11:34:08 -0300 Subject: [PATCH] Adding MeterProviderBuilder (#1149) updating changelog Co-authored-by: Cijo Thomas --- examples/Console/TestPrometheusExporter.cs | 11 ++- src/OpenTelemetry/CHANGELOG.md | 7 +- src/OpenTelemetry/Metrics/MeterBuilder.cs | 67 -------------- .../Metrics/MeterProviderBuilder.cs | 90 +++++++++++++++++++ src/OpenTelemetry/Sdk.cs | 40 +-------- .../PrometheusExporterTests.cs | 12 +-- .../Metrics/CounterCleanUpTests.cs | 22 ++++- .../Metrics/MeterFactoryTests.cs | 5 +- .../Metrics/MeterProviderTests.cs | 10 ++- .../Metrics/MetricsTest.cs | 21 ++++- 10 files changed, 155 insertions(+), 130 deletions(-) delete mode 100644 src/OpenTelemetry/Metrics/MeterBuilder.cs create mode 100644 src/OpenTelemetry/Metrics/MeterProviderBuilder.cs diff --git a/examples/Console/TestPrometheusExporter.cs b/examples/Console/TestPrometheusExporter.cs index ca36e570628..26ff08b3a9a 100644 --- a/examples/Console/TestPrometheusExporter.cs +++ b/examples/Console/TestPrometheusExporter.cs @@ -58,12 +58,11 @@ internal static async Task RunAsync(int port, int pushIntervalInSecs, in // Application which decides to enable OpenTelemetry metrics // would setup a MeterProvider and make it default. // All meters from this factory will be configured with the common processing pipeline. - MeterProvider.SetDefault(Sdk.CreateMeterProvider(mb => - { - mb.SetMetricProcessor(processor); - mb.SetMetricExporter(promExporter); - mb.SetMetricPushInterval(TimeSpan.FromSeconds(pushIntervalInSecs)); - })); + MeterProvider.SetDefault(Sdk.CreateMeterProviderBuilder() + .SetProcessor(processor) + .SetExporter(promExporter) + .SetPushInterval(TimeSpan.FromSeconds(pushIntervalInSecs)) + .Build()); // The following shows how libraries would obtain a MeterProvider. // MeterProvider is the entry point, which provides Meter. diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index d816da31890..cede9b5202b 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,8 +2,6 @@ ## Unreleased -* Modified Sampler implementation to match the spec - ([#1037](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1037)) * Changed `ActivityProcessor` to implement `IDisposable` ([#975](https://github.com/open-telemetry/opentelemetry-dotnet/pull/975)) * Samplers now get the actual TraceId of the Activity to be created. @@ -23,6 +21,8 @@ [#1035](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1035)) * Changed `AddActivitySource` to `AddSource` with params support ([#1036](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1036)) +* Modified Sampler implementation to match the spec + ([#1037](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1037)) * Refactored simple export and batch export APIs ([#1078](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1078) [#1081](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1081) @@ -34,6 +34,9 @@ [#1127](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1127) [#1129](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1129) [#1135](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1135)) +* Changed `MeterProviderBuilder` and `MeterProviderSdk` design to simply the + flow and usage + ([#1149](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1149)) * Renamed `ParentOrElseSampler` to `ParentBasedSampler` ([#1173](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1173)) * Renamed `ProbabilitySampler` to `TraceIdRatioBasedSampler` diff --git a/src/OpenTelemetry/Metrics/MeterBuilder.cs b/src/OpenTelemetry/Metrics/MeterBuilder.cs deleted file mode 100644 index a3a28fd6141..00000000000 --- a/src/OpenTelemetry/Metrics/MeterBuilder.cs +++ /dev/null @@ -1,67 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System; -using OpenTelemetry.Metrics.Export; - -namespace OpenTelemetry.Metrics -{ - public class MeterBuilder - { - internal MeterBuilder() - { - } - - internal MetricProcessor MetricProcessor { get; private set; } - - internal MetricExporter MetricExporter { get; private set; } - - internal TimeSpan MetricPushInterval { get; private set; } - - /// - /// Configures metric processor. (aka batcher). - /// - /// MetricProcessor instance. - /// The meter builder instance for chaining. - public MeterBuilder SetMetricProcessor(MetricProcessor metricProcessor) - { - this.MetricProcessor = metricProcessor; - return this; - } - - /// - /// Configures Metric Exporter. - /// - /// MetricExporter instance. - /// The meter builder instance for chaining. - public MeterBuilder SetMetricExporter(MetricExporter metricExporter) - { - this.MetricExporter = metricExporter; - return this; - } - - /// - /// Sets the push interval. - /// - /// push interval. - /// The meter builder instance for chaining. - public MeterBuilder SetMetricPushInterval(TimeSpan pushInterval) - { - this.MetricPushInterval = pushInterval; - return this; - } - } -} diff --git a/src/OpenTelemetry/Metrics/MeterProviderBuilder.cs b/src/OpenTelemetry/Metrics/MeterProviderBuilder.cs new file mode 100644 index 00000000000..6fcd3a1fd24 --- /dev/null +++ b/src/OpenTelemetry/Metrics/MeterProviderBuilder.cs @@ -0,0 +1,90 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Collections.Generic; +using System.Threading; +using OpenTelemetry.Metrics.Export; +using static OpenTelemetry.Metrics.MeterProviderSdk; + +namespace OpenTelemetry.Metrics +{ + /// + /// Build MeterProvider with Exporter, Processor and PushInterval. + /// + public class MeterProviderBuilder + { + private static readonly TimeSpan DefaultPushInterval = TimeSpan.FromSeconds(60); + private MetricProcessor metricProcessor; + private MetricExporter metricExporter; + private TimeSpan metricPushInterval; + + internal MeterProviderBuilder() + { + this.metricExporter = new NoopMetricExporter(); + this.metricProcessor = new NoopMetricProcessor(); + this.metricPushInterval = DefaultPushInterval; + } + + /// + /// Sets processor. + /// + /// Processor instance. + /// Returns for chaining. + public MeterProviderBuilder SetProcessor(MetricProcessor processor) + { + this.metricProcessor = processor ?? new NoopMetricProcessor(); + return this; + } + + /// + /// Sets exporter. + /// + /// Exporter instance. + /// Returns for chaining. + public MeterProviderBuilder SetExporter(MetricExporter exporter) + { + this.metricExporter = exporter ?? new NoopMetricExporter(); + return this; + } + + /// + /// Sets push interval. + /// + /// Push interval. + /// Returns for chaining. + public MeterProviderBuilder SetPushInterval(TimeSpan pushInterval) + { + this.metricPushInterval = pushInterval == default ? DefaultPushInterval : pushInterval; + return this; + } + + public MeterProvider Build() + { + var cancellationTokenSource = new CancellationTokenSource(); + var meterRegistry = new Dictionary(); + + var controller = new PushMetricController( + meterRegistry, + this.metricProcessor, + this.metricExporter, + this.metricPushInterval, + cancellationTokenSource); + + return new MeterProviderSdk(this.metricProcessor, meterRegistry, controller, cancellationTokenSource); + } + } +} diff --git a/src/OpenTelemetry/Sdk.cs b/src/OpenTelemetry/Sdk.cs index b1f3728381c..9585f84a0d6 100644 --- a/src/OpenTelemetry/Sdk.cs +++ b/src/OpenTelemetry/Sdk.cs @@ -14,13 +14,8 @@ // limitations under the License. // -using System; -using System.Collections.Generic; -using System.Threading; using OpenTelemetry.Metrics; -using OpenTelemetry.Metrics.Export; using OpenTelemetry.Trace; -using static OpenTelemetry.Metrics.MeterProviderSdk; namespace OpenTelemetry { @@ -29,45 +24,18 @@ namespace OpenTelemetry /// public static class Sdk { - private static readonly TimeSpan DefaultPushInterval = TimeSpan.FromSeconds(60); - /// /// Gets a value indicating whether instrumentation is suppressed (disabled). /// public static bool SuppressInstrumentation => SuppressInstrumentationScope.IsSuppressed; /// - /// Creates MeterProvider with the configuration provided. - /// Configuration involves MetricProcessor, Exporter and push internval. + /// Creates MeterProviderBuilder which should be used to build MeterProvider. /// - /// Action to configure MeterBuilder. - /// MeterProvider instance, which must be disposed upon shutdown. - public static MeterProvider CreateMeterProvider(Action configure) + /// MeterProviderBuilder instance, which should be used to build MeterProvider. + public static MeterProviderBuilder CreateMeterProviderBuilder() { - if (configure == null) - { - throw new ArgumentNullException(nameof(configure)); - } - - var meterBuilder = new MeterBuilder(); - configure(meterBuilder); - - var metricProcessor = meterBuilder.MetricProcessor ?? new NoopMetricProcessor(); - var metricExporter = meterBuilder.MetricExporter ?? new NoopMetricExporter(); - var cancellationTokenSource = new CancellationTokenSource(); - var meterRegistry = new Dictionary(); - - // We only have PushMetricController now with only configurable thing being the push interval - var controller = new PushMetricController( - meterRegistry, - metricProcessor, - metricExporter, - meterBuilder.MetricPushInterval == default ? DefaultPushInterval : meterBuilder.MetricPushInterval, - cancellationTokenSource); - - var meterProviderSdk = new MeterProviderSdk(metricProcessor, meterRegistry, controller, cancellationTokenSource); - - return meterProviderSdk; + return new MeterProviderBuilder(); } /// diff --git a/test/OpenTelemetry.Exporter.Prometheus.Tests/PrometheusExporterTests.cs b/test/OpenTelemetry.Exporter.Prometheus.Tests/PrometheusExporterTests.cs index b8dcdac73ac..53ef2fb28ab 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.Tests/PrometheusExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.Tests/PrometheusExporterTests.cs @@ -113,12 +113,12 @@ public async Task E2ETestMiddleware() private static void CollectMetrics(UngroupedBatcher simpleProcessor, MetricExporter exporter) { - var meter = Sdk.CreateMeterProvider(mb => - { - mb.SetMetricProcessor(simpleProcessor); - mb.SetMetricExporter(exporter); - mb.SetMetricPushInterval(TimeSpan.FromMilliseconds(MetricPushIntervalMsec)); - }).GetMeter("library1"); + var meter = Sdk.CreateMeterProviderBuilder() + .SetProcessor(simpleProcessor) + .SetExporter(exporter) + .SetPushInterval(TimeSpan.FromMilliseconds(MetricPushIntervalMsec)) + .Build() + .GetMeter("library1"); var testCounter = meter.CreateInt64Counter("testCounter"); var testMeasure = meter.CreateInt64Measure("testMeasure"); diff --git a/test/OpenTelemetry.Tests/Metrics/CounterCleanUpTests.cs b/test/OpenTelemetry.Tests/Metrics/CounterCleanUpTests.cs index 4cc924df41c..5304e9bb3c0 100644 --- a/test/OpenTelemetry.Tests/Metrics/CounterCleanUpTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/CounterCleanUpTests.cs @@ -37,7 +37,11 @@ public CounterCleanUpTests(ITestOutputHelper output) public void LongCounterBoundInstrumentsStatusUpdatedCorrectlySingleThread() { var testProcessor = new TestMetricProcessor(); - var meter = Sdk.CreateMeterProvider(mb => mb.SetMetricProcessor(testProcessor)).GetMeter("library1") as MeterSdk; + var meter = Sdk.CreateMeterProviderBuilder() + .SetProcessor(testProcessor) + .Build() + .GetMeter("library1") as MeterSdk; + var testCounter = meter.CreateInt64Counter("testCounter") as CounterMetricSdkBase; var labels1 = new List>(); labels1.Add(new KeyValuePair("dim1", "value1")); @@ -108,7 +112,10 @@ public void LongCounterBoundInstrumentsStatusUpdatedCorrectlySingleThread() public void DoubleCounterBoundInstrumentsStatusUpdatedCorrectlySingleThread() { var testProcessor = new TestMetricProcessor(); - var meter = Sdk.CreateMeterProvider(mb => mb.SetMetricProcessor(testProcessor)).GetMeter("library1") as MeterSdk; + var meter = Sdk.CreateMeterProviderBuilder() + .SetProcessor(testProcessor) + .Build() + .GetMeter("library1") as MeterSdk; var testCounter = meter.CreateDoubleCounter("testCounter") as CounterMetricSdkBase; var labels1 = new List>(); @@ -180,7 +187,11 @@ public void DoubleCounterBoundInstrumentsStatusUpdatedCorrectlySingleThread() public void LongCounterBoundInstrumentsStatusUpdatedCorrectlyMultiThread() { var testProcessor = new TestMetricProcessor(); - var meter = Sdk.CreateMeterProvider(mb => mb.SetMetricProcessor(testProcessor)).GetMeter("library1") as MeterSdk; + var meter = Sdk.CreateMeterProviderBuilder() + .SetProcessor(testProcessor) + .Build() + .GetMeter("library1") as MeterSdk; + var testCounter = meter.CreateInt64Counter("testCounter") as CounterMetricSdkBase; var labels1 = new List>(); @@ -249,7 +260,10 @@ public void LongCounterBoundInstrumentsStatusUpdatedCorrectlyMultiThread() public void DoubleCounterBoundInstrumentsStatusUpdatedCorrectlyMultiThread() { var testProcessor = new TestMetricProcessor(); - var meter = Sdk.CreateMeterProvider(mb => mb.SetMetricProcessor(testProcessor)).GetMeter("library1") as MeterSdk; + var meter = Sdk.CreateMeterProviderBuilder() + .SetProcessor(testProcessor) + .Build() + .GetMeter("library1") as MeterSdk; var testCounter = meter.CreateDoubleCounter("testCounter") as CounterMetricSdkBase; var labels1 = new List>(); diff --git a/test/OpenTelemetry.Tests/Metrics/MeterFactoryTests.cs b/test/OpenTelemetry.Tests/Metrics/MeterFactoryTests.cs index 58a46ab18ce..d47a6514735 100644 --- a/test/OpenTelemetry.Tests/Metrics/MeterFactoryTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MeterFactoryTests.cs @@ -28,7 +28,10 @@ public class MeterFactoryTests public void DefaultMeterShouldBeCollectedAsWell() { var testProcessor = new TestMetricProcessor(); - var meterProvider = (MeterProviderSdk)Sdk.CreateMeterProvider(mb => mb.SetMetricProcessor(testProcessor)); + var meterProvider = Sdk.CreateMeterProviderBuilder() + .SetProcessor(testProcessor) + .Build() as MeterProviderSdk; + var controller = meterProvider.PushMetricController; var defaultMeter = meterProvider.GetMeter(string.Empty) as MeterSdk; diff --git a/test/OpenTelemetry.Tests/Metrics/MeterProviderTests.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderTests.cs index 92289ff3711..8d40b50733f 100644 --- a/test/OpenTelemetry.Tests/Metrics/MeterProviderTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MeterProviderTests.cs @@ -41,7 +41,7 @@ public void MeterProvider_Default() [Fact] public void MeterProvider_SetDefault() { - var meterProvider = Sdk.CreateMeterProvider(b => { }); + var meterProvider = Sdk.CreateMeterProviderBuilder().Build(); MeterProvider.SetDefault(meterProvider); var defaultMeter = MeterProvider.Default.GetMeter(string.Empty); @@ -63,8 +63,9 @@ public void MeterProvider_SetDefaultNull() [Fact] public void MeterProvider_SetDefaultTwice_Throws() { - MeterProvider.SetDefault(Sdk.CreateMeterProvider(b => { })); - Assert.Throws(() => MeterProvider.SetDefault(Sdk.CreateMeterProvider(b => { }))); + var meterProvider = Sdk.CreateMeterProviderBuilder().Build(); + MeterProvider.SetDefault(meterProvider); + Assert.Throws(() => MeterProvider.SetDefault(meterProvider)); } [Fact] @@ -74,7 +75,8 @@ public void MeterProvider_UpdateDefault_CachedTracer() var noOpCounter = defaultMeter.CreateDoubleCounter("ctr"); Assert.IsType>(noOpCounter); - MeterProvider.SetDefault(Sdk.CreateMeterProvider(b => { })); + var meterProvider = Sdk.CreateMeterProviderBuilder().Build(); + MeterProvider.SetDefault(meterProvider); var counter = defaultMeter.CreateDoubleCounter("ctr"); Assert.IsType(counter); diff --git a/test/OpenTelemetry.Tests/Metrics/MetricsTest.cs b/test/OpenTelemetry.Tests/Metrics/MetricsTest.cs index edf6c340554..2123786782a 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricsTest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricsTest.cs @@ -28,7 +28,11 @@ public class MetricsTest public void CounterSendsAggregateToRegisteredProcessor() { var testProcessor = new TestMetricProcessor(); - var meter = Sdk.CreateMeterProvider(mb => mb.SetMetricProcessor(testProcessor)).GetMeter("library1") as MeterSdk; + var meter = Sdk.CreateMeterProviderBuilder() + .SetProcessor(testProcessor) + .Build() + .GetMeter("library1") as MeterSdk; + var testCounter = meter.CreateInt64Counter("testCounter"); var labels1 = new List>(); @@ -77,7 +81,10 @@ public void CounterSendsAggregateToRegisteredProcessor() public void MeasureSendsAggregateToRegisteredProcessor() { var testProcessor = new TestMetricProcessor(); - var meter = Sdk.CreateMeterProvider(mb => mb.SetMetricProcessor(testProcessor)).GetMeter("library1") as MeterSdk; + var meter = Sdk.CreateMeterProviderBuilder() + .SetProcessor(testProcessor) + .Build() + .GetMeter("library1") as MeterSdk; var testMeasure = meter.CreateInt64Measure("testMeasure"); var labels1 = new List>(); @@ -122,7 +129,10 @@ public void MeasureSendsAggregateToRegisteredProcessor() public void LongObserverSendsAggregateToRegisteredProcessor() { var testProcessor = new TestMetricProcessor(); - var meter = Sdk.CreateMeterProvider(mb => mb.SetMetricProcessor(testProcessor)).GetMeter("library1") as MeterSdk; + var meter = Sdk.CreateMeterProviderBuilder() + .SetProcessor(testProcessor) + .Build() + .GetMeter("library1") as MeterSdk; var testObserver = meter.CreateInt64Observer("testObserver", this.TestCallbackLong); meter.Collect(); @@ -148,7 +158,10 @@ public void LongObserverSendsAggregateToRegisteredProcessor() public void DoubleObserverSendsAggregateToRegisteredProcessor() { var testProcessor = new TestMetricProcessor(); - var meter = Sdk.CreateMeterProvider(mb => mb.SetMetricProcessor(testProcessor)).GetMeter("library1") as MeterSdk; + var meter = Sdk.CreateMeterProviderBuilder() + .SetProcessor(testProcessor) + .Build() + .GetMeter("library1") as MeterSdk; var testObserver = meter.CreateDoubleObserver("testObserver", this.TestCallbackDouble); meter.Collect();