Background - A common evolution to logging
Recently we needed to implement a decent logging solution for a new application. The application (like so many) had evolved from a proof of concept (P.O.C) code base. 5 weeks after the cut over from P.O.C to code that was intended to ship, we had a plethora of Debug.WriteLine, Console.WriteLine and Trace.WriteLines sprinkled through the code base. Most of the *.WriteLine code was to help debug- code that implemented 3rd party code
- code that had no tests (booo!) and F5 testing seemed to be "good enough"
At this point in time the team had decided we had several real problems here:
- We had various "debugging" systems (Console, debug and trace)
- We had hard coded our implementations of "logging". If logging a specific piece of functionality is a requirement we really should test that we are logging the correct thing!
- As the WPF guy I am interested in any WPF binding errors (in the VS output window), but the service/agent guy were flooding the console screen with their pub/sub debug messages. How could I filter out their messages, and how could they filter out mine?
- We were catching the un-handled exception at the dispatcher, not at the point it was thrown. This mean we had a huge stack trace (noise) pointing back to the problem and we also were missing some useful data to debug the exception such as the inputs to the method that threw the exception.
Setting the requirements
From the problems we have above the team decided that our requirements for the logging system were:- Testable
- Provide different levels(severity) of logging. Trace, Debug and Exception.
- Filter on logging level
- Log to various targets. Email, File and Console.
- Provide different areas or categories to log as. e.g. Presentation, Agent, Services...
- Filter on logging Category (so I don't see the agent debug messages and agent developers don't see my presentation guff)
- Log un-handled exceptions so we can analyse then when they occur on live
- Log the inputs to the method that threw the exception so we can identify it it is data related
- Replace console/debug/trace calls with new logger.
Journey to a solution
Logging System
First thing I will tackle is the testability. I will create my own interface called ILogger that will do my logging for mepublic interface ILogger { void Write(string message); }
Well that was easy! Next, Provide different levels of logging. I am going to do that by replacing my Write method with level specific methods:
public interface ILogger { void Trace(string message); void Debug(string message); void Error(string message); }
To now add Categories. Here I will add a category parameter to each of the methods. I would advise against using enums for your categories, especially if your logger becomes a shared API. Enums can not be extended and really tie you down.
public interface ILogger { void Trace(string category, string message); void Debug(string category, string message); void Error(string category, string message); }
Hmmm. It now looks like the write we are making a mess for the poor developer who has to implement this interface. I think I may have been on the right track with the write method in the first place. I am going to change that back a Write method that takes 3 parameters. I will offer extension methods to give users the ability to make calls like the one above (a little bit more work for me but less work for everyone else). See here for more info on extending interfaces with Extension methods.
public interface ILogger { void Write(LogLevel level, string category, string message); }
Some may be thinking that "Hang on. Strings are bad! Surely a category Enum has to be better?". To answer this I would recommend using an internal class like this
class Category { public const string Agent = "Agent"; public const string Presentation = "Presentation"; public const string Agent = "Service"; }
If you want to expose categories at your public API, create a public version of this class that consumers can use an even inherit from to add their own categories too. If you are thinking "shouldnt they be public static readonly?", I will cover that later.
Checking off aginst our requirements list we can cross somethings off
Testable(Interfaces make things testable)Provide different levels(severity) of logging. Trace, Debug and Exception.- Filter on logging level
- Log to various targets. Email, File and Console.
Provide different areas or categories to log as. eg Presentation, Agent, Services...- Filter on logging Category (so I dont see the agent debug messages and agent developers don't see my presentation guff)
Great! Thanks to the 3rd party logging system we have ticked off all of our system requirements, now for our implementation requirements.
Implementation of Logging
Now that we have an interface to code against, let us now look at how we would use it.Ed- This is completely back to front. You should normally look at how you would use it first and then create the implementation. TDD is a great methodology for this. We are approaching it back-to-front in this post because I think it is easier to consume for the reader.
So the main pain point we have is logging exceptions that occur at the agents (the classes that get data from services and map them to client side entities). This is due to poor 3rd party documentation, Live being slightly different to Dev and QA and integration testing being harder to perform than unit testing.
Prior to our new Logging system some agent code might look like this:
public void AmendPortfolio(PortfolioAmmendment portfolio) { Console.WriteLine("AmendPortfolio {0}", portfolio.Id); var data = portfolio.MapToServiceData(); //Mapper extension method. _service.AmendBasket(_session, data, true); //Call to 3rd party system Console.WriteLine("AmendPortfolio complete"); }
Now if we swapped out the console calls with out logger and then put in some exception logging it may look like this
public void AmendPortfolio(PortfolioAmmendment portfolio) { _logger.Debug(Category.Agent, "AmendPortfolio {0}", portfolio.Id); try { var data = portfolio.MapToServiceData(); //Mapper extension method. _service.AmendBasket(_session, data, true); //Call to 3rd party system } catch(Exception ex) { _logger.Error(Category.Agent, ex.ToString); throw; } _logger.Debug(Category.Agent, "AmendPortfolio complete"); }
Oh dear. We now have more code that does logging than code that does real work. While we have satisfied our requirements, we have doubled our work load. Not good. Back to the drawing board.
AOP
Some will have heard of Aspect Orientated Programming. It seemed like 5 years ago it was going to change everything. Well it mainly just changed logging. AOP is a style of programming that allows code to be injected at a given interception point. In our case the interception point would be the start of our agent method, the end of the agent method and when the method throws and exception (which is really the same as the end of the method). As far as I know there are 2 popular ways to achieve this- at run time using a tool like Castle Windsor or Microsoft Unity
- at compile time using a tool like PostSharp
First a quick introduction on how I have previously done AOP logging with PostSharp. I would create an Attribute that I would apply to classes or methods that I wanted to be logged. The attribute would satisfy the requirements of PostSharp so that code would be injected at compile time to do my logging.
code like this
[Logged] public void AmendPortfolio(PortfolioAmmendment portfolio) { var data = portfolio.MapToServiceData(); //Mapper extension method. _service.AmendBasket(_session, data, true); //Call to 3rd party system }
which at compile time would alter the IL to represent something more like this:
public void AmendPortfolio(PortfolioAmmendment portfolio) { Logger.Debug("AmendPortfolio({0})", portfolio.Id); try { var data = portfolio.MapToServiceData(); //Mapper extension method. _service.AmendBasket(_session, data, true); //Call to 3rd party system } catch(Exception ex) { Logger.Error(ex.ToString); throw; } Logger.Debug("AmendPortfolio complete"); }Well that looks perfect doesn't it? Not really. We dont have a category specified and we have a hard coded reference the static class Logger from Enterprise Library Logging. It no longer points to our _logger member variable which was of type ILogger. This makes our testing harder to do. If testing your logger is not really something you care about (which is fine) then this AOP solution might be for you. If you do want to be more specific about logging then we need to find a way of getting hold of the instance of ILogger. As PostSharp is a compile time AOP framework, it is a touch harder to integrate than if we used Unity or Windsor. The main problem is how do we get a handle on the Logger? A solution we came up with was to create an ILogged interface
public interface ILogged { ILogger Logger { get; } }By doing this we expose the Logger so we can use it in the Aspect/Attribute. Now if we look at out method that we are trying to log in the greater context of the class it resides in we can see what the implementation may now look like.
[Logged] public class PortfolioAgent : IPortfolioAgent, ILogged { private readonly ILogger _logger; private readonly ISomeService _service; public PortfolioAgent(ILogger logger, ISomeService _service) { _logger = logger; _service = service; } public void AmendPortfolio(PortfolioAmmendment portfolio) { var data = portfolio.MapToServiceData(); //Mapper extension method. _service.AmendBasket(_session, data, true); //Call to 3rd party system } }
That looks kinda cool to me. One thing my colleagues made a note of is that they would prefer if in this scenario we used property injection for the logger. This is fine with me as long as it is easy to use and we can still test it. The ILogged interface does not preclude using property injection, it is just not my preference. Another thing to note is the lack of Category. Easy fix there is to add a property to our LoggedAttribute of string Category.
[Logged(Category=Category.Agent)] public class PortfolioAgent : IPortfolioAgent, ILogged { ... }
If you remember earlier that I mentioned public const vs public static readonly. This is the reason why I choose to use const types so that they could be used in the attribute.
I am pretty happy with this now. I think we tick off all of our requirements and have not added a lot of complexity to our code. The one last thing that bugs me is that I have to use the LoggedAttribute and the ILogged interface as a couple. If I forget to use one without the other, I either will get no logging, a nasty runtime exception or if I code a aspect correctly I can get a compile time error (the most desirable). At first I coded the attribute to do the latter (compile time error), but then realised that all of my agents were in one project and all used the same Category. To make life a little bit easier I moved the attribute to the AssemblyInfo and had the Aspect apply itself automatically to any class the implemented ILogged. This maybe a step too far towards black magic for some teams, so do what fits best.
Have a look at the sample code here. Both styles are shown in the example so take your pick of which suits you best.