Skip to content

Commit

Permalink
#34: simple legacy-preserving switch to limit logger activations
Browse files Browse the repository at this point in the history
  • Loading branch information
srogovtsev committed Jun 24, 2020
1 parent 2975c9a commit 0aafd1b
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 20 deletions.
12 changes: 9 additions & 3 deletions src/AutofacSerilogIntegration/ContextualLoggingModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ internal class ContextualLoggingModule : Module
readonly bool _autowireProperties;
readonly bool _skipRegistration;
readonly bool _dispose;
readonly bool _onlyKnownConsumers;

[Obsolete("Do not use this constructor. This is required by the Autofac assembly scanning")]
public ContextualLoggingModule()
Expand All @@ -26,11 +27,12 @@ public ContextualLoggingModule()
_skipRegistration = true;
}

internal ContextualLoggingModule(ILogger logger = null, bool autowireProperties = false, bool dispose = false)
internal ContextualLoggingModule(ILogger logger = null, bool autowireProperties = false, bool dispose = false, bool onlyKnownConsumers = false)
{
_logger = logger;
_autowireProperties = autowireProperties;
_dispose = dispose;
_onlyKnownConsumers = onlyKnownConsumers;
_skipRegistration = false;
}

Expand Down Expand Up @@ -94,8 +96,7 @@ protected override void AttachToComponentRegistration(IComponentRegistryBuilder

PropertyInfo[] targetProperties = null;

var ra = registration.Activator as ReflectionActivator;
if (ra != null)
if (registration.Activator is ReflectionActivator ra)
{
// As of Autofac v4.7.0 "FindConstructors" will throw "NoConstructorsFoundException" instead of returning an empty array
// See: https://github.com/autofac/Autofac/pull/895 & https://github.com/autofac/Autofac/issues/733
Expand Down Expand Up @@ -130,6 +131,11 @@ protected override void AttachToComponentRegistration(IComponentRegistryBuilder
if (!usesLogger)
return;
}
else
{
if (_onlyKnownConsumers)
return;
}

registration.Preparing += (sender, args) =>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ public static class SerilogContainerBuilderExtensions
/// <param name="autowireProperties">If true, properties on reflection-based components of type <see cref="ILogger"/> will
/// be injected.</param>
/// <param name="dispose"></param>
/// <param name="onlyKnownConsumers">
/// If true, only the registrations that can be verified to use <see cref="ILogger"/> will be modified to access the logger,
/// thus avoiding unnecessary logger calls.
/// </param>
/// <returns>An object supporting method chaining.</returns>
public static IModuleRegistrar RegisterLogger(this ContainerBuilder builder, ILogger logger = null, bool autowireProperties = false, bool dispose = false)
public static IModuleRegistrar RegisterLogger(this ContainerBuilder builder, ILogger logger = null, bool autowireProperties = false, bool dispose = false, bool onlyKnownConsumers = false)
{
if (builder == null) throw new ArgumentNullException("builder");
return builder.RegisterModule(new ContextualLoggingModule(logger, autowireProperties, dispose));
if (builder == null) throw new ArgumentNullException(nameof(builder));
return builder.RegisterModule(new ContextualLoggingModule(logger, autowireProperties, dispose, onlyKnownConsumers));
}
}
}
57 changes: 43 additions & 14 deletions test/AutofacSerilogIntegration.Tests/ActivatorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,16 @@ public ActivatorTests()
_logger.SetReturnsDefault(_logger.Object);
}

private void ResolveInstance<TDependency>(Action<ContainerBuilder> configureContainer)
private void ResolveInstance<TDependency>(Action<ContainerBuilder> configureContainer, bool? onlyKnownConsumers)
{
var containerBuilder = new ContainerBuilder();

if (onlyKnownConsumers == null)
containerBuilder.RegisterLogger(_logger.Object);
else
containerBuilder.RegisterLogger(_logger.Object, onlyKnownConsumers: onlyKnownConsumers.Value);

containerBuilder.RegisterType<Component<TDependency>>();
containerBuilder.RegisterLogger(_logger.Object);
configureContainer(containerBuilder);
using (var container = containerBuilder.Build())
{
Expand All @@ -31,36 +36,60 @@ private void ResolveInstance<TDependency>(Action<ContainerBuilder> configureCont
private void VerifyLoggerCreation<TContext>(Func<Times> times)
=> _logger.Verify(logger => logger.ForContext(typeof(TContext)), times);

[Fact]
public void Default_ReflectionActivator_DependencyWithLogger_ShouldCreateLogger()
[Theory]
[InlineData(null)]
[InlineData(false)]
[InlineData(true)]
public void ReflectionActivator_DependencyWithLogger_ShouldCreateLogger(bool? onlyKnownConsumers)
{
ResolveInstance<DependencyWithLogger>(containerBuilder => containerBuilder.RegisterType<DependencyWithLogger>());
ResolveInstance<DependencyWithLogger>(containerBuilder => containerBuilder.RegisterType<DependencyWithLogger>(), onlyKnownConsumers);
VerifyLoggerCreation<DependencyWithLogger>(Times.AtLeastOnce);
}

[Fact]
public void Default_ReflectionActivator_DependencyWithoutLogger_ShouldNotCreateLogger()
[Theory]
[InlineData(null)]
[InlineData(false)]
[InlineData(true)]
public void ReflectionActivator_DependencyWithoutLogger_ShouldNotCreateLogger(bool? onlyKnownConsumers)
{
ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.RegisterType<DependencyWithoutLogger>());
ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.RegisterType<DependencyWithoutLogger>(), onlyKnownConsumers);
VerifyLoggerCreation<DependencyWithoutLogger>(Times.Never);
}

[Fact]
[Theory]
[InlineData(null)]
[InlineData(false)]
//legacy behavior
public void Default_ProvidedInstanceActivator_DependencyWithoutLogger_CreatesLogger()
public void Default_ProvidedInstanceActivator_DependencyWithoutLogger_CreatesLogger(bool? onlyKnownConsumers)
{
ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.RegisterInstance(new DependencyWithoutLogger()));
ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.RegisterInstance(new DependencyWithoutLogger()), onlyKnownConsumers);
VerifyLoggerCreation<DependencyWithoutLogger>(Times.AtLeastOnce);
}

[Fact]
[Theory]
[InlineData(null)]
[InlineData(false)]
//legacy behavior
public void Default_DelegateActivator_DependencyWithoutLogger_CreatesLogger()
public void Default_DelegateActivator_DependencyWithoutLogger_CreatesLogger(bool? onlyKnownConsumers)
{
ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.Register(_ => new DependencyWithoutLogger()));
ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.Register(_ => new DependencyWithoutLogger()), onlyKnownConsumers);
VerifyLoggerCreation<DependencyWithoutLogger>(Times.AtLeastOnce);
}

[Fact]
public void OnlyKnownCustomers_ProvidedInstanceActivator_DependencyWithoutLogger_ShouldNotCreateLogger()
{
ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.RegisterInstance(new DependencyWithoutLogger()), onlyKnownConsumers: true);
VerifyLoggerCreation<DependencyWithoutLogger>(Times.Never);
}

[Fact]
public void OnlyKnownCustomers_DelegateActivator_DependencyWithoutLogger_ShouldNotCreateLogger()
{
ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.Register(_ => new DependencyWithoutLogger()), onlyKnownConsumers: true);
VerifyLoggerCreation<DependencyWithoutLogger>(Times.Never);
}

private class Component<TDependency>
{
public Component(TDependency dependency)
Expand Down

0 comments on commit 0aafd1b

Please sign in to comment.