This project is read-only.

Constructor Argument - Logger

Jan 29, 2014 at 9:50 PM
I am new to Simple Injector. I have the following code using NINJECT.

public static ILogger Create(string loggerName)
    {
        var log = Kernel.Get<ILogger>(new ConstructorArgument("Logger", loggerName));
        return log;
    }
I would like to convert this to support Simple Injector.

Any one point me to the right direction?

Thanks
Jan 29, 2014 at 10:08 PM
Can you show how this Create method is used?
Jan 29, 2014 at 10:47 PM
Here is my code .....

namespace Demo.Core.Log
{
/// <summary>
/// Creates a log object for those instances where one cannot be injected (i.e. app startup).
/// Generally you should just ctor inject ILogger
/// </summary>
public static class AppLogFactory
{
    public static ILogger Create()
    {
        var declaringType = new StackTrace(1, false).GetFrame(1).GetMethod().DeclaringType;

        if (declaringType != null)
        {
            var loggerName = declaringType.FullName;
            return Create(loggerName);
        }

        throw new InvalidOperationException("Could not determine declaring type; specify logger name explicitly");
    }

    public static ILogger Create<T>()
    {
        return Create(typeof(T).FullName);
    }



    public static ILogger Create(string loggerName)
    {
        var container = MvcApplication.Container;

       // .Get<ILogger>(new ConstructorArgument(LogConstants.LoggerNameParam, loggerName));
        //return log;


  }

}




public class LogConstants
{
    public const string LoggerNameParam = "LoggerName";
}
}
Jan 29, 2014 at 11:06 PM
And who is calling AppLogFactory.Create?
Jan 29, 2014 at 11:21 PM
I am sorry. I should have given the calling part also.

public class MvcApplication : HttpApplication
{
    protected void Application_Start()
    {
        var log = AppLogFactory.Create<MvcApplication>();
        log.Info("Application starting");

        AreaRegistration.RegisterAllAreas();
        WebApiConfig.Register(GlobalConfiguration.Configuration);
        FilterConfig.RegisterGlobalFilters(GlobalFilters.Filters);
        RouteConfig.RegisterRoutes(RouteTable.Routes);
        BundleConfig.RegisterBundles(BundleTable.Bundles);

        log.Info("Application started");
    }

Jan 30, 2014 at 7:00 AM
Is this the only consumer of that ILogger interface? How is it currently registered in Ninject and how does the logger implementation look like?
Jan 30, 2014 at 6:34 PM
public interface ILog
{
void Debug(string format, params object[] args);
void Error(string format, params object[] args);
void Fatal(string format, params object[] args);
void Info(string format, params object[] args);
void Trace(string format, params object[] args);
void Warn(string format, params object[] args);

// custom
void Write(LogType type, object properties, string message, params object[] args);

bool IsDebugEnabled { get; }
bool IsErrorEnabled { get; }
bool IsTraceEnabled { get; }
}

public class Log : Logger, ILog
{
public void Write(LogType type, object properties, string message, params object[] args)
{
    var info = new LogEventInfo(LogLevel.FromOrdinal((int)type), Name, CultureInfo.CurrentCulture, message, args);

    if (null != properties)
    {
        foreach (PropertyDescriptor propertyDescriptor in TypeDescriptor.GetProperties(properties))
        {
            var value = propertyDescriptor.GetValue(properties);
            info.Properties[propertyDescriptor.Name] = value;
        }
    }

    Log(info);
}
}

public enum LogType
{
Trace, Debug, Info, Warn, Error, Fatal
}
public class ActionTrackerAttribute : ActionFilterAttribute
{
    private Stopwatch Watch { get; set; }

    private ILog Log { get; set; }
    private ActionExecutingContext FilterContext { get; set; }

    private string ActionName
    {
        get
        {
            return FilterContext.ActionDescriptor.ActionName;
        }
    }

    private string ControllerName
    {
        get
        {
            return FilterContext.ActionDescriptor.ControllerDescriptor.ControllerName;
        }
    }

    private Uri Url
    {
        get { return FilterContext.RequestContext.HttpContext.Request.Url; }
    }

    public override void OnActionExecuting(ActionExecutingContext filterContext)
    {
        base.OnActionExecuting(filterContext);

        try
        {
            FilterContext = filterContext;
            Log = AppLogFactory.Create<ActionTrackerAttribute>();
            Log.Trace("Executing {0}.{1}", ControllerName, ActionName);
            Watch = Stopwatch.StartNew();
        }
        catch (Exception ex)
        {
            Trace.WriteLine(ex);
        }
    }

    public override void OnResultExecuted(ResultExecutedContext filterContext)
    {
        base.OnResultExecuted(filterContext);

        try
        {
            Watch.Stop();
            Log.Info("Executed {0}.{1} for {2} in {3:##0.000} second(s)", ControllerName, ActionName, Url,
                     Watch.Elapsed.TotalSeconds);
        }
        catch (Exception ex)
        {
            Trace.WriteLine(ex);
        }
    }
}


public class LogModule : NinjectModule
{
    public override void Load()
    {
        Bind<ILog>().ToMethod(CreateLog);

        Kernel.BindFilter<AppErrorHandlerAttribute>(FilterScope.Controller, 0);
    }

    private static ILog CreateLog(IContext ctx)
    {
        var p = ctx.Parameters.FirstOrDefault(x => x.Name == LogConstants.LoggerNameParam);
        var loggerName = (null != p) ? p.GetValue(ctx, null).ToString() : null;

        if (string.IsNullOrWhiteSpace(loggerName))
        {
            if (null == ctx.Request.ParentRequest)
            {
                throw new NullReferenceException(
                    "ParentRequest is null; unable to determine logger name; if not injecting into a ctor "
                    + "a parameter for the logger name must be provided");
            }

            var service = ctx.Request.ParentRequest.Service;
            loggerName = service.FullName;
        }

        var log = (ILog)LogManager.GetLogger(loggerName, typeof(Log));
        return log;
    }
}


public class FilterConfig
{
    public static void RegisterGlobalFilters(GlobalFilterCollection filters)
    {

        filters.Add(new ActionTrackerAttribute());
    }
}

protected void Application_Start()
    {

        var log = AppLogFactory.Create<MvcApplication>();
        log.Info("Application starting");
}
Jan 30, 2014 at 8:46 PM
I'm sorry for all these questions, but I needed to get some context to give some useful feedback on what you're doing.

Out of the box, Simple Injector has no support for passing in runtime values while requesting an instance from the container. This feature is missing deliberately and it tries to steer you into a different direction. There are ways to implement this, but I think it's better to try something else instead.

One thing I noticed is your use of the AppLogFactory.Create method from within the ActionTrackerAttribute. This is a specialized form of the Service Locator anti-pattern (to be more exact in DI terminology, your static factory is an Ambient Context). The use of this pattern is suboptimal. First of all, your class now takes an extra dependency, making it (slightly) more complex. But more importantly, depending on this static class makes your code harder to test. If you have this code inside your ActionTrackerAttribute, I bet you have this call in a lot of classes in your application, or -when you don't- you will have in a couple of months. I think in your case it would be much better use property injection, which is something you are basically already doing, since you expose ILog as a property.

Besides this, from the LogModule.CreateLog method it becomes clear to me that what you are trying to achieve is Context Based Injection (or Contextual Binding in Ninject terminology). This basically means that the type you create changes based on its context (most often its parent in which it is injected).

For doing context based injection with Simple Injector, you need to add the RegisterWithContext extension method from the Context Based Injection wiki page. After copy-pasting the extension method into your project, you will be able to do the following registration:
container.RegisterWithContext<ILog>(context => 
{
    // I assume the Log class to have a ctor with one argument.
    return new Log(context.ImplementationType.FullName);
});
When ILog is injected into a consumer -say ActionTrackerAttribute for instance- the registered delegate will be called passing on information about ILog's parent (ActionTrackerAttribute in this case).

By default, Simple Injector will not do property injection. Again this design is deliberate (as explained here). There are multiple ways to enable property injection, but the best way is to force dependencies to be injected (explicit property injection), which will let the container fail fast when a property can't be injected; compared to implicit property injection, where the container skips and continues (the default behavior for most DI container, but this is rather error prone).

One way to enable property injection is by marking those properties with 'some' attribute and configure the container to inject properties that are marked with that attribute. You can for instance make use of the System.ComponentModel.Composition.ImportAttribute that is part of the .NET framework. This can be done by creating the following class:
class ImportPropertySelectionBehavior 
    : SimpleInjector.Advanced.IPropertySelectionBehavior {
    public bool SelectProperty(Type type, PropertyInfo prop) {
        return prop.GetCustomAttributes(typeof(ImportAttribute)).Any();
    }
}
And register it as follows:
var container = new Container();

container.Options.PropertySelectionBehavior = new ImportPropertySelectionBehavior();
Now you can define your ActionTrackerAttribute as follows:
public class ActionTrackerAttribute : ActionFilterAttribute {
    private readonly Stopwatch watch = new Stopwatch();

    [Import]
    public ILog Log { get; set; }

    public override void OnActionExecuting(ActionExecutingContext filterContext) {
        this.watch.Start();

        base.OnActionExecuting(filterContext);

        this.Log.Trace("Executing {0}.{1}", 
            filterContext.ActionDescriptor.ControllerDescriptor.ControllerName, 
            filterContext.ActionDescriptor.ActionName);
    }

    public override void OnResultExecuted(ResultExecutedContext filterContext) {
        base.OnResultExecuted(filterContext);

        this.watch.Stop();
        
        this.Log.Info("Executed {0}.{1} for {2} in {3:##0.000} second(s)", 
            filterContext.ActionDescriptor.ControllerDescriptor.ControllerName, 
            filterContext.ActionDescriptor.ActionName, 
            filterContext.RequestContext.HttpContext.Request.Url,
            this.watch.Elapsed.TotalSeconds);
    }
}
The next question is of course how to inject dependencies into Attributes, since .NET will not do this by default. The trick is to create a special MVC FilterAttributeFilterProvider as shown here and register it as follows:
var filterProvider = 
    new SimpleInjectorFilterAttributeFilterProvider(container);

container.RegisterSingle<IFilterProvider>(filterProvider);

var providers = FilterProviders.Providers
    .OfType<FilterAttributeFilterProvider>().ToList();

providers.ForEach(provider => FilterProviders.Providers.Remove(provider));

FilterProviders.Providers.Add(filterProvider);
That SimpleInjectorFilterAttributeFilterProvider will ensure that any MVC filter attributes that are placed on controllers, are initialized conform the Simple Injector configuration. This basically means that properties are injected as configured using our custom ImportPropertySelectionBehavior. There's more than just property injection, but that is out of the scope of this question.

Since our ILog is configured as contextual dependency, this means that when injected into the ActionTrackerAttribute, it will be injected as new Log("MyNamespace.ActionTrackerAttribute").

This is what you need to get property injection working on attributes.

As an alternative approach you can also extract the behavior from your attribute (separate data and behavior). This is a bit more advanced, but might be interesting to look at. Here's an example.

I can't help noticing the design of your ILog interface though. This interface is quite wide (and therefore violates the Interface Segregation Principle) and this makes creating implementations much harder (which might limit you while doing unit testing or when writing a decorator over logging implementations). You seem to have solved this with a base class (the same way as log4net does this), but you might want to think about a design such as this one. This prevents you from having both a base class and a interface.

There are other improvements that might be interesting to look at. In general, I would say that the consumer shouldn't care about how the logger is configured; whether debug, error or trace is enabled. It should just log and the logger should find out. Furthermore the logging implementation itself should probably not even check this, since you can wrap the implementation in a decorator that filters messages that are below the threshold. I know these query-properties are often included for performance, but I hardly ever seen this to become a performance bottleneck. Especially since having a good application design would prevent you from having to log much anyway.

Even the contextual information in the logger is IMO a sign that you're logging too much. Is it really useful to know that the log information came from the ActionTrackerAttribute? The logged message should be clear enough to let the user understand what is going on. From which class this information originates should either be uninteresting (in case of the ActionTrackerAttribute) or very obvious (for instance because you logged a UserMovedCommand, which basically means that the decorator for the ICommandHandler<UserMovedCommand> logged this information).
Jan 30, 2014 at 8:54 PM
Edited Jan 30, 2014 at 9:04 PM
Thank you so much for the detailed information. I am modifying my code based on your comment
Marked as answer by mdoss70 on 1/30/2014 at 1:04 PM