Skip to content

Commit ae03840

Browse files
CodeBlanchCodeBlanch
authored
[sdk-1.5.0-hotfix] Fix LogRecord.State being null when TState matches known interfaces (#4609)
1 parent 494323b commit ae03840

File tree

4 files changed

+62
-16
lines changed

4 files changed

+62
-16
lines changed

‎src/OpenTelemetry/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22

33
## Unreleased
44

5+
* Fixed a breaking change causing `LogRecord.State` to be `null` where it was
6+
previously set to a valid value when
7+
`OpenTelemetryLoggerOptions.ParseStateValues` is `false` and states implement
8+
`IReadOnlyList` or `IEnumerable` of `KeyValuePair<string, object>`s.
9+
([#4609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4609))
10+
511
## 1.5.0
612

713
Released 2023-Jun-05

‎src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,8 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
9595

9696
LogRecordData.SetActivityContext(ref data, activity);
9797

98-
var attributes = record.Attributes = this.options.IncludeAttributes
99-
? ProcessState(record, ref iloggerData, in state, this.options.ParseStateValues)
100-
: null;
98+
var attributes = record.Attributes =
99+
ProcessState(record, ref iloggerData, in state, this.options.IncludeAttributes, this.options.ParseStateValues);
101100

102101
if (!TryGetOriginalFormatFromAttributes(attributes, out var originalFormat))
103102
{
@@ -153,9 +152,15 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData,
153152
LogRecord logRecord,
154153
ref LogRecord.LogRecordILoggerData iLoggerData,
155154
in TState state,
155+
bool includeAttributes,
156156
bool parseStateValues)
157157
{
158-
iLoggerData.State = null;
158+
if (!includeAttributes
159+
|| (!typeof(TState).IsValueType && state is null))
160+
{
161+
iLoggerData.State = null;
162+
return null;
163+
}
159164

160165
/* TODO: Enable this if/when LogRecordAttributeList becomes public.
161166
if (typeof(TState) == typeof(LogRecordAttributeList))
@@ -172,10 +177,20 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData,
172177
else */
173178
if (state is IReadOnlyList<KeyValuePair<string, object?>> stateList)
174179
{
180+
// Note: This is to preserve legacy behavior where State is exposed
181+
// if we didn't parse state. We use stateList here to prevent a
182+
// second boxing of struct TStates.
183+
iLoggerData.State = !parseStateValues ? stateList : null;
184+
175185
return stateList;
176186
}
177187
else if (state is IEnumerable<KeyValuePair<string, object?>> stateValues)
178188
{
189+
// Note: This is to preserve legacy behavior where State is exposed
190+
// if we didn't parse state. We use stateValues here to prevent a
191+
// second boxing of struct TStates.
192+
iLoggerData.State = !parseStateValues ? stateValues : null;
193+
179194
var attributeStorage = logRecord.AttributeStorage;
180195
if (attributeStorage == null)
181196
{
@@ -187,7 +202,7 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData,
187202
return attributeStorage;
188203
}
189204
}
190-
else if (!parseStateValues || state is null)
205+
else if (!parseStateValues)
191206
{
192207
// Note: This is to preserve legacy behavior where State is
193208
// exposed if we didn't parse state.
@@ -196,9 +211,13 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData,
196211
}
197212
else
198213
{
214+
// Note: We clear State because the LogRecord we are processing may
215+
// have come from the pool and may have State from a prior log.
216+
iLoggerData.State = null;
217+
199218
try
200219
{
201-
PropertyDescriptorCollection itemProperties = TypeDescriptor.GetProperties(state);
220+
PropertyDescriptorCollection itemProperties = TypeDescriptor.GetProperties(state!);
202221

203222
var attributeStorage = logRecord.AttributeStorage ??= new List<KeyValuePair<string, object?>>(itemProperties.Count);
204223

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public void AddOtlpLogExporterReceivesAttributesWithParseStateValueSetToFalse()
6565
Assert.Single(logRecords);
6666
var logRecord = logRecords[0];
6767
#pragma warning disable CS0618 // Type or member is obsolete
68-
Assert.Null(logRecord.State);
68+
Assert.NotNull(logRecord.State);
6969
#pragma warning restore CS0618 // Type or member is obsolete
7070
Assert.NotNull(logRecord.Attributes);
7171
}

‎test/OpenTelemetry.Tests/Logs/LogRecordTest.cs

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public void CheckStateForUnstructuredLog(bool includeFormattedMessage)
8282
const string message = "Hello, World!";
8383
logger.LogInformation(message);
8484

85-
Assert.Null(exportedItems[0].State);
85+
Assert.NotNull(exportedItems[0].State);
8686

8787
var attributes = exportedItems[0].Attributes;
8888
Assert.NotNull(attributes);
@@ -113,7 +113,7 @@ public void CheckStateForUnstructuredLogWithStringInterpolation(bool includeForm
113113
var message = $"Hello from potato {0.99}.";
114114
logger.LogInformation(message);
115115

116-
Assert.Null(exportedItems[0].State);
116+
Assert.NotNull(exportedItems[0].State);
117117

118118
var attributes = exportedItems[0].Attributes;
119119
Assert.NotNull(attributes);
@@ -143,7 +143,7 @@ public void CheckStateForStructuredLogWithTemplate(bool includeFormattedMessage)
143143
const string message = "Hello from {name} {price}.";
144144
logger.LogInformation(message, "tomato", 2.99);
145145

146-
Assert.Null(exportedItems[0].State);
146+
Assert.NotNull(exportedItems[0].State);
147147

148148
var attributes = exportedItems[0].Attributes;
149149
Assert.NotNull(attributes);
@@ -185,7 +185,7 @@ public void CheckStateForStructuredLogWithStrongType(bool includeFormattedMessag
185185
var food = new Food { Name = "artichoke", Price = 3.99 };
186186
logger.LogInformation("{food}", food);
187187

188-
Assert.Null(exportedItems[0].State);
188+
Assert.NotNull(exportedItems[0].State);
189189

190190
var attributes = exportedItems[0].Attributes;
191191
Assert.NotNull(attributes);
@@ -226,7 +226,7 @@ public void CheckStateForStructuredLogWithAnonymousType(bool includeFormattedMes
226226
var anonymousType = new { Name = "pumpkin", Price = 5.99 };
227227
logger.LogInformation("{food}", anonymousType);
228228

229-
Assert.Null(exportedItems[0].State);
229+
Assert.NotNull(exportedItems[0].State);
230230

231231
var attributes = exportedItems[0].Attributes;
232232
Assert.NotNull(attributes);
@@ -271,7 +271,7 @@ public void CheckStateForStructuredLogWithGeneralType(bool includeFormattedMessa
271271
};
272272
logger.LogInformation("{food}", food);
273273

274-
Assert.Null(exportedItems[0].State);
274+
Assert.NotNull(exportedItems[0].State);
275275

276276
var attributes = exportedItems[0].Attributes;
277277
Assert.NotNull(attributes);
@@ -321,7 +321,13 @@ public void CheckStateForExceptionLogged()
321321
const string message = "Exception Occurred";
322322
logger.LogInformation(exception, message);
323323

324-
Assert.Null(exportedItems[0].State);
324+
Assert.NotNull(exportedItems[0].State);
325+
326+
var state = exportedItems[0].State;
327+
var itemCount = state.GetType().GetProperty("Count").GetValue(state);
328+
329+
// state only has {OriginalFormat}
330+
Assert.Equal(1, itemCount);
325331

326332
var attributes = exportedItems[0].Attributes;
327333
Assert.NotNull(attributes);
@@ -334,6 +340,7 @@ public void CheckStateForExceptionLogged()
334340
Assert.Equal(exceptionMessage, loggedException.Message);
335341

336342
Assert.Equal(message, exportedItems[0].Body);
343+
Assert.Equal(message, state.ToString());
337344
Assert.Null(exportedItems[0].FormattedMessage);
338345
}
339346

@@ -711,7 +718,14 @@ public void VerifyParseStateValues_UsingStandardExtensions(bool parseStateValues
711718
var logRecord = exportedItems[0];
712719

713720
Assert.NotNull(logRecord.StateValues);
714-
Assert.Null(logRecord.State);
721+
if (parseStateValues)
722+
{
723+
Assert.Null(logRecord.State);
724+
}
725+
else
726+
{
727+
Assert.NotNull(logRecord.State);
728+
}
715729

716730
Assert.NotNull(logRecord.StateValues);
717731
Assert.Equal(3, logRecord.StateValues.Count);
@@ -725,7 +739,14 @@ public void VerifyParseStateValues_UsingStandardExtensions(bool parseStateValues
725739
logRecord = exportedItems[1];
726740

727741
Assert.NotNull(logRecord.StateValues);
728-
Assert.Null(logRecord.State);
742+
if (parseStateValues)
743+
{
744+
Assert.Null(logRecord.State);
745+
}
746+
else
747+
{
748+
Assert.NotNull(logRecord.State);
749+
}
729750

730751
Assert.NotNull(logRecord.StateValues);
731752
Assert.Equal(4, logRecord.StateValues.Count);

0 commit comments

Comments
 (0)