diff --git a/examples/Example.MinimalApi/appsettings.json b/examples/Example.MinimalApi/appsettings.json index 9bba77a..393893c 100644 --- a/examples/Example.MinimalApi/appsettings.json +++ b/examples/Example.MinimalApi/appsettings.json @@ -2,11 +2,11 @@ "Logging": { "LogLevel": { "Default": "Information", - "Microsoft.AspNetCore": "Warning" + "Microsoft.AspNetCore": "Warning", + "Elastic.OpenTelemetry": "Information" } }, "AllowedHosts": "*", - "ServiceName": "elastic-minimal-api-example", "AspNetCoreInstrumentation": { "RecordException": "true" } diff --git a/src/Elastic.OpenTelemetry/Diagnostics/LoggerMessages.cs b/src/Elastic.OpenTelemetry/Diagnostics/LoggerMessages.cs index 5e66c09..5f8c1da 100644 --- a/src/Elastic.OpenTelemetry/Diagnostics/LoggerMessages.cs +++ b/src/Elastic.OpenTelemetry/Diagnostics/LoggerMessages.cs @@ -4,15 +4,27 @@ using System.Diagnostics; using Elastic.OpenTelemetry.Diagnostics.Logging; -using Elastic.OpenTelemetry.Processors; using Microsoft.Extensions.Logging; namespace Elastic.OpenTelemetry.Diagnostics; internal static partial class LoggerMessages { - [LoggerMessage(EventId = 100, Level = LogLevel.Trace, Message = $"{nameof(TransactionIdProcessor)} added 'transaction.id' tag to Activity.")] - internal static partial void TransactionIdProcessorTagAdded(this ILogger logger); +#pragma warning disable SYSLIB1006 // Multiple logging methods cannot use the same event id within a class + // We explictly reuse the same event ID and this is the same log message, but with different types for the structured data + + [LoggerMessage(EventId = 100, Level = LogLevel.Trace, Message = "{ProcessorName} found `{AttributeName}` attribute with value '{AttributeValue}' on the span.")] + internal static partial void FoundTag(this ILogger logger, string processorName, string attributeName, string attributeValue); + + [LoggerMessage(EventId = 100, Level = LogLevel.Trace, Message = "{ProcessorName} found `{AttributeName}` attribute with value '{AttributeValue}' on the span.")] + internal static partial void FoundTag(this ILogger logger, string processorName, string attributeName, int attributeValue); + + [LoggerMessage(EventId = 101, Level = LogLevel.Trace, Message = "{ProcessorName} set `{AttributeName}` attribute with value '{AttributeValue}' on the span.")] + internal static partial void SetTag(this ILogger logger, string processorName, string attributeName, string attributeValue); + + [LoggerMessage(EventId = 101, Level = LogLevel.Trace, Message = "{ProcessorName} set `{AttributeName}` attribute with value '{AttributeValue}' on the span.")] + internal static partial void SetTag(this ILogger logger, string processorName, string attributeName, int attributeValue); +#pragma warning restore SYSLIB1006 // Multiple logging methods cannot use the same event id within a class [LoggerMessage(EventId = 20, Level = LogLevel.Trace, Message = "Added '{ProcessorTypeName}' processor to '{BuilderTypeName}'.")] public static partial void LogProcessorAdded(this ILogger logger, string processorTypeName, string builderTypeName); diff --git a/src/Elastic.OpenTelemetry/Diagnostics/Logging/CompositeLogger.cs b/src/Elastic.OpenTelemetry/Diagnostics/Logging/CompositeLogger.cs index ecc6607..3d4af29 100644 --- a/src/Elastic.OpenTelemetry/Diagnostics/Logging/CompositeLogger.cs +++ b/src/Elastic.OpenTelemetry/Diagnostics/Logging/CompositeLogger.cs @@ -17,6 +17,7 @@ internal sealed class CompositeLogger(ILogger? additionalLogger) : IDisposable, { public FileLogger FileLogger { get; } = new(); + private ILogger? _additionalLogger = additionalLogger; private bool _isDisposed; public void Dispose() @@ -39,22 +40,22 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except if (FileLogger.IsEnabled(logLevel)) FileLogger.Log(logLevel, eventId, state, exception, formatter); - if (additionalLogger == null) + if (_additionalLogger == null) return; - if (additionalLogger.IsEnabled(logLevel)) - additionalLogger.Log(logLevel, eventId, state, exception, formatter); + if (_additionalLogger.IsEnabled(logLevel)) + _additionalLogger.Log(logLevel, eventId, state, exception, formatter); } public bool LogFileEnabled => FileLogger.FileLoggingEnabled; public string LogFilePath => FileLogger.LogFilePath ?? string.Empty; - public void SetAdditionalLogger(ILogger? logger) => additionalLogger ??= logger; + public void SetAdditionalLogger(ILogger? logger) => _additionalLogger ??= logger; - public bool IsEnabled(LogLevel logLevel) => FileLogger.IsEnabled(logLevel) || (additionalLogger?.IsEnabled(logLevel) ?? false); + public bool IsEnabled(LogLevel logLevel) => FileLogger.IsEnabled(logLevel) || (_additionalLogger?.IsEnabled(logLevel) ?? false); public IDisposable BeginScope(TState state) where TState : notnull => - new CompositeDisposable(FileLogger.BeginScope(state), additionalLogger?.BeginScope(state)); + new CompositeDisposable(FileLogger.BeginScope(state), _additionalLogger?.BeginScope(state)); private class CompositeDisposable(params IDisposable?[] disposables) : IDisposable { diff --git a/src/Elastic.OpenTelemetry/Diagnostics/Logging/LogState.cs b/src/Elastic.OpenTelemetry/Diagnostics/Logging/LogState.cs index 9fabb66..7ba27a2 100644 --- a/src/Elastic.OpenTelemetry/Diagnostics/Logging/LogState.cs +++ b/src/Elastic.OpenTelemetry/Diagnostics/Logging/LogState.cs @@ -10,6 +10,7 @@ namespace Elastic.OpenTelemetry.Diagnostics.Logging; internal class LogState : IReadOnlyList> { private readonly Activity? _activity; + public Activity? Activity { get => _activity; @@ -44,7 +45,7 @@ public string? SpanId init => _spanId = value; } - private readonly List> _values = new(); + private readonly List> _values = []; public IEnumerator> GetEnumerator() => _values.GetEnumerator(); diff --git a/src/Elastic.OpenTelemetry/ElasticOpenTelemetryBuilder.cs b/src/Elastic.OpenTelemetry/ElasticOpenTelemetryBuilder.cs index b0f8c4e..5d45ff2 100644 --- a/src/Elastic.OpenTelemetry/ElasticOpenTelemetryBuilder.cs +++ b/src/Elastic.OpenTelemetry/ElasticOpenTelemetryBuilder.cs @@ -86,6 +86,7 @@ public ElasticOpenTelemetryBuilder(ElasticOpenTelemetryOptions options) logging.IncludeScopes = true; //TODO add processor that adds service.id }); + openTelemetry .WithLogging(logging => { diff --git a/src/Elastic.OpenTelemetry/Extensions/TracerProviderBuilderExtensions.cs b/src/Elastic.OpenTelemetry/Extensions/TracerProviderBuilderExtensions.cs index 7a1bb51..080e943 100644 --- a/src/Elastic.OpenTelemetry/Extensions/TracerProviderBuilderExtensions.cs +++ b/src/Elastic.OpenTelemetry/Extensions/TracerProviderBuilderExtensions.cs @@ -20,8 +20,14 @@ public static class TracerProviderBuilderExtensions /// /// Include Elastic trace processors to ensure data is enriched and extended. /// - public static TracerProviderBuilder AddElasticProcessors(this TracerProviderBuilder builder, ILogger? logger = null) => - builder.LogAndAddProcessor(new TransactionIdProcessor(logger ?? NullLogger.Instance), logger ?? NullLogger.Instance); + public static TracerProviderBuilder AddElasticProcessors(this TracerProviderBuilder builder, ILogger? logger = null) + { + logger ??= NullLogger.Instance; + + return builder + .LogAndAddProcessor(new TransactionIdProcessor(logger), logger) + .LogAndAddProcessor(new ElasticCompatibilityProcessor(logger), logger); + } private static TracerProviderBuilder LogAndAddProcessor(this TracerProviderBuilder builder, BaseProcessor processor, ILogger logger) { diff --git a/src/Elastic.OpenTelemetry/Processors/ElasticCompatibilityProcessor.cs b/src/Elastic.OpenTelemetry/Processors/ElasticCompatibilityProcessor.cs new file mode 100644 index 0000000..d48e53c --- /dev/null +++ b/src/Elastic.OpenTelemetry/Processors/ElasticCompatibilityProcessor.cs @@ -0,0 +1,137 @@ +// 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; +using Elastic.OpenTelemetry.Diagnostics; +using Microsoft.Extensions.Logging; +using OpenTelemetry; +using static Elastic.OpenTelemetry.SemanticConventions.TraceSemanticConventions; + +namespace Elastic.OpenTelemetry.Processors; + +/// +/// This processor ensures that the data is compatible with Elastic backends. +/// +/// It checks for the presence of the older semantic conventions and if they are not present, it will +/// add them. This is only necessary for compatibility with older versions of the intake OTel endpoints +/// on Elastic APM. These issues will be fixed centrally in future versions of the intake code. +/// +/// +/// +public class ElasticCompatibilityProcessor(ILogger logger) : BaseProcessor +{ + private readonly ILogger _logger = logger; + + /// + public override void OnEnd(Activity activity) + { + if (activity.Kind == ActivityKind.Server) + { + // For inbound HTTP requests (server), ASP.NET Core sets the newer semantic conventions in + // the latest versions. For now, we need to ensure the older semantic conventions are also + // included on the spans sent to the Elastic backend as the intake system is currently + // unaware of the newer semantic conventions. We send the older attributes to ensure that + // the UI functions as expected. The http and net host conventions are required to build + // up the URL displayed in the trace sample UI within Kibana. This will be fixed in future + // version of apm-data. + + string? httpScheme = null; + string? httpTarget = null; + string? urlScheme = null; + string? urlPath = null; + string? urlQuery = null; + string? netHostName = null; + int? netHostPort = null; + string? serverAddress = null; + int? serverPort = null; + + // We loop once, collecting all the attributes we need for the older and newer + // semantic conventions. This is a bit more verbose but ensures we don't iterate + // the tags multiple times. + foreach (var tag in activity.TagObjects) + { + if (tag.Key == HttpScheme) + httpScheme = ProcessStringAttribute(tag); + + if (tag.Key == HttpTarget) + httpTarget = ProcessStringAttribute(tag); + + if (tag.Key == UrlScheme) + urlScheme = ProcessStringAttribute(tag); + + if (tag.Key == UrlPath) + urlPath = ProcessStringAttribute(tag); + + if (tag.Key == UrlQuery) + urlQuery = ProcessStringAttribute(tag); + + if (tag.Key == NetHostName) + netHostName = ProcessStringAttribute(tag); + + if (tag.Key == ServerAddress) + serverAddress = ProcessStringAttribute(tag); + + if (tag.Key == NetHostPort) + netHostPort = ProcessIntAttribute(tag); + + if (tag.Key == ServerPort) + serverPort = ProcessIntAttribute(tag); + } + + // Set the older semantic convention attributes + if (httpScheme is null && urlScheme is not null) + SetStringAttribute(HttpScheme, urlScheme); + + if (httpTarget is null && urlPath is not null) + { + var target = urlPath; + + if (urlQuery is not null) + target += $"?{urlQuery}"; + + SetStringAttribute(HttpTarget, target); + } + + if (netHostName is null && serverAddress is not null) + SetStringAttribute(NetHostName, serverAddress); + + if (netHostPort is null && serverPort is not null) + SetIntAttribute(NetHostPort, serverPort.Value); + } + + string? ProcessStringAttribute(KeyValuePair tag) + { + if (tag.Value is string value) + { + _logger.FoundTag(nameof(ElasticCompatibilityProcessor), tag.Key, value); + return value; + } + + return null; + } + + int? ProcessIntAttribute(KeyValuePair tag) + { + if (tag.Value is int value) + { + _logger.FoundTag(nameof(ElasticCompatibilityProcessor), tag.Key, value); + return value; + } + + return null; + } + + void SetStringAttribute(string attributeName, string value) + { + _logger.SetTag(nameof(ElasticCompatibilityProcessor), attributeName, value); + activity.SetTag(attributeName, value); + } + + void SetIntAttribute(string attributeName, int value) + { + _logger.SetTag(nameof(ElasticCompatibilityProcessor), attributeName, value); + activity.SetTag(attributeName, value); + } + } +} diff --git a/src/Elastic.OpenTelemetry/Processors/TransactionIdProcessor.cs b/src/Elastic.OpenTelemetry/Processors/TransactionIdProcessor.cs index 4f82a69..720d214 100644 --- a/src/Elastic.OpenTelemetry/Processors/TransactionIdProcessor.cs +++ b/src/Elastic.OpenTelemetry/Processors/TransactionIdProcessor.cs @@ -1,6 +1,7 @@ // 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; using Elastic.OpenTelemetry.Diagnostics; using Microsoft.Extensions.Logging; @@ -30,6 +31,6 @@ public override void OnStart(Activity activity) return; activity.SetTag(TransactionIdTagName, _currentTransactionId.Value.Value.ToString()); - logger.TransactionIdProcessorTagAdded(); + logger.SetTag(nameof(TransactionIdProcessor), TransactionIdTagName, _currentTransactionId.Value.Value.ToString()); } } diff --git a/src/Elastic.OpenTelemetry/SemanticConventions/TraceSemanticConventions.cs b/src/Elastic.OpenTelemetry/SemanticConventions/TraceSemanticConventions.cs new file mode 100644 index 0000000..c03c546 --- /dev/null +++ b/src/Elastic.OpenTelemetry/SemanticConventions/TraceSemanticConventions.cs @@ -0,0 +1,25 @@ +// 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 + +namespace Elastic.OpenTelemetry.SemanticConventions; + +internal static class TraceSemanticConventions +{ + // HTTP + public const string HttpScheme = "http.scheme"; + public const string HttpTarget = "http.target"; + + // NET + public const string NetHostName = "net.host.name"; + public const string NetHostPort = "net.host.port"; + + // SERVER + public const string ServerAddress = "server.address"; + public const string ServerPort = "server.port"; + + // URL + public const string UrlPath = "url.path"; + public const string UrlQuery = "url.query"; + public const string UrlScheme = "url.scheme"; +} diff --git a/tests/Elastic.OpenTelemetry.EndToEndTests/README.md b/tests/Elastic.OpenTelemetry.EndToEndTests/README.md index a29a992..44aaede 100644 --- a/tests/Elastic.OpenTelemetry.EndToEndTests/README.md +++ b/tests/Elastic.OpenTelemetry.EndToEndTests/README.md @@ -8,8 +8,8 @@ Requires an already running serverless observability project on cloud. The configuration can be provided either as asp.net secrets or environment variables. ```bash -dotnet user-secrets set "E2E:Endpoint" "" --project tests/Elastic.OpenTelemetry.IntegrationTests -dotnet user-secrets set "E2E:Authorization" "
" --project tests/Elastic.OpenTelemetry.IntegrationTests +dotnet user-secrets set "E2E:Endpoint" "" --project tests/Elastic.OpenTelemetry.EndToEndTests +dotnet user-secrets set "E2E:Authorization" "
" --project tests/Elastic.OpenTelemetry.EndToEndTests ``` The equivalent environment variables are `E2E__ENDPOINT` and `E2E__AUTHORIZATION`. For local development setting @@ -38,8 +38,8 @@ Once invited and accepted the invited email can be used to login during the auto These can be provided again as user secrets: ```bash -dotnet user-secrets set "E2E:BrowserEmail" "" --project tests/Elastic.OpenTelemetry.IntegrationTests -dotnet user-secrets set "E2E:BrowserPassword" "" --project tests/Elastic.OpenTelemetry.IntegrationTests +dotnet user-secrets set "E2E:BrowserEmail" "" --project tests/Elastic.OpenTelemetry.EndToEndTests +dotnet user-secrets set "E2E:BrowserPassword" "" --project tests/Elastic.OpenTelemetry.EndToEndTests ``` or environment variables (`E2E__BROWSEREMAIL` and `E2E__BROWSERPASSWORD`). diff --git a/tests/Elastic.OpenTelemetry.Tests/Processors/ElasticCompatibilityProcessorTests.cs b/tests/Elastic.OpenTelemetry.Tests/Processors/ElasticCompatibilityProcessorTests.cs new file mode 100644 index 0000000..0b7ba49 --- /dev/null +++ b/tests/Elastic.OpenTelemetry.Tests/Processors/ElasticCompatibilityProcessorTests.cs @@ -0,0 +1,98 @@ +// 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 Elastic.OpenTelemetry.SemanticConventions; +using Microsoft.Extensions.Logging.Abstractions; + +namespace Elastic.OpenTelemetry.Tests.Processors; + +public sealed class ElasticCompatibilityProcessorTests : IDisposable +{ + private readonly ActivitySource _activitySource; + private readonly ActivityListener _listener; + + public ElasticCompatibilityProcessorTests() + { + // It's a bit annoying as we have to create an ActivitySource and ActivityListener + // just so we can create an Activity with the ActivityKind set. The Activity ctor + // doesn't allow us to set the ActivityKind directly. + _activitySource = new ActivitySource("test"); + + // We need a listener so that CreateActivity returns a non-null Activity + _listener = new ActivityListener() + { + ShouldListenTo = _ => true, + Sample = (ref ActivityCreationOptions _) => ActivitySamplingResult.AllData + }; + + ActivitySource.AddActivityListener(_listener); + } + + [Fact] + public void AddsExpectedNetHostAttributes() + { + const string hostName = "/"; + const int port = 80; + + var activity = _activitySource.CreateActivity("test", ActivityKind.Server)!; + + activity.SetTag(TraceSemanticConventions.ServerAddress, hostName); + activity.SetTag(TraceSemanticConventions.ServerPort, port); + + var sut = new ElasticCompatibilityProcessor(NullLogger.Instance); + sut.OnEnd(activity); + + // We can test with Tags (rather than TagObjects) here as we know these are string values + activity.Tags.Single(t => t.Key == TraceSemanticConventions.NetHostName).Value.Should().Be(hostName); + + activity.TagObjects.Single(t => t.Key == TraceSemanticConventions.NetHostPort).Value + .Should().BeOfType().Subject.Should().Be(port); + } + + [Fact] + public void AddsExpectedHttpAttributes_WhenUrlQuery_IsNotPresent() + { + const string scheme = "https"; + const string path = "/my/path"; + + var activity = _activitySource.CreateActivity("test", ActivityKind.Server)!; + + activity.SetTag(TraceSemanticConventions.UrlScheme, scheme); + activity.SetTag(TraceSemanticConventions.UrlPath, path); + + var sut = new ElasticCompatibilityProcessor(NullLogger.Instance); + sut.OnEnd(activity); + + // We can test with Tags (rather than TagObjects) here as we know these are string values + activity.Tags.Single(t => t.Key == TraceSemanticConventions.HttpScheme).Value.Should().Be(scheme); + activity.Tags.Single(t => t.Key == TraceSemanticConventions.HttpTarget).Value.Should().Be(path); + } + + [Fact] + public void AddsExpectedHttpAttributes_WhenUrlQuery_IsPresent() + { + const string scheme = "https"; + const string path = "/my/path"; + const string query = "q=OpenTelemetry"; + + var activity = _activitySource.CreateActivity("test", ActivityKind.Server)!; + + activity.SetTag(TraceSemanticConventions.UrlScheme, scheme); + activity.SetTag(TraceSemanticConventions.UrlPath, path); + activity.SetTag(TraceSemanticConventions.UrlQuery, query); + + var sut = new ElasticCompatibilityProcessor(NullLogger.Instance); + sut.OnEnd(activity); + + // We can test with Tags (rather than TagObjects) here as we know these are string values + activity.Tags.Single(t => t.Key == TraceSemanticConventions.HttpScheme).Value.Should().Be(scheme); + activity.Tags.Single(t => t.Key == TraceSemanticConventions.HttpTarget).Value.Should().Be($"{path}?{query}"); + } + + public void Dispose() + { + _activitySource.Dispose(); + _listener.Dispose(); + } +}