sentry-dotnet icon indicating copy to clipboard operation
sentry-dotnet copied to clipboard

[Log4Net] Support custom `SentryMessage` generation

Open Pretasoc opened this issue 3 years ago • 2 comments

Currently the SentryAppender always uses LogEvent.RenderedMessage property, when the message for an SentryEvent is created. This causes some problem for one application of mine ahead, which heavily uses formatted strings to generate log messages. The problem is, that a lot of events don't get recognized as the same, as they vary a lot. This could of couse be solved by using some custom fingerprinting, but it would be easier to utilize the existing LogEvent.MessageObject property, which may contain the unformatted log message and its parameters, depending on the logger implementation.

Possible solutions

As there are a lot of logger implementations in log4net there is no generic useful assumption, what type to expect from MessageObject so a flexible approach is needed. I have some ideas for a possible solution listed below.

MessageFactory Callback

Add a new property Func<object, SentryMessage?>? MessageFactory { get; set; } to the SentryAppender class. This function (if its not null) would then be called inside Append() before trying to read the message from RenderedMessage and the result would be used (again if its not null) instead of RenderedMessage.

The factory function may be moved into a new interface, to support setting the factory from the log4net xml configuration using a type name.

Levering the TypeConverter class

An other approach would be to call the TypeDescriptor.GetConverter(...) and see, wether there is a TypeConverter, thats supports converting to a SentryMessage instance

Own opinion

Id prefer the first proposal, as is rather easy to implement and would't require any reflection, which would be inevitable at least at one point using the TypeConverterapproach. The added complexity needed for caching is also a drawback. On the other side that would bring an easy way to support more than type asMessageObject`. But that could be also done in a custom implementation of my first idea.

I'm intrested, what you've got to say about it and i'd implement the feature, if there is a consence.

Pretasoc avatar Jan 13 '22 20:01 Pretasoc

Fingerprint isn't the only way in this case. We have this built in a way that messages group by the template for Serilog and NLog. Only log4net happend to be limited.

e.g: Serilog: https://github.com/getsentry/sentry-dotnet/blob/6f7d05cbb29b73453a5c7db0e105df9c78bd923c/src/Sentry.Serilog/SentrySink.cs#L82-L86

In the case of log4net: https://github.com/getsentry/sentry-dotnet/blob/6f7d05cbb29b73453a5c7db0e105df9c78bd923c/src/Sentry.Log4Net/SentryAppender.cs#L99

One option here is to expose a new Option for the log4net integration to use a new implementation that would attempt to use MessageObject as the template, if that's indeed how this works (sorry I haven't used log4net in almost 10 years).

Would you be willing to open a PR to show what you have in mind?

bruno-garcia avatar Jan 13 '22 20:01 bruno-garcia

@Pretasoc - If you wouldn't mind a draft PR on this, that would be great. Thanks!

mattjohnsonpint avatar Mar 17 '22 18:03 mattjohnsonpint