diff --git a/cf-java-logging-support-core/src/main/java/com/sap/hcp/cf/logging/common/converter/DefaultPropertiesConverter.java b/cf-java-logging-support-core/src/main/java/com/sap/hcp/cf/logging/common/converter/DefaultPropertiesConverter.java index 7ca5e1c4..6f69f8c2 100644 --- a/cf-java-logging-support-core/src/main/java/com/sap/hcp/cf/logging/common/converter/DefaultPropertiesConverter.java +++ b/cf-java-logging-support-core/src/main/java/com/sap/hcp/cf/logging/common/converter/DefaultPropertiesConverter.java @@ -14,11 +14,13 @@ import com.fasterxml.jackson.jr.ob.JSON; import com.fasterxml.jackson.jr.ob.JSONComposer; import com.fasterxml.jackson.jr.ob.comp.ObjectComposer; +import com.sap.hcp.cf.logging.common.Defaults; import com.sap.hcp.cf.logging.common.LogContext; public class DefaultPropertiesConverter { private final Set exclusions = new HashSet(); + private boolean sendDefaultValues = false; public DefaultPropertiesConverter() { } @@ -31,6 +33,10 @@ public void setExclusions(List exclusionList) { } } + public void setSendDefaultValues(boolean sendDefaultValues) { + this.sendDefaultValues = sendDefaultValues; + } + private static class LoggerHolder { static final Logger LOG = LoggerFactory.getLogger(LoggerHolder.class.getEnclosingClass()); } @@ -46,7 +52,14 @@ public void convert(StringBuilder appendTo, Map eventProperties) ObjectComposer> oc = JSON.std.composeString().startObject(); for (Entry p: properties.entrySet()) { if (!exclusions.contains(p.getKey())) { - oc.put(p.getKey(), p.getValue()); + if (sendDefaultValues) { + oc.put(p.getKey(), p.getValue()); + } else { + String defaultValue = getDefaultValue(p.getKey()); + if (!defaultValue.equals(p.getValue())) { + oc.put(p.getKey(), p.getValue()); + } + } } } String result = oc.end().finish().trim(); @@ -73,4 +86,9 @@ private Map mergeContextMaps(Map eventMap) { } return result; } + + private String getDefaultValue(String key) { + String defaultValue = LogContext.getDefault(key); + return defaultValue == null ? Defaults.UNKNOWN : defaultValue; + } } diff --git a/cf-java-logging-support-core/src/test/java/com/sap/hcp/cf/logging/common/converter/DefaultPropertiesConverterTest.java b/cf-java-logging-support-core/src/test/java/com/sap/hcp/cf/logging/common/converter/DefaultPropertiesConverterTest.java index cc525229..d7b7a048 100644 --- a/cf-java-logging-support-core/src/test/java/com/sap/hcp/cf/logging/common/converter/DefaultPropertiesConverterTest.java +++ b/cf-java-logging-support-core/src/test/java/com/sap/hcp/cf/logging/common/converter/DefaultPropertiesConverterTest.java @@ -1,11 +1,12 @@ package com.sap.hcp.cf.logging.common.converter; import static com.sap.hcp.cf.logging.common.converter.UnmarshallUtilities.unmarshal; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import java.util.Arrays; import java.util.Collections; @@ -22,184 +23,200 @@ public class DefaultPropertiesConverterTest { - private static final String HACK_ATTEMPT = "}{:\",\""; - - private DefaultPropertiesConverter converter; - - @Before - public void initConverter() { - this.converter = new DefaultPropertiesConverter(); - } - - @Before - public void cleadMdc() { - MDC.clear(); - } - - @Test - public void emptyProperties() throws Exception { - StringBuilder sb = new StringBuilder(); - - converter.convert(sb, Collections.emptyMap()); - - assertThat(unmarshal(sb), hasDefaultProperties()); - } - - @Test - public void singleMdcEntry() throws Exception { - StringBuilder sb = new StringBuilder(); - MDC.put("some key", "some value"); - - converter.convert(sb, Collections.emptyMap()); - - assertThat(unmarshal(sb), hasEntry("some key", "some value")); - } - - @Test - public void twoMdcEntries() throws Exception { - StringBuilder sb = new StringBuilder(); - MDC.put("some key", "some value"); - MDC.put("other key", "other value"); - - converter.convert(sb, Collections.emptyMap()); - - assertThat(unmarshal(sb), - allOf(hasEntry("some key", "some value"), hasEntry("other key", "other value"))); - } - - @Test - public void singleExplicitEntry() throws Exception { - StringBuilder sb = new StringBuilder(); - @SuppressWarnings("serial") - Map explicitFields = new HashMap() { - { - put("explicit key", "explicit value"); - } - }; - - converter.convert(sb, explicitFields); - - assertThat(unmarshal(sb), hasEntry("explicit key", "explicit value")); - } - - @Test - public void mergesDifferentMdcAndExplicitEntries() throws Exception { - StringBuilder sb = new StringBuilder(); - @SuppressWarnings("serial") - Map explicitFields = new HashMap() { - { - put("explicit key", "explicit value"); - } - }; - MDC.put("some key", "some value"); - - converter.convert(sb, explicitFields); - - assertThat(unmarshal(sb), - allOf(hasEntry("some key", "some value"), hasEntry("explicit key", "explicit value"))); - } - - @Test - public void explicitValuesOverwritesMdc() throws Exception { - StringBuilder sb = new StringBuilder(); - @SuppressWarnings("serial") - Map explicitFields = new HashMap() { - { - put("some key", "explicit value"); - } - }; - MDC.put("some key", "some value"); - - converter.convert(sb, explicitFields); - - assertThat(unmarshal(sb), hasEntry("some key", "explicit value")); - } - - @Test - public void dropsExclusions() throws Exception { - StringBuilder sb = new StringBuilder(); - @SuppressWarnings("serial") - Map explicitFields = new HashMap() { - { - put("excluded explicit key", "excluded explicit value"); - put("retained explicit key", "retained explicit value"); - } - }; - MDC.put("retained mdc key", "retained mdc value"); - MDC.put("excluded mdc key", "excluded mdc value"); - - converter.setExclusions(Arrays.asList("excluded explicit key", "excluded mdc key")); - converter.convert(sb, explicitFields); - - assertThat(unmarshal(sb), - allOf(hasEntry("retained mdc key", "retained mdc value"), - not(hasEntry("excluded mdc key", "excluded mdc value")), - hasEntry("retained explicit key", "retained explicit value"), - not(hasEntry("excluded explicit key", "excluded explicit value")))); - - } - - @Test - public void properlyEscapesKeys() throws Exception { - StringBuilder sb = new StringBuilder(); - @SuppressWarnings("serial") - Map explicitFields = new HashMap() { - { - put("explicit" + HACK_ATTEMPT, "explicit value"); - } - }; - MDC.put("mdc" + HACK_ATTEMPT, "mdc value"); - - converter.convert(sb, explicitFields); - - assertThat(unmarshal(sb), - allOf(hasEntry("mdc" + HACK_ATTEMPT, "mdc value"), - hasEntry("explicit" + HACK_ATTEMPT, "explicit value"))); - } - - @Test - public void properlyEscapesValues() throws Exception { - StringBuilder sb = new StringBuilder(); - @SuppressWarnings("serial") - Map explicitFields = new HashMap() { - { - put("explicit key", "explicit" + HACK_ATTEMPT); - } - }; - MDC.put("mdc key", "mdc" + HACK_ATTEMPT); - - converter.convert(sb, explicitFields); - - assertThat(unmarshal(sb), - allOf(hasEntry("mdc key", "mdc" + HACK_ATTEMPT), hasEntry("explicit key", "explicit" + HACK_ATTEMPT))); - } - - @Test - public void properlyEscapesExclusions() throws Exception { - StringBuilder sb = new StringBuilder(); - @SuppressWarnings("serial") - Map explicitFields = new HashMap() { - { - put("explicit" + HACK_ATTEMPT, "explicit value"); - } - }; - MDC.put("mdc" + HACK_ATTEMPT, "mdc value"); - - converter.setExclusions(Arrays.asList("explicit" + HACK_ATTEMPT, "mdc" + HACK_ATTEMPT)); - converter.convert(sb, explicitFields); - - assertThat(unmarshal(sb), allOf(not(hasEntry("mdc" + HACK_ATTEMPT, "mdc value")), - not(hasEntry("explicit" + HACK_ATTEMPT, "explicit value")))); - } - - @SuppressWarnings("unchecked") - private static Matcher> hasDefaultProperties() { - return allOf(hasEntry(Fields.CORRELATION_ID, Defaults.UNKNOWN), hasEntry(Fields.TENANT_ID, Defaults.UNKNOWN), - hasEntry(Fields.COMPONENT_ID, Defaults.UNKNOWN), hasEntry(Fields.COMPONENT_NAME, Defaults.UNKNOWN), - hasEntry(Fields.COMPONENT_TYPE, Defaults.COMPONENT_TYPE), - hasEntry(Fields.COMPONENT_INSTANCE, Defaults.COMPONENT_INDEX), - hasEntry(Fields.CONTAINER_ID, Defaults.UNKNOWN), hasEntry(Fields.ORGANIZATION_ID, Defaults.UNKNOWN), - hasEntry(Fields.ORGANIZATION_NAME, Defaults.UNKNOWN), hasEntry(Fields.SPACE_ID, Defaults.UNKNOWN), - hasEntry(Fields.SPACE_NAME, Defaults.UNKNOWN), not(hasKey(Fields.REQUEST_ID))); - } + private static final String HACK_ATTEMPT = "}{:\",\""; + + private DefaultPropertiesConverter converter; + + @Before + public void initConverter() { + this.converter = new DefaultPropertiesConverter(); + } + + @Before + public void cleadMdc() { + MDC.clear(); + } + + @Test + public void emptyProperties() throws Exception { + StringBuilder sb = new StringBuilder(); + + converter.convert(sb, Collections.emptyMap()); + + assertTrue("Should have empty properties by default", unmarshal(sb).isEmpty()); + } + + @Test + public void singleMdcEntry() throws Exception { + StringBuilder sb = new StringBuilder(); + MDC.put("some key", "some value"); + + converter.convert(sb, Collections.emptyMap()); + + assertThat(unmarshal(sb), hasEntry("some key", "some value")); + } + + @Test + public void twoMdcEntries() throws Exception { + StringBuilder sb = new StringBuilder(); + MDC.put("some key", "some value"); + MDC.put("other key", "other value"); + + converter.convert(sb, Collections.emptyMap()); + + assertThat(unmarshal(sb), allOf(hasEntry("some key", "some value"), hasEntry("other key", "other value"))); + } + + @Test + public void singleExplicitEntry() throws Exception { + StringBuilder sb = new StringBuilder(); + @SuppressWarnings("serial") + Map explicitFields = new HashMap() { + { + put("explicit key", "explicit value"); + } + }; + + converter.convert(sb, explicitFields); + + assertThat(unmarshal(sb), hasEntry("explicit key", "explicit value")); + } + + @Test + public void mergesDifferentMdcAndExplicitEntries() throws Exception { + StringBuilder sb = new StringBuilder(); + @SuppressWarnings("serial") + Map explicitFields = new HashMap() { + { + put("explicit key", "explicit value"); + } + }; + MDC.put("some key", "some value"); + + converter.convert(sb, explicitFields); + + assertThat(unmarshal(sb), allOf(hasEntry("some key", "some value"), hasEntry("explicit key", + "explicit value"))); + } + + @Test + public void explicitValuesOverwritesMdc() throws Exception { + StringBuilder sb = new StringBuilder(); + @SuppressWarnings("serial") + Map explicitFields = new HashMap() { + { + put("some key", "explicit value"); + } + }; + MDC.put("some key", "some value"); + + converter.convert(sb, explicitFields); + + assertThat(unmarshal(sb), hasEntry("some key", "explicit value")); + } + + @Test + public void dropsExclusions() throws Exception { + StringBuilder sb = new StringBuilder(); + @SuppressWarnings("serial") + Map explicitFields = new HashMap() { + { + put("excluded explicit key", "excluded explicit value"); + put("retained explicit key", "retained explicit value"); + } + }; + MDC.put("retained mdc key", "retained mdc value"); + MDC.put("excluded mdc key", "excluded mdc value"); + + converter.setExclusions(Arrays.asList("excluded explicit key", "excluded mdc key")); + converter.convert(sb, explicitFields); + + assertThat(unmarshal(sb), allOf(hasEntry("retained mdc key", "retained mdc value"), not(hasEntry( + "excluded mdc key", + "excluded mdc value")), + hasEntry("retained explicit key", "retained explicit value"), not(hasEntry( + "excluded explicit key", + "excluded explicit value")))); + + } + + @Test + public void properlyEscapesKeys() throws Exception { + StringBuilder sb = new StringBuilder(); + @SuppressWarnings("serial") + Map explicitFields = new HashMap() { + { + put("explicit" + HACK_ATTEMPT, "explicit value"); + } + }; + MDC.put("mdc" + HACK_ATTEMPT, "mdc value"); + + converter.convert(sb, explicitFields); + + assertThat(unmarshal(sb), allOf(hasEntry("mdc" + HACK_ATTEMPT, "mdc value"), hasEntry("explicit" + HACK_ATTEMPT, + "explicit value"))); + } + + @Test + public void properlyEscapesValues() throws Exception { + StringBuilder sb = new StringBuilder(); + @SuppressWarnings("serial") + Map explicitFields = new HashMap() { + { + put("explicit key", "explicit" + HACK_ATTEMPT); + } + }; + MDC.put("mdc key", "mdc" + HACK_ATTEMPT); + + converter.convert(sb, explicitFields); + + assertThat(unmarshal(sb), allOf(hasEntry("mdc key", "mdc" + HACK_ATTEMPT), hasEntry("explicit key", "explicit" + + HACK_ATTEMPT))); + } + + @Test + public void properlyEscapesExclusions() throws Exception { + StringBuilder sb = new StringBuilder(); + @SuppressWarnings("serial") + Map explicitFields = new HashMap() { + { + put("explicit" + HACK_ATTEMPT, "explicit value"); + } + }; + MDC.put("mdc" + HACK_ATTEMPT, "mdc value"); + + converter.setExclusions(Arrays.asList("explicit" + HACK_ATTEMPT, "mdc" + HACK_ATTEMPT)); + converter.convert(sb, explicitFields); + + assertThat(unmarshal(sb), allOf(not(hasEntry("mdc" + HACK_ATTEMPT, "mdc value")), not(hasEntry("explicit" + + HACK_ATTEMPT, + "explicit value")))); + } + + @Test + public void fullDefaultPropertiesIfConfigured() throws Exception { + StringBuilder sb = new StringBuilder(); + + converter.setSendDefaultValues(true); + converter.convert(sb, Collections.emptyMap()); + + assertThat(unmarshal(sb), hasDefaultProperties()); + + } + + @SuppressWarnings("unchecked") + private static Matcher> hasDefaultProperties() { + return allOf(hasEntry(Fields.CORRELATION_ID, Defaults.UNKNOWN), // + hasEntry(Fields.TENANT_ID, Defaults.UNKNOWN), // + hasEntry(Fields.COMPONENT_ID, Defaults.UNKNOWN), // + hasEntry(Fields.COMPONENT_NAME, Defaults.UNKNOWN), // + hasEntry(Fields.COMPONENT_TYPE, Defaults.COMPONENT_TYPE), // + hasEntry(Fields.COMPONENT_INSTANCE, Defaults.COMPONENT_INDEX), // + hasEntry(Fields.CONTAINER_ID, Defaults.UNKNOWN), // + hasEntry(Fields.ORGANIZATION_ID, Defaults.UNKNOWN), // + hasEntry(Fields.ORGANIZATION_NAME, Defaults.UNKNOWN), // + hasEntry(Fields.SPACE_ID, Defaults.UNKNOWN), // + hasEntry(Fields.SPACE_NAME, Defaults.UNKNOWN), // + not(hasKey(Fields.REQUEST_ID))); + } } diff --git a/cf-java-logging-support-jersey/src/test/java/com/sap/hcp/cf/logging/jersey/filter/RequestMetricsClientFilterTest.java b/cf-java-logging-support-jersey/src/test/java/com/sap/hcp/cf/logging/jersey/filter/RequestMetricsClientFilterTest.java index c6fb8701..28595741 100644 --- a/cf-java-logging-support-jersey/src/test/java/com/sap/hcp/cf/logging/jersey/filter/RequestMetricsClientFilterTest.java +++ b/cf-java-logging-support-jersey/src/test/java/com/sap/hcp/cf/logging/jersey/filter/RequestMetricsClientFilterTest.java @@ -1,11 +1,11 @@ package com.sap.hcp.cf.logging.jersey.filter; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsNot.not; import static org.hamcrest.core.IsNull.nullValue; -import static org.junit.Assert.assertThat; import java.util.HashSet; import java.util.Set; @@ -70,7 +70,7 @@ public void ChainedResourcePerformanceLogTest() { correlationIds.add(id); } assertThat(correlationIds.size(), is(1)); - assertThat(getField(Fields.TENANT_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.TENANT_ID), is(nullValue())); } @Test @@ -79,7 +79,7 @@ public void PerformanceLogTest() { final Response response = getClient().target(getBaseUri() + "testresource").request().header( HttpHeaders.CORRELATION_ID.getName(), "1").get(); - assertThat(getField(Fields.COMPONENT_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); assertThat(getField(Fields.RESPONSE_SIZE_B), is("4")); assertThat(getField(Fields.RESPONSE_TIME_MS), not(nullValue())); assertThat(getField(Fields.RESPONSE_STATUS), is(Integer.toString(TestResource.EXPECTED_STATUS_CODE))); @@ -87,7 +87,7 @@ public void PerformanceLogTest() { assertThat(getField(Fields.DIRECTION), is(Direction.OUT.toString())); assertThat(getField(Fields.METHOD), is(TestResource.EXPECTED_REQUEST_METHOD)); assertThat(getField(Fields.LAYER), is(ClientRequestContextAdapter.LAYER_NAME)); - assertThat(getField(Fields.TENANT_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.TENANT_ID), is(nullValue())); } @Test @@ -96,7 +96,7 @@ public void ResponseTimeTest() { final Response response = target("testresource").request().delete(); assertThat(new Double(getField(Fields.RESPONSE_TIME_MS)), greaterThan(TestResource.EXPECTED_REQUEST_TIME)); - assertThat(getField(Fields.TENANT_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.TENANT_ID), is(nullValue())); } @Test diff --git a/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverter.java b/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverter.java index bad4315f..ebebf43b 100644 --- a/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverter.java +++ b/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverter.java @@ -36,7 +36,8 @@ public class ContextPropsConverter extends LogEventPatternConverter { public ContextPropsConverter(String[] options) { super(WORD, WORD); if (options != null) { - converter.setExclusions(Arrays.asList(options)); + converter.setSendDefaultValues(Boolean.parseBoolean(options[0])); + converter.setExclusions(Arrays.asList(Arrays.copyOfRange(options, 1, options.length))); } } @@ -48,7 +49,13 @@ public static ContextPropsConverter newInstance(final String[] options) { public void format(LogEvent event, StringBuilder toAppendTo) { Map contextData = event.getContextData().toMap(); addCustomFieldsFromArguments(contextData, event); + int lengthBefore = toAppendTo.length(); converter.convert(toAppendTo, contextData); + // append comma only, if at least one field was added + // otherwise, there will be to commas ",," rendering the JSON invalid + if (toAppendTo.length() > lengthBefore) { + toAppendTo.append(","); + } } private void addCustomFieldsFromArguments(Map contextData, LogEvent event) { diff --git a/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/JsonPatternLayout.java b/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/JsonPatternLayout.java index 1f4247f6..b8cb39e4 100644 --- a/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/JsonPatternLayout.java +++ b/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/JsonPatternLayout.java @@ -23,65 +23,75 @@ @Plugin(name = "JsonPatternLayout", category = "Core", elementType = "Layout", printObject = true) public final class JsonPatternLayout extends AbstractStringLayout { - private final PatternSelector markerSelector; - private final PatternSelector execptionSelector; + private final PatternSelector markerSelector; + private final PatternSelector execptionSelector; - private final Configuration config; - private final CustomFieldsAdapter customFieldsAdapter; + private final Configuration config; + private final CustomFieldsAdapter customFieldsAdapter; + private final boolean sendDefaultValues; - protected JsonPatternLayout(Configuration config, Charset charset, CustomField... customFieldMdcKeyNames) { - super(charset); - this.config = config; - this.customFieldsAdapter = new CustomFieldsAdapter(customFieldMdcKeyNames); - markerSelector = createPatternSelector(customFieldMdcKeyNames); - execptionSelector = createExceptionSelector(customFieldMdcKeyNames); - } + protected JsonPatternLayout(Configuration config, Charset charset, boolean sendDefaultValues, + CustomField... customFieldMdcKeyNames) { + super(charset); + this.config = config; + this.sendDefaultValues = sendDefaultValues; + this.customFieldsAdapter = new CustomFieldsAdapter(customFieldMdcKeyNames); + markerSelector = createPatternSelector(customFieldMdcKeyNames); + execptionSelector = createExceptionSelector(customFieldMdcKeyNames); + } - @Override - public String toSerializable(LogEvent event) { - PatternSelector selector = getSelector(event); - final StringBuilder buf = getStringBuilder(); - PatternFormatter[] formatters = selector.getFormatters(event); - final int len = formatters.length; - for (int i = 0; i < len; i++) { - formatters[i].format(event, buf); - } - String str = buf.toString(); - return str; - } + @Override + public String toSerializable(LogEvent event) { + PatternSelector selector = getSelector(event); + final StringBuilder buf = getStringBuilder(); + PatternFormatter[] formatters = selector.getFormatters(event); + final int len = formatters.length; + for (int i = 0; i < len; i++) { + formatters[i].format(event, buf); + } + String str = buf.toString(); + return str; + } - @PluginFactory - public static JsonPatternLayout createLayout(@PluginAttribute(value = "charset") final Charset charset, - @PluginElement(value = "customField") CustomField[] customFieldMdcKeyNames, - @PluginConfiguration final Configuration config) { - return new JsonPatternLayout(config, charset, customFieldMdcKeyNames); - } + @PluginFactory + public static JsonPatternLayout createLayout(@PluginAttribute(value = "charset") final Charset charset, + @PluginAttribute(value = "sendDefaultValues") final boolean sendDefaultValues, + @PluginElement(value = "customField") CustomField[] customFieldMdcKeyNames, + @PluginConfiguration final Configuration config) { + return new JsonPatternLayout(config, charset, sendDefaultValues, customFieldMdcKeyNames); + } - private MarkerPatternSelector createPatternSelector(CustomField... customFieldMdcKeyNames) { - PatternMatch[] pMatches = new PatternMatch[1]; - String requestPattern = new LayoutPatternBuilder().addRequestMetrics().addContextProperties(emptyList()) - .suppressExceptions().build(); - pMatches[0] = new PatternMatch(Markers.REQUEST_MARKER.getName(), requestPattern); - List customFields = customFieldsAdapter.getCustomFieldKeyNames(); - List excludedFields = customFieldsAdapter.getExcludedFieldKeyNames(); - String applicationPattern = new LayoutPatternBuilder().addBasicApplicationLogs() - .addContextProperties(excludedFields).addCustomFields(customFields).suppressExceptions().build(); - return new MarkerPatternSelector.Builder().setProperties(pMatches).setDefaultPattern(applicationPattern) - .setAlwaysWriteExceptions(false).setNoConsoleNoAnsi(false).setConfiguration(config).build(); - } + private MarkerPatternSelector createPatternSelector(CustomField... customFieldMdcKeyNames) { + PatternMatch[] pMatches = new PatternMatch[1]; + String requestPattern = new LayoutPatternBuilder(sendDefaultValues).addContextProperties(emptyList()) + .addRequestMetrics().suppressExceptions() + .build(); + pMatches[0] = new PatternMatch(Markers.REQUEST_MARKER.getName(), requestPattern); + List customFields = customFieldsAdapter.getCustomFieldKeyNames(); + List excludedFields = customFieldsAdapter.getExcludedFieldKeyNames(); + String applicationPattern = new LayoutPatternBuilder(sendDefaultValues).addBasicApplicationLogs() + .addContextProperties(excludedFields) + .addCustomFields(customFields) + .suppressExceptions().build(); + return new MarkerPatternSelector.Builder().setProperties(pMatches).setDefaultPattern(applicationPattern) + .setAlwaysWriteExceptions(false).setNoConsoleNoAnsi(false) + .setConfiguration(config).build(); + } - private PatternSelector createExceptionSelector(CustomField... customFieldMdcKeyNames) { - List customFields = customFieldsAdapter.getCustomFieldKeyNames(); - List excludedFields = customFieldsAdapter.getExcludedFieldKeyNames(); - String exceptionPattern = new LayoutPatternBuilder().addBasicApplicationLogs() - .addContextProperties(excludedFields).addCustomFields(customFields).addStacktraces().build(); - return new MarkerPatternSelector(new PatternMatch[0], exceptionPattern, false, false, config); - } + private PatternSelector createExceptionSelector(CustomField... customFieldMdcKeyNames) { + List customFields = customFieldsAdapter.getCustomFieldKeyNames(); + List excludedFields = customFieldsAdapter.getExcludedFieldKeyNames(); + String exceptionPattern = new LayoutPatternBuilder(sendDefaultValues).addBasicApplicationLogs() + .addContextProperties(excludedFields) + .addCustomFields(customFields) + .addStacktraces().build(); + return new MarkerPatternSelector(new PatternMatch[0], exceptionPattern, false, false, config); + } - private PatternSelector getSelector(LogEvent event) { - if (event.getThrownProxy() != null || event.getThrown() != null) { - return execptionSelector; - } - return markerSelector; - } -} \ No newline at end of file + private PatternSelector getSelector(LogEvent event) { + if (event.getThrownProxy() != null || event.getThrown() != null) { + return execptionSelector; + } + return markerSelector; + } +} diff --git a/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilder.java b/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilder.java index 4c15c6d5..8bc391fa 100644 --- a/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilder.java +++ b/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilder.java @@ -1,5 +1,7 @@ package com.sap.hcp.cf.log4j2.layout; +import static java.util.Arrays.asList; + import java.util.List; import com.sap.hcp.cf.log4j2.converter.ContextPropsConverter; @@ -20,14 +22,16 @@ public final class LayoutPatternBuilder { private static final String COMMON_POSTFIX_PATTERN = "}%n"; private final StringBuilder sb; + private final boolean sendDefaultValues; /* * -- this defines the common prefix to all variants. -- the final line will * add non-predefined context parameters from the MDC -- as this list may be * empty, we use "replace" to at a colon if it's not */ - public LayoutPatternBuilder() { - this.sb = new StringBuilder("{ "); + public LayoutPatternBuilder(boolean sendDefaultValues) { + this.sendDefaultValues = sendDefaultValues; + this.sb = new StringBuilder("{ "); appendQuoted(Fields.WRITTEN_AT, "%d{yyyy-MM-dd'T'HH:mm:ss.SSSX}{UTC}").append(","); appendUnquoted(Fields.WRITTEN_TS, "%tstamp").append(","); } @@ -50,8 +54,8 @@ public LayoutPatternBuilder addBasicApplicationLogs() { public LayoutPatternBuilder addContextProperties(List exclusions) { sb.append("%").append(ContextPropsConverter.WORD); + appendParameters(asList(Boolean.toString(sendDefaultValues))); appendParameters(exclusions); - sb.append(","); return this; } diff --git a/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/TestContextPropsConverter.java b/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverterTest.java similarity index 53% rename from cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/TestContextPropsConverter.java rename to cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverterTest.java index 61d0d5da..88df33e9 100644 --- a/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/TestContextPropsConverter.java +++ b/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverterTest.java @@ -1,27 +1,37 @@ package com.sap.hcp.cf.log4j2.converter; import static com.sap.hcp.cf.logging.common.customfields.CustomField.customField; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.not; import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertThat; import java.io.IOException; +import java.util.Map; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.pattern.LogEventPatternConverter; import org.junit.Test; import org.slf4j.MDC; import com.fasterxml.jackson.jr.ob.JSONObjectException; -public class TestContextPropsConverter extends AbstractConverterTest { +public class ContextPropsConverterTest extends AbstractConverterTest { - private static final String[] EXCLUDE_SOME_KEY = new String[] { SOME_KEY }; - private static final String[] NO_EXCLUSIONS = new String[0]; + private static final String[] EXCLUDE_SOME_KEY = new String[] { Boolean.FALSE.toString(), SOME_KEY }; + private static final String[] NO_EXCLUSIONS = new String[] { Boolean.FALSE.toString() }; @Test public void testEmpty() throws JSONObjectException, IOException { ContextPropsConverter cpc = new ContextPropsConverter(NO_EXCLUSIONS); MDC.clear(); + assertThat(mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))).size(), is(0)); + } + + @Test + public void testEmptyWithDefaults() throws JSONObjectException, IOException { + ContextPropsConverter cpc = new ContextPropsConverter(new String[] { Boolean.TRUE.toString() }); + MDC.clear(); assertThat(mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))), is(mdcMap())); } @@ -30,7 +40,9 @@ public void testSingleArg() throws JSONObjectException, IOException { ContextPropsConverter cpc = new ContextPropsConverter(NO_EXCLUSIONS); MDC.clear(); MDC.put(SOME_KEY, SOME_VALUE); - assertThat(mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))), is(mdcMap())); + Map resultMap = mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))); + assertThat(resultMap, hasEntry(SOME_KEY, SOME_VALUE)); + assertThat(resultMap.size(), is(1)); } @Test @@ -39,7 +51,11 @@ public void testTwoArgs() throws JSONObjectException, IOException { MDC.clear(); MDC.put(SOME_KEY, SOME_VALUE); MDC.put(SOME_OTHER_KEY, SOME_OTHER_VALUE); - assertThat(mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))), is(mdcMap())); + Map resultMap = mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))); + assertThat(resultMap, hasEntry(SOME_KEY, SOME_VALUE)); + assertThat(resultMap, hasEntry(SOME_OTHER_KEY, SOME_OTHER_VALUE)); + assertThat(resultMap.size(), is(2)); + } @Test @@ -48,7 +64,10 @@ public void testStrangeArgs() throws JSONObjectException, IOException { MDC.clear(); MDC.put(SOME_KEY, SOME_VALUE); MDC.put(STRANGE_SEQ, STRANGE_SEQ); - assertThat(mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))), is(mdcMap())); + Map resultMap = mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))); + assertThat(resultMap, hasEntry(SOME_KEY, SOME_VALUE)); + assertThat(resultMap, hasEntry(STRANGE_SEQ, STRANGE_SEQ)); + assertThat(resultMap.size(), is(2)); } @Test @@ -57,17 +76,21 @@ public void testExclusion() throws JSONObjectException, IOException { MDC.clear(); MDC.put(SOME_KEY, SOME_VALUE); MDC.put(SOME_OTHER_KEY, SOME_OTHER_VALUE); - assertThat(mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))), is(mdcMap(EXCLUDE_SOME_KEY))); + Map resultMap = mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))); + assertThat(resultMap, hasEntry(SOME_OTHER_KEY, SOME_OTHER_VALUE)); + assertThat(resultMap.size(), is(1)); } @Test public void testExclusionStrangeSeq() throws JSONObjectException, IOException { - String[] exclusions = new String[] { STRANGE_SEQ }; + String[] exclusions = new String[] { Boolean.FALSE.toString(), STRANGE_SEQ }; ContextPropsConverter cpc = new ContextPropsConverter(exclusions); MDC.clear(); MDC.put(SOME_KEY, SOME_VALUE); MDC.put(STRANGE_SEQ, STRANGE_SEQ); - assertThat(mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))), is(mdcMap(exclusions))); + Map resultMap = mapFrom(format(cpc, makeEvent(TEST_MSG_NO_ARGS, NO_ARGS))); + assertThat(resultMap, hasEntry(SOME_KEY, SOME_VALUE)); + assertThat(resultMap.size(), is(1)); } @Test @@ -84,4 +107,10 @@ public void testRegisteredCustomField() throws Exception { not(hasEntry(SOME_KEY, SOME_VALUE))); } + @Override + protected String format(LogEventPatternConverter cpc, LogEvent event) { + String converted = super.format(cpc, event); + return converted.length() > 0 ? converted.substring(0, converted.length() - 1) : converted; + } + } diff --git a/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilderTest.java b/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilderTest.java index 7ceec364..1eaf3940 100644 --- a/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilderTest.java +++ b/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilderTest.java @@ -20,7 +20,7 @@ public class LayoutPatternBuilderTest { @Test public void minimalPattern() throws Exception { - String pattern = new LayoutPatternBuilder().build(); + String pattern = new LayoutPatternBuilder(false).build(); assertThat(pattern, is(COMMON_PREFIX + COMMON_SUFFIX)); assertThat(pattern, specificPart(is(isEmptyString()))); @@ -28,87 +28,96 @@ public void minimalPattern() throws Exception { @Test public void requestPattern() throws Exception { - String pattern = new LayoutPatternBuilder().addRequestMetrics().build(); + String pattern = new LayoutPatternBuilder(false).addRequestMetrics().build(); assertThat(pattern, specificPart(is(",\"type\":\"request\",%jsonmsg{flatten}"))); } @Test public void basicApplicationLogs() throws Exception { - String pattern = new LayoutPatternBuilder().addBasicApplicationLogs().build(); + String pattern = new LayoutPatternBuilder(false).addBasicApplicationLogs().build(); assertThat(pattern, specificPart(is( ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape}"))); } @Test - public void contextProperties() throws Exception { - String pattern = new LayoutPatternBuilder().addContextProperties(Arrays.asList("this key", "that key")).build(); + public void contextPropertiesWithoutDefaultValues() throws Exception { + String pattern = new LayoutPatternBuilder(false).addContextProperties(Arrays.asList("this key", "that key")) + .build(); - assertThat(pattern, specificPart(is(",%ctxp{this key}{that key}"))); + assertThat(pattern, specificPart(is(",%ctxp{false}{this key}{that key}"))); } @Test + public void contextPropertiesWithDefaultValues() throws Exception { + String pattern = new LayoutPatternBuilder(true).addContextProperties(Arrays.asList("this key", "that key")) + .build(); + + assertThat(pattern, specificPart(is(",%ctxp{true}{this key}{that key}"))); + } + + @Test public void customFields() throws Exception { - String pattern = new LayoutPatternBuilder().addCustomFields(Arrays.asList("this key", "that key")).build(); + String pattern = new LayoutPatternBuilder(false).addCustomFields(Arrays.asList("this key", "that key")).build(); assertThat(pattern, specificPart(is(",\"#cf\":{%cf{this key}{that key}}"))); } @Test public void emptyCustomFields() throws Exception { - String pattern = new LayoutPatternBuilder().addCustomFields(Collections.emptyList()).build(); + String pattern = new LayoutPatternBuilder(false).addCustomFields(Collections.emptyList()).build(); assertThat(pattern, specificPart(is(""))); } @Test public void nullCustomFields() throws Exception { - String pattern = new LayoutPatternBuilder().addCustomFields(null).build(); + String pattern = new LayoutPatternBuilder(false).addCustomFields(null).build(); assertThat(pattern, specificPart(is(""))); } @Test public void stacktrace() throws Exception { - String pattern = new LayoutPatternBuilder().addStacktraces().build(); + String pattern = new LayoutPatternBuilder(false).addStacktraces().build(); assertThat(pattern, specificPart(is(",\"stacktrace\":%stacktrace"))); } @Test public void suppressExceptions() throws Exception { - String pattern = new LayoutPatternBuilder().suppressExceptions().build(); + String pattern = new LayoutPatternBuilder(false).suppressExceptions().build(); assertThat(pattern, specificPart(is("%ex{0} "))); } @Test public void requestMetricsScenario() throws Exception { - String pattern = new LayoutPatternBuilder().addRequestMetrics().addContextProperties(emptyList()) + String pattern = new LayoutPatternBuilder(false).addRequestMetrics().addContextProperties(emptyList()) .suppressExceptions().build(); - assertThat(pattern, specificPart(is(",\"type\":\"request\",%jsonmsg{flatten},%ctxp{}%ex{0} "))); + assertThat(pattern, specificPart(is(",\"type\":\"request\",%jsonmsg{flatten},%ctxp{false}{}%ex{0} "))); } @Test public void applicationScenario() throws Exception { - String pattern = new LayoutPatternBuilder().addBasicApplicationLogs() + String pattern = new LayoutPatternBuilder(false).addBasicApplicationLogs() .addContextProperties(asList("excluded-field")).addCustomFields(asList("custom-field")) .suppressExceptions().build(); assertThat(pattern, specificPart(is( - ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{excluded-field},\"#cf\":{%cf{custom-field}}%ex{0} "))); + ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{false}{excluded-field}\"#cf\":{%cf{custom-field}}%ex{0} "))); } @Test public void exceptionScenario() throws Exception { - String pattern = new LayoutPatternBuilder().addBasicApplicationLogs() + String pattern = new LayoutPatternBuilder(false).addBasicApplicationLogs() .addContextProperties(asList("excluded-field")).addCustomFields(asList("custom-field")).addStacktraces() .build(); assertThat(pattern, specificPart(is( - ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{excluded-field},\"#cf\":{%cf{custom-field}},\"stacktrace\":%stacktrace"))); + ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{false}{excluded-field}\"#cf\":{%cf{custom-field}},\"stacktrace\":%stacktrace"))); } private static Matcher specificPart(Matcher expected) { diff --git a/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/logging/common/TestAppLog.java b/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/logging/common/TestAppLog.java index 854ef17a..6405fb60 100644 --- a/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/logging/common/TestAppLog.java +++ b/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/logging/common/TestAppLog.java @@ -5,6 +5,7 @@ import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsNull.notNullValue; @@ -29,9 +30,9 @@ public void test() { logMsg = "Running test()"; LOGGER.info(logMsg); assertThat(getMessage(), is(logMsg)); - assertThat(getField(Fields.COMPONENT_ID), is("-")); - assertThat(getField(Fields.COMPONENT_NAME), is("-")); - assertThat(getField(Fields.COMPONENT_INSTANCE), is("0")); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.COMPONENT_NAME), is(nullValue())); + assertThat(getField(Fields.COMPONENT_INSTANCE), is(nullValue())); assertThat(getField(Fields.WRITTEN_TS), is(notNullValue())); } @@ -42,9 +43,9 @@ public void testCategorties() { LOGGER.info(cat0, logMsg); assertThat(getMessage(), is(logMsg)); - assertThat(getField(Fields.COMPONENT_ID), is("-")); - assertThat(getField(Fields.COMPONENT_NAME), is("-")); - assertThat(getField(Fields.COMPONENT_INSTANCE), is("0")); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.COMPONENT_NAME), is(nullValue())); + assertThat(getField(Fields.COMPONENT_INSTANCE), is(nullValue())); assertThat(getField(Fields.WRITTEN_TS), is(notNullValue())); assertThat(getList(Fields.CATEGORIES), contains(cat0.getName())); @@ -53,9 +54,9 @@ public void testCategorties() { LOGGER.info(cat1, logMsg); assertThat(getMessage(), is(logMsg)); - assertThat(getField(Fields.COMPONENT_ID), is("-")); - assertThat(getField(Fields.COMPONENT_NAME), is("-")); - assertThat(getField(Fields.COMPONENT_INSTANCE), is("0")); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.COMPONENT_NAME), is(nullValue())); + assertThat(getField(Fields.COMPONENT_INSTANCE), is(nullValue())); assertThat(getField(Fields.WRITTEN_TS), is(notNullValue())); assertThat(getList(Fields.CATEGORIES), contains(cat1.getName(), cat0.getName())); } @@ -69,9 +70,9 @@ public void testMDC() { LOGGER.info(logMsg); long afterTS = now(); assertThat(getMessage(), is(logMsg)); - assertThat(getField(Fields.COMPONENT_ID), is("-")); - assertThat(getField(Fields.COMPONENT_NAME), is("-")); - assertThat(getField(Fields.COMPONENT_INSTANCE), is("0")); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.COMPONENT_NAME), is(nullValue())); + assertThat(getField(Fields.COMPONENT_INSTANCE), is(nullValue())); assertThat(getField(Fields.WRITTEN_TS), is(notNullValue())); assertThat(getField(Fields.WRITTEN_TS), greaterThanOrEqualTo(Long.toString(beforeTS))); assertThat(Long.toString(afterTS), greaterThanOrEqualTo(getField(Fields.WRITTEN_TS))); @@ -84,9 +85,9 @@ public void testUnregisteredCustomField() { LOGGER.info(logMsg, CustomField.customField(SOME_KEY, SOME_VALUE)); assertThat(getMessage(), is(logMsg)); assertThat(getField(SOME_KEY), is(SOME_VALUE)); - assertThat(getField(Fields.COMPONENT_ID), is("-")); - assertThat(getField(Fields.COMPONENT_NAME), is("-")); - assertThat(getField(Fields.COMPONENT_INSTANCE), is("0")); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.COMPONENT_NAME), is(nullValue())); + assertThat(getField(Fields.COMPONENT_INSTANCE), is(nullValue())); assertThat(getField(Fields.WRITTEN_TS), is(notNullValue())); assertThat(getField(Fields.WRITTEN_TS), greaterThanOrEqualTo(Long.toString(beforeTS))); assertThat(Long.toString(beforeTS), lessThanOrEqualTo(getField(Fields.WRITTEN_TS))); @@ -103,9 +104,9 @@ public void testCustomFieldOverwritesMdc() throws Exception { CustomField.customField(RETAINED_FIELD_KEY, SOME_OTHER_VALUE), CustomField.customField(SOME_KEY, SOME_OTHER_VALUE)); assertThat(getMessage(), is(logMsg)); - assertThat(getField(Fields.COMPONENT_ID), is("-")); - assertThat(getField(Fields.COMPONENT_NAME), is("-")); - assertThat(getField(Fields.COMPONENT_INSTANCE), is("0")); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.COMPONENT_NAME), is(nullValue())); + assertThat(getField(Fields.COMPONENT_INSTANCE), is(nullValue())); assertThat(getField(Fields.WRITTEN_TS), is(notNullValue())); assertThat(getField(Fields.WRITTEN_TS), greaterThanOrEqualTo(Long.toString(beforeTS))); assertThat(Long.toString(beforeTS), lessThanOrEqualTo(getField(Fields.WRITTEN_TS))); @@ -125,9 +126,9 @@ public void testStacktrace() { logMsg = "Running testStacktrace()"; LOGGER.error(logMsg, ex); assertThat(getMessage(), is(logMsg)); - assertThat(getField(Fields.COMPONENT_ID), is("-")); - assertThat(getField(Fields.COMPONENT_NAME), is("-")); - assertThat(getField(Fields.COMPONENT_INSTANCE), is("0")); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.COMPONENT_NAME), is(nullValue())); + assertThat(getField(Fields.COMPONENT_INSTANCE), is(nullValue())); assertThat(getField(Fields.STACKTRACE), is(notNullValue())); assertThat(getField(Fields.WRITTEN_TS), is(notNullValue())); } diff --git a/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/converter/ContextPropsConverter.java b/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/converter/ContextPropsConverter.java index 2666c813..22be4df8 100644 --- a/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/converter/ContextPropsConverter.java +++ b/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/converter/ContextPropsConverter.java @@ -67,6 +67,7 @@ private void addCustomFieldsFromArgument(Map propertyMap, ILoggi public void start() { customFieldsAdapter.initialize(getContext()); converter.setExclusions(calculateExclusions()); + converter.setSendDefaultValues(customFieldsAdapter.isSendDefaultValues()); super.start(); } diff --git a/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/converter/CustomFieldsAdapter.java b/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/converter/CustomFieldsAdapter.java index 503f27fe..a8a0a0ac 100644 --- a/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/converter/CustomFieldsAdapter.java +++ b/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/converter/CustomFieldsAdapter.java @@ -15,9 +15,11 @@ public class CustomFieldsAdapter { public static final String OPTION_MDC_CUSTOM_FIELDS = "customFieldMdcKeyNames"; public static final String OPTION_MDC_RETAINED_FIELDS = "retainFieldMdcKeyNames"; + public static final String OPTION_SEND_DEFAULT_VALUES = "sendDefaultValues"; private List customFieldMdcKeyNames = emptyList(); private List customFieldExclusions = emptyList(); + private boolean sendDefaultValues; public void initialize(Context context) { if (context == null) { @@ -25,6 +27,7 @@ public void initialize(Context context) { } customFieldExclusions = calculateExclusions(context); customFieldMdcKeyNames = getListItemsAsStrings(context, OPTION_MDC_CUSTOM_FIELDS); + sendDefaultValues = getBoolean(context, OPTION_SEND_DEFAULT_VALUES); } private List calculateExclusions(Context context) { @@ -47,6 +50,14 @@ private List getListItemsAsStrings(Context context, String key) { return emptyList(); } + private boolean getBoolean(Context context, String key) { + Object object = context.getObject(key); + if (object instanceof Boolean) { + return ((Boolean) object).booleanValue(); + } + return false; + } + public List getCustomFieldExclusions() { return customFieldExclusions; } @@ -55,4 +66,8 @@ public List getCustomFieldMdcKeyNames() { return customFieldMdcKeyNames; } + public boolean isSendDefaultValues() { + return sendDefaultValues; + } + } diff --git a/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/encoder/JsonEncoder.java b/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/encoder/JsonEncoder.java index 32449288..e5b506c6 100644 --- a/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/encoder/JsonEncoder.java +++ b/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/encoder/JsonEncoder.java @@ -43,6 +43,7 @@ */ public class JsonEncoder extends LayoutWrappingEncoder { + public static class JsonLayout extends LayoutBase { private final Map layouts = new HashMap(); @@ -121,19 +122,25 @@ private void initPatterns() { private List customFieldMdcKeyNames = new ArrayList<>(); private List retainFieldMdcKeyNames = new ArrayList<>(); + private boolean sendDefaultValues = false; public void addCustomFieldMdcKeyName(String name) { - customFieldMdcKeyNames.add(name); + customFieldMdcKeyNames.add(name); } public void addRetainFieldMdcKeyName(String name) { retainFieldMdcKeyNames.add(name); } + public void setSendDefaultValues(boolean sendDefaultValues) { + this.sendDefaultValues = sendDefaultValues; + } + @Override public void start() { context.putObject(CustomFieldsAdapter.OPTION_MDC_CUSTOM_FIELDS, customFieldMdcKeyNames); context.putObject(CustomFieldsAdapter.OPTION_MDC_RETAINED_FIELDS, retainFieldMdcKeyNames); + context.putObject(CustomFieldsAdapter.OPTION_SEND_DEFAULT_VALUES, sendDefaultValues); JsonLayout jsonLayout = new JsonLayout(); jsonLayout.setContext(context); diff --git a/cf-java-logging-support-logback/src/test/java/com/sap/hcp/cf/logback/converter/ContextPropsConverterTest.java b/cf-java-logging-support-logback/src/test/java/com/sap/hcp/cf/logback/converter/ContextPropsConverterTest.java index 0c6cd0ce..573c5d29 100644 --- a/cf-java-logging-support-logback/src/test/java/com/sap/hcp/cf/logback/converter/ContextPropsConverterTest.java +++ b/cf-java-logging-support-logback/src/test/java/com/sap/hcp/cf/logback/converter/ContextPropsConverterTest.java @@ -1,9 +1,9 @@ package com.sap.hcp.cf.logback.converter; import static java.util.Arrays.asList; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasItems; -import static org.junit.Assert.assertThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.verify; @@ -122,7 +122,14 @@ public void testAddsCustomField() throws Exception { verify(defaultConverter).convert(any(StringBuilder.class), eventProperties.capture()); assertThat(eventProperties.getValue(), hasEntry("this key", "this value")); - } + @Test + public void setsSendDefaultValuesFromCustomFieldsAdapter() throws Exception { + when(customFieldsAdapter.isSendDefaultValues()).thenReturn(true); + + converter.start(); + + verify(defaultConverter).setSendDefaultValues(true); + } } diff --git a/cf-java-logging-support-logback/src/test/java/com/sap/hcp/cf/logback/converter/CustomFieldsAdapterTest.java b/cf-java-logging-support-logback/src/test/java/com/sap/hcp/cf/logback/converter/CustomFieldsAdapterTest.java index 8ae72357..4799a2ee 100644 --- a/cf-java-logging-support-logback/src/test/java/com/sap/hcp/cf/logback/converter/CustomFieldsAdapterTest.java +++ b/cf-java-logging-support-logback/src/test/java/com/sap/hcp/cf/logback/converter/CustomFieldsAdapterTest.java @@ -3,16 +3,17 @@ import static com.sap.hcp.cf.logback.converter.CustomFieldsAdapter.OPTION_MDC_CUSTOM_FIELDS; import static com.sap.hcp.cf.logback.converter.CustomFieldsAdapter.OPTION_MDC_RETAINED_FIELDS; +import static com.sap.hcp.cf.logback.converter.CustomFieldsAdapter.OPTION_SEND_DEFAULT_VALUES; import static java.util.Arrays.asList; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.when; -import java.util.HashMap; import java.util.List; -import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; @@ -27,15 +28,6 @@ @RunWith(MockitoJUnitRunner.class) public class CustomFieldsAdapterTest { - @SuppressWarnings("serial") - private static final Map ALL_ENTRIES = new HashMap() { - { - put("this key", "this value"); - put("that key", "that value"); - put("other key", "other value"); - } - }; - @Mock private Context context; @@ -48,7 +40,7 @@ public void emptyExclusionsWithoutContext() throws Exception { List exclusions = adapter.getCustomFieldExclusions(); - assertThat(exclusions, is(empty())); + assertThat(exclusions, is(empty())); } @Test @@ -121,4 +113,32 @@ public void neverExcludesLogContextFieldsEvenWhenConfigured() throws Exception { assertThat(exclusions, is(empty())); } + @Test + public void missingSendDefaultValueOption() throws Exception { + assertFalse("Should not send default values without config.", adapter.isSendDefaultValues()); + } + + @Test + public void falseSendDefaultValueOption() throws Exception { + when(context.getObject(OPTION_SEND_DEFAULT_VALUES)).thenReturn(Boolean.FALSE); + adapter.initialize(context); + assertFalse("Should not send default values when configured false.", adapter.isSendDefaultValues()); + } + + @Test + public void trueSendDefaultValueOption() throws Exception { + when(context.getObject(OPTION_SEND_DEFAULT_VALUES)).thenReturn(true); + adapter.initialize(context); + assertTrue("Should send default values when configured true.", adapter.isSendDefaultValues()); + } + + @Test + public void faultySendDefaultValueOption() throws Exception { + when(context.getObject(OPTION_SEND_DEFAULT_VALUES)).thenReturn(new Object()); + adapter.initialize(context); + assertFalse("Should not send default values when configured with generic object.", adapter + .isSendDefaultValues()); + + } + } diff --git a/cf-java-logging-support-logback/src/test/resources/logback-test.xml b/cf-java-logging-support-logback/src/test/resources/logback-test.xml index 22d7dfcc..6285886f 100644 --- a/cf-java-logging-support-logback/src/test/resources/logback-test.xml +++ b/cf-java-logging-support-logback/src/test/resources/logback-test.xml @@ -3,6 +3,7 @@ + true custom-field test-field retained-field diff --git a/cf-java-logging-support-servlet/src/test/java/com/sap/hcp/cf/logging/servlet/filter/RequestLoggingFilterTest.java b/cf-java-logging-support-servlet/src/test/java/com/sap/hcp/cf/logging/servlet/filter/RequestLoggingFilterTest.java index 21add618..2f38eafa 100644 --- a/cf-java-logging-support-servlet/src/test/java/com/sap/hcp/cf/logging/servlet/filter/RequestLoggingFilterTest.java +++ b/cf-java-logging-support-servlet/src/test/java/com/sap/hcp/cf/logging/servlet/filter/RequestLoggingFilterTest.java @@ -92,8 +92,8 @@ public void testSimple() throws IOException, ServletException { assertThat(getField(Fields.CORRELATION_ID), not(isEmptyOrNullString())); assertThat(getField(Fields.REQUEST_ID), is(nullValue())); assertThat(getField(Fields.REMOTE_HOST), is(Defaults.UNKNOWN)); - assertThat(getField(Fields.COMPONENT_ID), is(Defaults.UNKNOWN)); - assertThat(getField(Fields.CONTAINER_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.CONTAINER_ID), is(nullValue())); assertThat(getField(Fields.REQUEST_SIZE_B), is("-1")); } @@ -115,8 +115,8 @@ public void doFilter(ServletRequest request, ServletResponse response) throws IO assertThat(getField(Fields.CORRELATION_ID), not(isEmptyOrNullString())); assertThat(getField(Fields.REQUEST_ID), is(nullValue())); assertThat(getField(Fields.REMOTE_HOST), is(Defaults.UNKNOWN)); - assertThat(getField(Fields.COMPONENT_ID), is(Defaults.UNKNOWN)); - assertThat(getField(Fields.CONTAINER_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.CONTAINER_ID), is(nullValue())); assertThat(getField(Fields.REQUEST_SIZE_B), is("1")); } @@ -137,10 +137,10 @@ public void doFilter(ServletRequest request, ServletResponse response) throws IO assertThat(getField(Fields.CORRELATION_ID), not(isEmptyOrNullString())); assertThat(getField(Fields.REQUEST_ID), is(nullValue())); assertThat(getField(Fields.REMOTE_HOST), is(Defaults.UNKNOWN)); - assertThat(getField(Fields.COMPONENT_ID), is(Defaults.UNKNOWN)); - assertThat(getField(Fields.CONTAINER_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.CONTAINER_ID), is(nullValue())); assertThat(getField(Fields.REQUEST_SIZE_B), is("4")); - assertThat(getField(Fields.TENANT_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.TENANT_ID), is(nullValue())); } @Test @@ -163,10 +163,10 @@ public void testWithActivatedOptionalFields() throws IOException, ServletExcepti assertThat(getField(Fields.CORRELATION_ID), is(REQUEST_ID)); assertThat(getField(Fields.REQUEST_ID), is(REQUEST_ID)); assertThat(getField(Fields.REMOTE_HOST), is(REMOTE_HOST)); - assertThat(getField(Fields.COMPONENT_ID), is(Defaults.UNKNOWN)); - assertThat(getField(Fields.CONTAINER_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.CONTAINER_ID), is(nullValue())); assertThat(getField(Fields.REFERER), is(REFERER)); - assertThat(getField(Fields.TENANT_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.TENANT_ID), is(nullValue())); } private void mockGetHeader(HttpHeader header, String value) { @@ -194,9 +194,9 @@ public void testWithSuppressedOptionalFields() throws IOException, ServletExcept assertThat(getField(Fields.REQUEST_ID), is(REQUEST_ID)); assertThat(getField(Fields.REMOTE_IP), is(Defaults.UNKNOWN)); assertThat(getField(Fields.REMOTE_HOST), is(Defaults.REDACTED)); - assertThat(getField(Fields.COMPONENT_ID), is(Defaults.UNKNOWN)); - assertThat(getField(Fields.CONTAINER_ID), is(Defaults.UNKNOWN)); - assertThat(getField(Fields.TENANT_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.COMPONENT_ID), is(nullValue())); + assertThat(getField(Fields.CONTAINER_ID), is(nullValue())); + assertThat(getField(Fields.TENANT_ID), is(nullValue())); } @Test @@ -208,7 +208,7 @@ public void testExplicitCorrelationId() throws IOException, ServletException { assertThat(getField(Fields.CORRELATION_ID), is(CORRELATION_ID)); assertThat(getField(Fields.CORRELATION_ID), not(REQUEST_ID)); assertThat(getField(Fields.REQUEST_ID), is(REQUEST_ID)); - assertThat(getField(Fields.TENANT_ID), is(Defaults.UNKNOWN)); + assertThat(getField(Fields.TENANT_ID), is(nullValue())); } @Test