[sdk-1.5.0-hotfix] Fix LogRecord.State being null when TState matches known interfaces

src/OpenTelemetry/CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+* Fixed a breaking change causing `LogRecord.State` to be `null` where it was
+  previously set to a valid value when
+  `OpenTelemetryLoggerOptions.ParseStateValues` is `false` and states implement
+  `IReadOnlyList` or `IEnumerable` of `KeyValuePair<string, object>`s.
+  ([#4609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4609))
+
 ## 1.5.0
 
 Released 2023-Jun-05

src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs

@@ -95,9 +95,8 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
 
             LogRecordData.SetActivityContext(ref data, activity);
 
-            var attributes = record.Attributes = this.options.IncludeAttributes
-                ? ProcessState(record, ref iloggerData, in state, this.options.ParseStateValues)
-                : null;
+            var attributes = record.Attributes =
+                ProcessState(record, ref iloggerData, in state, this.options.IncludeAttributes, this.options.ParseStateValues);
 
             if (!TryGetOriginalFormatFromAttributes(attributes, out var originalFormat))
             {
@@ -153,9 +152,15 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData,
         LogRecord logRecord,
         ref LogRecord.LogRecordILoggerData iLoggerData,
         in TState state,
+        bool includeAttributes,
         bool parseStateValues)
     {
-        iLoggerData.State = null;
+        if (!includeAttributes
+            || (!typeof(TState).IsValueType && state is null))
+        {
+            iLoggerData.State = null;
+            return null;
+        }
 
         /* TODO: Enable this if/when LogRecordAttributeList becomes public.
         if (typeof(TState) == typeof(LogRecordAttributeList))
@@ -172,10 +177,20 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData,
         else */
         if (state is IReadOnlyList<KeyValuePair<string, object?>> stateList)
         {
+            // Note: This is to preserve legacy behavior where State is exposed
+            // if we didn't parse state. We use stateList here to prevent a
+            // second boxing of struct TStates.
+            iLoggerData.State = !parseStateValues ? stateList : null;
+
             return stateList;
         }
         else if (state is IEnumerable<KeyValuePair<string, object?>> stateValues)
         {
+            // Note: This is to preserve legacy behavior where State is exposed
+            // if we didn't parse state. We use stateValues here to prevent a
+            // second boxing of struct TStates.
+            iLoggerData.State = !parseStateValues ? stateValues : null;
+
             var attributeStorage = logRecord.AttributeStorage;
             if (attributeStorage == null)
             {
@@ -187,7 +202,7 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData,
                 return attributeStorage;
             }
         }
-        else if (!parseStateValues || state is null)
+        else if (!parseStateValues)
         {
             // Note: This is to preserve legacy behavior where State is
             // exposed if we didn't parse state.
@@ -196,9 +211,13 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData,
         }
         else
         {
+            // Note: We clear State because the LogRecord we are processing may
+            // have come from the pool and may have State from a prior log.
+            iLoggerData.State = null;
+
             try
             {
-                PropertyDescriptorCollection itemProperties = TypeDescriptor.GetProperties(state);
+                PropertyDescriptorCollection itemProperties = TypeDescriptor.GetProperties(state!);
 
                 var attributeStorage = logRecord.AttributeStorage ??= new List<KeyValuePair<string, object?>>(itemProperties.Count);
 

test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs

@@ -65,7 +65,7 @@ public void AddOtlpLogExporterReceivesAttributesWithParseStateValueSetToFalse()
             Assert.Single(logRecords);
             var logRecord = logRecords[0];
 #pragma warning disable CS0618 // Type or member is obsolete
-            Assert.Null(logRecord.State);
+            Assert.NotNull(logRecord.State);
 #pragma warning restore CS0618 // Type or member is obsolete
             Assert.NotNull(logRecord.Attributes);
         }

test/OpenTelemetry.Tests/Logs/LogRecordTest.cs

@@ -82,7 +82,7 @@ public void CheckStateForUnstructuredLog(bool includeFormattedMessage)
             const string message = "Hello, World!";
             logger.LogInformation(message);
 
-            Assert.Null(exportedItems[0].State);
+            Assert.NotNull(exportedItems[0].State);
 
             var attributes = exportedItems[0].Attributes;
             Assert.NotNull(attributes);
@@ -113,7 +113,7 @@ public void CheckStateForUnstructuredLogWithStringInterpolation(bool includeForm
             var message = $"Hello from potato {0.99}.";
             logger.LogInformation(message);
 
-            Assert.Null(exportedItems[0].State);
+            Assert.NotNull(exportedItems[0].State);
 
             var attributes = exportedItems[0].Attributes;
             Assert.NotNull(attributes);
@@ -143,7 +143,7 @@ public void CheckStateForStructuredLogWithTemplate(bool includeFormattedMessage)
             const string message = "Hello from {name} {price}.";
             logger.LogInformation(message, "tomato", 2.99);
 
-            Assert.Null(exportedItems[0].State);
+            Assert.NotNull(exportedItems[0].State);
 
             var attributes = exportedItems[0].Attributes;
             Assert.NotNull(attributes);
@@ -185,7 +185,7 @@ public void CheckStateForStructuredLogWithStrongType(bool includeFormattedMessag
             var food = new Food { Name = "artichoke", Price = 3.99 };
             logger.LogInformation("{food}", food);
 
-            Assert.Null(exportedItems[0].State);
+            Assert.NotNull(exportedItems[0].State);
 
             var attributes = exportedItems[0].Attributes;
             Assert.NotNull(attributes);
@@ -226,7 +226,7 @@ public void CheckStateForStructuredLogWithAnonymousType(bool includeFormattedMes
             var anonymousType = new { Name = "pumpkin", Price = 5.99 };
             logger.LogInformation("{food}", anonymousType);
 
-            Assert.Null(exportedItems[0].State);
+            Assert.NotNull(exportedItems[0].State);
 
             var attributes = exportedItems[0].Attributes;
             Assert.NotNull(attributes);
@@ -271,7 +271,7 @@ public void CheckStateForStructuredLogWithGeneralType(bool includeFormattedMessa
             };
             logger.LogInformation("{food}", food);
 
-            Assert.Null(exportedItems[0].State);
+            Assert.NotNull(exportedItems[0].State);
 
             var attributes = exportedItems[0].Attributes;
             Assert.NotNull(attributes);
@@ -321,7 +321,13 @@ public void CheckStateForExceptionLogged()
             const string message = "Exception Occurred";
             logger.LogInformation(exception, message);
 
-            Assert.Null(exportedItems[0].State);
+            Assert.NotNull(exportedItems[0].State);
+
+            var state = exportedItems[0].State;
+            var itemCount = state.GetType().GetProperty("Count").GetValue(state);
+
+            // state only has {OriginalFormat}
+            Assert.Equal(1, itemCount);
 
             var attributes = exportedItems[0].Attributes;
             Assert.NotNull(attributes);
@@ -334,6 +340,7 @@ public void CheckStateForExceptionLogged()
             Assert.Equal(exceptionMessage, loggedException.Message);
 
             Assert.Equal(message, exportedItems[0].Body);
+            Assert.Equal(message, state.ToString());
             Assert.Null(exportedItems[0].FormattedMessage);
         }
 
@@ -711,7 +718,14 @@ public void VerifyParseStateValues_UsingStandardExtensions(bool parseStateValues
             var logRecord = exportedItems[0];
 
             Assert.NotNull(logRecord.StateValues);
-            Assert.Null(logRecord.State);
+            if (parseStateValues)
+            {
+                Assert.Null(logRecord.State);
+            }
+            else
+            {
+                Assert.NotNull(logRecord.State);
+            }
 
             Assert.NotNull(logRecord.StateValues);
             Assert.Equal(3, logRecord.StateValues.Count);
@@ -725,7 +739,14 @@ public void VerifyParseStateValues_UsingStandardExtensions(bool parseStateValues
             logRecord = exportedItems[1];
 
             Assert.NotNull(logRecord.StateValues);
-            Assert.Null(logRecord.State);
+            if (parseStateValues)
+            {
+                Assert.Null(logRecord.State);
+            }
+            else
+            {
+                Assert.NotNull(logRecord.State);
+            }
 
             Assert.NotNull(logRecord.StateValues);
             Assert.Equal(4, logRecord.StateValues.Count);