[sdk] Merge 1.5.1 fixes to main (1.6 dev)

[CodeBlanch]

Relates to #4609
Relates to #4614
Relates to #4615

Changes

• Brings the changes done in main-1.5.0 branch (1.5.1 hotfix) to main (1.6 dev line).
• Adds a couple tweaks for LogRecordAttributeList which is part of 1.6 dev.

Merge requirement checklist

• CONTRIBUTING guidelines followed (nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes

[codecov]

Codecov Report

Merging #4619 (6a8fbaa) into main (1395b7f) will increase coverage by 0.02%.
The diff coverage is 87.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4619      +/-   ##
==========================================
+ Coverage   84.85%   84.88%   +0.02%     
==========================================
  Files         313      313              
  Lines       12604    12609       +5     
==========================================
+ Hits        10695    10703       +8     
+ Misses       1909     1906       -3     

Impacted Files	Coverage Δ
...lemetry/Logs/ILogger/OpenTelemetryLoggerOptions.cs	85.00% <ø> (ø)
.../OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs	86.79% <80.00%> (+0.55%)	⬆️
...c/OpenTelemetry.Api/Logs/LogRecordAttributeList.cs	69.02% <100.00%> (+0.27%)	⬆️
...nTelemetry/Internal/OpenTelemetrySdkEventSource.cs	81.18% <100.00%> (+1.40%)	⬆️

... and 2 files with indirect coverage changes

[utpilla]

How does this work with LoggerSdk.EmitLog? This would be changing the behavior of LoggerSdk.EmitLog to return an empty list instead of null when the attribute count is zero. Is that what we want?

opentelemetry-dotnet/src/OpenTelemetry/Logs/LoggerSdk.cs

Line 56 in 303a357

logRecord.Attributes = attributes.Export(ref logRecord.AttributeStorage);

LogRecord.Attributes description says that its value is set depending upon the options: IncludeAttributes or ParseStateValues. Would these options be considered when using LoggerSdk.EmitLog?

[CodeBlanch]

There's a lot to unpack here 🤣

How does this work with LoggerSdk.EmitLog? This would be changing the behavior of LoggerSdk.EmitLog to return an empty list instead of null when the attribute count is zero. Is that what we want?

I don't think it is an issue. It is all net-new so up to us to define the behavior. When calling Logger.EmitLog (at the moment) there are always attributes. It is either user-supplied (may have 0 count or more) or it is default (0 count). It is a struct so there isn't a way to pass null. With this change the LogRecord will mirror that behavior. It will always have attributes either empty (0) or some list using the pool storage.

I made this change because the exporter which causes the 1.5.1 breakfix is doing...

var list = (IReadOnlyList<KeyValuePair<string, object>>)logRecord.State;

It isn't checking for null and it isn't looking at Attributes or StateValues. If some users take a newer SDK and starts using some Logger.EmitLog thing that exporter would break again. I needed to make sure this path also had a valid logRecord.State. Since State would have a value I decided Attributes / StateValues should as well. It felt confusing the other way around.

LogRecord.Attributes description says that its value is set depending upon the options: IncludeAttributes or ParseStateValues. Would these options be considered when using LoggerSdk.EmitLog?

OpenTelemetryLoggerOptions only applies to the ILogger integration. I'll open a follow-up PR to clarify the XML comments on LogRecord because you are right they are kind of misleading.

Now you can use LogRecordAttributeList with the ILogger integration! You can do logger.Log<LogRecordAttributeList>(...) which is using LogRecordAttributeList as TState in the ILogger API. This is really the only way at the moment to get truly allocation-free logging in OpenTelemetry .NET. If you set OpenTelemetryLoggerOptions.IncludeAttributes = false AND you use LogRecordAttributeList as the TState THEN IncludeAttributes is respected and the LogRecord ends up with null values for State, Attributes, and StateValues.