Skip to content
This repository has been archived by the owner on Feb 8, 2022. It is now read-only.

Resolve host name once per appender instead of once per log message #18

Merged
merged 4 commits into from
Jul 6, 2018
Merged

Resolve host name once per appender instead of once per log message #18

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5c542cf
Fix bug where hostname was determined for every single logger message
Jun 25, 2018
6653e75
Make more variables final
Jun 25, 2018
1c51cd5
Revert the change to additionalFields - it was not correct
Jul 5, 2018
174c4b4
revert change to jdk 1.8
Jul 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 13 additions & 23 deletions src/main/java/de/appelgriepsch/logback/GelfAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ public class GelfAppender extends AppenderBase<ILoggingEvent> {
private int sendBufferSize = -1;
private boolean tcpNoDelay = false;
private boolean tcpKeepAlive = false;
private Map<String, Object> additionalFields = new HashMap<>();
private ThrowableProxyConverter throwableConverter = new ThrowableProxyConverter();
private final Map<String, Object> additionalFields = new HashMap<>();
private final ThrowableProxyConverter throwableConverter = new ThrowableProxyConverter();
private Layout<ILoggingEvent> layout;

private GelfTransport client;
Expand All @@ -68,7 +68,7 @@ protected void append(ILoggingEvent event) {
copy.setTimeStamp(event.getTimeStamp());
copy.setMDCPropertyMap(event.getMDCPropertyMap());

final GelfMessageBuilder builder = new GelfMessageBuilder(this.layout.doLayout(copy), hostName()).timestamp(
final GelfMessageBuilder builder = new GelfMessageBuilder(this.layout.doLayout(copy), hostName).timestamp(
event.getTimeStamp() / 1000d)
.level(GelfMessageLevel.fromNumericLevel(toGelfNumericValue(event.getLevel())))
.additionalField("loggerName", event.getLoggerName())
Expand All @@ -81,12 +81,10 @@ protected void append(ILoggingEvent event) {
}

if (includeMDC) {
for (Map.Entry<String, String> entry : event.getMDCPropertyMap().entrySet()) {
builder.additionalField(entry.getKey(), entry.getValue());
}
this.additionalFields.putAll(event.getMDCPropertyMap());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good catch. Working on fixing this now.

}

StackTraceElement[] callerData = event.getCallerData();
final StackTraceElement[] callerData = event.getCallerData();

if (includeSource && event.hasCallerData() && callerData.length > 0) {
StackTraceElement source = callerData[0];
Expand All @@ -97,7 +95,7 @@ protected void append(ILoggingEvent event) {
builder.additionalField("sourceLineNumber", source.getLineNumber());
}

IThrowableProxy thrown = event.getThrowableProxy();
final IThrowableProxy thrown = event.getThrowableProxy();

if (includeStackTrace && thrown != null) {
String convertedThrowable = throwableConverter.convert(event);
Expand Down Expand Up @@ -134,6 +132,13 @@ public void start() {
patternLayout.start();
this.layout = patternLayout;
}
if (this.hostName == null || this.hostName.trim().isEmpty()) {
try {
this.hostName = InetAddress.getLocalHost().getHostName();
} catch (UnknownHostException e) {
this.hostName = "localhost";
}
}
createGelfClient();
throwableConverter.start();
super.start();
Expand All @@ -148,21 +153,6 @@ public void stop() {
throwableConverter.stop();
}


private String hostName() {

if (hostName == null || hostName.trim().isEmpty()) {
try {
return InetAddress.getLocalHost().getHostName();
} catch (UnknownHostException e) {
return "localhost";
}
}

return hostName;
}


private void createGelfClient() {

client = GelfTransports.create(getGelfConfiguration());
Expand Down