diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 5eee0c5b453..b2aa7b15ea9 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -8,6 +8,16 @@ `IReadOnlyList` or `IEnumerable` of `KeyValuePair`s. ([#4609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4609)) +* **Breaking Change** Removed the support for parsing `TState` types passed to + the `ILogger.Log` API when `ParseStateValues` is true and `TState` + does not implement either `IReadOnlyList>` or + `IEnumerable>`. This feature was first introduced + in the `1.5.0` stable release with + [#4334](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4334) and + has been removed because it makes the OpenTelemetry .NET SDK incompatible with + native AOT. + ([#4614](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4614)) + ## 1.5.0 Released 2023-Jun-05 diff --git a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs index a13bc1adba0..db4aff9ce65 100644 --- a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs +++ b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs @@ -158,6 +158,17 @@ public void LoggerProviderException(string methodName, Exception ex) } } + [NonEvent] + public void LoggerProcessStateSkipped() + { + if (this.IsEnabled(EventLevel.Warning, EventKeywords.All)) + { + this.LoggerProcessStateSkipped( + typeof(TState).FullName!, + "because it does not implement a supported interface (either IReadOnlyList> or IEnumerable>)"); + } + } + [Event(4, Message = "Unknown error in SpanProcessor event '{0}': '{1}'.", Level = EventLevel.Error)] public void SpanProcessorException(string evnt, string ex) { @@ -325,6 +336,12 @@ public void LoggerProviderException(string methodName, string ex) this.WriteEvent(50, methodName, ex); } + [Event(51, Message = "Skipped processing log state of type '{0}' {1}.", Level = EventLevel.Warning)] + public void LoggerProcessStateSkipped(string type, string reason) + { + this.WriteEvent(51, type, reason); + } + #if DEBUG public class OpenTelemetryEventListener : EventListener { diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs index e66c548d2bc..9d15e3f1355 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs @@ -16,7 +16,6 @@ #nullable enable -using System.ComponentModel; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; @@ -207,6 +206,7 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData, // Note: This is to preserve legacy behavior where State is // exposed if we didn't parse state. iLoggerData.State = state; + return null; } else @@ -215,36 +215,9 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData, // have come from the pool and may have State from a prior log. iLoggerData.State = null; - try - { - PropertyDescriptorCollection itemProperties = TypeDescriptor.GetProperties(state!); - - var attributeStorage = logRecord.AttributeStorage ??= new List>(itemProperties.Count); - - foreach (PropertyDescriptor? itemProperty in itemProperties) - { - if (itemProperty == null) - { - continue; - } - - object? value = itemProperty.GetValue(state); - if (value == null) - { - continue; - } - - attributeStorage.Add(new KeyValuePair(itemProperty.Name, value)); - } + OpenTelemetrySdkEventSource.Log.LoggerProcessStateSkipped(); - return attributeStorage; - } - catch (Exception parseException) - { - OpenTelemetrySdkEventSource.Log.LoggerParseStateException(parseException); - - return Array.Empty>(); - } + return Array.Empty>(); } } diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerOptions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerOptions.cs index f2a392e52e7..d3ff3681f2a 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerOptions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerOptions.cs @@ -58,13 +58,21 @@ public class OpenTelemetryLoggerOptions /// /// Notes: /// - /// Parsing is only executed when the state logged does NOT - /// implement or As of OpenTelemetry v1.5 state parsing is handled automatically if + /// the state logged implements or where T is KeyValuePair<string, - /// object>. + /// object> than will be set + /// regardless of the value of . /// When is set to will always be . + /// langword="null"/>. When is set to will always be set to + /// the logged state to support legacy exporters which access directly. Exporters should NOT access directly because is NOT safe and may lead to + /// exceptions or incorrect data especially when using batching. Exporters + /// should use to safely access any data + /// attached to log messages. /// /// public bool ParseStateValues { get; set; } diff --git a/test/OpenTelemetry.AotCompatibility.Tests/AotCompatibilityTests.cs b/test/OpenTelemetry.AotCompatibility.Tests/AotCompatibilityTests.cs index 2b9cf77d9c3..0e54f129a10 100644 --- a/test/OpenTelemetry.AotCompatibility.Tests/AotCompatibilityTests.cs +++ b/test/OpenTelemetry.AotCompatibility.Tests/AotCompatibilityTests.cs @@ -85,7 +85,7 @@ public void EnsureAotCompatibility() Assert.True(process.ExitCode == 0, "Publishing the AotCompatibility app failed. See test output for more details."); var warnings = expectedOutput.ToString().Split('\n', '\r').Where(line => line.Contains("warning IL")); - Assert.Equal(43, warnings.Count()); + Assert.Equal(42, warnings.Count()); } } } diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index d9e79fee60d..b5dd80f329d 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -100,7 +100,11 @@ public void AddOtlpLogExporterParseStateValueCanBeTurnedOff(bool parseState) { Assert.Null(logRecord.State); Assert.NotNull(logRecord.Attributes); - Assert.Contains(logRecord.Attributes, kvp => kvp.Key == "propertyA" && (string)kvp.Value == "valueA"); + + // Note: We currently do not support parsing custom states which do + // not implement the standard interfaces. We return empty attributes + // for these. + Assert.Empty(logRecord.Attributes); } else { @@ -140,7 +144,11 @@ public void AddOtlpLogExporterParseStateValueCanBeTurnedOffHosting(bool parseSta { Assert.Null(logRecord.State); Assert.NotNull(logRecord.Attributes); - Assert.Contains(logRecord.Attributes, kvp => kvp.Key == "propertyA" && (string)kvp.Value == "valueA"); + + // Note: We currently do not support parsing custom states which do + // not implement the standard interfaces. We return empty attributes + // for these. + Assert.Empty(logRecord.Attributes); } else { diff --git a/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs b/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs index a9861d03aca..a828e7af63e 100644 --- a/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs +++ b/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs @@ -826,7 +826,7 @@ public void ParseStateValuesUsingIEnumerableTest() } [Fact] - public void ParseStateValuesUsingCustomTest() + public void ParseStateValuesUsingNonconformingCustomTypeTest() { using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.ParseStateValues = true); var logger = loggerFactory.CreateLogger(); @@ -848,12 +848,11 @@ public void ParseStateValuesUsingCustomTest() Assert.Null(logRecord.State); Assert.NotNull(logRecord.StateValues); - Assert.Equal(1, logRecord.StateValues.Count); - - KeyValuePair actualState = logRecord.StateValues[0]; - Assert.Equal("Property", actualState.Key); - Assert.Equal("Value", actualState.Value); + // Note: We currently do not support parsing custom states which do + // not implement the standard interfaces. We return empty attributes + // for these. + Assert.Empty(logRecord.StateValues); } [Fact]