Skip to content

Commit

Permalink
encoder uses layout for all message formating, but still prevents bla… (
Browse files Browse the repository at this point in the history 
#102)

encoder uses layout for all message formating, but still prevents blank messages (#100)
  • Loading branch information
@fupgang
fupgang authored Mar 16, 2024
1 parent 7973aba commit 372c9a7
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 102 deletions.
98 changes: 26 additions & 72 deletions src/main/java/de/siegmar/logbackgelf/GelfEncoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.classic.util.LevelToSyslogSeverity;
import ch.qos.logback.core.CoreConstants;
import ch.qos.logback.core.Layout;
import ch.qos.logback.core.encoder.EncoderBase;
import de.siegmar.logbackgelf.mappers.CallerDataFieldMapper;
import de.siegmar.logbackgelf.mappers.KeyValueFieldMapper;
Expand Down Expand Up @@ -117,14 +118,16 @@ public class GelfEncoder extends EncoderBase<ILoggingEvent> {
private boolean appendNewline;

/**
* Short message format. Default: {@value DEFAULT_SHORT_PATTERN}.
* Short message format.
* Default is a {@link PatternLayout} with {@value DEFAULT_SHORT_PATTERN}.
*/
private PatternLayout shortPatternLayout;
private Layout<ILoggingEvent> shortMessageLayout;

/**
* Full message format (Stacktrace). Default: {@value DEFAULT_FULL_PATTERN}.
* Full message format (Stacktrace).
* Default is a {@link PatternLayout} with {@value DEFAULT_FULL_PATTERN}.
*/
private PatternLayout fullPatternLayout;
private Layout<ILoggingEvent> fullMessageLayout;

/**
* Log numbers as String. Default: false.
Expand All @@ -136,12 +139,6 @@ public class GelfEncoder extends EncoderBase<ILoggingEvent> {
*/
private final Map<String, Object> staticFields = new HashMap<>();

/**
* Max length for short message (truncate after this length).
* Default: {@code 0} (no truncate).
*/
private int maxShortMessageLength;

private final List<GelfFieldMapper<?>> builtInFieldMappers = new ArrayList<>();

private final List<GelfFieldMapper<?>> fieldMappers = new ArrayList<>();
Expand Down Expand Up @@ -250,20 +247,20 @@ public void setNumbersAsString(final boolean numbersAsString) {
this.numbersAsString = numbersAsString;
}

public PatternLayout getShortPatternLayout() {
return shortPatternLayout;
public Layout<ILoggingEvent> getShortMessageLayout() {
return shortMessageLayout;
}

public void setShortPatternLayout(final PatternLayout shortPatternLayout) {
this.shortPatternLayout = shortPatternLayout;
public void setShortMessageLayout(final Layout<ILoggingEvent> shortMessageLayout) {
this.shortMessageLayout = shortMessageLayout;
}

public PatternLayout getFullPatternLayout() {
return fullPatternLayout;
public Layout<ILoggingEvent> getFullMessageLayout() {
return fullMessageLayout;
}

public void setFullPatternLayout(final PatternLayout fullPatternLayout) {
this.fullPatternLayout = fullPatternLayout;
public void setFullMessageLayout(final Layout<ILoggingEvent> fullMessageLayout) {
this.fullMessageLayout = fullMessageLayout;
}

public Map<String, Object> getStaticFields() {
Expand All @@ -289,17 +286,6 @@ public void addStaticField(final String staticField) {
addStaticField(split[0].trim(), split[1].trim());
}

public int getMaxShortMessageLength() {
return maxShortMessageLength;
}

public void setMaxShortMessageLength(final int maxShortMessageLength) {
if (maxShortMessageLength < 0) {
throw new IllegalArgumentException("maxShortMessageLength must not be < 0");
}
this.maxShortMessageLength = maxShortMessageLength;
}

public List<GelfFieldMapper<?>> getFieldMappers() {
return Collections.unmodifiableList(fieldMappers);
}
Expand Down Expand Up @@ -343,11 +329,11 @@ public void start() {
if (originHost == null || originHost.isBlank()) {
originHost = Optional.ofNullable(context.getProperty(CoreConstants.HOSTNAME_KEY)).orElse("unknown");
}
if (shortPatternLayout == null) {
shortPatternLayout = buildPattern(DEFAULT_SHORT_PATTERN);
if (shortMessageLayout == null) {
shortMessageLayout = buildPattern(DEFAULT_SHORT_PATTERN);
}
if (fullPatternLayout == null) {
fullPatternLayout = buildPattern(DEFAULT_FULL_PATTERN);
if (fullMessageLayout == null) {
fullMessageLayout = buildPattern(DEFAULT_FULL_PATTERN);
}
addBuiltInFieldMappers();

Expand Down Expand Up @@ -426,54 +412,22 @@ protected GelfMessage buildGelfMessage(final long timestamp, final int logLevel,
}

protected String normalizeShortMessage(final String shortMessage) {
// Graylog doesn't like newlines in short messages: https://github.com/Graylog2/graylog2-server/issues/4842
final String sanitizedShortMessage = sanitizeShortMessage(shortMessage);

// Short message is mandatory per GELF spec
if (sanitizedShortMessage.isEmpty()) {
addWarn("Log message was empty - replaced to prevent Graylog error");
return "Empty message replaced by logback-gelf";
}

return sanitizedShortMessage;
}

private String sanitizeShortMessage(final String sanitizedShortMessage) {
if (sanitizedShortMessage.isEmpty()) {
return sanitizedShortMessage;
}

final int len = maxShortMessageLength == 0
? sanitizedShortMessage.length()
: Math.min(sanitizedShortMessage.length(), maxShortMessageLength);
final char[] tmp = new char[len];

int iDst = 0;
boolean whitspaceLast = false;
boolean whitespaceStart = true;
for (int iSrc = 0; iSrc < sanitizedShortMessage.length() && iDst < tmp.length; iSrc++) {
final char c = sanitizedShortMessage.charAt(iSrc);
if (Character.isWhitespace(c)) {
if (!whitespaceStart && !whitspaceLast) {
tmp[iDst++] = ' ';
}
whitspaceLast = true;
} else {
tmp[iDst++] = c;
whitspaceLast = false;
whitespaceStart = false;
}
// Graylog doesn't like a single newline as short message: https://github.com/Graylog2/graylog2-server/issues/4842
if (shortMessage.isBlank()) {
addWarn("Log message was blank - replaced to prevent Graylog error");
return "Blank message replaced by logback-gelf";
}

return new String(tmp, 0, iDst).trim();
return shortMessage;
}

protected String buildShortMessage(final ILoggingEvent event) {
return shortPatternLayout.doLayout(event);
return shortMessageLayout.doLayout(event);
}

protected String buildFullMessage(final ILoggingEvent event) {
return fullPatternLayout.doLayout(event);
return fullMessageLayout.doLayout(event);
}

protected Map<String, Object> collectAdditionalFields(final ILoggingEvent event) {
Expand Down
31 changes: 13 additions & 18 deletions src/test/java/de/siegmar/logbackgelf/GelfEncoderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,6 @@ static void basicValidation(final String json) {
);
}

@Test
void shortenShortMessageDefault() {
final String expectedShortMessage = "A".repeat(250);
final String actualShortMessage = " " + expectedShortMessage + "123\r\n";


final GelfEncoder gelfEncoder = new GelfEncoder();
gelfEncoder.setMaxShortMessageLength(250);

assertThat(gelfEncoder.normalizeShortMessage(actualShortMessage))
.hasSameSizeAs(expectedShortMessage)
.isEqualTo(expectedShortMessage);
}

@Test
void exception() {
encoder.start();
Expand Down Expand Up @@ -456,18 +442,19 @@ void originHostDefaultToLocalHostNameIfEmpty(final String configuredHostname) {
.isNotEqualTo("unknown");
}

@Test
void blankShortmessage() {
@ParameterizedTest
@ValueSource(strings = { " \t ", "", "\n" })
void blankShortMessage(final String message) {
encoder.start();

final LoggerContext lc = (LoggerContext) LoggerFactory.getILoggerFactory();
final Logger logger = lc.getLogger(LOGGER_NAME);

final LoggingEvent event = new LoggingEvent(LOGGER_NAME, logger, Level.DEBUG, " \t ", null, new Object[]{1});
final LoggingEvent event = new LoggingEvent(LOGGER_NAME, logger, Level.DEBUG, message, null, new Object[]{1});
final String logMsg = encodeToStr(event);

assertThatJson(logMsg).node("short_message")
.isEqualTo("Empty message replaced by logback-gelf");
.isEqualTo("Blank message replaced by logback-gelf");
}

@Test
Expand Down Expand Up @@ -495,4 +482,12 @@ void addFieldMapper() {
assertThat(encoder.getFieldMappers()).containsExactly(fieldMapper);
}

@Test
void hasDefaultPatternLayout() {
encoder.start();

assertThat(encoder.getFullMessageLayout()).isNotNull();
assertThat(encoder.getShortMessageLayout()).isNotNull();
}

}
8 changes: 4 additions & 4 deletions src/test/resources/tcp-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
<includeCallerData>false</includeCallerData>
<includeRootCauseData>false</includeRootCauseData>
<includeLevelName>false</includeLevelName>
<shortPatternLayout class="ch.qos.logback.classic.PatternLayout">
<shortMessageLayout class="ch.qos.logback.classic.PatternLayout">
<pattern>%m%nopex</pattern>
</shortPatternLayout>
<fullPatternLayout class="ch.qos.logback.classic.PatternLayout">
</shortMessageLayout>
<fullMessageLayout class="ch.qos.logback.classic.PatternLayout">
<pattern>%m%n</pattern>
</fullPatternLayout>
</fullMessageLayout>
<numbersAsString>false</numbersAsString>
<staticField>app_name:backend</staticField>
<staticField>os_arch:${os.arch}</staticField>
Expand Down
8 changes: 4 additions & 4 deletions src/test/resources/tcp_tls-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
<includeCallerData>false</includeCallerData>
<includeRootCauseData>false</includeRootCauseData>
<includeLevelName>false</includeLevelName>
<shortPatternLayout class="ch.qos.logback.classic.PatternLayout">
<shortMessageLayout class="ch.qos.logback.classic.PatternLayout">
<pattern>%m%nopex</pattern>
</shortPatternLayout>
<fullPatternLayout class="ch.qos.logback.classic.PatternLayout">
</shortMessageLayout>
<fullMessageLayout class="ch.qos.logback.classic.PatternLayout">
<pattern>%m%n</pattern>
</fullPatternLayout>
</fullMessageLayout>
<numbersAsString>false</numbersAsString>
<staticField>app_name:backend</staticField>
<staticField>os_arch:${os.arch}</staticField>
Expand Down
8 changes: 4 additions & 4 deletions src/test/resources/udp-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
<includeCallerData>false</includeCallerData>
<includeRootCauseData>false</includeRootCauseData>
<includeLevelName>false</includeLevelName>
<shortPatternLayout class="ch.qos.logback.classic.PatternLayout">
<shortMessageLayout class="ch.qos.logback.classic.PatternLayout">
<pattern>%m%nopex</pattern>
</shortPatternLayout>
<fullPatternLayout class="ch.qos.logback.classic.PatternLayout">
</shortMessageLayout>
<fullMessageLayout class="ch.qos.logback.classic.PatternLayout">
<pattern>%m%n</pattern>
</fullPatternLayout>
</fullMessageLayout>
<numbersAsString>false</numbersAsString>
<staticField>app_name:backend</staticField>
<staticField>os_arch:${os.arch}</staticField>
Expand Down

0 comments on commit 372c9a7

Please sign in to comment.