Merge pull request #75 from newrelic/NEWRELIC-3252-mdc-support

Newrelic-3252 Add configurable mdc and stack trace size support

Author: jtduffy

CHANGELOG.md

@@ -4,6 +4,17 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## 3.0.0
+* Exception Stack Trace Size - this has been changed from a default of `10` to `300`. This applies to all logging libraries supported by the `java-log-extension` project.
+    * This is configurable via:
+      * System property: `-Dnewrelic.log_extension.max_stack_size=integer`
+      * Environment Variable: `NEW_RELIC_LOG_EXTENSION_MAX_STACK_SIZE=integer`
+* Mapped Diagnostic Context (MDC) - decorating logs with MDC data is now generally supported but disabled by default. MDC keys will be prefixed by `context.` to prevent clashes with New Relic specific attributes. This applies to all logging libraries supported by the `java-log-extension` project, except for Java Util Logging (JUL) which does not provide an MDC mechanism. 
+  * This is configurable via:
+      * System property: `-Dnewrelic.log_extension.add_mdc=boolean`
+      * Environment Variable: `NEW_RELIC_LOG_EXTENSION_ADD_MDC=boolean`
+  * Note: This is considered a breaking change as previously some of the logging libraries automatically added MDC. If you upgrade to this version of the `java-log-extension` and wish to have MDC added to your logs then you will need to explicitly enable it. Currently, this will add all MDC as filtering out specific keys is not yet supported.
+
 ## 2.6.0
 * Removed the log forwarder. Please use the [New Relic Java Agent](https://github.com/newrelic/newrelic-java-agent) if you want log forwarding to be used.
 * Upgrade Gradle to version 7.5.1

README.md

@@ -18,6 +18,28 @@ We support:
 * [Logback 1.1](logback11/README.md)
 * [Dropwizard 1.3](dropwizard/README.md)
 
+## Configuration Options
+The following config options apply to all supported logging frameworks, with some noted exceptions.
+
+### Mapped Diagnostic Context (MDC)
+Not all logging frameworks provide an MDC mechanism (e.g. JUL) but for those that do you can configure
+the logging extension to add the MDC data in addition to the trace context data from New Relic.
+
+Default for adding MDC is `false`, change this to `true` if you wish to include MDC data on log events. It can be configured by
+environment variable (`NEW_RELIC_LOG_EXTENSION_ADD_MDC=boolean`) or system property (`-Dnewrelic.log_extension.add_mdc=boolean`).
+
+Note that, if set to `true` all MDC data will be added to log events. Currently, there is no filtering capability for excluding specific MDC entries.
+
+MDC data will be added to log events with a `context.` prefix to distinguish it as user defined context data as well as to prevent potential
+collisions with New Relic specific context keys.
+
+### Exception Stack Trace Size
+You can configure the logging extension to control the max stack trace size for exceptions added to log events.
+
+Default max stack trace size is `300`. It is recommended that you do not exceed this value or data could be dropped or truncated as well as
+lead to higher log event ingest costs. Max stack trace size can be configured by environment variable (`NEW_RELIC_LOG_EXTENSION_MAX_STACK_SIZE=integer`)
+or system property (`-Dnewrelic.log_extension.max_stack_size=integer`).
+
 ## Support
 
 Should you need assistance with New Relic products, you are in good hands with several diagnostic tools and support channels.

core-test/src/main/java/com/newrelic/logging/core/LogAsserts.java

@@ -14,6 +14,7 @@
 
 import static com.newrelic.logging.core.ElementName.TIMESTAMP;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -33,6 +34,37 @@ public static void assertFieldValues(String result, Map<String, Object> expected
         }
     }
 
+    /**
+     * Assert whether a specified field (aka attribute key) exists, or not, in the resulting log attributes that are recorded.
+     *
+     * @param field       String representing a field to check the existence of
+     * @param result      String representing the actual results to assert against
+     * @param shouldExist boolean true if field should exist in the result, else false
+     * @throws IOException sometimes
+     */
+    public static boolean assertFieldExistence(String field, String result, boolean shouldExist) throws IOException {
+        assertEquals('\n', result.charAt(result.length() - 1));
+        boolean fieldExists = false;
+
+        try (JsonParser parser = new JsonFactory().createParser(result)) {
+            assertEquals(parser.nextToken(), JsonToken.START_OBJECT);
+
+            JsonToken token;
+            while ((token = parser.nextToken()) != JsonToken.END_OBJECT) {
+                if (token.equals(JsonToken.FIELD_NAME)) {
+                    String elementName = parser.getValueAsString();
+                    if (shouldExist && elementName.contains(field)) {
+                        assertEquals(field, elementName);
+                        fieldExists = true;
+                    } else {
+                        assertNotEquals(field, elementName);
+                    }
+                }
+            }
+        }
+        return fieldExists;
+    }
+
     private static void assertValueIfPresent(String elementName, JsonParser parser, Map<String, Object> expectedValues) throws IOException {
         JsonToken valueToken = parser.nextToken();
         if (elementName.equals(TIMESTAMP)) {
@@ -57,7 +89,7 @@ private static void assertValueIfPresent(String elementName, JsonParser parser,
                     assertEquals(parser.getValueAsString(), expectedValue);
                     break;
                 case "java.util.regex.Pattern":
-                    Pattern pattern = (Pattern)expectedValue;
+                    Pattern pattern = (Pattern) expectedValue;
                     assertEquals(valueToken, JsonToken.VALUE_STRING);
                     assertTrue(
                             pattern.matcher(parser.getValueAsString()).matches(),

core/build.gradle.kts

@@ -13,4 +13,6 @@ configure<JavaPluginConvention> {
 
 dependencies {
     testImplementation("org.junit.jupiter:junit-jupiter:5.6.2")
-}
+    // Allows for easy testing of values based on Environment Variables and System Properties
+    testImplementation("com.github.stefanbirkner:system-lambda:1.2.1")
+}

core/src/main/java/com/newrelic/logging/core/ExceptionUtil.java

@@ -5,8 +5,10 @@
 
 package com.newrelic.logging.core;
 
+import static com.newrelic.logging.core.LogExtensionConfig.getMaxStackSize;
+
 public class ExceptionUtil {
-    public static final int MAX_STACK_SIZE = 10;
+    public static final int MAX_STACK_SIZE_DEFAULT = 300;
     public static String getErrorStack(Throwable throwable) {
         if (throwable == null) {
             return null;
@@ -17,7 +19,7 @@ public static String getErrorStack(Throwable throwable) {
     }
 
     public static String getErrorStack(StackTraceElement[] stack) {
-        return getErrorStack(stack, MAX_STACK_SIZE);
+        return getErrorStack(stack, getMaxStackSize());
     }
 
     public static String getErrorStack(StackTraceElement[] stack, Integer maxStackSize) {
@@ -27,7 +29,7 @@ public static String getErrorStack(StackTraceElement[] stack, Integer maxStackSi
 
         StringBuilder stackBuilder = new StringBuilder();
         for(int i = 0; i < Math.min(maxStackSize, stack.length); i++) {
-            stackBuilder.append("  at " + stack[i].toString() + "\n");
+            stackBuilder.append("  at ").append(stack[i].toString()).append("\n");
         }
         return stackBuilder.toString();
     }

core/src/main/java/com/newrelic/logging/core/LogExtensionConfig.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright 2023 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.newrelic.logging.core;
+
+public class LogExtensionConfig {
+    public static final String CONTEXT_PREFIX = "context.";
+    private static final String CONFIG_PREFIX_ENV_VAR = "NEW_RELIC_LOG_EXTENSION_";
+    private static final String CONFIG_PREFIX_SYS_PROP = "newrelic.log_extension.";
+    public static final String MAX_STACK_SIZE_ENV_VAR = CONFIG_PREFIX_ENV_VAR + "MAX_STACK_SIZE";
+    public static final String MAX_STACK_SIZE_SYS_PROP = CONFIG_PREFIX_SYS_PROP + "max_stack_size";
+    public static final String ADD_MDC_ENV_VAR = CONFIG_PREFIX_ENV_VAR + "ADD_MDC";
+    public static final String ADD_MDC_SYS_PROP = CONFIG_PREFIX_SYS_PROP + "add_mdc";
+    public static final boolean ADD_MDC_DEFAULT = false;
+
+    /**
+     * Get an int representing the max stack size for errors that should be added to logs
+     * <p>
+     * Precedence: Env var > Sys prop > Default
+     *
+     * @return int representing max stack size
+     */
+    public static int getMaxStackSize() {
+        String envVar = System.getenv(MAX_STACK_SIZE_ENV_VAR);
+        String sysProp = System.getProperty(MAX_STACK_SIZE_SYS_PROP);
+
+        if (isInteger(envVar)) {
+            return Integer.parseInt(envVar);
+        } else if (isInteger(sysProp)) {
+            return Integer.parseInt(sysProp);
+        } else {
+            return ExceptionUtil.MAX_STACK_SIZE_DEFAULT;
+        }
+    }
+
+    /**
+     * Get a boolean indicating if MDC should be added to logs
+     * <p>
+     * Precedence: Env var > Sys prop > Default
+     *
+     * @return true if MDC should be added to logs, else false
+     */
+    public static boolean shouldAddMDC() {
+        String envVar = System.getenv(ADD_MDC_ENV_VAR);
+        String sysProp = System.getProperty(ADD_MDC_SYS_PROP);
+
+        if (envVar != null) {
+            return Boolean.parseBoolean(envVar);
+        } else if (sysProp != null) {
+            return Boolean.parseBoolean(sysProp);
+        } else {
+            return ADD_MDC_DEFAULT;
+        }
+    }
+
+    /**
+     * Validate that a String value evaluates to an integer
+     *
+     * @param val String to be evaluated as an int
+     * @return true if val is an int, else false
+     */
+    static boolean isInteger(String val) {
+        if (val == null) {
+            return false;
+        }
+        try {
+            Integer.parseInt(val);
+        } catch (NumberFormatException nfe) {
+            return false;
+        }
+        return true;
+    }
+}

core/src/test/java/com/newrelic/logging/core/LogExtensionConfigTest.java

@@ -0,0 +1,103 @@
+/*
+ * Copyright 2023 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.newrelic.logging.core;
+
+import org.junit.jupiter.api.Test;
+
+import static com.github.stefanbirkner.systemlambda.SystemLambda.withEnvironmentVariable;
+import static com.newrelic.logging.core.ExceptionUtil.MAX_STACK_SIZE_DEFAULT;
+import static com.newrelic.logging.core.LogExtensionConfig.ADD_MDC_DEFAULT;
+import static com.newrelic.logging.core.LogExtensionConfig.ADD_MDC_ENV_VAR;
+import static com.newrelic.logging.core.LogExtensionConfig.ADD_MDC_SYS_PROP;
+import static com.newrelic.logging.core.LogExtensionConfig.MAX_STACK_SIZE_ENV_VAR;
+import static com.newrelic.logging.core.LogExtensionConfig.MAX_STACK_SIZE_SYS_PROP;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class LogExtensionConfigTest {
+
+    @Test
+    void testGetMaxStackSizeDefault() {
+        int actualSize = LogExtensionConfig.getMaxStackSize();
+        assertEquals(MAX_STACK_SIZE_DEFAULT, actualSize);
+    }
+
+    @Test
+    void testGetMaxStackSizeSysProp() {
+        int expectedSize = 5;
+        System.setProperty(MAX_STACK_SIZE_SYS_PROP, String.valueOf(expectedSize));
+        int actualSize = LogExtensionConfig.getMaxStackSize();
+        assertEquals(expectedSize, actualSize);
+
+        System.clearProperty(MAX_STACK_SIZE_SYS_PROP);
+    }
+
+    @Test
+    void testGetMaxStackSizeEnvVar() throws Exception {
+        int expectedSize = 7;
+        int actualSize = withEnvironmentVariable(MAX_STACK_SIZE_ENV_VAR, String.valueOf(expectedSize)).execute(LogExtensionConfig::getMaxStackSize);
+        assertEquals(expectedSize, actualSize);
+    }
+
+    @Test
+    void testGetMaxStackSizeEnvVarAndSysProp() throws Exception {
+        // Set via sys prop
+        int sysPropSize = 90;
+        System.setProperty(MAX_STACK_SIZE_SYS_PROP, String.valueOf(sysPropSize));
+
+        // Environment variable takes precedence over system property
+        int expectedSize = 3;
+        int actualSize = withEnvironmentVariable(MAX_STACK_SIZE_ENV_VAR, String.valueOf(expectedSize)).execute(LogExtensionConfig::getMaxStackSize);
+        assertEquals(expectedSize, actualSize);
+
+        System.clearProperty(MAX_STACK_SIZE_SYS_PROP);
+    }
+
+    @Test
+    void testShouldAddMDCDefault() {
+        boolean actualAddMDCValue = LogExtensionConfig.shouldAddMDC();
+        assertEquals(ADD_MDC_DEFAULT, actualAddMDCValue);
+    }
+
+    @Test
+    void testShouldAddMDCSysProp() {
+        boolean expectedAddMDCValue = true;
+        System.setProperty(ADD_MDC_SYS_PROP, String.valueOf(expectedAddMDCValue));
+        boolean actualAddMDCValue = LogExtensionConfig.shouldAddMDC();
+        assertEquals(expectedAddMDCValue, actualAddMDCValue);
+
+        System.clearProperty(ADD_MDC_SYS_PROP);
+    }
+
+    @Test
+    void testShouldAddMDCEnvVar() throws Exception {
+        boolean expectedAddMDCValue = true;
+        boolean actualAddMDCValue = withEnvironmentVariable(ADD_MDC_ENV_VAR, String.valueOf(expectedAddMDCValue)).execute(LogExtensionConfig::shouldAddMDC);
+        assertEquals(expectedAddMDCValue, actualAddMDCValue);
+    }
+
+    @Test
+    void testShouldAddMDCEnvVarAndSysProp() throws Exception {
+        // Set via sys prop
+        boolean sysPropAddMDCValue = false;
+        System.setProperty(ADD_MDC_SYS_PROP, String.valueOf(sysPropAddMDCValue));
+
+        // Environment variable takes precedence over system property
+        boolean expectedAddMDCValue = true;
+        boolean actualAddMDCValue = withEnvironmentVariable(ADD_MDC_ENV_VAR, String.valueOf(expectedAddMDCValue)).execute(LogExtensionConfig::shouldAddMDC);
+        assertEquals(expectedAddMDCValue, actualAddMDCValue);
+
+        System.clearProperty(ADD_MDC_SYS_PROP);
+    }
+
+    @Test
+    void isInteger() {
+        assertFalse(LogExtensionConfig.isInteger("FOO"));
+        assertFalse(LogExtensionConfig.isInteger("90.12"));
+        assertTrue(LogExtensionConfig.isInteger("200"));
+    }
+}