From 2da04dd8511ffce564a7ca5de4694eda270ce2b1 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 13 Dec 2022 14:59:29 -0800 Subject: [PATCH 1/5] Allow the hosting startup to throw for valid issues. --- .../HostingExtensionsEventSource.cs | 31 +------ .../Implementation/TelemetryHostedService.cs | 15 +--- .../TelemetryHostedServiceTests.cs | 84 +++++++++++++++++++ 3 files changed, 91 insertions(+), 39 deletions(-) create mode 100644 test/OpenTelemetry.Extensions.Hosting.Tests/TelemetryHostedServiceTests.cs diff --git a/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs b/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs index f23f3a86226..b4795b35989 100644 --- a/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs +++ b/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs @@ -15,7 +15,6 @@ // using System.Diagnostics.Tracing; -using OpenTelemetry.Internal; namespace OpenTelemetry.Extensions.Hosting.Implementation { @@ -27,34 +26,10 @@ internal sealed class HostingExtensionsEventSource : EventSource { public static HostingExtensionsEventSource Log = new(); - [NonEvent] - public void FailedInitialize(Exception ex) + [Event(1, Message = "OpenTelemetry TraverProvider and/or MeterProvider were not found in application services. SDK will remain disabled.", Level = EventLevel.Warning)] + public void SdkNotRegistered() { - if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) - { - this.FailedInitialize(ex.ToInvariantString()); - } - } - - [NonEvent] - public void FailedOpenTelemetrySDK(Exception ex) - { - if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) - { - this.FailedOpenTelemetrySDK(ex.ToInvariantString()); - } - } - - [Event(1, Message = "An exception occurred while initializing OpenTelemetry Tracing. OpenTelemetry tracing will remain disabled. Exception: '{0}'.", Level = EventLevel.Error)] - public void FailedInitialize(string exception) - { - this.WriteEvent(1, exception); - } - - [Event(2, Message = "An exception occurred while retrieving OpenTelemetry Tracer. OpenTelemetry tracing will remain disabled. Exception: '{0}'.", Level = EventLevel.Error)] - public void FailedOpenTelemetrySDK(string exception) - { - this.WriteEvent(2, exception); + this.WriteEvent(1); } } } diff --git a/src/OpenTelemetry.Extensions.Hosting/Implementation/TelemetryHostedService.cs b/src/OpenTelemetry.Extensions.Hosting/Implementation/TelemetryHostedService.cs index 612175c1a58..0c484efa01c 100644 --- a/src/OpenTelemetry.Extensions.Hosting/Implementation/TelemetryHostedService.cs +++ b/src/OpenTelemetry.Extensions.Hosting/Implementation/TelemetryHostedService.cs @@ -33,16 +33,9 @@ public TelemetryHostedService(IServiceProvider serviceProvider) public Task StartAsync(CancellationToken cancellationToken) { - try - { - // The sole purpose of this HostedService is to ensure all - // instrumentations, exporters, etc., are created and started. - Initialize(this.serviceProvider); - } - catch (Exception ex) - { - HostingExtensionsEventSource.Log.FailedOpenTelemetrySDK(ex); - } + // The sole purpose of this HostedService is to ensure all + // instrumentations, exporters, etc., are created and started. + Initialize(this.serviceProvider); return Task.CompletedTask; } @@ -61,7 +54,7 @@ internal static void Initialize(IServiceProvider serviceProvider) if (meterProvider == null && tracerProvider == null) { - throw new InvalidOperationException("Could not resolve either MeterProvider or TracerProvider through application ServiceProvider, OpenTelemetry SDK has not been initialized."); + HostingExtensionsEventSource.Log.SdkNotRegistered(); } } } diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/TelemetryHostedServiceTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/TelemetryHostedServiceTests.cs new file mode 100644 index 00000000000..9eb1d1d765d --- /dev/null +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/TelemetryHostedServiceTests.cs @@ -0,0 +1,84 @@ +// +// 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 Microsoft.Extensions.Hosting; +using OpenTelemetry.Trace; +using Xunit; + +namespace OpenTelemetry.Extensions.Hosting.Tests; + +public class TelemetryHostedServiceTests +{ + [Fact] + public async Task StartWithoutProvidersDoesNotThrow() + { + var builder = new HostBuilder().ConfigureServices(services => + { + services.AddOpenTelemetry() + .StartWithHost(); + }); + + var host = builder.Build(); + + await host.StartAsync().ConfigureAwait(false); + + await host.StopAsync().ConfigureAwait(false); + } + + [Fact] + public async Task StartWithExceptionsThrows() + { + bool expectedInnerExceptionThrown = false; + + var builder = new HostBuilder().ConfigureServices(services => + { + services.AddOpenTelemetry() + .WithTracing(builder => + { + builder.ConfigureBuilder((sp, sdkBuilder) => + { + try + { + // Note: This throws because services cannot be + // registered after IServiceProvider has been + // created. + sdkBuilder.SetSampler(); + } + catch (NotSupportedException) + { + expectedInnerExceptionThrown = true; + throw; + } + }); + }) + .StartWithHost(); + }); + + var host = builder.Build(); + + await Assert.ThrowsAsync(() => host.StartAsync()).ConfigureAwait(false); + + await host.StopAsync().ConfigureAwait(false); + + Assert.True(expectedInnerExceptionThrown); + } + + private sealed class MySampler : Sampler + { + public override SamplingResult ShouldSample(in SamplingParameters samplingParameters) + => new SamplingResult(SamplingDecision.RecordAndSample); + } +} From 5e701ae596ce178f30c7326750d87cf5271fbdfc Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 13 Dec 2022 15:02:44 -0800 Subject: [PATCH 2/5] CHANGELOG stub. --- src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md b/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md index d2f5477ca0d..3c3c5910396 100644 --- a/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md +++ b/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* If the OpenTelemetry SDK cannot start it will now throw exceptions and prevent + the host from starting. + ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ## 1.4.0-rc.1 Released 2022-Dec-12 From 63f0a83dbfd83a3b7b6909826a9112a3c37f17e1 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 13 Dec 2022 15:58:22 -0800 Subject: [PATCH 3/5] CHANGELOG patch. --- src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md b/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md index 3c3c5910396..8cd9562ab8e 100644 --- a/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md +++ b/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md @@ -4,7 +4,7 @@ * If the OpenTelemetry SDK cannot start it will now throw exceptions and prevent the host from starting. - ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ([#4006](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4006)) ## 1.4.0-rc.1 From da4019344c9777c93b8ed73cc36c5a291c4b7173 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 14 Dec 2022 13:00:46 -0800 Subject: [PATCH 4/5] Warning cleanup. --- .../TelemetryHostedServiceTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/TelemetryHostedServiceTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/TelemetryHostedServiceTests.cs index 9eb1d1d765d..f6b0c6c8ec0 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/TelemetryHostedServiceTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/TelemetryHostedServiceTests.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); From 6c1b07525c2d39f6f6a4233345e1d5bf19786f79 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 14 Dec 2022 13:03:27 -0800 Subject: [PATCH 5/5] Code review. --- .../Implementation/HostingExtensionsEventSource.cs | 10 ++++++++-- .../Implementation/TelemetryHostedService.cs | 10 +++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs b/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs index b4795b35989..f6f86038cfe 100644 --- a/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs +++ b/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs @@ -26,10 +26,16 @@ internal sealed class HostingExtensionsEventSource : EventSource { public static HostingExtensionsEventSource Log = new(); - [Event(1, Message = "OpenTelemetry TraverProvider and/or MeterProvider were not found in application services. SDK will remain disabled.", Level = EventLevel.Warning)] - public void SdkNotRegistered() + [Event(1, Message = "OpenTelemetry TracerProvider was not found in application services. Tracing will remain disabled.", Level = EventLevel.Warning)] + public void TracerProviderNotRegistered() { this.WriteEvent(1); } + + [Event(2, Message = "OpenTelemetry MeterProvider was not found in application services. Metrics will remain disabled.", Level = EventLevel.Warning)] + public void MeterProviderNotRegistered() + { + this.WriteEvent(2); + } } } diff --git a/src/OpenTelemetry.Extensions.Hosting/Implementation/TelemetryHostedService.cs b/src/OpenTelemetry.Extensions.Hosting/Implementation/TelemetryHostedService.cs index 0c484efa01c..78e50395b59 100644 --- a/src/OpenTelemetry.Extensions.Hosting/Implementation/TelemetryHostedService.cs +++ b/src/OpenTelemetry.Extensions.Hosting/Implementation/TelemetryHostedService.cs @@ -50,11 +50,15 @@ internal static void Initialize(IServiceProvider serviceProvider) Debug.Assert(serviceProvider != null, "serviceProvider was null"); var meterProvider = serviceProvider.GetService(); - var tracerProvider = serviceProvider.GetService(); + if (meterProvider == null) + { + HostingExtensionsEventSource.Log.MeterProviderNotRegistered(); + } - if (meterProvider == null && tracerProvider == null) + var tracerProvider = serviceProvider.GetService(); + if (tracerProvider == null) { - HostingExtensionsEventSource.Log.SdkNotRegistered(); + HostingExtensionsEventSource.Log.TracerProviderNotRegistered(); } } }