From c372fe85ff49e01f4504befa073c253c2ac93f28 Mon Sep 17 00:00:00 2001 From: Oliver Siegmar Date: Sun, 17 Jan 2021 14:43:16 +0100 Subject: [PATCH] Refactored field mapper handling. * Separate built-in field mappers (additional field mappers cannot overwrite values from built-in mappers) * Handle mapping exceptions * Ensure field validation and conversion happens for all field mappers * Removed GelfFieldHelper (inlined it in GelfEncoder again) --- .../de/siegmar/logbackgelf/GelfEncoder.java | 167 ++++++++++++------ .../siegmar/logbackgelf/GelfFieldHelper.java | 106 ----------- .../mappers/MdcDataFieldMapper.java | 20 +-- 3 files changed, 116 insertions(+), 177 deletions(-) delete mode 100644 src/main/java/de/siegmar/logbackgelf/GelfFieldHelper.java diff --git a/src/main/java/de/siegmar/logbackgelf/GelfEncoder.java b/src/main/java/de/siegmar/logbackgelf/GelfEncoder.java index ff0c07d..20330a5 100644 --- a/src/main/java/de/siegmar/logbackgelf/GelfEncoder.java +++ b/src/main/java/de/siegmar/logbackgelf/GelfEncoder.java @@ -19,13 +19,15 @@ package de.siegmar.logbackgelf; +import java.math.BigDecimal; import java.net.UnknownHostException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; +import java.util.regex.Pattern; import ch.qos.logback.classic.PatternLayout; import ch.qos.logback.classic.spi.ILoggingEvent; @@ -43,6 +45,7 @@ @SuppressWarnings("checkstyle:classdataabstractioncoupling") public class GelfEncoder extends EncoderBase { + private static final Pattern VALID_ADDITIONAL_FIELD_PATTERN = Pattern.compile("^[\\w.-]*$"); private static final String DEFAULT_SHORT_PATTERN = "%m%nopex"; private static final String DEFAULT_FULL_PATTERN = "%m%n"; @@ -116,14 +119,19 @@ public class GelfEncoder extends EncoderBase { */ private PatternLayout fullPatternLayout; + /** + * Log numbers as String. Default: false. + */ + private boolean numbersAsString; + /** * Additional, static fields to send to graylog. Defaults: none. */ - private Map staticFields = new HashMap<>(); + private final Map staticFields = new HashMap<>(); - private final List> fieldMappers = new ArrayList<>(); + private final List> builtInFieldMappers = new ArrayList<>(); - private final GelfFieldHelper fieldHelper = new GelfFieldHelper(this); + private final List> fieldMappers = new ArrayList<>(); public String getOriginHost() { return originHost; @@ -214,11 +222,11 @@ public void setAppendNewline(final boolean appendNewline) { } public boolean isNumbersAsString() { - return fieldHelper.isNumbersAsString(); + return numbersAsString; } public void setNumbersAsString(final boolean numbersAsString) { - this.fieldHelper.setNumbersAsString(numbersAsString); + this.numbersAsString = numbersAsString; } public PatternLayout getShortPatternLayout() { @@ -238,26 +246,61 @@ public void setFullPatternLayout(final PatternLayout fullPatternLayout) { } public Map getStaticFields() { - return staticFields; - } - - public void setStaticFields(final Map staticFields) { - this.staticFields = Objects.requireNonNull(staticFields); + return Collections.unmodifiableMap(staticFields); } public void addStaticField(final String staticField) { final String[] split = staticField.split(":", 2); if (split.length == 2) { - fieldHelper.addField(staticFields, split[0].trim(), split[1].trim()); + try { + addField(staticFields, split[0].trim(), split[1].trim()); + } catch (final IllegalArgumentException e) { + addWarn("Could not add field " + staticField, e); + } } else { addWarn("staticField must be in format key:value - rejecting '" + staticField + "'"); } } + public List> getFieldMappers() { + return Collections.unmodifiableList(fieldMappers); + } + public void addFieldMapper(final GelfFieldMapper fieldMapper) { fieldMappers.add(fieldMapper); } + private void addField(final Map dst, final String fieldName, final Object fieldValue) { + if (fieldName.isEmpty()) { + throw new IllegalArgumentException("fieldName key must not be empty"); + } + if ("id".equalsIgnoreCase(fieldName)) { + throw new IllegalArgumentException("fieldName key name 'id' is prohibited"); + } + if (!VALID_ADDITIONAL_FIELD_PATTERN.matcher(fieldName).matches()) { + throw new IllegalArgumentException("fieldName key '" + fieldName + "' is illegal. " + + "Keys must apply to regex ^[\\w.-]*$"); + } + + if (dst.get(fieldName) != null) { + throw new IllegalArgumentException("Field mapper tried to set already defined key '" + fieldName + "'."); + } + + dst.put(fieldName, convertToNumberIfNeeded(fieldValue)); + } + + private Object convertToNumberIfNeeded(final Object value) { + if (numbersAsString || !(value instanceof String)) { + return value; + } + + try { + return new BigDecimal((String) value); + } catch (final NumberFormatException e) { + return value; + } + } + @Override public void start() { if (originHost == null || originHost.trim().isEmpty()) { @@ -287,6 +330,35 @@ private PatternLayout buildPattern(final String pattern) { return patternLayout; } + private void addBuiltInFieldMappers() { + builtInFieldMappers.add(new SimpleFieldMapper<>(loggerNameKey, ILoggingEvent::getLoggerName)); + builtInFieldMappers.add(new SimpleFieldMapper<>(threadNameKey, ILoggingEvent::getThreadName)); + + if (includeLevelName) { + builtInFieldMappers.add(new SimpleFieldMapper<>(levelNameKey, event -> event.getLevel().toString())); + } + + if (includeRawMessage) { + builtInFieldMappers.add(new SimpleFieldMapper<>("raw_message", ILoggingEvent::getMessage)); + } + + if (includeCallerData) { + builtInFieldMappers.add(new CallerDataFieldMapper()); + } + + if (includeRootCauseData) { + builtInFieldMappers.add(new RootExceptionDataFieldMapper()); + } + + if (includeMarker) { + builtInFieldMappers.add(new MarkerFieldMapper("marker")); + } + + if (includeMdcData) { + builtInFieldMappers.add(new MdcDataFieldMapper()); + } + } + @Override public byte[] headerBytes() { return null; @@ -294,20 +366,18 @@ public byte[] headerBytes() { @Override public byte[] encode(final ILoggingEvent event) { - final String shortMessage = shortPatternLayout.doLayout(event); - final String fullMessage = fullPatternLayout.doLayout(event); final Map additionalFields = new HashMap<>(staticFields); - - fieldMappers.forEach(fieldMapper -> fieldMapper.mapField(event, (key, value) -> { - final Object oldValue = additionalFields.put(key, value); - if (oldValue != null) { - addWarn("additional field with key '" + key + "' is already set"); - additionalFields.put(key, oldValue); - } - })); - - final GelfMessage gelfMessage = new GelfMessage(originHost, shortMessage, fullMessage, - event.getTimeStamp(), LevelToSyslogSeverity.convert(event), additionalFields); + addFieldMapperData(event, additionalFields, builtInFieldMappers); + addFieldMapperData(event, additionalFields, fieldMappers); + + final GelfMessage gelfMessage = new GelfMessage( + originHost, + shortPatternLayout.doLayout(event), + fullPatternLayout.doLayout(event), + event.getTimeStamp(), + LevelToSyslogSeverity.convert(event), + additionalFields + ); String jsonStr = gelfMessageToJson(gelfMessage); if (appendNewline) { @@ -317,6 +387,24 @@ public byte[] encode(final ILoggingEvent event) { return jsonStr.getBytes(StandardCharsets.UTF_8); } + @SuppressWarnings("checkstyle:IllegalCatch") + private void addFieldMapperData(final ILoggingEvent event, final Map additionalFields, + final List> mappers) { + for (final GelfFieldMapper fieldMapper : mappers) { + try { + fieldMapper.mapField(event, (key, value) -> { + try { + addField(additionalFields, key, value); + } catch (final IllegalArgumentException e) { + addWarn("Could not add field " + key, e); + } + }); + } catch (final Exception e) { + addError("Exception in field mapper", e); + } + } + } + /** * Allow subclasses to customize the message before it is converted to String. * @@ -332,33 +420,4 @@ public byte[] footerBytes() { return null; } - private void addBuiltInFieldMappers() { - addFieldMapper(new SimpleFieldMapper<>(loggerNameKey, ILoggingEvent::getLoggerName)); - addFieldMapper(new SimpleFieldMapper<>(threadNameKey, ILoggingEvent::getThreadName)); - - if (includeRawMessage) { - addFieldMapper(new SimpleFieldMapper<>("raw_message", ILoggingEvent::getMessage)); - } - - if (includeMarker) { - addFieldMapper(new MarkerFieldMapper("marker")); - } - - if (includeLevelName) { - addFieldMapper(new SimpleFieldMapper<>(levelNameKey, event -> event.getLevel().levelStr)); - } - - if (includeMdcData) { - addFieldMapper(new MdcDataFieldMapper(fieldHelper)); - } - - if (includeCallerData) { - addFieldMapper(new CallerDataFieldMapper()); - } - - if (includeRootCauseData) { - addFieldMapper(new RootExceptionDataFieldMapper()); - } - } - } diff --git a/src/main/java/de/siegmar/logbackgelf/GelfFieldHelper.java b/src/main/java/de/siegmar/logbackgelf/GelfFieldHelper.java deleted file mode 100644 index 2685a8d..0000000 --- a/src/main/java/de/siegmar/logbackgelf/GelfFieldHelper.java +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Logback GELF - zero dependencies Logback GELF appender library. - * Copyright (C) 2016 Oliver Siegmar - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - */ - -package de.siegmar.logbackgelf; - -import java.math.BigDecimal; -import java.util.Map; -import java.util.regex.Pattern; - -import ch.qos.logback.core.spi.ContextAwareBase; - -/** - * A helper utility for field validation and conversion. - */ -public class GelfFieldHelper { - - private static final Pattern VALID_ADDITIONAL_FIELD_PATTERN = Pattern.compile("^[\\w.-]*$"); - - private final ContextAwareBase logger; - - /** - * Log numbers as String. Default: false. - */ - private boolean numbersAsString; - - GelfFieldHelper(final ContextAwareBase logger) { - this.logger = logger; - } - - /** - * @param fieldName name of the field to check - * @return true in case field name is valid; false otherwise - */ - public boolean isValidFieldName(final String fieldName) { - boolean isValid = true; - if (fieldName.isEmpty()) { - logger.addWarn("staticField key must not be empty"); - isValid = false; - } else if ("id".equalsIgnoreCase(fieldName)) { - logger.addWarn("staticField key name 'id' is prohibited"); - isValid = false; - } else if (!VALID_ADDITIONAL_FIELD_PATTERN.matcher(fieldName).matches()) { - logger.addWarn("staticField key '" + fieldName + "' is illegal. " - + "Keys must apply to regex ^[\\w.-]*$"); - isValid = false; - } - return isValid; - } - - /** - * Check validity of field name and in case it is valid, add it to the provided map - * (converting the value to a number, if needed). - * - * @param dst the map where to add the field - * @param fieldName field name - * @param fieldValue field value - */ - public void addField(final Map dst, final String fieldName, final String fieldValue) { - if (isValidFieldName(fieldName) && fieldValue != null) { - dst.put(fieldName, convertToNumberIfNeeded(fieldValue)); - } - } - - /** - * In case {@link #isValidFieldName(String)} is enabled, try to convert the provided value to a number. - * - * @param value the value to convert - * @return the provided value as {@link String}, in case {@link #isValidFieldName(String)} is disabled or - * conversion fails; otherwise return the provided value as {@link BigDecimal}. - */ - public Object convertToNumberIfNeeded(final String value) { - if (!numbersAsString) { - try { - return new BigDecimal(value); - } catch (final NumberFormatException e) { - return value; - } - } - return value; - } - - public void setNumbersAsString(final boolean numbersAsString) { - this.numbersAsString = numbersAsString; - } - - public boolean isNumbersAsString() { - return numbersAsString; - } - -} diff --git a/src/main/java/de/siegmar/logbackgelf/mappers/MdcDataFieldMapper.java b/src/main/java/de/siegmar/logbackgelf/mappers/MdcDataFieldMapper.java index df4b0bd..2dbf305 100644 --- a/src/main/java/de/siegmar/logbackgelf/mappers/MdcDataFieldMapper.java +++ b/src/main/java/de/siegmar/logbackgelf/mappers/MdcDataFieldMapper.java @@ -19,32 +19,18 @@ package de.siegmar.logbackgelf.mappers; -import java.util.Map; import java.util.Optional; import java.util.function.BiConsumer; import ch.qos.logback.classic.spi.ILoggingEvent; -import de.siegmar.logbackgelf.GelfFieldHelper; import de.siegmar.logbackgelf.GelfFieldMapper; -public class MdcDataFieldMapper implements GelfFieldMapper { - - private final GelfFieldHelper fieldHelper; - - public MdcDataFieldMapper(final GelfFieldHelper fieldHelper) { - this.fieldHelper = fieldHelper; - } +public class MdcDataFieldMapper implements GelfFieldMapper { @Override - public void mapField(final ILoggingEvent event, final BiConsumer valueHandler) { + public void mapField(final ILoggingEvent event, final BiConsumer valueHandler) { Optional.ofNullable(event.getMDCPropertyMap()) - .ifPresent(p -> { - for (final Map.Entry entry : p.entrySet()) { - if (fieldHelper.isValidFieldName(entry.getKey())) { - valueHandler.accept(entry.getKey(), fieldHelper.convertToNumberIfNeeded(entry.getValue())); - } - } - }); + .ifPresent(p -> p.forEach(valueHandler)); } }