Skip to content

Commit

Permalink
[Group 4] Enable nullable annotations for `Microsoft.Extensions.Loggi…
Browse files Browse the repository at this point in the history
…ng.EventSource` (#66802)

* Annotate src

* Annotate ref

* Make internal parameters non-nullable if they never receive null
  • Loading branch information
maxkoshevoi committed Mar 23, 2022
1 parent 0d4b279 commit 103fb84
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 42 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace Microsoft.Extensions.Logging
{
internal sealed class EventLogFiltersConfigureOptionsChangeSource: IOptionsChangeTokenSource<LoggerFilterOptions>
internal sealed class EventLogFiltersConfigureOptionsChangeSource : IOptionsChangeTokenSource<LoggerFilterOptions>
{
private readonly LoggingEventSource _eventSource;

Expand All @@ -18,6 +18,6 @@ public EventLogFiltersConfigureOptionsChangeSource(LoggingEventSource eventSourc

public IChangeToken GetChangeToken() => _eventSource.GetFilterChangeToken();

public string Name { get; }
public string? Name { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal sealed class EventSourceLogger : ILogger
private readonly LoggingEventSource _eventSource;
private readonly int _factoryID;

public EventSourceLogger(string categoryName, int factoryID, LoggingEventSource eventSource, EventSourceLogger next)
public EventSourceLogger(string categoryName, int factoryID, LoggingEventSource eventSource, EventSourceLogger? next)
{
CategoryName = categoryName;

Expand All @@ -43,20 +43,20 @@ public EventSourceLogger(string categoryName, int factoryID, LoggingEventSource
public LogLevel Level { get; set; }

// Loggers created by a single provider form a linked list
public EventSourceLogger Next { get; }
public EventSourceLogger? Next { get; }

public bool IsEnabled(LogLevel logLevel)
{
return logLevel != LogLevel.None && logLevel >= Level;
}

public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
{
if (!IsEnabled(logLevel))
{
return;
}
string message = null;
string? message = null;

// See if they want the formatted message
if (_eventSource.IsEnabled(EventLevel.Critical, LoggingEventSource.Keywords.FormattedMessage))
Expand All @@ -75,7 +75,7 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
if (_eventSource.IsEnabled(EventLevel.Critical, LoggingEventSource.Keywords.Message))
{
ExceptionInfo exceptionInfo = GetExceptionInfo(exception);
IReadOnlyList<KeyValuePair<string, string>> arguments = GetProperties(state);
IReadOnlyList<KeyValuePair<string, string?>> arguments = GetProperties(state);

_eventSource.Message(
logLevel,
Expand All @@ -94,16 +94,16 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
if (exception != null)
{
ExceptionInfo exceptionInfo = GetExceptionInfo(exception);
KeyValuePair<string, string>[] exceptionInfoData = new[]
KeyValuePair<string, string?>[] exceptionInfoData = new[]
{
new KeyValuePair<string, string>("TypeName", exceptionInfo.TypeName),
new KeyValuePair<string, string>("Message", exceptionInfo.Message),
new KeyValuePair<string, string>("HResult", exceptionInfo.HResult.ToString()),
new KeyValuePair<string, string>("VerboseMessage", exceptionInfo.VerboseMessage),
new KeyValuePair<string, string?>("TypeName", exceptionInfo.TypeName),
new KeyValuePair<string, string?>("Message", exceptionInfo.Message),
new KeyValuePair<string, string?>("HResult", exceptionInfo.HResult.ToString()),
new KeyValuePair<string, string?>("VerboseMessage", exceptionInfo.VerboseMessage),
};
exceptionJson = ToJson(exceptionInfoData);
}
IReadOnlyList<KeyValuePair<string, string>> arguments = GetProperties(state);
IReadOnlyList<KeyValuePair<string, string?>> arguments = GetProperties(state);
message ??= formatter(state, exception);
_eventSource.MessageJson(
logLevel,
Expand All @@ -129,15 +129,15 @@ public IDisposable BeginScope<TState>(TState state)
// If JsonMessage is on, use JSON format
if (_eventSource.IsEnabled(EventLevel.Critical, LoggingEventSource.Keywords.JsonMessage))
{
IReadOnlyList<KeyValuePair<string, string>> arguments = GetProperties(state);
IReadOnlyList<KeyValuePair<string, string?>> arguments = GetProperties(state);
_eventSource.ActivityJsonStart(id, _factoryID, CategoryName, ToJson(arguments));
return new ActivityScope(_eventSource, CategoryName, id, _factoryID, true);
}

if (_eventSource.IsEnabled(EventLevel.Critical, LoggingEventSource.Keywords.Message) ||
_eventSource.IsEnabled(EventLevel.Critical, LoggingEventSource.Keywords.FormattedMessage))
{
IReadOnlyList<KeyValuePair<string, string>> arguments = GetProperties(state);
IReadOnlyList<KeyValuePair<string, string?>> arguments = GetProperties(state);
_eventSource.ActivityStart(id, _factoryID, CategoryName, arguments);
return new ActivityScope(_eventSource, CategoryName, id, _factoryID, false);
}
Expand Down Expand Up @@ -185,37 +185,37 @@ public void Dispose()
/// <param name="exception">The exception to get information for.</param>
/// <returns>ExceptionInfo object represending a .NET Exception</returns>
/// <remarks>ETW does not support a concept of a null value. So we use an un-initialized object if there is no exception in the event data.</remarks>
private ExceptionInfo GetExceptionInfo(Exception exception)
private ExceptionInfo GetExceptionInfo(Exception? exception)
{
return exception != null ? new ExceptionInfo(exception) : ExceptionInfo.Empty;
}

/// <summary>
/// Converts an ILogger state object into a set of key-value pairs (That can be send to a EventSource)
/// </summary>
private IReadOnlyList<KeyValuePair<string, string>> GetProperties(object state)
private IReadOnlyList<KeyValuePair<string, string?>> GetProperties(object? state)
{
if (state is IReadOnlyList<KeyValuePair<string, object>> keyValuePairs)
if (state is IReadOnlyList<KeyValuePair<string, object?>> keyValuePairs)
{
var arguments = new KeyValuePair<string, string>[keyValuePairs.Count];
var arguments = new KeyValuePair<string, string?>[keyValuePairs.Count];
for (int i = 0; i < keyValuePairs.Count; i++)
{
KeyValuePair<string, object> keyValuePair = keyValuePairs[i];
arguments[i] = new KeyValuePair<string, string>(keyValuePair.Key, keyValuePair.Value?.ToString());
KeyValuePair<string, object?> keyValuePair = keyValuePairs[i];
arguments[i] = new KeyValuePair<string, string?>(keyValuePair.Key, keyValuePair.Value?.ToString());
}
return arguments;
}

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

private string ToJson(IReadOnlyList<KeyValuePair<string, string>> keyValues)
private string ToJson(IReadOnlyList<KeyValuePair<string, string?>> keyValues)
{
using var stream = new MemoryStream();
using var writer = new Utf8JsonWriter(stream);

writer.WriteStartObject();
foreach (KeyValuePair<string, string> keyValue in keyValues)
foreach (KeyValuePair<string, string?> keyValue in keyValues)
{
writer.WriteString(keyValue.Key, keyValue.Value);
}
Expand All @@ -228,7 +228,7 @@ private string ToJson(IReadOnlyList<KeyValuePair<string, string>> keyValues)
buffer = new ArraySegment<byte>(stream.ToArray());
}

return Encoding.UTF8.GetString(buffer.Array, buffer.Offset, buffer.Count);
return Encoding.UTF8.GetString(buffer.Array!, buffer.Offset, buffer.Count);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class EventSourceLoggerProvider : ILoggerProvider
// A small integer that uniquely identifies the LoggerFactory associated with this LoggingProvider.
private readonly int _factoryID;

private EventSourceLogger _loggers; // Linked list of loggers that I have created
private EventSourceLogger? _loggers; // Linked list of loggers that I have created
private readonly LoggingEventSource _eventSource;

public EventSourceLoggerProvider(LoggingEventSource eventSource!!)
Expand All @@ -36,7 +36,7 @@ public ILogger CreateLogger(string categoryName)
public void Dispose()
{
// Turn off any logging
for (EventSourceLogger logger = _loggers; logger != null; logger = logger.Next)
for (EventSourceLogger? logger = _loggers; logger != null; logger = logger.Next)
{
logger.Level = LogLevel.None;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace Microsoft.Extensions.Logging.EventSource
/// <summary>
/// Represents information about exceptions that is captured by EventSourceLogger
/// </summary>
[System.Diagnostics.Tracing.EventData(Name ="ExceptionInfo")]
[System.Diagnostics.Tracing.EventData(Name = "ExceptionInfo")]
internal sealed class ExceptionInfo
{
public static ExceptionInfo Empty { get; } = new ExceptionInfo();
Expand All @@ -25,9 +25,9 @@ public ExceptionInfo(Exception exception)
VerboseMessage = exception.ToString();
}

public string TypeName { get; }
public string Message { get; }
public string? TypeName { get; }
public string? Message { get; }
public int HResult { get; }
public string VerboseMessage { get; } // This is the ToString() of the Exception
public string? VerboseMessage { get; } // This is the ToString() of the Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public static class Keywords
// base ctor might call OnEventCommand and set filter spec
// having assignment in ctor would overwrite the value
private LoggerFilterRule[] _filterSpec = Array.Empty<LoggerFilterRule>();
private CancellationTokenSource _cancellationTokenSource;
private CancellationTokenSource? _cancellationTokenSource;
private const string UseAppFilters = "UseAppFilters";
private const string WriteEventCoreSuppressionJustification = "WriteEventCore is safe when eventData object is a primitive type which is in this case.";
private const string WriteEventDynamicDependencySuppressionJustification = "DynamicDependency attribute will ensure that the required properties are not trimmed.";
Expand All @@ -129,7 +129,7 @@ private LoggingEventSource() : base(EventSourceSettings.EtwSelfDescribingEventFo
[Event(1, Keywords = Keywords.FormattedMessage, Level = EventLevel.LogAlways)]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = WriteEventCoreSuppressionJustification)]
internal unsafe void FormattedMessage(LogLevel Level, int FactoryID, string LoggerName, int EventId, string EventName, string FormattedMessage)
internal unsafe void FormattedMessage(LogLevel Level, int FactoryID, string LoggerName, int EventId, string? EventName, string FormattedMessage)
{
if (IsEnabled())
{
Expand Down Expand Up @@ -164,7 +164,7 @@ internal unsafe void FormattedMessage(LogLevel Level, int FactoryID, string Logg
[DynamicDependency(DynamicallyAccessedMemberTypes.PublicProperties, typeof(KeyValuePair<string, string>))]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = WriteEventDynamicDependencySuppressionJustification)]
internal void Message(LogLevel Level, int FactoryID, string LoggerName, int EventId, string EventName, ExceptionInfo Exception, IEnumerable<KeyValuePair<string, string>> Arguments)
internal void Message(LogLevel Level, int FactoryID, string LoggerName, int EventId, string? EventName, ExceptionInfo Exception, IEnumerable<KeyValuePair<string, string?>> Arguments)
{
if (IsEnabled())
{
Expand All @@ -179,7 +179,7 @@ internal void Message(LogLevel Level, int FactoryID, string LoggerName, int Even
[DynamicDependency(DynamicallyAccessedMemberTypes.PublicProperties, typeof(KeyValuePair<string, string>))]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = WriteEventDynamicDependencySuppressionJustification)]
internal void ActivityStart(int ID, int FactoryID, string LoggerName, IEnumerable<KeyValuePair<string, string>> Arguments)
internal void ActivityStart(int ID, int FactoryID, string LoggerName, IEnumerable<KeyValuePair<string, string?>> Arguments)
{
if (IsEnabled())
{
Expand Down Expand Up @@ -213,7 +213,7 @@ internal unsafe void ActivityStop(int ID, int FactoryID, string LoggerName)
[Event(5, Keywords = Keywords.JsonMessage, Level = EventLevel.LogAlways)]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = WriteEventCoreSuppressionJustification)]
internal unsafe void MessageJson(LogLevel Level, int FactoryID, string LoggerName, int EventId, string EventName, string ExceptionJson, string ArgumentsJson, string FormattedMessage)
internal unsafe void MessageJson(LogLevel Level, int FactoryID, string LoggerName, int EventId, string? EventName, string ExceptionJson, string ArgumentsJson, string FormattedMessage)
{
if (IsEnabled())
{
Expand Down Expand Up @@ -298,9 +298,9 @@ internal unsafe void ActivityJsonStop(int ID, int FactoryID, string LoggerName)
/// <inheritdoc />
protected override void OnEventCommand(EventCommandEventArgs command)
{
if (command.Command == EventCommand.Update || command.Command == EventCommand.Enable)
if (command.Command is EventCommand.Update or EventCommand.Enable)
{
if (!command.Arguments.TryGetValue("FilterSpecs", out string filterSpec))
if (!command.Arguments!.TryGetValue("FilterSpecs", out string? filterSpec))
{
filterSpec = string.Empty; // This means turn on everything.
}
Expand All @@ -318,7 +318,7 @@ protected override void OnEventCommand(EventCommandEventArgs command)
/// </summary>
/// <param name="filterSpec">The filter specification to set.</param>
[NonEvent]
private void SetFilterSpec(string filterSpec)
private void SetFilterSpec(string? filterSpec)
{
_filterSpec = ParseFilterSpec(filterSpec, GetDefaultLevel());

Expand All @@ -335,7 +335,7 @@ internal IChangeToken GetFilterChangeToken()
[NonEvent]
private void FireChangeToken()
{
CancellationTokenSource tcs = Interlocked.Exchange(ref _cancellationTokenSource, null);
CancellationTokenSource? tcs = Interlocked.Exchange(ref _cancellationTokenSource, null);
tcs?.Cancel();
}

Expand All @@ -351,7 +351,7 @@ private void FireChangeToken()
/// The first specification that 'loggers' Name matches is used.
/// </summary>
[NonEvent]
private static LoggerFilterRule[] ParseFilterSpec(string filterSpec, LogLevel defaultLevel)
private static LoggerFilterRule[] ParseFilterSpec(string? filterSpec, LogLevel defaultLevel)
{
if (filterSpec == string.Empty)
{
Expand Down Expand Up @@ -501,7 +501,7 @@ private static unsafe void SetEventData<T>(ref EventData eventData, ref T value,
{
if (typeof(T) == typeof(string))
{
string str = value as string;
string str = (value as string)!;
#if DEBUG
fixed (char* rePinnedString = str)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>enable</Nullable>
<EnableDefaultItems>true</EnableDefaultItems>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- Use targeting pack references instead of granular ones in the project file. -->
Expand Down

0 comments on commit 103fb84

Please sign in to comment.