Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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