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.

Sign up for GitHub

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

Jump to bottom

Update log4net module to handle registrations in nested lifetime scopes #59

Closed
wants to merge 1 commit into from
Closed

Update log4net module to handle registrations in nested lifetime scopes #59

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 26 additions & 8 deletions docs/examples/log4net.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,42 @@ Here's a sample module that configures Autofac to inject ``ILog`` parameters bas

private static void OnComponentPreparing(object sender, PreparingEventArgs e)
{
e.Parameters = e.Parameters.Union(
new[]
{
new ResolvedParameter(
(p, i) => p.ParameterType == typeof(ILog),
(p, i) => LogManager.GetLogger(p.Member.DeclaringType)
),
});
e.Parameters = e.Parameters.Union(new[]
{
new ResolvedParameter(
(p, i) => p.ParameterType == typeof(ILog),
(p, i) => LogManager.GetLogger(p.Member.DeclaringType))
});
}

protected override void Load(ContainerBuilder builder)
{
// Handle registrations in nested lifetime scopes.
builder.RegisterBuildCallback(c => c.ChildLifetimeScopeBeginning += OnChildLifetimeScopeBeginning);
Copy link
Member

Choose a reason for hiding this comment

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

This should register for the container creating child scopes but I don't think it handles those children creating scopes, like...

using(var scope1 = container.BeginLifetimeScope())
{
  // scope1 should have things attached
  using(var scope2 = scope1.BeginLifetimeScope())
  {
    // I don't think scope2 will be registered
  }
}

One of the reasons the example isn't super robust is... it's an example, and we don't have unit tests or anything to verify that a super rich example like this actually handles everything.

Additional related issues:

Copy link
Author

Choose a reason for hiding this comment

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

You are right - it wasn't handling children scopes. I've updated PR to handle that.

}

protected override void AttachToComponentRegistration(IComponentRegistry componentRegistry, IComponentRegistration registration)
{
AttachToRegistration(registration);
}

private static void AttachToRegistration(IComponentRegistration registration)
{
// Handle constructor parameters.
registration.Preparing += OnComponentPreparing;

// Handle properties.
registration.Activated += (sender, e) => InjectLoggerProperties(e.Instance);
}

private static void OnChildLifetimeScopeBeginning(object sender, LifetimeScopeBeginningEventArgs e)
{
e.LifetimeScope.ChildLifetimeScopeBeginning += OnChildLifetimeScopeBeginning;
foreach (var registration in e.LifetimeScope.ComponentRegistry.Registrations)
{
AttachToRegistration(registration);
}
}
}

**Performance Note**: At the time of this writing, calling ``LogManager.GetLogger(type)`` has a slight performance hit as the internal log manager locks the collection of loggers to retrieve the appropriate logger. An enhancement to the module would be to add caching around logger instances so you can reuse them without the lock hit in the ``LogManager`` call.
Expand Down