Consider having easy way to access and log custom data as a sort of pseudo context

[eugeneagafonov]

While developing several projects for my clients I've found out that injecting logging services into controllers and services seems to be uncomfortable to use, I have to add a logging service into every constructor of each controller and service. Property injecting works slightly better, but the best way I've found is providing static class which serves as a pseudo context, having scope based service resolution under the hood. It goes like this:

public static class EnvContext 
{
  public static IEnvironment Current 
  {
     get 
     {
         //get dependency resolver or whatever di framework
         var resolver = GlobalConfiguration.DependencyResolver // or whatever
         using (var scope = resolver.GetRequestLifetimeScope())
         {
             // get services needed, instantiate per request scope 
             // (in non http environments that would be a different scope)

            // use data from those services, cache data per lifetime scope
             return new CustomEnvironment();
         }
     }
  }  
}

public interface IEnvironment
{
  // Information about Azure environment - node data, etc.
  AzureEnvironment Azure { get; }

  // information about Http environment - url, headers, http method, etc
  HttpEnvironment Http { get; }

  // custom user information
  CustomUser User { get; }  

  // current logger, we could use several approaches to get it while constructing the context
  ILogger Logger { get; }

  // any other useful data
}

Then in any controller or service I can use it like this

// for example
var shippingAddress = EnvContext.Current.User.ShippingAddress;

// logs Azure, Http and User info automatically (as configured) and serializes into JSON or whatever
EnvContext.Current.Logger.WriteWarning("Something suspicious is going on");

// logs a string
EnvContext.Current.Logger.WriteWarningString("No additional info required");

This was a code written in issue editor to illustrate a concept. It is still fully testable, since it used DI under the hood, and it is comfortable to use. I think it makes sense providing a mechanics for a user to configure such a context and then have it out of the box in aspnet vnext

Thanks!

[davidfowl]

How is that code still testable?

[eugeneagafonov]

in test environment you register mock services which are being used in EnvContext.Current

[davidfowl]

That seems pretty anti di. Even property injection would be better imo. This seems like something we wouldn't bake into the framework or would recommend to people. Totally doable of course and might may be required in cases where you aren't being activated by the di system directly.

We do have a single static service provider available that updates with the current scope but we strongly recommend against using it where possible (like a controller).

[eugeneagafonov]

So the only problem is how to find the proper resolver, instead of doing GlobalConfig.DependencyResolver? I'm not going to argue, though I think it's not anti di at all, but the problem exists, and I would love to hear how to get rid of endless constructor parameters in every controller and every external code.

Can you make a property injection on a base class for a controller?
It can partially resolve a problem if you can do something like this:

 public class ControllerBase
 {
     // injected
     protected IEnvironment Environment 
     {
         get;
         set; // want to have private set here, is that possible?
     }
 }

 public class HomeController : ControllerBase
 {
    public IActionResult Index()
    {
        return Content("Hello " + Environment.User.Name);
    }
 }

[tugberkugurlu]

@eugeneagafonov

While developing several projects for my clients I've found out that injecting logging services into controllers and services seems to be uncomfortable to use,

I would like the ctor to be the only place where I look at to find out which external components that component has dependencies on. In this new world, I archived this 99.9%. So, please do not take this away from me 😄

[eugeneagafonov]

@tugberkugurlu well I think no chance for me ;) But I'd love to hear an alternative solution then. Or do you think there is no such problem?

[eugeneagafonov]

Let's see a problem from a wider angle. Logging is something that is needed literally everywhere in your code. That's why I'm writing here, not to aspnet repo itsef. Another example is security, manual profiling, etc. Leaving beside AOP approach, what were common approaches to logging? Trace was a static class, which was configured in config or code, many popular frameworks, for example, NLog:

Logger logger = LogManager.GetLogger("MyClassName");

It was implemented in that way for a reason, not only because DI was not in trend at that times. You can't keep logging in only asp.net environment, it needs to go outside of aspnet. Of course I can use a separate logging in my azure worker role, but it is natural to have it in a way System.Diagnostics.Trace had. If I needed only logging mechanics, I would just keep using Trace, but I need to log additional info every time, and for that I need to have that information, so I need to go through and through from the very beginning providing it to the place in code where I log something.

In aspnet code I tried to use constructor injection, and have about 3 services I almost always needed to provide. While I fully understand advantages and disadvantages of that kind of application composition, I still think the problem does exist. So, would love to hear what would you recommend in such a case.

I've already gave a try to a code snippet in VS, which adds implementation of ILogger, IUserInfoProvider and IEnvironmentInfoProvider

[eugeneagafonov]

In case someone is interested discussing this at MVP Summit I'm going to keep hanging in Kodiak today and tomorrow :)

[tugberkugurlu]

It was implemented in that way for a reason, not only because DI was not in trend at that times. You can't keep logging in only asp.net environment, it needs to go outside of aspnet.

It's already outside the ASP.NET 😄 AFAIK, anything under Microsoft.Framework namespace is supposted to be go beyond ASP.NET like logging and caching. You can even start using this new logging infrastructure in a good-old .NET application now.

Was this the problem? I'm still not seeing what the problem is.

[eugeneagafonov]

No the problem is that i have to provide too many dependencies into every bit of large codebase. My bad failing to explain this. Maybe it could be illustrated with this simple problem: DateTime.Now (or UtcNow) cannot be mocked, so to follow DI principles we have to create IDateTimeFactory (or whatever) and provide it into every bit of code which relies on it. But if we could configure the value returned by DateTime.Now in tests it wouldn't be necessary, and would save a lot of efforts writing and maintaining constantly repeating code.
It is clear for me that my suggestion is not considered as something valuable, so we could move this discussion (if anyone is interested to) to somewhere more appropriate place (perhaps I could go with a blog post on this when get a little bit of free time)

[tugberkugurlu]

Well, if you ask me, DateTime.Now case is also a bad example for this problem as it's also a bad API to use. However, we are all using it regardless in most of the cases (including me 😄). It is fair to say that NodaTime's approach on this problem is very logical and DI friendly.

I am just casting my vote here: I'm happy to inject ILoggerFactory implementation every place where I need a logger.

[PureKrome]

Same here @tugberkugurlu - Happy to inject it via a ctor. 👍

The problem you speak of @eugeneagafonov is that your ctor's could have lots of interfaces defined in there. This could lead to constructor over-injection.

My TL;DR; of that anti-pattern is basically this: inject dependencies that are to be used all over the class/service. Logging is a perfect example of this. But if you have 10 methods and 2 only use an IFoo? don't inject it via the ctor. Pass it in, in those 2 methods.

Why inject everything, when it's only used here and there, in side that class?

So - an ILoggerFactory is actually a perfect example IMO where a service is injected and would be used all over the class.

I also think that people start to get tired of having to create more mocks for their classes because the ctor's now require them ... assuming one does testing. So refer back to the anit-pattern, mentioned above.

[eugeneagafonov]

I think DateTime.Now is an excellent example. I used to replace it with Func<DateTime> which was injected into all ctors, and this was one of the reasons I reconsidered this approach and started to use contexts. And this library, NodaTime from what I saw uses the same approach I use:

Instant now = SystemClock.Instance.Now

@PureKrome thanks for "over-injection" article, however I'm not sure how it is relevant to this case - logging and accompanying services are needed all over the code.

[tugberkugurlu]

@eugeneagafonov I still insist that DateTime.Now is a bad example 😄 and the way you used SystemClock is also a bad example. If you are using NodaTime in your application, you would inject IClock implementation into your componnets. The only place where you use SystemClock.Instance explicitly is the IoC container registration where you would register SystemClock.Instance as a singleton for IClock.