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

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);