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-1.5.0-hotfix] Removed support for parsing custom log states using reflection #4614

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 10 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@
`IReadOnlyList` or `IEnumerable` of `KeyValuePair<string, object>`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<TState>` API when `ParseStateValues` is true and `TState`
does not implement either `IReadOnlyList<KeyValuePair<string, object>>` or
`IEnumerable<KeyValuePair<string, object>>`. 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.
([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX))

## 1.5.0

Released 2023-Jun-05
Expand Down
17 changes: 17 additions & 0 deletions src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,17 @@ public void LoggerProviderException(string methodName, Exception ex)
}
}

[NonEvent]
public void LoggerProcessStateSkipped<TState>()
{
if (this.IsEnabled(EventLevel.Warning, EventKeywords.All))
{
this.LoggerProcessStateSkipped(
typeof(TState).FullName!,
"because it does not implement either IReadOnlyList<KeyValuePair<string, object>> or IEnumerable<KeyValuePair<string, object>>");
utpilla marked this conversation as resolved.
Show resolved Hide resolved
}
}

[Event(4, Message = "Unknown error in SpanProcessor event '{0}': '{1}'.", Level = EventLevel.Error)]
public void SpanProcessorException(string evnt, string ex)
{
Expand Down Expand Up @@ -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
{
Expand Down
33 changes: 3 additions & 30 deletions src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

#nullable enable

using System.ComponentModel;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -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
Expand All @@ -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<KeyValuePair<string, object?>>(itemProperties.Count);

foreach (PropertyDescriptor? itemProperty in itemProperties)
{
if (itemProperty == null)
{
continue;
}

object? value = itemProperty.GetValue(state);
if (value == null)
{
continue;
}

attributeStorage.Add(new KeyValuePair<string, object?>(itemProperty.Name, value));
}
OpenTelemetrySdkEventSource.Log.LoggerProcessStateSkipped<TState>();

return attributeStorage;
}
catch (Exception parseException)
{
OpenTelemetrySdkEventSource.Log.LoggerParseStateException<TState>(parseException);

return Array.Empty<KeyValuePair<string, object?>>();
}
return Array.Empty<KeyValuePair<string, object?>>();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down
11 changes: 5 additions & 6 deletions test/OpenTelemetry.Tests/Logs/LogRecordTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,7 @@ public void ParseStateValuesUsingIEnumerableTest()
}

[Fact]
public void ParseStateValuesUsingCustomTest()
public void ParseStateValuesUsingNonconformingCustomTypeTest()
{
using var loggerFactory = InitializeLoggerFactory(out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);
var logger = loggerFactory.CreateLogger<LogRecordTest>();
Expand All @@ -848,12 +848,11 @@ public void ParseStateValuesUsingCustomTest()

Assert.Null(logRecord.State);
Assert.NotNull(logRecord.StateValues);
Assert.Equal(1, logRecord.StateValues.Count);

KeyValuePair<string, object> 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]
Expand Down