[sdk-logs] Keep LogRecord.State & LogRecord.Attributes in sync if either is updated by a log processor (#5169)

Author: CodeBlanch

src/OpenTelemetry/CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## Unreleased
 
+* Fixed an issue where `LogRecord.Attributes` (or `LogRecord.StateValues` alias)
+  could become out of sync with `LogRecord.State` if either is set directly via
+  the public setters. This was done to further mitigate issues introduced in
+  1.5.0 causing attributes added using custom processor(s) to be missing after
+  upgrading. For details see:
+  [#5169](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5169)
+
 ## 1.7.0
 
 Released 2023-Dec-08

src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs

@@ -79,7 +79,7 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
 
             LogRecordData.SetActivityContext(ref data, activity);
 
-            var attributes = record.Attributes =
+            var attributes = record.AttributeData =
                 ProcessState(record, ref iloggerData, in state, this.options.IncludeAttributes, this.options.ParseStateValues);
 
             if (!TryGetOriginalFormatFromAttributes(attributes, out var originalFormat))
@@ -133,7 +133,7 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData,
         }
     }
 
-    private static IReadOnlyList<KeyValuePair<string, object?>>? ProcessState<TState>(
+    internal static IReadOnlyList<KeyValuePair<string, object?>>? ProcessState<TState>(
         LogRecord logRecord,
         ref LogRecord.LogRecordILoggerData iLoggerData,
         in TState state,

src/OpenTelemetry/Logs/LogRecord.cs

@@ -18,6 +18,7 @@ public sealed class LogRecord
 {
     internal LogRecordData Data;
     internal LogRecordILoggerData ILoggerData;
+    internal IReadOnlyList<KeyValuePair<string, object?>>? AttributeData;
     internal List<KeyValuePair<string, object?>>? AttributeStorage;
     internal List<object?>? ScopeStorage;
     internal int PoolReferenceCount = int.MaxValue;
@@ -75,7 +76,7 @@ internal LogRecord(
                 this.Data.Body = template;
             }
 
-            this.Attributes = stateValues;
+            this.AttributeData = stateValues;
         }
     }
 
@@ -228,13 +229,30 @@ public string? Body
     /// through <see cref="ILogger"/>.</item>
     /// <item>Set to <see langword="null"/> when <see
     /// cref="OpenTelemetryLoggerOptions.ParseStateValues"/> is enabled.</item>
+    /// <item><see cref="Attributes"/> are automatically updated if <see
+    /// cref="State"/> is set directly.</item>
     /// </list>
     /// </remarks>
     [Obsolete("State cannot be accessed safely outside of an ILogger.Log call stack. Use Attributes instead to safely access the data attached to a LogRecord. State will be removed in a future version.")]
     public object? State
     {
         get => this.ILoggerData.State;
-        set => this.ILoggerData.State = value;
+        set
+        {
+            if (ReferenceEquals(this.ILoggerData.State, value))
+            {
+                return;
+            }
+
+            if (this.AttributeData is not null)
+            {
+                this.AttributeData = OpenTelemetryLogger.ProcessState(this, ref this.ILoggerData, value, includeAttributes: true, parseStateValues: false);
+            }
+            else
+            {
+                this.ILoggerData.State = value;
+            }
+        }
     }
 
     /// <summary>
@@ -252,15 +270,37 @@ public object? State
     /// Gets or sets the attributes attached to the log.
     /// </summary>
     /// <remarks>
-    /// Note: Set when <see
-    /// cref="OpenTelemetryLoggerOptions.IncludeAttributes"/> is enabled and
-    /// log record state implements <see cref="IReadOnlyList{T}"/> or <see
+    /// Notes:
+    /// <list type="bullet">
+    /// <item>Set when <see
+    /// cref="OpenTelemetryLoggerOptions.IncludeAttributes"/> is enabled and log
+    /// record state implements <see cref="IReadOnlyList{T}"/> or <see
     /// cref="IEnumerable{T}"/> of <see cref="KeyValuePair{TKey, TValue}"/>s
     /// (where TKey is <c>string</c> and TValue is <c>object</c>) or <see
     /// cref="OpenTelemetryLoggerOptions.ParseStateValues"/> is enabled
-    /// otherwise <see langword="null"/>.
+    /// otherwise <see langword="null"/>.</item>
+    /// <item><see cref="State"/> is automatically updated if <see
+    /// cref="Attributes"/> are set directly.</item>
+    /// </list>
     /// </remarks>
-    public IReadOnlyList<KeyValuePair<string, object?>>? Attributes { get; set; }
+    public IReadOnlyList<KeyValuePair<string, object?>>? Attributes
+    {
+        get => this.AttributeData;
+        set
+        {
+            if (ReferenceEquals(this.AttributeData, value))
+            {
+                return;
+            }
+
+            if (this.ILoggerData.State is not null)
+            {
+                this.ILoggerData.State = value;
+            }
+
+            this.AttributeData = value;
+        }
+    }
 
     /// <summary>
     /// Gets or sets the log <see cref="System.Exception"/>.
@@ -411,7 +451,7 @@ internal LogRecord Copy()
         {
             Data = this.Data,
             ILoggerData = this.ILoggerData.Copy(),
-            Attributes = this.Attributes == null ? null : new List<KeyValuePair<string, object?>>(this.Attributes),
+            AttributeData = this.AttributeData is null ? null : new List<KeyValuePair<string, object?>>(this.AttributeData),
             Logger = this.Logger,
         };
     }
@@ -422,7 +462,7 @@ internal LogRecord Copy()
     /// </summary>
     private void BufferLogAttributes()
     {
-        var attributes = this.Attributes;
+        var attributes = this.AttributeData;
         if (attributes == null || attributes == this.AttributeStorage)
         {
             return;
@@ -437,7 +477,7 @@ private void BufferLogAttributes()
         // https://github.com/open-telemetry/opentelemetry-dotnet/issues/2905.
         attributeStorage.AddRange(attributes);
 
-        this.Attributes = attributeStorage;
+        this.AttributeData = attributeStorage;
     }
 
     /// <summary>

src/OpenTelemetry/Logs/LoggerSdk.cs

@@ -38,7 +38,7 @@ public override void EmitLog(in LogRecordData data, in LogRecordAttributeList at
 
             logRecord.Logger = this;
 
-            logRecord.Attributes = attributes.Export(ref logRecord.AttributeStorage);
+            logRecord.AttributeData = attributes.Export(ref logRecord.AttributeStorage);
 
             processor.OnEnd(logRecord);