exceptionless / Exceptionless.Net

Exceptionless clients for the .NET platform
https://exceptionless.com
Other
551 stars 142 forks source link

NLogExceptionlessLog Missing Exception object #298

Closed snakefoot closed 1 year ago

snakefoot commented 1 year ago

This should be changed:

https://github.com/exceptionless/Exceptionless.Net/blob/17841c9b07858cb3aa5f5f4d5cf038afff5a7bf2/src/Platforms/Exceptionless.NLog/NLogExceptionlessLog.cs#L17-L22

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.

niemyjski commented 1 year ago

@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?

snakefoot commented 1 year ago

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)

niemyjski commented 1 year ago

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

snakefoot commented 1 year ago

Created #299