[sdk-logs] Update LogRecord to keep CategoryName and Logger in sync (#5317)

Author: CodeBlanch

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpLogRecordTransformer.cs

@@ -18,8 +18,6 @@ internal sealed class OtlpLogRecordTransformer
 {
     internal static readonly ConcurrentBag<OtlpLogs.ScopeLogs> LogListPool = new();
 
-    private const string DefaultScopeName = "";
-
     private readonly SdkLimitOptions sdkLimitOptions;
     private readonly ExperimentalOptions experimentalOptions;
 
@@ -49,7 +47,7 @@ internal OtlpCollector.ExportLogsServiceRequest BuildExportRequest(
             var otlpLogRecord = this.ToOtlpLog(logRecord);
             if (otlpLogRecord != null)
             {
-                var scopeName = logRecord.CategoryName ?? logRecord.Logger?.Name ?? DefaultScopeName;
+                var scopeName = logRecord.Logger.Name;
                 if (!logsByCategory.TryGetValue(scopeName, out var scopeLogs))
                 {
                     scopeLogs = this.GetLogListFromPool(scopeName);

src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt

@@ -1,6 +1,6 @@
 OpenTelemetry.Logs.LoggerProviderBuilderExtensions
 OpenTelemetry.Logs.LoggerProviderExtensions
-OpenTelemetry.Logs.LogRecord.Logger.get -> OpenTelemetry.Logs.Logger?
+OpenTelemetry.Logs.LogRecord.Logger.get -> OpenTelemetry.Logs.Logger!
 OpenTelemetry.Logs.LogRecord.Severity.get -> OpenTelemetry.Logs.LogRecordSeverity?
 OpenTelemetry.Logs.LogRecord.Severity.set -> void
 OpenTelemetry.Logs.LogRecord.SeverityText.get -> string?

src/OpenTelemetry/CHANGELOG.md

@@ -24,6 +24,10 @@
   when configuring a view.
   ([#5312](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5312))
 
+* Updated `LogRecord` to keep `CategoryName` and `Logger` in sync when using the
+  experimental Log Bridge API.
+  [#5317](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5317)
+
 ## 1.7.0
 
 Released 2023-Dec-08

src/OpenTelemetry/Internal/InstrumentationScopeLogger.cs

@@ -0,0 +1,29 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+using System.Collections.Concurrent;
+using OpenTelemetry.Logs;
+
+namespace OpenTelemetry.Internal;
+
+internal sealed class InstrumentationScopeLogger : Logger
+{
+    private static readonly ConcurrentDictionary<string, InstrumentationScopeLogger> Cache = new();
+
+    private InstrumentationScopeLogger(string name)
+        : base(name)
+    {
+    }
+
+    public static InstrumentationScopeLogger Default { get; } = new(string.Empty);
+
+    public static InstrumentationScopeLogger GetInstrumentationScopeLoggerForName(string? name)
+    {
+        return string.IsNullOrWhiteSpace(name)
+            ? Default
+            : Cache.GetOrAdd(name!, static n => new(n));
+    }
+
+    public override void EmitLog(in LogRecordData data, in LogRecordAttributeList attributes)
+        => throw new NotSupportedException();
+}

src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs

@@ -24,7 +24,7 @@ internal sealed class OpenTelemetryLogger : ILogger
 
     private readonly LoggerProviderSdk provider;
     private readonly OpenTelemetryLoggerOptions options;
-    private readonly string categoryName;
+    private readonly InstrumentationScopeLogger instrumentationScope;
 
     internal OpenTelemetryLogger(
         LoggerProviderSdk provider,
@@ -37,7 +37,7 @@ internal OpenTelemetryLogger(
 
         this.provider = provider!;
         this.options = options!;
-        this.categoryName = categoryName!;
+        this.instrumentationScope = InstrumentationScopeLogger.GetInstrumentationScopeLoggerForName(categoryName);
     }
 
     internal IExternalScopeProvider? ScopeProvider { get; set; }
@@ -65,7 +65,6 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
             iloggerData.TraceState = this.options.IncludeTraceState && activity != null
                 ? activity.TraceStateString
                 : null;
-            iloggerData.CategoryName = this.categoryName;
             iloggerData.EventId = eventId;
             iloggerData.Exception = exception;
             iloggerData.ScopeProvider = this.options.IncludeScopes ? this.ScopeProvider : null;
@@ -97,7 +96,7 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
                     : null;
             }
 
-            record.Logger = LoggerInstrumentationScope.Instance;
+            record.Logger = this.instrumentationScope;
 
             processor.OnEnd(record);
 
@@ -239,19 +238,4 @@ public void Dispose()
         {
         }
     }
-
-    private sealed class LoggerInstrumentationScope : Logger
-    {
-        private LoggerInstrumentationScope(string name, string version)
-            : base(name)
-        {
-            this.SetInstrumentationScope(version);
-        }
-
-        public static LoggerInstrumentationScope Instance { get; }
-            = new("OpenTelemetry", Sdk.InformationalVersion);
-
-        public override void EmitLog(in LogRecordData data, in LogRecordAttributeList attributes)
-            => throw new NotSupportedException();
-    }
 }

src/OpenTelemetry/Logs/LogRecord.cs

@@ -60,7 +60,6 @@ internal LogRecord(
         this.ILoggerData = new()
         {
             TraceState = activity?.TraceStateString,
-            CategoryName = categoryName,
             FormattedMessage = formattedMessage,
             EventId = eventId,
             Exception = exception,
@@ -79,6 +78,8 @@ internal LogRecord(
 
             this.AttributeData = stateValues;
         }
+
+        this.Logger = InstrumentationScopeLogger.GetInstrumentationScopeLoggerForName(categoryName);
     }
 
     internal enum LogRecordSource
@@ -153,16 +154,31 @@ public string? TraceState
         set => this.ILoggerData.TraceState = value;
     }
 
+#if EXPOSE_EXPERIMENTAL_FEATURES
     /// <summary>
     /// Gets or sets the log category name.
     /// </summary>
     /// <remarks>
-    /// Note: <see cref="CategoryName"/> is only set when emitting logs through <see cref="ILogger"/>.
+    /// Note: <see cref="CategoryName"/> is an alias for the <see
+    /// cref="Logger.Name"/> accessed via the <see cref="Logger"/> property.
+    /// Setting a new value for <see cref="CategoryName"/> will result in a new
+    /// <see cref="Logger"/> being set.
     /// </remarks>
+#else
+    /// <summary>
+    /// Gets or sets the log category name.
+    /// </summary>
+#endif
     public string? CategoryName
     {
-        get => this.ILoggerData.CategoryName;
-        set => this.ILoggerData.CategoryName = value;
+        get => this.Logger.Name;
+        set
+        {
+            if (this.Logger.Name != value)
+            {
+                this.Logger = InstrumentationScopeLogger.GetInstrumentationScopeLoggerForName(value);
+            }
+        }
     }
 
     /// <summary>
@@ -379,18 +395,26 @@ public Exception? Exception
 
 #if EXPOSE_EXPERIMENTAL_FEATURES
     /// <summary>
-    /// Gets the <see cref="Logs.Logger"/> which emitted the <see cref="LogRecord"/>.
+    /// Gets the <see cref="Logs.Logger"/> associated with the <see
+    /// cref="LogRecord"/>.
     /// </summary>
-    /// <remarks><inheritdoc cref="Sdk.CreateLoggerProviderBuilder" path="/remarks"/></remarks>
+    /// <remarks>
+    /// <para><inheritdoc cref="Sdk.CreateLoggerProviderBuilder" path="/remarks"/></para>
+    /// Note: When using the Log Bridge API (for example <see
+    /// cref="Logger.EmitLog(in LogRecordData)"/>) <see cref="Logger"/> is
+    /// typically the <see cref="Logs.Logger"/> which emitted the <see
+    /// cref="LogRecord"/> however the value may be different if <see
+    /// cref="CategoryName"/> is modified.</remarks>
 #if NET8_0_OR_GREATER
     [Experimental(DiagnosticDefinitions.LogsBridgeExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
 #endif
-    public Logger? Logger { get; internal set; }
+    public Logger Logger { get; internal set; } = InstrumentationScopeLogger.Default;
 #else
     /// <summary>
-    /// Gets or sets the <see cref="Logs.Logger"/> which emitted the <see cref="LogRecord"/>.
+    /// Gets or sets the <see cref="Logs.Logger"/> associated with the <see
+    /// cref="LogRecord"/>.
     /// </summary>
-    internal Logger? Logger { get; set; }
+    internal Logger Logger { get; set; } = InstrumentationScopeLogger.Default;
 #endif
 
     /// <summary>
@@ -523,7 +547,6 @@ private void BufferLogScopes()
     internal struct LogRecordILoggerData
     {
         public string? TraceState;
-        public string? CategoryName;
         public EventId EventId;
         public string? FormattedMessage;
         public Exception? Exception;
@@ -536,7 +559,6 @@ public LogRecordILoggerData Copy()
             var copy = new LogRecordILoggerData
             {
                 TraceState = this.TraceState,
-                CategoryName = this.CategoryName,
                 EventId = this.EventId,
                 FormattedMessage = this.FormattedMessage,
                 Exception = this.Exception,

test/OpenTelemetry.Tests/Logs/LogRecordTest.cs

@@ -979,8 +979,41 @@ public void LogRecordInstrumentationScopeTest()
 
         Assert.NotNull(logRecord);
         Assert.NotNull(logRecord.Logger);
-        Assert.Equal("OpenTelemetry", logRecord.Logger.Name);
-        Assert.Equal(Sdk.InformationalVersion, logRecord.Logger.Version);
+        Assert.Equal("OpenTelemetry.Logs.Tests.LogRecordTest", logRecord.Logger.Name);
+        Assert.Null(logRecord.Logger.Version);
+    }
+
+    [Fact]
+    public void LogRecordCategoryNameAliasForInstrumentationScopeTests()
+    {
+        LogRecord logRecord = new();
+
+        Assert.Equal(string.Empty, logRecord.CategoryName);
+        Assert.Equal(logRecord.CategoryName, logRecord.Logger.Name);
+
+        logRecord.CategoryName = "Testing";
+
+        Assert.Equal("Testing", logRecord.CategoryName);
+        Assert.Equal(logRecord.CategoryName, logRecord.Logger.Name);
+
+        logRecord.CategoryName = null;
+
+        Assert.Equal(string.Empty, logRecord.CategoryName);
+        Assert.Equal(logRecord.CategoryName, logRecord.Logger.Name);
+
+        var exportedItems = new List<LogRecord>();
+        using (var loggerProvider = Sdk.CreateLoggerProviderBuilder()
+            .AddProcessor(new BatchLogRecordExportProcessor(new InMemoryExporter<LogRecord>(exportedItems)))
+            .Build())
+        {
+            var logger = loggerProvider.GetLogger("TestName");
+            logger.EmitLog(default);
+        }
+
+        Assert.Single(exportedItems);
+
+        Assert.Equal("TestName", exportedItems[0].CategoryName);
+        Assert.Equal(exportedItems[0].CategoryName, exportedItems[0].Logger.Name);
     }
 
     private static ILoggerFactory InitializeLoggerFactory(out List<LogRecord> exportedItems, Action<OpenTelemetryLoggerOptions> configure = null)