Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Make AddFilter chainable, add some doc comments, add UseConfiguration… #597

Merged
merged 4 commits into from
Apr 25, 2017
Merged

Make AddFilter chainable, add some doc comments, add UseConfiguration… #597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4cc77e9
Make AddFilter chainable, add some doc comments, add UseConfiguration…
BrennanConroy Apr 11, 2017
87fafde
feedback
BrennanConroy Apr 24, 2017
1bbc30d
fb
BrennanConroy Apr 25, 2017
8c0cde4
fb
BrennanConroy Apr 25, 2017
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
17 changes: 9 additions & 8 deletions samples/SampleApp/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@ public Program()
.AddJsonFile("logging.json", optional: false, reloadOnChange: true)
.Build();

// A dependency injection based application would get ILoggerFactory injected instead.
// A Web App based program would configure logging via the WebHostBuilder.
// Create a logger factory with filters that can be applied across all logger providers.
var factory = new LoggerFactory(loggingConfiguration.GetSection("Logging"));
factory.AddFilter(new Dictionary<string, LogLevel>
{
{ "Microsoft", LogLevel.Warning },
{ "System", LogLevel.Warning },
{ "SampleApp.Program", LogLevel.Debug }
});
var factory = new LoggerFactory()
.UseConfiguration(loggingConfiguration.GetSection("Logging"))
Copy link
Member

@glennc glennc Apr 12, 2017

Choose a reason for hiding this comment

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

Not sure of this name. It kind of implies that you are configuring the factory, like adding providers, instead of just filtering.

.AddFilter(new Dictionary<string, LogLevel>
{
{ "Microsoft", LogLevel.Warning },
{ "System", LogLevel.Warning },
{ "SampleApp.Program", LogLevel.Debug }
});

// providers may be added to a LoggerFactory before any loggers are created
#if NET46
Expand Down
265 changes: 121 additions & 144 deletions src/Microsoft.Extensions.Logging/LoggerFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ public class LoggerFactory : ILoggerFactory
private KeyValuePair<ILoggerProvider, string>[] _providers = new KeyValuePair<ILoggerProvider, string>[0];
private readonly object _sync = new object();
private volatile bool _disposed;
private readonly IConfiguration _configuration;
private IConfiguration _configuration;
private IChangeToken _changeToken;
private IDisposable _changeTokenRegistration;
private Dictionary<string, LogLevel> _defaultFilter;
private Func<string, string, LogLevel, bool> _genericFilters;
private Dictionary<string, Func<string, LogLevel, bool>> _providerFilters = new Dictionary<string, Func<string, LogLevel, bool>>();
Expand All @@ -41,11 +42,40 @@ public LoggerFactory(IConfiguration configuration)
throw new ArgumentNullException(nameof(configuration));
}

UseConfiguration(configuration);
}

/// <summary>
/// Replaces the <see cref="IConfiguration"/> used for filtering.
Copy link
Contributor

@Eilon Eilon Apr 19, 2017

Choose a reason for hiding this comment

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

I'm a bit late on the design here, but why does this replace the configuration used for filtering? What if I have 2 sets of config that I want to use from 2 different sections? This seems to me like it should all be additive, always.

Copy link
Member Author

@BrennanConroy BrennanConroy Apr 19, 2017

Choose a reason for hiding this comment

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

I think usage will mostly be factory.UseConfiguration(config.GetSection("Logging"))
with the config having:

{
  "OtherStuff": {
  },
  "Logging": {
    "LogLevel": {
        "System": "Trace"
    }
  }
}

When would you want to use multiple sections?

Copy link
Contributor

@Eilon Eilon Apr 19, 2017

Choose a reason for hiding this comment

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

Why would we specifically disallow it?

Copy link
Contributor

@Eilon Eilon Apr 19, 2017

Choose a reason for hiding this comment

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

I'm ok punting on this now; let's log a bug to track this discussion.

/// </summary>
/// <param name="configuration">The new configuration to use.</param>
/// <returns>The <see cref="LoggerFactory"/> so that additional calls can be chained.</returns>
public LoggerFactory UseConfiguration(IConfiguration configuration)
{
if (configuration == _configuration)
{
return this;
}

// unregister the previous configuration callback if there was one
_changeTokenRegistration?.Dispose();

_configuration = configuration;
_changeToken = configuration.GetReloadToken();
_changeToken.RegisterChangeCallback(OnConfigurationReload, null);

if (configuration == null)
{
_changeToken = null;
_changeTokenRegistration = null;
}
else
{
_changeToken = _configuration.GetReloadToken();
_changeTokenRegistration = _changeToken?.RegisterChangeCallback(OnConfigurationReload, null);
}

LoadDefaultConfigValues();

return this;
}

public ILogger CreateLogger(string categoryName)
Expand Down Expand Up @@ -132,8 +162,22 @@ public void AddProvider(string providerName, ILoggerProvider provider)
}
}

public void AddFilter(string providerName, string categoryName, Func<LogLevel, bool> filter)
/// <summary>
/// Adds a filter that applies to <paramref name="providerName"/> and <paramref name="categoryName"/> with the given
/// <paramref name="filter"/>.
/// </summary>
/// <param name="providerName">The name of the provider.</param>
/// <param name="categoryName">The name of the logger category.</param>
/// <param name="filter">The filter that applies to logs for <paramref name="providerName"/> and <paramref name="categoryName"/>.
/// Returning true means allow log through, false means reject log.</param>
/// <returns>The <see cref="LoggerFactory"/> so that additional calls can be chained.</returns>
public LoggerFactory AddFilter(string providerName, string categoryName, Func<LogLevel, bool> filter)
{
if (filter == null)
{
throw new ArgumentNullException(nameof(filter));
}

lock (_sync)
{
if (_categoryFilters.TryGetValue(categoryName, out var previousFilter))
Expand Down Expand Up @@ -166,46 +210,25 @@ public void AddFilter(string providerName, string categoryName, Func<LogLevel, b
};
}
}

return this;
}

public void AddFilter(string providerName, string categoryName, LogLevel minLevel)
/// <summary>
/// Adds a filter that applies to <paramref name="providerName"/> with the given <paramref name="filter"/>.
/// </summary>
/// <param name="providerName">The name of the provider.</param>
/// <param name="filter">The filter that applies to logs for <paramref name="providerName"/>.
/// The string argument is the category being logged to.
/// Returning true means allow log through, false means reject log.</param>
/// <returns>The <see cref="LoggerFactory"/> so that additional calls can be chained.</returns>
public LoggerFactory AddFilter(string providerName, Func<string, LogLevel, bool> filter)
{
lock (_sync)
if (filter == null)
{
if (_categoryFilters.TryGetValue(categoryName, out var previousFilter))
{
_categoryFilters[categoryName] = (currentProviderName, level) =>
{
if (previousFilter(currentProviderName, level))
{
if (string.Equals(providerName, currentProviderName))
{
return level >= minLevel;
}

return true;
}

return false;
};
}
else
{
_categoryFilters[categoryName] = (currentProviderName, level) =>
{
if (string.Equals(providerName, currentProviderName))
{
return level >= minLevel;
}

return true;
};
}
throw new ArgumentNullException(nameof(filter));
}
}

public void AddFilter(string providerName, Func<string, LogLevel, bool> filter)
{
lock (_sync)
{
if (_providerFilters.TryGetValue(providerName, out var value))
Expand All @@ -225,46 +248,24 @@ public void AddFilter(string providerName, Func<string, LogLevel, bool> filter)
_providerFilters[providerName] = (category, level) => filter(category, level);
}
}

return this;
}

public void AddFilter(string providerName, Func<LogLevel, bool> filter)
/// <summary>
/// Adds a filter that applies to all logs.
/// </summary>
/// <param name="filter">The filter that applies to logs.
/// The first string is the provider name and the second string is the category name being logged to.
/// Returning true means allow log through, false means reject log.</param>
/// <returns>The <see cref="LoggerFactory"/> so that additional calls can be chained.</returns>
public LoggerFactory AddFilter(Func<string, string, LogLevel, bool> filter)
{
lock (_sync)
if (filter == null)
{
if (_categoryFilters.TryGetValue("Default", out var value))
{
_categoryFilters["Default"] = (currentProviderName, level) =>
{
if (value(currentProviderName, level))
{
if (string.Equals(providerName, currentProviderName))
{
return filter(level);
}

return true;
}

return false;
};
}
else
{
_categoryFilters["Default"] = (currentProviderName, level) =>
{
if (string.Equals(providerName, currentProviderName))
{
return filter(level);
}

return true;
};
}
throw new ArgumentNullException(nameof(filter));
}
}

public void AddFilter(Func<string, string, LogLevel, bool> filter)
{
lock (_sync)
{
var previousFilters = _genericFilters;
Expand All @@ -278,10 +279,23 @@ public void AddFilter(Func<string, string, LogLevel, bool> filter)
return false;
};
}

return this;
}

public void AddFilter(IDictionary<string, LogLevel> filter)
/// <summary>
/// Adds a filter to all logs.
/// </summary>
/// <param name="filter">The filter that applies to logs.
/// The key is the category and the <see cref="LogLevel"/> is the minimum level allowed.</param>
/// <returns>The <see cref="LoggerFactory"/> so that additional calls can be chained.</returns>
public LoggerFactory AddFilter(IDictionary<string, LogLevel> filter)
{
if (filter == null)
{
throw new ArgumentNullException(nameof(filter));
}

lock (_sync)
{
foreach (var kvp in filter)
Expand All @@ -304,84 +318,39 @@ public void AddFilter(IDictionary<string, LogLevel> filter)
}
}
}

return this;
}

public void AddFilter(string providerName, IDictionary<string, LogLevel> filter)
/// <summary>
/// Adds a filter that applies to <paramref name="providerName"/> and <paramref name="categoryName"/>, allowing logs with the given
/// minimum <see cref="LogLevel"/> or higher.
/// </summary>
/// <param name="providerName">The name of the provider.</param>
/// <param name="categoryName">The name of the logger category.</param>
/// <param name="minLevel">The minimum <see cref="LogLevel"/> that logs from
/// <paramref name="providerName"/> and <paramref name="categoryName"/> are allowed.</param>
public LoggerFactory AddFilter(string providerName, string categoryName, LogLevel minLevel)
{
lock (_sync)
{
foreach (var kvp in filter)
{
if (_categoryFilters.TryGetValue(kvp.Key, out var currentFilter))
{
_categoryFilters[kvp.Key] = (currentProviderName, level) =>
{
if (currentFilter(currentProviderName, level))
{
if (string.Equals(providerName, currentProviderName))
{
return level >= kvp.Value;
}

return true;
}

return false;
};
}
else
{
_categoryFilters[kvp.Key] = (currentProviderName, level) =>
{
if (string.Equals(providerName, currentProviderName))
{
return level >= kvp.Value;
}

return true;
};
}
}
}
return AddFilter(providerName, categoryName, level => level >= minLevel);
}

public void AddFilter(Func<string, bool> providerNames, IDictionary<string, LogLevel> filter)
/// <summary>
/// Adds a filter that applies to <paramref name="providerName"/> with the given
/// <paramref name="filter"/>.
/// </summary>
/// <param name="providerName">The name of the provider.</param>
/// <param name="filter">The filter that applies to logs for <paramref name="providerName"/>.
/// Returning true means allow log through, false means reject log.</param>
public LoggerFactory AddFilter(string providerName, Func<LogLevel, bool> filter)
{
lock (_sync)
if (filter == null)
{
foreach (var kvp in filter)
{
if (_categoryFilters.TryGetValue(kvp.Key, out var currentFilter))
{
_categoryFilters[kvp.Key] = (providerName, level) =>
{
if (providerNames(providerName))
{
if (currentFilter(providerName, level))
{
return level >= kvp.Value;
}

return false;
}

return true;
};
}
else
{
_categoryFilters[kvp.Key] = (providerName, level) =>
{
if (providerNames(providerName))
{
return level >= kvp.Value;
}

return true;
};
}
}
throw new ArgumentNullException(nameof(filter));
}

// Using 'Default' for the category name means this filter will apply for all category names
return AddFilter(providerName, "Default", filter);
}

// TODO: Figure out how to do this better, perhaps a new IConfigurableLogger interface?
Expand Down Expand Up @@ -479,7 +448,7 @@ private void OnConfigurationReload(object state)
finally
{
// The token will change each time it reloads, so we need to register again.
_changeToken.RegisterChangeCallback(OnConfigurationReload, null);
_changeTokenRegistration = _changeToken.RegisterChangeCallback(OnConfigurationReload, null);
}
}

Expand Down Expand Up @@ -519,6 +488,12 @@ private static IEnumerable<string> GetKeyPrefixes(string name)
private void LoadDefaultConfigValues()
{
var replacementDefaultFilters = new Dictionary<string, LogLevel>();
if (_configuration == null)
{
_defaultFilter = replacementDefaultFilters;
return;
}

var logLevelSection = _configuration.GetSection("LogLevel");

if (logLevelSection != null)
Expand Down Expand Up @@ -547,6 +522,8 @@ public void Dispose()
{
_disposed = true;

_changeTokenRegistration?.Dispose();

foreach (var provider in _providers)
{
try
Expand Down
Loading