Skip to content

Commit

Permalink
[sdk-logs] Keep LogRecord.State & LogRecord.Attributes in sync if eit…
Browse files Browse the repository at this point in the history
…her is updated by a log processor (#5169)
  • Loading branch information
CodeBlanch authored Dec 15, 2023
1 parent 7eeae5b commit af83eb1
Show file tree
Hide file tree
Showing 5 changed files with 326 additions and 13 deletions.
7 changes: 7 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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,
Expand Down
60 changes: 50 additions & 10 deletions src/OpenTelemetry/Logs/LogRecord.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -75,7 +76,7 @@ internal LogRecord(
this.Data.Body = template;
}

this.Attributes = stateValues;
this.AttributeData = stateValues;
}
}

Expand Down Expand Up @@ -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>
Expand All @@ -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"/>.
Expand Down Expand Up @@ -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,
};
}
Expand All @@ -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;
Expand All @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Logs/LoggerSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Loading

0 comments on commit af83eb1

Please sign in to comment.