Structured logging

[gregkalapos]

Currently the logging interface is very simple. It's basically 1 method, with it's string parameter (which contains the log level, all the parameters, etc). Users have access to the log, and they can redirect it wherever they want by doing this:

Agent.Config.Logger = new CustomLogCollector();

class CustomLogCollector : AbstractLogger
{
	protected override void PrintLogline(string logline)
	{
		//send the logline or process it...
	}
}

@SeanFarrow suggested here that:

Even if I derive from AbstractLogger to send the log somewhere else, I’m stuck with a string and would have to use something else to extract data using regex or an alternative input such as LogStash.

Let's discuss if we could make this a nicer interface! I'm totally open to provide an interface with more details.

@SeanFarrow feel free to suggest a specific interface if you have something in mind.

Thanks

[Mpdreamz]

I think a single method is a feature we should strive to keep it. In fact I'd propose only shipping a single interface getting rid of AbstractLogger all together.

public interface IApmLogger
{
     void Log<TState>(ApmLogLevel level, TState state, Func<TState, string> formatter) 
}

public class MyCustomLogger : IApmLogger
{
    public void Log<TState>(ApmLogLevel level, TState state, Func<TState, string> formatter) 
    {

    }
}

Obviously heavy inspired by Microsoft.Extensions.Logging.ILogging.Log

I am usually not a fan of smurf typing but in the case of logging adapters I am since most libraries end up with similar named logging interfaces.

Apm.Agent.Config.LogLevel should not be a concern of the agent but the logging adapter.

It being a single method we can then expose this configuration as a method with two overloads either taking an IApmLogger or the single method as Action<> (which we can wrap into an eg internal ApmWrappedLogger instance`.

[SeanFarrow]

Should we be logging any exceptions, or do they not apply?

[Mpdreamz]

TState could very well be an Exception. Microsoft.Extensions.Logging.ILogging.Log has these separated not sure we need too but perhaps reusing their full signature (additional Exception arg) is best indeed.

[SeanFarrow]

Makes sense. From: Martijn Laarman <[email protected]> Sent: 10 January 2019 09:53 To: elastic/apm-agent-dotnet <[email protected]> Cc: Sean Farrow <[email protected]>; Mention <[email protected]> Subject: Re: [elastic/apm-agent-dotnet] Discussion: Structured logging (#61) TState could very well be an Exception. Microsoft.Extensions.Logging.ILogging.Log has these separated not sure we need too. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#61 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABY1fhjDMGvSBDPd31PLx2W9AZlyp6lsks5vBw2GgaJpZM4Z4J65>.