[
https://issues.apache.org/jira/browse/LOG4NET-565?focusedWorklogId=671300&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-671300
]
ASF GitHub Bot logged work on LOG4NET-565:
------------------------------------------
Author: ASF GitHub Bot
Created on: 28/Oct/21 08:50
Start Date: 28/Oct/21 08:50
Worklog Time Spent: 10m
Work Description: fluffynuts commented on pull request #73:
URL: https://github.com/apache/logging-log4net/pull/73#issuecomment-953642068
@tschettler thanks for the response and the thorough code example. The
example does, however, back up exactly what my thoughts are on the whole story:
the ultimate result is simply that service location is pushed out of _your_
code into _log4net's_ code. This means that people who want this get a cleanup
at the cost of every other log4net user - log4net is used in a wide array of
places, so I have to apply very cautious reasoning when examining requests like
this.
The fact is that, somewhere, something is doing a
`ServiceLocator.Resolve<T>` for the more complex `T` (in this case, your
`HttpContextPatternLayoutConverter`. There are alternative ways to accomplish
what you want without introducing the complexity and potential security holes
of being able to push an outside-in service-locator into log4net.
For example:
```csharp
public class HttpContextPatternLayoutConverter : PatternLayoutConverter {
public HttpContextPatternLayoutConverter():
this(Container.Resolve<IHttpContextAccessor>())
{
}
public HttpContextPatternLayoutConverter (IHttpContextAccessor
httpContextAccessor) {
_httpContextAccessor = httpContextAccessor;
}
protected override void Convert (TextWriter writer, LoggingEvent
loggingEvent) {
var httpContext = _httpContextAccessor.HttpContext;
if (httpContext == null) {
writer.Write (SystemInfo.NotAvailableText);
} else {
Convert (writer, loggingEvent, httpContext);
}
}
...
}
```
I hear you saying "but the point is to not have service-locator in my code!"
and yes, this means that you have service-locator in your code - this is a very
small step up from "poor man's DI" which I used a very, very long time ago to
get older mvc controllers testable before figuring out the correct way to do DI.
you could also have:
```csharp
public class HttpContextPatternLayoutConverterFacade: PatternLayoutConverter
{
private HttpContextPatternLayoutConverter _actual;
public HttpContextPatternLayoutConverterFacade()
{
// this is pretty-much the code proposed to live within log4net
_actual =
GlobalContainer.Resolve<HttpContextPatternLayoutConverter>();
}
public override void Convert(TextWriter writer, LoggingEvent
loggingEvent)
{
_actual.Convert(writer, loggingEvent);
}
}
```
The above is what I meant by achieving the same goals in user-land.
Remember: there are _many, many_ consumers of `log4net`, and many of them have
no need whatsoever for IOC, but all would have to take on the potential worry
that a third-party dependency could gain unnecessary privilege in their system.
I think that a better solution, might be for log4net to have "proper" DI and
expose an api like Serilog does - one that's very familiar from use of DI
frameworks that I've dealt with (I haven't collected 'em all, but I have worked
a lot with Castle.Windsor, StructureMap and DryIOC, and a bit with Autofac and,
back in the mists of time, unity):
Instead of allowing the hosting application to completely take over how all
things are constructed, log4net could:
- expose factory functions for those services
- scan constructors of types it's about to create to match up with factories
or simply new them up
This is the absolute lowest-level DI I can imagine, where lifespan is
controlled by the logic within the factory function. The second part -
constructing items within log4net-land - isn't so difficult.
Imagineered code:
```csharp
using log4net.DependencyInjection;
namespace YourApplication.IOC
{
public class Bootstrapper
{
public static void Bootstrap()
{
var container = new WhateverContainerYouLikeToUse();
// all your existing logic still happens here
// transient registration, or simply relying on
whatever lifespan you've
// registered with in your container:
Log4NetContainer.Instance.Register(container.Resolve<TService>);
// singleton, irrespective of container
var singleton = container.Resolve<TService2>();
Log4NetContainer.Instance.Register(() => singleton);
}
}
}
```
of course, the bare-minimum factory method exposure could be extended to
provide friendlier methods:
```csharp
Log4NetContainer.Instance.Register(singleton);
// parameterless constructor, service -> impl
Log4NetContainer.Instance.Register<TService3, TImplementation3>();
```
or even to do magic like scans with 1-1 mappings being automatically
resolved within a configured assembly, but now we're stepping out of bare-bones
and into nice-to-have and perhaps it's best to start with the minimum viable
product?
Now, I know that this also exposes the ability of a third-party to change
how services are constructed, but it makes it considerably harder to do
something unexpected / nefarious because the third-party now has to know about
types in _your_ system.
Thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 671300)
Time Spent: 1h 20m (was: 1h 10m)
> Dependency Injection support appender and logger types
> ------------------------------------------------------
>
> Key: LOG4NET-565
> URL: https://issues.apache.org/jira/browse/LOG4NET-565
> Project: Log4net
> Issue Type: Improvement
> Components: Appenders, Core
> Reporter: Hitesh Chauhan
> Priority: Major
> Labels: Enhancement
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> I have seen demand for dependency injection support in log4net. I have added
> that behavior to my local repository. And I would like to push that to
> log4net library.
> e.g. appender configuration
> <appender
> name="ServiceAppender"
> type="LoggingServiceAppender"
> serviceLocatorType="Log4NetServiceLocator">
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
