From 1d8bd3f7098ac3e497c4cbba84bce33b082ebe48 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Wed, 6 Mar 2024 10:06:46 +0100 Subject: [PATCH] Introduce AgentBuilderOptions (#50) * Introduce AgentBuilderOptions Sadly we can not defer OltpExport to AgentBuilder.Build() in *all* IHostingEnvironments. Atleast under WebApplicationHosts the IHostedLifecycleSerivce.StartingAsync happens on app.Run() at which stage the IServiceCollection is marked as readonly. To that end we now expose overloads to AddOpenTelemetry taking AgentBuilderOptions. These are typically only needed for expert level control and not part of the documented getting started approach. This PR also addresses all warnings in the solution (branch started off for just that sorry!) * remove recursive SkipOtlpRegistration extension method * fix double logger initialization in logger tests --- .editorconfig | 1 + .../Controllers/E2EController.cs | 4 +- .../Controllers/HomeController.cs | 10 +- .../Program.cs | 8 -- src/Elastic.OpenTelemetry/Agent.cs | 2 +- .../AgentBuilder.Build.cs | 32 ------ src/Elastic.OpenTelemetry/AgentBuilder.cs | 97 +++++++++++++------ .../ElasticOtelDistroService.cs | 38 -------- .../OpenTelemetryServicesExtensions.cs | 23 +++-- .../ServiceCollectionExtensions.cs | 15 ++- .../Diagnostics/Logging/FileLogger.cs | 2 +- .../Diagnostics/Logging/LogFormatter.cs | 3 - .../Diagnostics/LoggingEventListener.cs | 17 ++-- .../Diagnostics/StringBuilderCache.cs | 2 - .../Elastic.OpenTelemetry.csproj | 4 + .../Extensions/ActivityExtensions.cs | 2 - .../Hosting/ElasticOtelDistroService.cs | 38 ++++++++ src/Elastic.OpenTelemetry/IAgent.cs | 4 +- .../IOpenTelemetryBuilder.cs | 8 +- .../Processors/SpanCompressionProcessor.cs | 51 +++++----- .../Resources/DefaultServiceDetector.cs | 2 - .../DistributedFixture/ITrafficSimulator.cs | 2 +- .../LoggingTests.cs | 10 +- .../ServiceCollectionTests.cs | 13 +-- .../TransactionIdProcessorTests.cs | 13 +-- 25 files changed, 193 insertions(+), 208 deletions(-) delete mode 100644 src/Elastic.OpenTelemetry/DependencyInjection/ElasticOtelDistroService.cs create mode 100644 src/Elastic.OpenTelemetry/Hosting/ElasticOtelDistroService.cs diff --git a/.editorconfig b/.editorconfig index fabbed8..795584d 100644 --- a/.editorconfig +++ b/.editorconfig @@ -191,6 +191,7 @@ resharper_csharp_accessor_owner_body=expression_body resharper_redundant_case_label_highlighting=do_not_show resharper_redundant_argument_default_value_highlighting=do_not_show +resharper_explicit_caller_info_argument_highlighting=hint dotnet_diagnostic.IDE0057.severity = none diff --git a/examples/Example.Elastic.OpenTelemetry.AspNetCore/Controllers/E2EController.cs b/examples/Example.Elastic.OpenTelemetry.AspNetCore/Controllers/E2EController.cs index 07f5bb9..d088529 100644 --- a/examples/Example.Elastic.OpenTelemetry.AspNetCore/Controllers/E2EController.cs +++ b/examples/Example.Elastic.OpenTelemetry.AspNetCore/Controllers/E2EController.cs @@ -19,7 +19,7 @@ public async Task Index() await Task.Delay(100); - using var childActivity = activity?.Source.StartActivity("childActivity", ActivityKind.Internal); + using var childActivity = activity?.Source.StartActivity(ActivityKind.Internal); await Task.Delay(200); return View(); @@ -34,7 +34,7 @@ public async Task Fail() await Task.Delay(100); - using var childActivity = activity?.Source.StartActivity("childActivity", ActivityKind.Internal); + using var childActivity = activity?.Source.StartActivity(ActivityKind.Internal); await Task.Delay(200); throw new Exception("Random failure"); diff --git a/examples/Example.Elastic.OpenTelemetry.AspNetCore/Controllers/HomeController.cs b/examples/Example.Elastic.OpenTelemetry.AspNetCore/Controllers/HomeController.cs index fe6fe22..6f1a26a 100644 --- a/examples/Example.Elastic.OpenTelemetry.AspNetCore/Controllers/HomeController.cs +++ b/examples/Example.Elastic.OpenTelemetry.AspNetCore/Controllers/HomeController.cs @@ -2,8 +2,8 @@ // 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; +using System.Net; using Example.Elastic.OpenTelemetry.AspNetCore.Models; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc; namespace Example.Elastic.OpenTelemetry.AspNetCore.Controllers; @@ -17,8 +17,7 @@ public async Task Index() { using var client = httpClientFactory.CreateClient(); - var activityFeature = HttpContext.Features.Get(); - + // ReSharper disable once ExplicitCallerInfoArgument using var activity = ActivitySource.StartActivity("DoingStuff", ActivityKind.Internal); activity?.SetTag("CustomTag", "TagValue"); @@ -26,10 +25,7 @@ public async Task Index() var response = await client.GetAsync("http://elastic.co"); await Task.Delay(50); - if (response.StatusCode == System.Net.HttpStatusCode.OK) - activity?.SetStatus(ActivityStatusCode.Ok); - else - activity?.SetStatus(ActivityStatusCode.Error); + activity?.SetStatus(response.StatusCode == HttpStatusCode.OK ? ActivityStatusCode.Ok : ActivityStatusCode.Error); return View(); } diff --git a/examples/Example.Elastic.OpenTelemetry.Worker/Program.cs b/examples/Example.Elastic.OpenTelemetry.Worker/Program.cs index 0d102f2..e69dcb2 100644 --- a/examples/Example.Elastic.OpenTelemetry.Worker/Program.cs +++ b/examples/Example.Elastic.OpenTelemetry.Worker/Program.cs @@ -2,7 +2,6 @@ // 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 Elastic.OpenTelemetry; using Example.Elastic.OpenTelemetry.Worker; using OpenTelemetry; using OpenTelemetry.Metrics; @@ -11,13 +10,6 @@ var builder = Host.CreateApplicationBuilder(args); -/* - * appBuilder.Services.AddOpenTelemetry() - .ConfigureResource(builder => builder.AddService(serviceName: "MyService")) - .WithTracing(builder => builder.AddConsoleExporter()) - .WithMetrics(builder => builder.AddConsoleExporter()); - */ - builder.Services.AddElasticOpenTelemetry(Worker.ActivitySourceName, "CustomMeter") .ConfigureResource(r => r.AddService(serviceName: "MyService")) .WithTracing(t => t.AddConsoleExporter()) diff --git a/src/Elastic.OpenTelemetry/Agent.cs b/src/Elastic.OpenTelemetry/Agent.cs index 74af972..ceaaf52 100644 --- a/src/Elastic.OpenTelemetry/Agent.cs +++ b/src/Elastic.OpenTelemetry/Agent.cs @@ -30,7 +30,7 @@ private static string ParseAssemblyInformationalVersion(string? informationalVer * The following parts are optional: pre-release label, pre-release version, git height, Git SHA of current commit */ - var indexOfPlusSign = informationalVersion!.IndexOf('+'); + var indexOfPlusSign = informationalVersion.IndexOf('+'); return indexOfPlusSign > 0 ? informationalVersion[..indexOfPlusSign] : informationalVersion; diff --git a/src/Elastic.OpenTelemetry/AgentBuilder.Build.cs b/src/Elastic.OpenTelemetry/AgentBuilder.Build.cs index 6a7dcb9..aef13c8 100644 --- a/src/Elastic.OpenTelemetry/AgentBuilder.Build.cs +++ b/src/Elastic.OpenTelemetry/AgentBuilder.Build.cs @@ -7,7 +7,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using OpenTelemetry; -using OpenTelemetry.Exporter; using OpenTelemetry.Metrics; using OpenTelemetry.Trace; @@ -16,14 +15,6 @@ namespace Elastic.OpenTelemetry; /// TODO public static class OpenTelemetryBuilderExtensions { - /// TODO - public static IOpenTelemetryBuilder SkipOtlpExporter(this IOpenTelemetryBuilder builder) - { - if (builder is not AgentBuilder agentBuilder) return builder; - - return agentBuilder.SkipOtlpExporter(); - } - /// TODO public static IOpenTelemetryBuilder WithLogger(this IOpenTelemetryBuilder builder, ILogger logger) { @@ -50,29 +41,6 @@ public static IAgent Build(this IOpenTelemetryBuilder builder, ILogger? logger = log.SetAdditionalLogger(logger); - var openTelemetry = - Microsoft.Extensions.DependencyInjection.OpenTelemetryServicesExtensions.AddOpenTelemetry(agentBuilder.Services); - - openTelemetry - .WithTracing(tracing => - { - if (!agentBuilder.SkipOtlpRegistration) - tracing.AddOtlpExporter(agentBuilder.OtlpExporterName, agentBuilder.OtlpExporterConfiguration); - log.LogAgentBuilderBuiltTracerProvider(); - }) - .WithMetrics(metrics => - { - if (!agentBuilder.SkipOtlpRegistration) - { - metrics.AddOtlpExporter(agentBuilder.OtlpExporterName, o => - { - o.ExportProcessorType = ExportProcessorType.Simple; - o.Protocol = OtlpExportProtocol.HttpProtobuf; - }); - } - log.LogAgentBuilderBuiltMeterProvider(); - }); - var sp = serviceProvider ?? agentBuilder.Services.BuildServiceProvider(); var tracerProvider = sp.GetService()!; var meterProvider = sp.GetService()!; diff --git a/src/Elastic.OpenTelemetry/AgentBuilder.cs b/src/Elastic.OpenTelemetry/AgentBuilder.cs index 90a5da0..4bfec04 100644 --- a/src/Elastic.OpenTelemetry/AgentBuilder.cs +++ b/src/Elastic.OpenTelemetry/AgentBuilder.cs @@ -6,26 +6,58 @@ using Elastic.OpenTelemetry.Diagnostics; using Elastic.OpenTelemetry.Diagnostics.Logging; using Elastic.OpenTelemetry.Extensions; +using Elastic.OpenTelemetry.Hosting; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using OpenTelemetry; using OpenTelemetry.Exporter; using OpenTelemetry.Metrics; -using OpenTelemetry.Resources; using OpenTelemetry.Trace; namespace Elastic.OpenTelemetry; + +/// +/// Expert options to provide to to control its initial OpenTelemetry registration +/// +public record AgentBuilderOptions +{ + /// + /// Provide an additional logger to the internal file logger. + /// + /// The agent will always log to file if a Path is provided using the ELASTIC_OTEL_LOG_DIRECTORY + /// environment variable. + /// + public ILogger? Logger { get; init; } + + /// + /// Provides an to register the agent into. + /// If null a new local instance will be used. + /// + public IServiceCollection? Services { get; init; } + + /// + /// The initial activity sources to listen to. + /// >These can always later be amended with + /// + public string[] ActivitySources { get; init; } = []; + + /// + /// Stops to register OLTP exporters, useful for testing scenarios + /// + public bool SkipOtlpExporter { get; init; } + + /// + /// Optional name which is used when retrieving OTLP options. + /// + public string? OtlpExporterName { get; init; } +} + /// /// Supports building instances which include Elastic defaults, but can also be customised. /// public class AgentBuilder : IOpenTelemetryBuilder { - private bool _skipOtlpRegistration; - - internal Action? OtlpExporterConfiguration { get; private set; } - internal string? OtlpExporterName { get; private set; } - internal bool SkipOtlpRegistration => _skipOtlpRegistration; internal AgentCompositeLogger Logger { get; } internal LoggingEventListener EventListener { get; } @@ -33,21 +65,24 @@ public class AgentBuilder : IOpenTelemetryBuilder public IServiceCollection Services { get; } /// TODO - public AgentBuilder(params string[] activitySourceNames) : this(null, null, activitySourceNames) { } + public AgentBuilder(params string[] activitySourceNames) : this(new AgentBuilderOptions + { + ActivitySources = activitySourceNames + }) { } /// TODO - public AgentBuilder(ILogger? logger = null, IServiceCollection? services = null, params string[] activitySourceNames) + public AgentBuilder(AgentBuilderOptions options) { - Logger = new AgentCompositeLogger(logger); + Logger = new AgentCompositeLogger(options.Logger); // Enables logging of OpenTelemetry-SDK event source events EventListener = new LoggingEventListener(Logger); Logger.LogAgentPreamble(); Logger.LogAgentBuilderInitialized(Environment.NewLine, new StackTrace(true)); - Services = services ?? new ServiceCollection(); + Services = options.Services ?? new ServiceCollection(); - if (services != null) + if (options.Services != null) Services.AddHostedService(); Services.AddSingleton(this); @@ -60,7 +95,7 @@ public AgentBuilder(ILogger? logger = null, IServiceCollection? services = null, { tracing.ConfigureResource(r => r.AddDistroAttributes()); - foreach (var source in activitySourceNames) + foreach (var source in options.ActivitySources) tracing.LogAndAddSource(source, Logger); tracing @@ -74,7 +109,7 @@ public AgentBuilder(ILogger? logger = null, IServiceCollection? services = null, { metrics.ConfigureResource(r => r.AddDistroAttributes()); - foreach (var source in activitySourceNames) + foreach (var source in options.ActivitySources) { Logger.LogMeterAdded(source, metrics.GetType().Name); metrics.AddMeter(source); @@ -86,24 +121,28 @@ public AgentBuilder(ILogger? logger = null, IServiceCollection? services = null, .AddRuntimeInstrumentation() .AddHttpClientInstrumentation(); }); - Logger.LogAgentBuilderRegisteredServices(); - } - - /// TODO - public AgentBuilder SkipOtlpExporter() - { - _skipOtlpRegistration = true; - return this; - } + openTelemetry + .WithTracing(tracing => + { + if (!options.SkipOtlpExporter) + tracing.AddOtlpExporter(options.OtlpExporterName, _ => { }); + Logger.LogAgentBuilderBuiltTracerProvider(); + }) + .WithMetrics(metrics => + { + if (!options.SkipOtlpExporter) + { + metrics.AddOtlpExporter(options.OtlpExporterName, o => + { + o.ExportProcessorType = ExportProcessorType.Simple; + o.Protocol = OtlpExportProtocol.HttpProtobuf; + }); + } + Logger.LogAgentBuilderBuiltMeterProvider(); + }); - /// - /// TODO - /// - public void ConfigureOtlpExporter(Action configure, string? name = null) - { - OtlpExporterConfiguration = configure ?? throw new ArgumentNullException(nameof(configure)); - OtlpExporterName = name; + Logger.LogAgentBuilderRegisteredServices(); } } diff --git a/src/Elastic.OpenTelemetry/DependencyInjection/ElasticOtelDistroService.cs b/src/Elastic.OpenTelemetry/DependencyInjection/ElasticOtelDistroService.cs deleted file mode 100644 index d7bc6f5..0000000 --- a/src/Elastic.OpenTelemetry/DependencyInjection/ElasticOtelDistroService.cs +++ /dev/null @@ -1,38 +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 Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Logging; - -namespace Elastic.OpenTelemetry -{ - internal sealed class ElasticOtelDistroService(IServiceProvider serviceProvider) : IHostedService, IHostedLifecycleService - { - private IAgent? _agent; - - public Task StartingAsync(CancellationToken cancellationToken) - { - var loggerFactory = serviceProvider.GetService(); - var logger = loggerFactory?.CreateLogger($"{nameof(Elastic)}.{nameof(OpenTelemetry)}"); - - _agent = serviceProvider.GetRequiredService().Build(logger, serviceProvider); - - //logger.LogInformation("Initialising Agent.Current."); - //Agent.SetAgent(_agent, logger); - - return Task.CompletedTask; - } - - public Task StartAsync(CancellationToken cancellationToken) => Task.CompletedTask; - public Task StartedAsync(CancellationToken cancellationToken) => Task.CompletedTask; - public Task StoppingAsync(CancellationToken cancellationToken) => Task.CompletedTask; - public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; - - public async Task StoppedAsync(CancellationToken cancellationToken) - { - if (_agent != null) - await _agent.DisposeAsync().ConfigureAwait(false); - } - } -} diff --git a/src/Elastic.OpenTelemetry/DependencyInjection/OpenTelemetryServicesExtensions.cs b/src/Elastic.OpenTelemetry/DependencyInjection/OpenTelemetryServicesExtensions.cs index 0e4fc57..8f84121 100644 --- a/src/Elastic.OpenTelemetry/DependencyInjection/OpenTelemetryServicesExtensions.cs +++ b/src/Elastic.OpenTelemetry/DependencyInjection/OpenTelemetryServicesExtensions.cs @@ -2,12 +2,15 @@ // 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 Elastic.OpenTelemetry; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; /// TODO +// ReSharper disable once CheckNamespace public static class OpenTelemetryServicesExtensions { + // ReSharper disable RedundantNameQualifier + /// /// /// Uses defaults particularly well suited for Elastic's Observability offering because Elastic.OpenTelemetry is referenced @@ -19,7 +22,6 @@ public static class OpenTelemetryServicesExtensions /// /// /// - // ReSharper disable RedundantNameQualifier public static global::OpenTelemetry.IOpenTelemetryBuilder AddOpenTelemetry( this global::Microsoft.Extensions.DependencyInjection.IServiceCollection services ) => services.AddElasticOpenTelemetry(); @@ -36,12 +38,10 @@ public static class OpenTelemetryServicesExtensions /// /// /// - // ReSharper disable RedundantNameQualifier public static global::OpenTelemetry.IOpenTelemetryBuilder AddOpenTelemetry( this global::Microsoft.Extensions.DependencyInjection.IServiceCollection services , params string[]? activitySourceNames ) => services.AddElasticOpenTelemetry(activitySourceNames); - // ReSharper enable RedundantNameQualifier /// /// @@ -51,16 +51,19 @@ public static class OpenTelemetryServicesExtensions /// /// /// - /// Provide an additional ILogger to listen in on the bootstrapping - /// Activity source names to subscribe too + /// Expert level options to control the bootstrapping of the Elastic Agent /// /// /// - // ReSharper disable RedundantNameQualifier public static global::OpenTelemetry.IOpenTelemetryBuilder AddOpenTelemetry( this global::Microsoft.Extensions.DependencyInjection.IServiceCollection services - , ILogger? logger - , params string[]? activitySourceNames - ) => services.AddElasticOpenTelemetry(logger, activitySourceNames); + , AgentBuilderOptions options + ) + { + if (options.Services == null) + options = options with { Services = services }; + return services.AddElasticOpenTelemetry(options); + } + // ReSharper enable RedundantNameQualifier } diff --git a/src/Elastic.OpenTelemetry/DependencyInjection/ServiceCollectionExtensions.cs b/src/Elastic.OpenTelemetry/DependencyInjection/ServiceCollectionExtensions.cs index e4ed626..c79b684 100644 --- a/src/Elastic.OpenTelemetry/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Elastic.OpenTelemetry/DependencyInjection/ServiceCollectionExtensions.cs @@ -2,12 +2,12 @@ // 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.Runtime.CompilerServices; using Elastic.OpenTelemetry; +using Elastic.OpenTelemetry.Hosting; using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Logging; using OpenTelemetry; +// ReSharper disable once CheckNamespace namespace Microsoft.Extensions.DependencyInjection; /// @@ -21,7 +21,7 @@ public static class ServiceCollectionExtensions /// /// public static IOpenTelemetryBuilder AddElasticOpenTelemetry(this IServiceCollection serviceCollection) => - serviceCollection.AddElasticOpenTelemetry(logger: null); + serviceCollection.AddElasticOpenTelemetry(new AgentBuilderOptions { Services = serviceCollection }); /// /// TODO @@ -30,23 +30,22 @@ public static IOpenTelemetryBuilder AddElasticOpenTelemetry(this IServiceCollect /// /// public static IOpenTelemetryBuilder AddElasticOpenTelemetry(this IServiceCollection serviceCollection, params string[]? activitySourceNames) => - serviceCollection.AddElasticOpenTelemetry(logger: null, activitySourceNames); + serviceCollection.AddElasticOpenTelemetry(new AgentBuilderOptions { Services = serviceCollection, ActivitySources = activitySourceNames ?? [] }); /// /// TODO /// /// - /// - /// + /// /// - public static IOpenTelemetryBuilder AddElasticOpenTelemetry(this IServiceCollection serviceCollection, ILogger? logger, params string[]? activitySourceNames) + public static IOpenTelemetryBuilder AddElasticOpenTelemetry(this IServiceCollection serviceCollection, AgentBuilderOptions options) { if (serviceCollection.Any(d => d.ServiceType == typeof(IHostedService) && d.ImplementationType == typeof(ElasticOtelDistroService))) { var sp = serviceCollection.BuildServiceProvider(); return sp.GetService()!; //already registered as singleton } - return new AgentBuilder(logger, services: serviceCollection, activitySourceNames ?? []); + return new AgentBuilder(options); } diff --git a/src/Elastic.OpenTelemetry/Diagnostics/Logging/FileLogger.cs b/src/Elastic.OpenTelemetry/Diagnostics/Logging/FileLogger.cs index 7a2ec44..ed14993 100644 --- a/src/Elastic.OpenTelemetry/Diagnostics/Logging/FileLogger.cs +++ b/src/Elastic.OpenTelemetry/Diagnostics/Logging/FileLogger.cs @@ -84,7 +84,7 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except public bool IsEnabled(LogLevel logLevel) => FileLoggingEnabled && ConfiguredLogLevel <= logLevel; - public IDisposable? BeginScope(TState state) where TState : notnull => _scopeProvider.Push(state); + public IDisposable BeginScope(TState state) where TState : notnull => _scopeProvider.Push(state); public string? LogFilePath { get; } diff --git a/src/Elastic.OpenTelemetry/Diagnostics/Logging/LogFormatter.cs b/src/Elastic.OpenTelemetry/Diagnostics/Logging/LogFormatter.cs index a1f1bc7..25a35da 100644 --- a/src/Elastic.OpenTelemetry/Diagnostics/Logging/LogFormatter.cs +++ b/src/Elastic.OpenTelemetry/Diagnostics/Logging/LogFormatter.cs @@ -49,9 +49,6 @@ public static string Format(LogLevel logLevel, EventId eventId, TState s return fullLogLine; } - private static void WriteLogPrefix(LogLevel logLevel, StringBuilder builder) => - WriteLogPrefix(Environment.CurrentManagedThreadId, DateTime.UtcNow, logLevel, builder); - private const string EmptySpanId = "------"; private static void WriteLogPrefix(int managedThreadId, DateTime dateTime, LogLevel level, StringBuilder builder, string spanId = "") diff --git a/src/Elastic.OpenTelemetry/Diagnostics/LoggingEventListener.cs b/src/Elastic.OpenTelemetry/Diagnostics/LoggingEventListener.cs index 3f0bb54..4dbecba 100644 --- a/src/Elastic.OpenTelemetry/Diagnostics/LoggingEventListener.cs +++ b/src/Elastic.OpenTelemetry/Diagnostics/LoggingEventListener.cs @@ -9,12 +9,16 @@ namespace Elastic.OpenTelemetry.Diagnostics; -internal sealed partial class LoggingEventListener : EventListener, IAsyncDisposable +internal sealed +#if NET8_0_OR_GREATER + partial +#endif + class LoggingEventListener : EventListener, IAsyncDisposable { public const string OpenTelemetrySdkEventSourceNamePrefix = "OpenTelemetry-"; private readonly ILogger _logger; - private readonly EventLevel _eventLevel = EventLevel.Informational; + private readonly EventLevel _eventLevel; private const string TraceParentRegularExpressionString = "^\\d{2}-[a-f0-9]{32}-[a-f0-9]{16}-\\d{2}$"; #if NET8_0_OR_GREATER @@ -77,7 +81,7 @@ protected override void OnEventWritten(EventWrittenEventArgs eventData) var builder = StringBuilderCache.Acquire(); #if NETSTANDARD2_0 || NETFRAMEWORK - var timestamp = DateTime.UtcNow; //best effort in absense of real event timestamp + var timestamp = DateTime.UtcNow; //best effort in absence of real event timestamp var osThreadId = 0L; #else var timestamp = eventData.TimeStamp; @@ -137,13 +141,12 @@ static LogLevel GetLogLevel(EventWrittenEventArgs eventData) => var payload = eventData.Payload[i]; if (payload is not null) - { +#if NETFRAMEWORK + // ReSharper disable once NullCoalescingConditionIsAlwaysNotNullAccordingToAPIContract +#endif builder.Append(payload.ToString() ?? "null"); - } else - { builder.Append("null"); - } } } } diff --git a/src/Elastic.OpenTelemetry/Diagnostics/StringBuilderCache.cs b/src/Elastic.OpenTelemetry/Diagnostics/StringBuilderCache.cs index 644d7f9..729ded8 100644 --- a/src/Elastic.OpenTelemetry/Diagnostics/StringBuilderCache.cs +++ b/src/Elastic.OpenTelemetry/Diagnostics/StringBuilderCache.cs @@ -46,9 +46,7 @@ public static StringBuilder Acquire(int capacity = DefaultCapacity) public static void Release(StringBuilder sb) { if (sb.Capacity <= MaxBuilderSize) - { CachedInstance = sb; - } } /// ToString() the StringBuilder, Release it to the cache, and return the resulting string. diff --git a/src/Elastic.OpenTelemetry/Elastic.OpenTelemetry.csproj b/src/Elastic.OpenTelemetry/Elastic.OpenTelemetry.csproj index eb2fd3c..fa71023 100644 --- a/src/Elastic.OpenTelemetry/Elastic.OpenTelemetry.csproj +++ b/src/Elastic.OpenTelemetry/Elastic.OpenTelemetry.csproj @@ -9,6 +9,10 @@ false + + $(NoWarn);nullable + + diff --git a/src/Elastic.OpenTelemetry/Extensions/ActivityExtensions.cs b/src/Elastic.OpenTelemetry/Extensions/ActivityExtensions.cs index 27b5533..dbfe9d5 100644 --- a/src/Elastic.OpenTelemetry/Extensions/ActivityExtensions.cs +++ b/src/Elastic.OpenTelemetry/Extensions/ActivityExtensions.cs @@ -16,9 +16,7 @@ public static bool TryCompress(this Activity buffered, Activity sibling) var property = buffered.GetCustomProperty("Composite"); if (property is Composite c) - { composite = c; - } var isAlreadyComposite = composite is not null; diff --git a/src/Elastic.OpenTelemetry/Hosting/ElasticOtelDistroService.cs b/src/Elastic.OpenTelemetry/Hosting/ElasticOtelDistroService.cs new file mode 100644 index 0000000..1125870 --- /dev/null +++ b/src/Elastic.OpenTelemetry/Hosting/ElasticOtelDistroService.cs @@ -0,0 +1,38 @@ +// 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 Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +namespace Elastic.OpenTelemetry.Hosting; + +internal sealed class ElasticOtelDistroService(IServiceProvider serviceProvider) : IHostedLifecycleService +{ + private IAgent? _agent; + + public Task StartingAsync(CancellationToken cancellationToken) + { + var loggerFactory = serviceProvider.GetService(); + var logger = loggerFactory?.CreateLogger($"{nameof(Elastic)}.{nameof(OpenTelemetry)}"); + + _agent = serviceProvider.GetRequiredService().Build(logger, serviceProvider); + + //logger.LogInformation("Initialising Agent.Current."); + //Agent.SetAgent(_agent, logger); + + return Task.CompletedTask; + } + + public Task StartAsync(CancellationToken cancellationToken) => Task.CompletedTask; + public Task StartedAsync(CancellationToken cancellationToken) => Task.CompletedTask; + public Task StoppingAsync(CancellationToken cancellationToken) => Task.CompletedTask; + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + public async Task StoppedAsync(CancellationToken cancellationToken) + { + if (_agent != null) + await _agent.DisposeAsync().ConfigureAwait(false); + } +} diff --git a/src/Elastic.OpenTelemetry/IAgent.cs b/src/Elastic.OpenTelemetry/IAgent.cs index e1a109f..410eb03 100644 --- a/src/Elastic.OpenTelemetry/IAgent.cs +++ b/src/Elastic.OpenTelemetry/IAgent.cs @@ -6,6 +6,4 @@ namespace Elastic.OpenTelemetry; /// /// An Elastic OpenTelemetry Distribution agent. /// -public interface IAgent : IDisposable, IAsyncDisposable -{ -} +public interface IAgent : IDisposable, IAsyncDisposable; diff --git a/src/Elastic.OpenTelemetry/IOpenTelemetryBuilder.cs b/src/Elastic.OpenTelemetry/IOpenTelemetryBuilder.cs index 79053df..528e4ba 100644 --- a/src/Elastic.OpenTelemetry/IOpenTelemetryBuilder.cs +++ b/src/Elastic.OpenTelemetry/IOpenTelemetryBuilder.cs @@ -53,9 +53,9 @@ public static IOpenTelemetryBuilder ConfigureResource( this IOpenTelemetryBuilder builder, Action configure) { - builder.Services.ConfigureOpenTelemetryMeterProvider(builder => builder.ConfigureResource(configure)); + builder.Services.ConfigureOpenTelemetryMeterProvider(metrics => metrics.ConfigureResource(configure)); - builder.Services.ConfigureOpenTelemetryTracerProvider(builder => builder.ConfigureResource(configure)); + builder.Services.ConfigureOpenTelemetryTracerProvider(tracing => tracing.ConfigureResource(configure)); //builder.Services.ConfigureOpenTelemetryLoggerProvider(builder => builder.ConfigureResource(configure)); @@ -80,7 +80,7 @@ public static IOpenTelemetryBuilder ConfigureResource( /// The supplied for chaining /// calls. public static IOpenTelemetryBuilder WithMetrics(this IOpenTelemetryBuilder builder) - => WithMetrics(builder, b => { }); + => WithMetrics(builder, _ => { }); /// /// Adds metric services into the builder. @@ -124,7 +124,7 @@ public static IOpenTelemetryBuilder WithMetrics( /// The supplied for chaining /// calls. public static IOpenTelemetryBuilder WithTracing(this IOpenTelemetryBuilder builder) - => WithTracing(builder, b => { }); + => WithTracing(builder, _ => { }); /// /// Adds tracing services into the builder. diff --git a/src/Elastic.OpenTelemetry/Processors/SpanCompressionProcessor.cs b/src/Elastic.OpenTelemetry/Processors/SpanCompressionProcessor.cs index a9310af..0353926 100644 --- a/src/Elastic.OpenTelemetry/Processors/SpanCompressionProcessor.cs +++ b/src/Elastic.OpenTelemetry/Processors/SpanCompressionProcessor.cs @@ -59,30 +59,29 @@ public override void OnEnd(Activity data) base.OnEnd(data); static bool IsCompressionEligible(Activity data, object? property) => - property is bool isExitSpan && isExitSpan && data.Status is ActivityStatusCode.Ok or ActivityStatusCode.Unset; - - void FlushBuffer(Activity data) - { - if (_compressionBuffer.TryGetValue(data, out var compressionBuffer)) - { - // This recreates the initial activity now we know it's final end time and can record it. - using var activity = compressionBuffer.Source.StartActivity(compressionBuffer.DisplayName, compressionBuffer.Kind, compressionBuffer.Parent!.Context, - compressionBuffer.TagObjects, compressionBuffer.Links, compressionBuffer.StartTimeUtc); - - var property = compressionBuffer.GetCustomProperty("Composite"); - - if (property is Composite composite) - { - activity?.SetTag("span_compression.strategy", composite.CompressionStrategy); - activity?.SetTag("span_compression.count", composite.Count); - activity?.SetTag("span_compression.duration", composite.DurationSum); - } - - var endTime = compressionBuffer.StartTimeUtc.Add(compressionBuffer.Duration); - activity?.SetEndTime(endTime); - activity?.Stop(); - _compressionBuffer.Remove(data); - } - } - } + property is true && data.Status is ActivityStatusCode.Ok or ActivityStatusCode.Unset; + } + + private void FlushBuffer(Activity data) + { + if (!_compressionBuffer.TryGetValue(data, out var compressionBuffer)) return; + + // This recreates the initial activity now we know it's final end time and can record it. + using var activity = compressionBuffer.Source.StartActivity(compressionBuffer.DisplayName, compressionBuffer.Kind, compressionBuffer.Parent!.Context, + compressionBuffer.TagObjects, compressionBuffer.Links, compressionBuffer.StartTimeUtc); + + var property = compressionBuffer.GetCustomProperty("Composite"); + + if (property is Composite composite) + { + activity?.SetTag("span_compression.strategy", composite.CompressionStrategy); + activity?.SetTag("span_compression.count", composite.Count); + activity?.SetTag("span_compression.duration", composite.DurationSum); + } + + var endTime = compressionBuffer.StartTimeUtc.Add(compressionBuffer.Duration); + activity?.SetEndTime(endTime); + activity?.Stop(); + _compressionBuffer.Remove(data); + } } diff --git a/src/Elastic.OpenTelemetry/Resources/DefaultServiceDetector.cs b/src/Elastic.OpenTelemetry/Resources/DefaultServiceDetector.cs index 971f1f2..5359017 100644 --- a/src/Elastic.OpenTelemetry/Resources/DefaultServiceDetector.cs +++ b/src/Elastic.OpenTelemetry/Resources/DefaultServiceDetector.cs @@ -22,9 +22,7 @@ static DefaultServiceDetector() { var processName = Process.GetCurrentProcess().ProcessName; if (!string.IsNullOrWhiteSpace(processName)) - { defaultServiceName = $"{defaultServiceName}:{processName}"; - } } catch { diff --git a/tests/Elastic.OpenTelemetry.EndToEndTests/DistributedFixture/ITrafficSimulator.cs b/tests/Elastic.OpenTelemetry.EndToEndTests/DistributedFixture/ITrafficSimulator.cs index 118a0c4..63ea77f 100644 --- a/tests/Elastic.OpenTelemetry.EndToEndTests/DistributedFixture/ITrafficSimulator.cs +++ b/tests/Elastic.OpenTelemetry.EndToEndTests/DistributedFixture/ITrafficSimulator.cs @@ -20,7 +20,7 @@ public async Task Start(DistributedApplicationFixture distributedInfra) { var get = await distributedInfra.AspNetApplication.HttpClient.GetAsync("e2e"); get.StatusCode.Should().Be(HttpStatusCode.OK); - var response = await get.Content.ReadAsStringAsync(); + _ = await get.Content.ReadAsStringAsync(); } } } diff --git a/tests/Elastic.OpenTelemetry.Tests/LoggingTests.cs b/tests/Elastic.OpenTelemetry.Tests/LoggingTests.cs index 7f5f125..8c6cfb5 100644 --- a/tests/Elastic.OpenTelemetry.Tests/LoggingTests.cs +++ b/tests/Elastic.OpenTelemetry.Tests/LoggingTests.cs @@ -2,7 +2,7 @@ // 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.Collections.ObjectModel; +using System.Net.Sockets; using OpenTelemetry; using Xunit.Abstractions; @@ -14,12 +14,12 @@ public class LoggingTests(ITestOutputHelper output) public async Task ObserveLogging() { var logger = new TestLogger(output); + var options = new AgentBuilderOptions { Logger = logger, SkipOtlpExporter = true }; const string activitySourceName = nameof(ObserveLogging); var activitySource = new ActivitySource(activitySourceName, "1.0.0"); - await using (new AgentBuilder(logger) - .SkipOtlpExporter() + await using (new AgentBuilder(options) .WithTracing(tpb => tpb .ConfigureResource(rb => rb.AddService("Test", "1.0.0")) .AddSource(activitySourceName) @@ -27,10 +27,8 @@ public async Task ObserveLogging() ) .Build()) { - using (var activity = activitySource.StartActivity("DoingStuff", ActivityKind.Internal)) - { + using (var activity = activitySource.StartActivity(ActivityKind.Internal)) activity?.SetStatus(ActivityStatusCode.Ok); - } } //assert preamble information gets logged diff --git a/tests/Elastic.OpenTelemetry.Tests/ServiceCollectionTests.cs b/tests/Elastic.OpenTelemetry.Tests/ServiceCollectionTests.cs index 49361dc..fd8dddc 100644 --- a/tests/Elastic.OpenTelemetry.Tests/ServiceCollectionTests.cs +++ b/tests/Elastic.OpenTelemetry.Tests/ServiceCollectionTests.cs @@ -2,9 +2,7 @@ // 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 Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Logging; using OpenTelemetry; using Xunit.Abstractions; @@ -14,7 +12,9 @@ public class ServiceCollectionTests(ITestOutputHelper output) { [Fact] public async Task ServiceCollectionAddIsSafeToCallMultipleTimes() - { + { + var options = new AgentBuilderOptions { Logger = new TestLogger(output), SkipOtlpExporter = true }; + const string activitySourceName = nameof(ServiceCollectionAddIsSafeToCallMultipleTimes); var activitySource = new ActivitySource(activitySourceName, "1.0.0"); @@ -23,8 +23,7 @@ public async Task ServiceCollectionAddIsSafeToCallMultipleTimes() var host = Host.CreateDefaultBuilder(); host.ConfigureServices(s => { - s.AddOpenTelemetry(new TestLogger(output)) - .SkipOtlpExporter() + s.AddOpenTelemetry(options) .WithTracing(tpb => tpb .ConfigureResource(rb => rb.AddService("Test", "1.0.0")) .AddSource(activitySourceName) @@ -39,10 +38,8 @@ public async Task ServiceCollectionAddIsSafeToCallMultipleTimes() using (var app = host.Build()) { _ = app.RunAsync(ctx.Token); - using (var activity = activitySource.StartActivity("DoingStuff", ActivityKind.Internal)) - { + using (var activity = activitySource.StartActivity(ActivityKind.Internal)) activity?.SetStatus(ActivityStatusCode.Ok); - } await ctx.DisposeAsync(); } diff --git a/tests/Elastic.OpenTelemetry.Tests/TransactionIdProcessorTests.cs b/tests/Elastic.OpenTelemetry.Tests/TransactionIdProcessorTests.cs index 302d1bd..f21b79e 100644 --- a/tests/Elastic.OpenTelemetry.Tests/TransactionIdProcessorTests.cs +++ b/tests/Elastic.OpenTelemetry.Tests/TransactionIdProcessorTests.cs @@ -12,30 +12,27 @@ public class TransactionIdProcessorTests(ITestOutputHelper output) [Fact] public void TransactionId_IsAddedToTags() { + var options = new AgentBuilderOptions { Logger = new TestLogger(output), SkipOtlpExporter = true }; const string activitySourceName = nameof(TransactionId_IsAddedToTags); var activitySource = new ActivitySource(activitySourceName, "1.0.0"); var exportedItems = new List(); - using var agent = new AgentBuilder() - .SkipOtlpExporter() + using var agent = new AgentBuilder(options) .WithTracing(tpb => { tpb .ConfigureResource(rb => rb.AddService("Test", "1.0.0")) .AddSource(activitySourceName) .AddInMemoryExporter(exportedItems); - output.WriteLine("Added in memory exporter"); }) .Build(); - using (var activity = activitySource.StartActivity("DoingStuff", ActivityKind.Internal)) - { - activity?.SetStatus(ActivityStatusCode.Ok); - } + using (var activity = activitySource.StartActivity(ActivityKind.Internal)) + activity?.SetStatus(ActivityStatusCode.Ok); - exportedItems.Should().HaveCount(1); + exportedItems.Should().HaveCount(1); var exportedActivity = exportedItems[0];