Resolve host name once per appender instead of once per log message #18
Conversation
|
Nice one man. Good find. And improvement is nice |
ghost
mentioned this pull request
| for (Map.Entry<String, String> entry : event.getMDCPropertyMap().entrySet()) { | ||
| builder.additionalField(entry.getKey(), entry.getValue()); | ||
| } | ||
| this.additionalFields.putAll(event.getMDCPropertyMap()); |
There was a problem hiding this comment.
I think this change introduces a bug. Previously the dynamic additional fields where only added to the builder, not to the appenders this.additionalFields (these are only the additional fields from the configuration file).
Say a log event comes with property foo included in the MDC, than this property is added to this.additionalFields and all those additional fields are included in the builder. That's fine so far. But when there's a second log event without the property foo, then this event's gelf message should not include foo (but it will with this change).
There was a problem hiding this comment.
Oh, good catch. Working on fixing this now.
|
I reverted the change to additionalFields |
This fixes a bug where the host name was resolved for each log message (if it was not set explicitly in the log config). For my benchmarks this increased throughput by ~15%.
I also made a couple more variables final.