jjchiw / gelf4net

GELF log4net Appender - graylog2
MIT License
63 stars 59 forks source link

Check the LoggingEvent.RenderedMessage property as well if MessageObject is null #36

Closed andrasnemes closed 7 years ago

andrasnemes commented 8 years ago

Hello,

In log4net a new LoggingEvent object can be constructed using the LoggingEventData struct:

LoggingEventData loggingEventData = new LoggingEventData(); loggingEventData.Message = "Here is a short message"; //other properties are also set, ignored here LoggingEvent loggingEvent = new LoggingEvent(loggingEventData);

Strangely enough the MessageObject property of LoggingEvent will be null, whereas the RenderedMessage property will be set to "Here is a short message".

Then in the AddLoggingEventToMessage method of GelfLayout (https://github.com/jjchiw/gelf4net/blob/master/src/Gelf4net/Layout/GelfLayout.cs , line 144) the code only checks the MessageObject property of LoggingEvent. If that is null then we pass a null message to Graylog.

I think it would make sense to check the RenderedMessage property as well. If MessageObject is null but RenderedMessage is not an empty string then gelfMessage.FullMessage should be populated with that value.

Thanks and best regards, Andras Nemes

jjchiw commented 8 years ago

Hi!

Interesting after reading the Contstructor documentation's remark.

This constructor is provided to allow a LoggingEvent to be created independently of the log4net framework. This can be useful if you require a custom serialization scheme.

Use the [M:GetLoggingEventData(FixFlags)] method to obtain an instance of the LoggingEventData class.

This constructor sets this objects Fix flags to All, this assumes that all the data relating to this event is passed in via the data parameter and no other data should be captured from the environment.

When we programmed the GelfLayout we didn't expect to be used independently of the log4net framework. I will implement it during the week. I'll update the issue when the new NuGet package is up.

:)

andrasnemes commented 8 years ago

Hello, thanks for your quick feedback.

Here comes a short background why I need this fix. We use the overloaded constructor of LoggingEvent described above out of necessity, not because we use gelf4net independently. The LocationInformation property seems to be incorrectly populated on some occasions when calling the "normal" constructor so I wanted to set it directly using LoggingEventData. The location information is now fine but MessageObject is null. I therefore also filed a request with the log4net team but I figured that their response time may be quite long. Hence I inserted this request hoping you guys will be faster :-)

Thanks for your help, Andras

jjchiw commented 8 years ago

Hi!

It's done README.md

Here it's the code if you want to read it GelfLayout.cs:146

And the packages are