exceptionless/Exceptionless.Net

NLogExceptionlessLog Missing Exception object

snakefoot opened this issue · 4 comments

This should be changed:

public void Error(string message, string source = null, Exception exception = null) {
if (LogLevel.Error < MinimumLogLevel)
return;
_logger.ForErrorEvent().Message(message).LoggerName(source).Log();

To this:

_logger.ForErrorEvent().Message(message).LoggerName(source).Exception(exception).Log(); 

Notice it is a little special that you modify the LoggerName, since this is usually derived from _logger. Can see that log4net does the following:

NLog.LogManager.GetLogger(source ?? GetType().ToString()).ForErrorEvent().Message(message).Exception(exception).Log(typeof(NLogExceptionlessLog)); 

Any reason why not doing the same for NLog ?

P.S the reason for using .Log(typeof(NLogExceptionlessLog)) is to make NLog Callsite work correctly.

@snakefoot thanks for reporting this, For the call site I don't see anywhere where it says a target should specify it. I would think it would be inferred as it's trying to capture the calling code call site (not the target as the call site) from what I understand. Is there more information for this or are you seeing an issue? Would you mind submitting a pr for this or opening an issue just for that?

NLogExceptionlessLog is not a NLog Target but a custom wrapper around the NLog Logger

When wrapping the NLog Logger, then must provide the type of the custom-wrapper when calling the NLog Logger. Forexample like this:

NLog.LogManager.GetLogger(source ?? typeof(NLogExceptionlessLog).ToString())
    .ForErrorEvent()
    .Message(message)
    .Exception(exception)
    .Log(typeof(NLogExceptionlessLog)); 

Also your custom logic with LoggerName is not recommended. I think NLogExceptionlessLog should behave like Log4netExceptionlessLog (Lookup the Logger from source-input)

@snakefoot thanks for this information, would you mind submitting a pr for this, it would help out a lot.

Created #299