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

[AOT] Removed support for ParseStateValues if the state neither implements IReadOnlyList nor IEnumberable when ParseStateValues is true. #4560

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

## Unreleased

* **Breaking Change** Removed 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 was done to make logging AOT
compatible. This feature was first introduced in `1.5.0` stable release with
[#4334](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4334).
([#4560](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4560))

* Add back support for Exemplars. See [exemplars](../../docs/metrics/customizing-the-sdk/README.md#exemplars)
for instructions to enable exemplars.
([#4553](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4553))
Expand Down
24 changes: 12 additions & 12 deletions src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,20 +141,20 @@ public void DroppedExportProcessorItems(string exportProcessorName, string expor
}

[NonEvent]
public void LoggerParseStateException<TState>(Exception exception)
public void LoggerProviderException(string methodName, Exception ex)
{
if (this.IsEnabled(EventLevel.Warning, EventKeywords.All))
if (this.IsEnabled(EventLevel.Error, EventKeywords.All))
{
this.LoggerParseStateException(typeof(TState).FullName!, exception.ToInvariantString());
this.LoggerProviderException(methodName, ex.ToInvariantString());
}
}

[NonEvent]
public void LoggerProviderException(string methodName, Exception ex)
public void LoggerProcessStateSkipped<TState>()
{
if (this.IsEnabled(EventLevel.Error, EventKeywords.All))
if (this.IsEnabled(EventLevel.Warning, EventKeywords.All))
{
this.LoggerProviderException(methodName, ex.ToInvariantString());
this.LoggerProcessStateSkipped(typeof(TState).FullName!, "This can be fixed by updating the state to be a type that implements either IReadOnlyList<KeyValuePair<string, object>> or IEnumerable<KeyValuePair<string, object>>.");
}
}

Expand Down Expand Up @@ -301,12 +301,6 @@ public void InvalidEnvironmentVariable(string key, string? value)
this.WriteEvent(47, key, value);
}

[Event(48, Message = "Exception thrown parsing log state of type '{0}'. Exception: '{1}'", Level = EventLevel.Warning)]
public void LoggerParseStateException(string type, string error)
{
this.WriteEvent(48, type, error);
}

[Event(49, Message = "LoggerProviderSdk event: '{0}'", Level = EventLevel.Verbose)]
public void LoggerProviderSdkEvent(string message)
{
Expand All @@ -319,6 +313,12 @@ public void LoggerProviderException(string methodName, string ex)
this.WriteEvent(50, methodName, ex);
}

[Event(51, Message = "Skip processing log state of type '{0}' because it does not implement either IReadOnlyList<KeyValuePair<string, object>> or IEnumerable<KeyValuePair<string, object>>. Suggested action: '{1}'.", Level = EventLevel.Warning)]
public void LoggerProcessStateSkipped(string type, string fix)
{
this.WriteEvent(51, type, fix);
}

#if DEBUG
public class OpenTelemetryEventListener : EventListener
{
Expand Down
33 changes: 2 additions & 31 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 @@ -194,36 +193,8 @@ internal static void SetLogRecordSeverityFields(ref LogRecordData logRecordData,
}
else
{
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));
}

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

return Array.Empty<KeyValuePair<string, object?>>();
}
OpenTelemetrySdkEventSource.Log.LoggerProcessStateSkipped<TState>();
Yun-Ting marked this conversation as resolved.
Show resolved Hide resolved
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(37, warnings.Count());
Assert.Equal(36, warnings.Count());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ 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");
}
else
{
Expand Down Expand Up @@ -141,7 +140,6 @@ 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");
}
else
{
Expand Down
31 changes: 0 additions & 31 deletions test/OpenTelemetry.Tests/Logs/LogRecordTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -804,37 +804,6 @@ public void ParseStateValuesUsingIEnumerableTest()
Assert.Equal(new KeyValuePair<string, object>("Key1", "Value1"), logRecord.StateValues[0]);
}

[Fact]
public void ParseStateValuesUsingCustomTest()
{
using var loggerFactory = InitializeLoggerFactory(out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);
var logger = loggerFactory.CreateLogger<LogRecordTest>();

// Tests unknown state parse path.

CustomState state = new CustomState
{
Property = "Value",
};

logger.Log(
LogLevel.Information,
0,
state,
null,
(s, e) => "OpenTelemetry!");
var logRecord = exportedItems[0];

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);
}

[Fact]
public void DisposingStateTest()
{
Expand Down