Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Message and exception provided via LoggingEvent created from LoggingEventData is not sent to Elastic Search #6

Closed
RedwoodForest opened this issue Aug 27, 2015 · 8 comments

Comments

@RedwoodForest
Copy link

We have a remote system that submits information to be logged to our server. This information includes the message and sometimes exception data.

On our server we then log the message using log4net like this:

var eventData = new LoggingEventData {
    LoggerName = diagnostic.loggerName,
    Message = diagnostic.message",
    ExceptionString = diagnostic.maybeExceptionString,
    TimeStamp = diagnostic.timeStamp,
    // etc...
};

var loggingEvent = new LoggingEvent(eventData);

loggingEvent.Fix = FixFlags.All;

ILogger logger = ... // Get ILogger
logger.Log(loggingEvent)

Everything works as expected when using a log4net FileAppender, but when submitted to Elastic Search using log4stash the message and exception are not sent.

I tracked this down to the ElasticSearchAppender implementation where it gets the message from loggingEvent.MessageObject.ToString() and the exception from loggingEvent.ExceptionObject.ToString(). A LoggingEvent created from LoggingEventData does not have values for these properties.

It seems like the correct place to get this information is loggingEvent.RenderedMessage and loggingEvent.GetExceptionString().

Note that GetExceptionString() returns an empty string rather than null if no exception information is provided, so you'll probably need to using string.IsNullOrEmpty() when checking whether or not there's exception information to send.

@RedwoodForest RedwoodForest changed the title Message and Exception provided via LoggingEvent created from LoggingEventData is not sent to Elastic Search Message and exception provided via LoggingEvent created from LoggingEventData is not sent to Elastic Search Aug 27, 2015
@urielha
Copy link
Owner

urielha commented Sep 2, 2015

Hi @lawrencejohnston,
I will look at it ASAP, in the past I removed the reference to loggingEvent.RenderedMessage cause I saw no difference..
Maybe it's my fault.

will update you soon.
Thanks!!

By the way,
Are you sure you want to do this: loggingEvent.Fix = FixFlags.All ?
Consider that this has a performance hit, maybe FixFlags.Partial will be enough for you :)

@RedwoodForest
Copy link
Author

Hi @urielha

Thanks!

I'll take a look at FixFlags.All vs FixFlags.Partial for my code. I'm using it to stop log4net from dynamically populating fields that I'm not setting explicitly in the event (like the thread name). It may be that as long as in my appenders I'm only using fields included in FixFlags.Partial that that would be fine.

@urielha
Copy link
Owner

urielha commented Sep 3, 2015

@lawrencejohnston ,
Are you using ILogger from a reason? why don't you use the regular ILog?

One more thing,
The object you are logging is of Exception type? or it's just the LoggingEvent?
I'm trying to build a test upon it so we can be sure we got the right behavior.

Thanks

@RedwoodForest
Copy link
Author

We're using ILogger because it has a Log method that accepts a LoggingEvent (rather than just a string and exception). We need to use a LoggingEvent because the data is coming from a remote system and already has TimeStamp/ThreadName/Exception (serialized to a string) information.

The object we're logging is a LoggingEvent with the ExceptionString property set. Something like this should work for your test:

var eventData = new LoggingEventData {
    ExceptionString = "Any string" // Can make it look more like Exception.ToString() if you want, but any string will do
    // Any other properties you want to test...
};

var loggingEvent = new LoggingEvent(eventData);

ILogger logger = ... // Get ILogger
logger.Log(loggingEvent)

@urielha
Copy link
Owner

urielha commented Sep 15, 2015

I'm little busy those days so I will look at it ASAP.
Thanks!

@urielha
Copy link
Owner

urielha commented Sep 27, 2015

Hi @lawrencejohnston,
It's taking me more time than I thought, but I think you can workaround this issue.

Try to insert the ExceptionString into a property of the loggingEvent:

var eventData = new LoggingEventData {
   // ....
};
eventData.Properties["TheException"] = "Exception string";
var loggingEvent = new LoggingEvent(eventData);

ILogger logger = ... // Get ILogger
logger.Log(loggingEvent)

let me know if this worked for you.

And one question:
can you tell me how exactly you extract the ILogger? are you using ILog.Logger member?

Best regards

@RedwoodForest
Copy link
Author

Yes, that's how we're getting an ILogger.

That workaround seems like it would work, but for now we're using a local fork with the changes described in the first post. As soon as the issues we've encountered are fixed and released we'll switch back to the NuGet version.

urielha added a commit that referenced this issue Oct 10, 2015
@urielha
Copy link
Owner

urielha commented Oct 10, 2015

Fixed with commit 560676d

@urielha urielha closed this as completed Oct 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants