Skip to content

Commit

Permalink
Merge pull request serilog#1033 from tsimbalar/967-no-sublogger-overr…
Browse files Browse the repository at this point in the history
…ides

Make it clear that Sub-logger MinimumLevel.Override is not supported
  • Loading branch information
nblumhardt authored Oct 9, 2017
2 parents 5bcf133 + 48b4757 commit b33a4aa
Show file tree
Hide file tree
Showing 7 changed files with 438 additions and 161 deletions.
31 changes: 22 additions & 9 deletions src/Serilog/Configuration/LoggerSinkConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,9 @@ public class LoggerSinkConfiguration

internal LoggerSinkConfiguration(LoggerConfiguration loggerConfiguration, Action<ILogEventSink> addSink, Action<LoggerConfiguration> applyInheritedConfiguration)
{
if (loggerConfiguration == null) throw new ArgumentNullException(nameof(loggerConfiguration));
if (addSink == null) throw new ArgumentNullException(nameof(addSink));
if (applyInheritedConfiguration == null) throw new ArgumentNullException(nameof(applyInheritedConfiguration));
_loggerConfiguration = loggerConfiguration;
_addSink = addSink;
_applyInheritedConfiguration = applyInheritedConfiguration;
_loggerConfiguration = loggerConfiguration ?? throw new ArgumentNullException(nameof(loggerConfiguration));
_addSink = addSink ?? throw new ArgumentNullException(nameof(addSink));
_applyInheritedConfiguration = applyInheritedConfiguration ?? throw new ArgumentNullException(nameof(applyInheritedConfiguration));
}

/// <summary>
Expand Down Expand Up @@ -128,9 +125,17 @@ public LoggerConfiguration Logger(
var lc = new LoggerConfiguration();

_applyInheritedConfiguration(lc);

configureLogger(lc);
return Sink(new SecondaryLoggerSink(lc.CreateLogger(), attemptDispose: true), restrictedToMinimumLevel, levelSwitch);

var subLogger = lc.CreateLogger();
if (subLogger.HasOverrideMap)
{
SelfLog.WriteLine("Minimum level overrides are not supported on sub-loggers " +
"and may be removed completely in a future version.");
}

var secondarySink = new SecondaryLoggerSink(subLogger, attemptDispose: true);
return Sink(secondarySink, restrictedToMinimumLevel, levelSwitch);
}

/// <summary>
Expand All @@ -149,7 +154,15 @@ public LoggerConfiguration Logger(
LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum)
{
if (logger == null) throw new ArgumentNullException(nameof(logger));
return Sink(new SecondaryLoggerSink(logger, attemptDispose: false), restrictedToMinimumLevel);

if (logger is Logger concreteLogger && concreteLogger.HasOverrideMap)
{
SelfLog.WriteLine("Minimum level overrides are not supported on sub-loggers " +
"and may be removed completely in a future version.");
}

var secondarySink = new SecondaryLoggerSink(logger, attemptDispose: false);
return Sink(secondarySink, restrictedToMinimumLevel);
}

/// <summary>
Expand Down
8 changes: 5 additions & 3 deletions src/Serilog/Core/Logger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ internal Logger(
_enricher = enricher;
}

internal bool HasOverrideMap => _overrideMap != null;

/// <summary>
/// Create a logger that enriches log events via the provided enrichers.
/// </summary>
Expand Down Expand Up @@ -273,11 +275,11 @@ public void Write(LogEventLevel level, string messageTemplate, params object[] p
/// <returns>True if the level is enabled; otherwise, false.</returns>
public bool IsEnabled(LogEventLevel level)
{
if ((int) level < (int) _minimumLevel)
if ((int)level < (int)_minimumLevel)
return false;

return _levelSwitch == null ||
(int) level >= (int) _levelSwitch.MinimumLevel;
(int)level >= (int)_levelSwitch.MinimumLevel;
}

/// <summary>
Expand Down Expand Up @@ -1359,7 +1361,7 @@ public bool BindProperty(string propertyName, object value, bool destructureObje
return false;
}

property =_messageTemplateProcessor.CreateProperty(propertyName, value, destructureObjects);
property = _messageTemplateProcessor.CreateProperty(propertyName, value, destructureObjects);
return true;
}

Expand Down
7 changes: 4 additions & 3 deletions src/Serilog/LoggerConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ public LoggerMinimumLevelConfiguration MinimumLevel
get
{
return new LoggerMinimumLevelConfiguration(this,
l => {
l =>
{
_minimumLevel = l;
_levelSwitch = null;
},
Expand Down Expand Up @@ -155,10 +156,10 @@ public Logger CreateLogger()
}

var converter = new PropertyValueConverter(
_maximumDestructuringDepth,
_maximumDestructuringDepth,
_maximumStringLength,
_maximumCollectionCount,
_additionalScalarTypes,
_additionalScalarTypes,
_additionalDestructuringPolicies,
auditing);
var processor = new MessageTemplateProcessor(converter);
Expand Down
189 changes: 189 additions & 0 deletions test/Serilog.Tests/Core/ChildLoggerKnownLimitationsTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
using System.Collections.Generic;
using System.Linq;
using Serilog.Core;
using Serilog.Events;
using Serilog.Tests.Support;
using Xunit;
using static Serilog.Events.LogEventLevel;

namespace Serilog.Tests.Core
{
public class ChildLoggerKnownLimitationsTests
{
[Fact]
public void SpecifyingMinimumLevelOverridesInWriteToLoggerWithConfigCallBackWritesWarningToSelfLog()
{
var outputs = new List<string>();
using (TemporarySelfLog.SaveTo(outputs))
{
var configCallBackSink = new CollectingSink();

var logger = new LoggerConfiguration()
.MinimumLevel.Verbose()
.MinimumLevel.Override("Foo.Bar", Warning)
.WriteTo.Logger(lc => lc
.MinimumLevel.Verbose()
.MinimumLevel.Override("Foo.Bar", Debug)
.WriteTo.Sink(configCallBackSink))
.CreateLogger();

var contextLogger = logger.ForContext(Constants.SourceContextPropertyName, "Foo.Bar");
contextLogger.Write(Some.InformationEvent());
}

Assert.EndsWith("Minimum level overrides are not supported on sub-loggers " +
"and may be removed completely in a future version.",
outputs.FirstOrDefault() ?? "");
}

[Fact]
public void SpecifyingMinimumLevelOverridesInWriteToLoggerWritesWarningToSelfLog()
{
var outputs = new List<string>();
using (TemporarySelfLog.SaveTo(outputs))
{
var subSink = new CollectingSink();

var subLogger = new LoggerConfiguration()
.MinimumLevel.Verbose()
.MinimumLevel.Override("Foo.Bar", Debug)
.WriteTo.Sink(subSink)
.CreateLogger();

var logger = new LoggerConfiguration()
.MinimumLevel.Verbose()
.MinimumLevel.Override("Foo.Bar", Warning)
.WriteTo.Logger(subLogger)
.CreateLogger();

var contextLogger = logger.ForContext(Constants.SourceContextPropertyName, "Foo.Bar");
contextLogger.Write(Some.InformationEvent());
}

Assert.EndsWith("Minimum level overrides are not supported on sub-loggers " +
"and may be removed completely in a future version.",
outputs.FirstOrDefault() ?? "");
}

public static IEnumerable<object[]> GetMinimumLevelOverrideInheritanceTestCases()
{
// Visualizing the pipeline from left to right ....
//
// Event --> Root Logger --> Child Logger
// lvl override/lvl override/levl
//
object[] T(string rs, int? rl, string cs, int? cl)
{
return new object[] { rs, rl, cs, cl };
}
// numbers are relative to incoming event level
// Information + 1 = Warning
// Information - 1 = Debug
//
// Incoming event is Information
// with SourceContext Root.N1.N2
//
// - no root overrides but children has its own
yield return T(null, +0, "Root", +1);
yield return T(null, +0, "Root.N1", +1);
yield return T(null, +0, "Root.N1.N2", +1);
// - root overrides let it through but child rejects it
yield return T("Root", +0, "Root", +1);
yield return T("Root.N1", +0, "Root", +1);
yield return T("Root.N1.N2", +0, "Root", +1);
yield return T("Root", +0, "Root.N1", +1);
yield return T("Root.N1", +0, "Root.N1", +1);
yield return T("Root.N1.N2", +0, "Root.N1", +1);
yield return T("Root", +0, "Root.N1.N2", +1);
yield return T("Root.N1", +0, "Root.N1.N2", +1);
yield return T("Root.N1.N2", +0, "Root.N1.N2", +1);
}

[Theory]
[MemberData(nameof(GetMinimumLevelOverrideInheritanceTestCases))]
public void WriteToLoggerWithConfigCallbackMinimumLevelOverrideInheritanceNotSupportedScenarios(
string rootOverrideSource,
int rootOverrideLevelIncrement,
string childOverrideSource,
int childOverrideLevelIncrement)
{
var incomingEventLevel = Information;
var rootOverrideLevel = incomingEventLevel + rootOverrideLevelIncrement;
var childOverrideLevel = incomingEventLevel + childOverrideLevelIncrement;

LogEvent evt = null;
var sink = new DelegatingSink(e => evt = e);

var rootLoggerConfig = new LoggerConfiguration()
.MinimumLevel.Is(LevelAlias.Minimum);

if (rootOverrideSource != null)
{
rootLoggerConfig.MinimumLevel.Override(rootOverrideSource, rootOverrideLevel);
}

var logger = rootLoggerConfig
.WriteTo.Logger(lc =>
{
lc.MinimumLevel.Is(LevelAlias.Minimum);
if (childOverrideSource != null)
{
lc.MinimumLevel.Override(childOverrideSource, childOverrideLevel);
}
lc.WriteTo.Sink(sink);
})
.CreateLogger();

logger
.ForContext(Constants.SourceContextPropertyName, "Root.N1.N2")
.Write(Some.LogEvent(level: incomingEventLevel));

// even though the user may expect no event
Assert.NotNull(evt);
}

[Theory]
[MemberData(nameof(GetMinimumLevelOverrideInheritanceTestCases))]
public void WriteToLoggerMinimumLevelOverrideInheritanceNotSupportedScenarios(
string rootOverrideSource,
int rootOverrideLevelIncrement,
string childOverrideSource,
int childOverrideLevelIncrement)
{
var incomingEventLevel = Information;
var rootOverrideLevel = incomingEventLevel + rootOverrideLevelIncrement;
var childOverrideLevel = incomingEventLevel + childOverrideLevelIncrement;

LogEvent evt = null;
var sink = new DelegatingSink(e => evt = e);

var childLoggerConfig = new LoggerConfiguration()
.MinimumLevel.Is(LevelAlias.Minimum);
if (childOverrideSource != null)
{
childLoggerConfig.MinimumLevel.Override(childOverrideSource, childOverrideLevel);
}
childLoggerConfig.WriteTo.Sink(sink);
var childLogger = childLoggerConfig.CreateLogger();

var rootLoggerConfig = new LoggerConfiguration()
.MinimumLevel.Is(LevelAlias.Minimum);

if (rootOverrideSource != null)
{
rootLoggerConfig.MinimumLevel.Override(rootOverrideSource, rootOverrideLevel);
}

var logger = rootLoggerConfig
.WriteTo.Logger(childLogger)
.CreateLogger();

logger
.ForContext(Constants.SourceContextPropertyName, "Root.N1.N2")
.Write(Some.LogEvent(level: incomingEventLevel));

// even though the use may expect no event
Assert.NotNull(evt);
}
}
}
Loading

0 comments on commit b33a4aa

Please sign in to comment.