From c7ed28aa7ecd1e36f4bb9c2a1425d93bd19088c7 Mon Sep 17 00:00:00 2001 From: Steve Gordon Date: Tue, 13 Feb 2024 14:59:34 +0000 Subject: [PATCH] Refactor diagnostics for simplicity (#36) --- .../Program.cs | 2 +- src/Elastic.OpenTelemetry/Agent.cs | 2 +- src/Elastic.OpenTelemetry/AgentBuilder.cs | 29 ++++++++++--------- .../ServiceCollectionExtensions.cs | 6 ++-- .../ElasticDiagnosticLoggingObserver.cs | 18 ++++++------ ....cs => ElasticOpenTelemetryDiagnostics.cs} | 9 ++++-- .../Diagnostics/LogFileWriter.cs | 1 - .../LoggingDiagnosticSourceListener.cs | 26 ----------------- .../TraceBuilderProviderExtensions.cs | 2 +- .../Processors/TransactionIdProcessor.cs | 2 +- 10 files changed, 38 insertions(+), 59 deletions(-) rename src/Elastic.OpenTelemetry/Diagnostics/{ElasticOpenTelemetryDiagnosticSource.cs => ElasticOpenTelemetryDiagnostics.cs} (92%) delete mode 100644 src/Elastic.OpenTelemetry/Diagnostics/LoggingDiagnosticSourceListener.cs diff --git a/examples/Example.Elastic.OpenTelemetry.Worker/Program.cs b/examples/Example.Elastic.OpenTelemetry.Worker/Program.cs index a9afd6a..ca33f58 100644 --- a/examples/Example.Elastic.OpenTelemetry.Worker/Program.cs +++ b/examples/Example.Elastic.OpenTelemetry.Worker/Program.cs @@ -5,7 +5,7 @@ var builder = Host.CreateApplicationBuilder(args); -builder.EnableElasticOpenTelemetry("CustomActivitySource"); +builder.AddElasticOpenTelemetry("CustomActivitySource"); builder.Services.AddHostedService(); diff --git a/src/Elastic.OpenTelemetry/Agent.cs b/src/Elastic.OpenTelemetry/Agent.cs index 9e22d7f..6681a25 100644 --- a/src/Elastic.OpenTelemetry/Agent.cs +++ b/src/Elastic.OpenTelemetry/Agent.cs @@ -4,7 +4,7 @@ using System.Reflection; using Microsoft.Extensions.Logging; -using static Elastic.OpenTelemetry.Diagnostics.ElasticOpenTelemetryDiagnosticSource; +using static Elastic.OpenTelemetry.Diagnostics.ElasticOpenTelemetryDiagnostics; namespace Elastic.OpenTelemetry; diff --git a/src/Elastic.OpenTelemetry/AgentBuilder.cs b/src/Elastic.OpenTelemetry/AgentBuilder.cs index 9c52517..c418065 100644 --- a/src/Elastic.OpenTelemetry/AgentBuilder.cs +++ b/src/Elastic.OpenTelemetry/AgentBuilder.cs @@ -14,7 +14,7 @@ using OpenTelemetry.Resources; using OpenTelemetry.Trace; -using static Elastic.OpenTelemetry.Diagnostics.ElasticOpenTelemetryDiagnosticSource; +using static Elastic.OpenTelemetry.Diagnostics.ElasticOpenTelemetryDiagnostics; namespace Elastic.OpenTelemetry; @@ -43,6 +43,7 @@ public class AgentBuilder private Action? _resourceBuilderAction; private Action? _otlpExporterConfiguration; private string? _otlpExporterName; + private readonly IDisposable? _diagnosticSourceSubscription; /// /// TODO @@ -55,8 +56,7 @@ public AgentBuilder() _ = new LoggingEventListener(LogFileWriter.Instance); // Enables logging of Elastic OpenTelemetry diagnostic source events - var listener = new LoggingDiagnosticSourceListener(LogFileWriter.Instance); - DiagnosticListener.AllListeners.Subscribe(listener); + _diagnosticSourceSubscription = EnableFileLogging(); } Log(AgentBuilderInitializedEvent, () => new DiagnosticEvent(new StackTrace(true))); @@ -193,7 +193,7 @@ public IAgent Build() Log(AgentBuilderBuiltTracerProviderEvent); - var agent = tracerProvider is not null ? new Agent(tracerProvider) : new Agent(); + var agent = tracerProvider is not null ? new Agent(_diagnosticSourceSubscription, tracerProvider) : new Agent(_diagnosticSourceSubscription); Log(AgentBuilderBuiltAgentEvent); @@ -212,7 +212,9 @@ public IServiceCollection Register(IServiceCollection serviceCollection) _ = serviceCollection .AddHostedService() - .AddSingleton() + // This is purely to register an instance of the agent such that should the service provider be disposed, the agent + // will also be disposed which in turn avoids further diagnostics subscriptions and file logging. + .AddSingleton(new Agent(_diagnosticSourceSubscription)) .AddSingleton() .AddOpenTelemetry() .WithTracing(TracerProviderBuilderAction); @@ -233,11 +235,6 @@ public IServiceCollection Register(IServiceCollection serviceCollection) .AddGrpcClientInstrumentation() .AddEntityFrameworkCoreInstrumentation(); // TODO - Should we add this by default? - // TODO - Update these to capture the builder type also - //Log.AddedInstrumentation("HttpClient"); - //Log.AddedInstrumentation("GrpcClient"); - //Log.AddedInstrumentation("EntityFrameworkCore"); - tracerProviderBuilder.AddElasticProcessors(); if (_resourceBuilderAction is not null) @@ -251,7 +248,6 @@ public IServiceCollection Register(IServiceCollection serviceCollection) tracerProviderBuilder.ConfigureResource(DefaultResourceBuilderConfiguration); } - // TODO - Can/should we use reflection to determine and log what is configured by the user action? _tracerProviderBuilderAction?.Invoke(tracerProviderBuilder); tracerProviderBuilder.AddElasticOtlpExporter(_otlpExporterConfiguration, _otlpExporterName); @@ -267,16 +263,19 @@ public void ConfigureOtlpExporter(Action configure, string? _otlpExporterName = name; } - private class Agent(TracerProvider? tracerProvider, MeterProvider? meterProvider) : IAgent + private class Agent(IDisposable? diagnosticSubscription, TracerProvider? tracerProvider, MeterProvider? meterProvider) : IAgent { + private readonly IDisposable? _diagnosticSubscription = diagnosticSubscription; private readonly TracerProvider? _tracerProvider = tracerProvider; private readonly MeterProvider? _meterProvider = meterProvider; - public Agent() : this(null, null) + public Agent(IDisposable? diagnosticSubscription) + : this(diagnosticSubscription,null, null) { } - internal Agent(TracerProvider tracerProvider) : this(tracerProvider, null) + internal Agent(IDisposable? diagnosticSubscription, TracerProvider tracerProvider) + : this(diagnosticSubscription, tracerProvider, null) { } @@ -284,6 +283,7 @@ public void Dispose() { _tracerProvider?.Dispose(); _meterProvider?.Dispose(); + _diagnosticSubscription?.Dispose(); LogFileWriter.Instance.Dispose(); } @@ -291,6 +291,7 @@ public async ValueTask DisposeAsync() { _tracerProvider?.Dispose(); _meterProvider?.Dispose(); + _diagnosticSubscription?.Dispose(); await LogFileWriter.Instance.DisposeAsync().ConfigureAwait(false); } } diff --git a/src/Elastic.OpenTelemetry/DependencyInjection/ServiceCollectionExtensions.cs b/src/Elastic.OpenTelemetry/DependencyInjection/ServiceCollectionExtensions.cs index 28153ee..7ee3ee6 100644 --- a/src/Elastic.OpenTelemetry/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Elastic.OpenTelemetry/DependencyInjection/ServiceCollectionExtensions.cs @@ -17,8 +17,8 @@ public static class ServiceCollectionExtensions /// /// /// - public static IHostApplicationBuilder EnableElasticOpenTelemetry(this IHostApplicationBuilder builder) => - EnableElasticOpenTelemetry(builder, []); + public static IHostApplicationBuilder AddElasticOpenTelemetry(this IHostApplicationBuilder builder) => + AddElasticOpenTelemetry(builder, []); /// /// TODO @@ -26,7 +26,7 @@ public static IHostApplicationBuilder EnableElasticOpenTelemetry(this IHostAppli /// /// /// - public static IHostApplicationBuilder EnableElasticOpenTelemetry(this IHostApplicationBuilder builder, params string[] activitySourceNames) + public static IHostApplicationBuilder AddElasticOpenTelemetry(this IHostApplicationBuilder builder, params string[] activitySourceNames) { builder.Services.AddElasticOpenTelemetry(activitySourceNames); return builder; diff --git a/src/Elastic.OpenTelemetry/Diagnostics/ElasticDiagnosticLoggingObserver.cs b/src/Elastic.OpenTelemetry/Diagnostics/ElasticDiagnosticLoggingObserver.cs index e80ae91..06c372d 100644 --- a/src/Elastic.OpenTelemetry/Diagnostics/ElasticDiagnosticLoggingObserver.cs +++ b/src/Elastic.OpenTelemetry/Diagnostics/ElasticDiagnosticLoggingObserver.cs @@ -17,39 +17,39 @@ public void OnNext(KeyValuePair data) switch (data.Key) { - case ElasticOpenTelemetryDiagnosticSource.AgentBuilderInitializedEvent: + case ElasticOpenTelemetryDiagnostics.AgentBuilderInitializedEvent: AgentBuilderInitialized(data); break; - case ElasticOpenTelemetryDiagnosticSource.AgentBuilderBuiltTracerProviderEvent: + case ElasticOpenTelemetryDiagnostics.AgentBuilderBuiltTracerProviderEvent: AgentBuilderBuiltTracerProvider(data); break; - case ElasticOpenTelemetryDiagnosticSource.AgentBuilderBuiltAgentEvent: + case ElasticOpenTelemetryDiagnostics.AgentBuilderBuiltAgentEvent: AgentBuilderBuiltAgent(data); break; - case ElasticOpenTelemetryDiagnosticSource.AgentBuildCalledMultipleTimesEvent: + case ElasticOpenTelemetryDiagnostics.AgentBuildCalledMultipleTimesEvent: AgentBuilderBuildCalledMultipleTimes(data); break; - case ElasticOpenTelemetryDiagnosticSource.AgentSetAgentCalledMultipleTimesEvent: + case ElasticOpenTelemetryDiagnostics.AgentSetAgentCalledMultipleTimesEvent: AgentBuilderSetAgentCalledMultipleTimes(data); break; - case ElasticOpenTelemetryDiagnosticSource.AgentBuilderRegisteredDistroServicesEvent: + case ElasticOpenTelemetryDiagnostics.AgentBuilderRegisteredDistroServicesEvent: AgentBuilderRegisteredDistroServices(data); break; - case ElasticOpenTelemetryDiagnosticSource.TransactionIdAddedEvent: + case ElasticOpenTelemetryDiagnostics.TransactionIdAddedEvent: TransactionIdAdded(data); break; - case ElasticOpenTelemetryDiagnosticSource.ProcessorAddedEvent: + case ElasticOpenTelemetryDiagnostics.ProcessorAddedEvent: ProcessorAdded(data); break; - case ElasticOpenTelemetryDiagnosticSource.SourceAddedEvent: + case ElasticOpenTelemetryDiagnostics.SourceAddedEvent: SourceAdded(data); break; diff --git a/src/Elastic.OpenTelemetry/Diagnostics/ElasticOpenTelemetryDiagnosticSource.cs b/src/Elastic.OpenTelemetry/Diagnostics/ElasticOpenTelemetryDiagnostics.cs similarity index 92% rename from src/Elastic.OpenTelemetry/Diagnostics/ElasticOpenTelemetryDiagnosticSource.cs rename to src/Elastic.OpenTelemetry/Diagnostics/ElasticOpenTelemetryDiagnostics.cs index b609b13..d794784 100644 --- a/src/Elastic.OpenTelemetry/Diagnostics/ElasticOpenTelemetryDiagnosticSource.cs +++ b/src/Elastic.OpenTelemetry/Diagnostics/ElasticOpenTelemetryDiagnostics.cs @@ -8,11 +8,16 @@ namespace Elastic.OpenTelemetry.Diagnostics; -internal static class ElasticOpenTelemetryDiagnosticSource +internal static class ElasticOpenTelemetryDiagnostics { + private static readonly DiagnosticListener Listener = new(DiagnosticSourceName); + public const string DiagnosticSourceName = "Elastic.OpenTelemetry"; - internal static readonly DiagnosticSource DiagnosticSource = new DiagnosticListener(DiagnosticSourceName); + internal static readonly DiagnosticSource DiagnosticSource = Listener; + + public static IDisposable EnableFileLogging() => + Listener.Subscribe(new ElasticDiagnosticLoggingObserver(LogFileWriter.Instance)); public static void Log(string name) { diff --git a/src/Elastic.OpenTelemetry/Diagnostics/LogFileWriter.cs b/src/Elastic.OpenTelemetry/Diagnostics/LogFileWriter.cs index 4e4cc66..96bc153 100644 --- a/src/Elastic.OpenTelemetry/Diagnostics/LogFileWriter.cs +++ b/src/Elastic.OpenTelemetry/Diagnostics/LogFileWriter.cs @@ -5,7 +5,6 @@ using System.Runtime.InteropServices; using System.Text; using System.Threading.Channels; -using System.Xml.Serialization; namespace Elastic.OpenTelemetry.Diagnostics; diff --git a/src/Elastic.OpenTelemetry/Diagnostics/LoggingDiagnosticSourceListener.cs b/src/Elastic.OpenTelemetry/Diagnostics/LoggingDiagnosticSourceListener.cs deleted file mode 100644 index 0c4f7bc..0000000 --- a/src/Elastic.OpenTelemetry/Diagnostics/LoggingDiagnosticSourceListener.cs +++ /dev/null @@ -1,26 +0,0 @@ -// Licensed to Elasticsearch B.V under one or more agreements. -// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. -// See the LICENSE file in the project root for more information -using System.Diagnostics; - -namespace Elastic.OpenTelemetry.Diagnostics; - -internal sealed class LoggingDiagnosticSourceListener(LogFileWriter logFileWriter) : IObserver -{ - private readonly object _lock = new(); - private readonly LogFileWriter _logFileWriter = logFileWriter; - - public void OnNext(DiagnosticListener listener) - { - if (listener.Name == ElasticOpenTelemetryDiagnosticSource.DiagnosticSourceName) - { - lock (_lock) - listener.Subscribe(new ElasticDiagnosticLoggingObserver(_logFileWriter)); - } - } - - public void OnCompleted() { } - - public void OnError(Exception error) { } -} - diff --git a/src/Elastic.OpenTelemetry/Extensions/TraceBuilderProviderExtensions.cs b/src/Elastic.OpenTelemetry/Extensions/TraceBuilderProviderExtensions.cs index 0a208ee..50352af 100644 --- a/src/Elastic.OpenTelemetry/Extensions/TraceBuilderProviderExtensions.cs +++ b/src/Elastic.OpenTelemetry/Extensions/TraceBuilderProviderExtensions.cs @@ -8,7 +8,7 @@ using System.Diagnostics; using Elastic.OpenTelemetry.Diagnostics; -using static Elastic.OpenTelemetry.Diagnostics.ElasticOpenTelemetryDiagnosticSource; +using static Elastic.OpenTelemetry.Diagnostics.ElasticOpenTelemetryDiagnostics; namespace Elastic.OpenTelemetry.Extensions; diff --git a/src/Elastic.OpenTelemetry/Processors/TransactionIdProcessor.cs b/src/Elastic.OpenTelemetry/Processors/TransactionIdProcessor.cs index d64253d..448a820 100644 --- a/src/Elastic.OpenTelemetry/Processors/TransactionIdProcessor.cs +++ b/src/Elastic.OpenTelemetry/Processors/TransactionIdProcessor.cs @@ -6,7 +6,7 @@ using Microsoft.Extensions.Logging; using OpenTelemetry; -using static Elastic.OpenTelemetry.Diagnostics.ElasticOpenTelemetryDiagnosticSource; +using static Elastic.OpenTelemetry.Diagnostics.ElasticOpenTelemetryDiagnostics; namespace Elastic.OpenTelemetry.Processors;