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.

Sign up for GitHub

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

setting to exclude MDC keys from being sent to Elastic #57

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

setting to exclude MDC keys from being sent to Elastic #57

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6cd8d99
setting to exclude MDC keys from being sent to Elastic
PeterFokkinga Aug 23, 2018
105d6e7
performance improvement in ClassicElasticsearchPublisher as suggested…
PeterFokkinga Jan 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ In your `logback.xml`:
<sleepTime>250</sleepTime> <!-- optional (in ms, default 250) -->
<rawJsonMessage>false</rawJsonMessage> <!-- optional (default false) -->
<includeMdc>false</includeMdc> <!-- optional (default false) -->
<excludedMdcKeys>stacktrace</excludedMdcKeys> <!-- optional (default empty) -->
<maxMessageSize>100</maxMessageSize> <!-- optional (default -1 -->
<authentication class="com.internetitem.logback.elasticsearch.config.BasicAuthentication" /> <!-- optional -->
<properties>
Expand Down Expand Up @@ -108,6 +109,7 @@ Configuration Reference
* `errorLoggerName` (optional): If set, any internal errors or problems will be logged to this logger
* `rawJsonMessage` (optional, default false): If set to `true`, the log message is interpreted as pre-formatted raw JSON message.
* `includeMdc` (optional, default false): If set to `true`, then all [MDC](http://www.slf4j.org/api/org/slf4j/MDC.html) values will be mapped to properties on the JSON payload.
* `excludedMdcKeys` (optional, default empty): comma separated (extra whitespace is fine) list of case sensitive MDC keys that should not be mapped automatically to properties; only useful when includeMdc is set to `true`
* `maxMessageSize` (optional, default -1): If set to a number greater than 0, truncate messages larger than this length, then append "`..`" to denote that the message was truncated
* `authentication` (optional): Add the ability to send authentication headers (see below)

Expand Down
15 changes: 8 additions & 7 deletions src/main/java/com/internetitem/logback/elasticsearch/AbstractElasticsearchAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
package com.internetitem.logback.elasticsearch;

import ch.qos.logback.core.UnsynchronizedAppenderBase;
import com.internetitem.logback.elasticsearch.config.*;
import com.internetitem.logback.elasticsearch.util.ErrorReporter;

import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;

import ch.qos.logback.core.UnsynchronizedAppenderBase;
import com.internetitem.logback.elasticsearch.config.Authentication;
import com.internetitem.logback.elasticsearch.config.ElasticsearchProperties;
import com.internetitem.logback.elasticsearch.config.HttpRequestHeaders;
import com.internetitem.logback.elasticsearch.config.Settings;
import com.internetitem.logback.elasticsearch.util.ErrorReporter;

public abstract class AbstractElasticsearchAppender<T> extends UnsynchronizedAppenderBase<T> {

protected Settings settings;
Expand Down Expand Up @@ -131,6 +128,10 @@ public void setIncludeMdc(boolean includeMdc) {
settings.setIncludeMdc(includeMdc);
}

public void setExcludedMdcKeys(String setExcludedMdcKeys) {
settings.setExcludedMdcKeys(setExcludedMdcKeys);
}

public void setAuthentication(Authentication auth) {
settings.setAuthentication(auth);
}
Expand Down
39 changes: 27 additions & 12 deletions src/main/java/com/internetitem/logback/elasticsearch/ClassicElasticsearchPublisher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
package com.internetitem.logback.elasticsearch;

import java.io.IOException;
import java.util.Map;

import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.Context;
import com.fasterxml.jackson.core.JsonGenerator;
import com.internetitem.logback.elasticsearch.config.ElasticsearchProperties;
import com.internetitem.logback.elasticsearch.config.HttpRequestHeaders;
import com.internetitem.logback.elasticsearch.config.Property;
import com.internetitem.logback.elasticsearch.config.Settings;
import com.internetitem.logback.elasticsearch.util.AbstractPropertyAndEncoder;
import com.internetitem.logback.elasticsearch.util.ClassicPropertyAndEncoder;
import com.internetitem.logback.elasticsearch.util.ErrorReporter;
import com.internetitem.logback.elasticsearch.config.*;
import com.internetitem.logback.elasticsearch.util.*;

import java.io.IOException;
import java.util.*;
Copy link

Choose a reason for hiding this comment

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

New to the project, but I don't think star-imports are used anywhere else (and haven't been used before in this file).



public class ClassicElasticsearchPublisher extends AbstractElasticsearchPublisher<ILoggingEvent> {

Expand Down Expand Up @@ -40,10 +36,29 @@ protected void serializeCommonFields(JsonGenerator gen, ILoggingEvent event) thr
gen.writeObjectField("message", formattedMessage);
}

if(settings.isIncludeMdc()) {
if (settings.isIncludeMdc()) {
List<String> excludedKeys = getExcludedMdcKeys();
for (Map.Entry<String, String> entry : event.getMDCPropertyMap().entrySet()) {
gen.writeObjectField(entry.getKey(), entry.getValue());
if (!excludedKeys.contains(entry.getKey())) {
gen.writeObjectField(entry.getKey(), entry.getValue());
}
}
}
}

private List<String> getExcludedMdcKeys() {
/*
* using a List instead of a Map because the assumption is that
* the number of excluded keys will be very small and not cause
* a performance issue
*/
List<String> result = new ArrayList<>();
if (settings.getExcludedMdcKeys() != null) {
String[] parts = settings.getExcludedMdcKeys().split(",");
for (String part : parts) {
result.add(part.trim());
}
}
return result;
Copy link

Choose a reason for hiding this comment

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

Is there a reason this list is re-allocated and populated anew every time a log entry is written? Why is this not parsed once when reading the config and then re-used on subsequent accesses? Looks like this could incur a performance overhead.

List instead of a map or set is probably fine, because this will likely only matter when there are hundreds of items in the collection and calculating the hash code of each tested key can be avoided.

}
}
9 changes: 9 additions & 0 deletions src/main/java/com/internetitem/logback/elasticsearch/config/Settings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public class Settings {
private boolean errorsToStderr;
private boolean includeCallerData;
private boolean includeMdc;
private String excludedMdcKeys;
private boolean rawJsonMessage;
private int maxQueueSize = 100 * 1024 * 1024;
private Authentication authentication;
Expand Down Expand Up @@ -155,6 +156,14 @@ public void setIncludeMdc(boolean includeMdc) {
this.includeMdc = includeMdc;
}

public String getExcludedMdcKeys() {
return excludedMdcKeys;
}

public void setExcludedMdcKeys(String excludedMdcKeys) {
this.excludedMdcKeys = excludedMdcKeys;
}

public int getMaxMessageSize() {
return maxMessageSize;
}
Expand Down
3 changes: 3 additions & 0 deletions src/test/java/com/internetitem/logback/elasticsearch/ElasticsearchAppenderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ public void should_delegate_setters_to_settings() throws MalformedURLException {
boolean errorsToStderr = false;
boolean rawJsonMessage = false;
boolean includeMdc = true;
String excludedMdcKeys = "stacktrace,url";
String index = "app-logs";
String type = "appenderType";
int maxQueueSize = 10;
Expand All @@ -202,6 +203,7 @@ public void should_delegate_setters_to_settings() throws MalformedURLException {
appender.setConnectTimeout(connectTimeout);
appender.setRawJsonMessage(rawJsonMessage);
appender.setIncludeMdc(includeMdc);
appender.setExcludedMdcKeys(excludedMdcKeys);

verify(settings, times(1)).setReadTimeout(readTimeout);
verify(settings, times(1)).setSleepTime(aSleepTime);
Expand All @@ -218,6 +220,7 @@ public void should_delegate_setters_to_settings() throws MalformedURLException {
verify(settings, times(1)).setConnectTimeout(connectTimeout);
verify(settings, times(1)).setRawJsonMessage(rawJsonMessage);
verify(settings, times(1)).setIncludeMdc(includeMdc);
verify(settings, times(1)).setExcludedMdcKeys(excludedMdcKeys);
}


Expand Down