From 525726b5d935430aeb825ba0ffdbc2c21d37f961 Mon Sep 17 00:00:00 2001 From: Louis Chu Date: Thu, 7 Mar 2024 16:03:33 -0800 Subject: [PATCH] Use pattern layout json msg format Signed-off-by: Louis Chu --- .../flint/core/logging/CustomJsonLayout.java | 99 ----------- .../flint/core/logging/CustomLogging.java | 159 ++++++++++++++++-- .../DimensionedCloudWatchReporter.java | 1 - .../core/logging/CustomJsonLayoutTest.java | 90 ---------- .../flint/core/logging/CustomLoggingTest.java | 77 +++++++++ 5 files changed, 221 insertions(+), 205 deletions(-) delete mode 100644 flint-core/src/main/java/org/opensearch/flint/core/logging/CustomJsonLayout.java delete mode 100644 flint-core/src/test/java/org/opensearch/flint/core/logging/CustomJsonLayoutTest.java create mode 100644 flint-core/src/test/java/org/opensearch/flint/core/logging/CustomLoggingTest.java diff --git a/flint-core/src/main/java/org/opensearch/flint/core/logging/CustomJsonLayout.java b/flint-core/src/main/java/org/opensearch/flint/core/logging/CustomJsonLayout.java deleted file mode 100644 index 40e72c99f..000000000 --- a/flint-core/src/main/java/org/opensearch/flint/core/logging/CustomJsonLayout.java +++ /dev/null @@ -1,99 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.flint.core.logging; - -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; -import org.apache.logging.log4j.core.Layout; -import org.apache.logging.log4j.core.LogEvent; -import org.apache.logging.log4j.core.config.plugins.Plugin; -import org.apache.logging.log4j.core.config.plugins.PluginAttribute; -import org.apache.logging.log4j.core.config.plugins.PluginFactory; -import org.apache.logging.log4j.core.layout.AbstractStringLayout; - -import java.nio.charset.Charset; -import java.util.HashMap; -import java.util.Map; - -/** - * CustomJsonLayout is a plugin for formatting log events as JSON strings. - * - *

The layout is designed to be used with OpenSearch Flint logging. It extracts environment-specific information, - * such as the cluster name, from the environment variable "FLINT_CLUSTER_NAME" and splits it into domain name and client ID.

- * - */ -@Plugin(name = "CustomJsonLayout", category = "Core", elementType = Layout.ELEMENT_TYPE, printObject = true) -public class CustomJsonLayout extends AbstractStringLayout { - private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); - private static final String CLIENT_ID; - private static final String DOMAIN_NAME; - private static final String UNKNOWN = "UNKNOWN"; - - static { - String value = System.getenv().getOrDefault("FLINT_CLUSTER_NAME", ""); - String[] parts = value.split(":"); - if (value.isEmpty() || parts.length != 2) { - CLIENT_ID = UNKNOWN; - DOMAIN_NAME = UNKNOWN; - } else { - CLIENT_ID = parts[0]; - DOMAIN_NAME = parts[1]; - } - } - - protected CustomJsonLayout(Charset charset) { - super(charset); - } - - /** - * Plugin factory method to create an instance of CustomJsonLayout. - * - * @param charset The charset for encoding the log event. If not specified, defaults to UTF-8. - * @return A new instance of CustomJsonLayout with the specified charset. - */ - @PluginFactory - public static CustomJsonLayout createLayout(@PluginAttribute(value = "charset", defaultString = "UTF-8") Charset charset) { - return new CustomJsonLayout(charset); - } - - /** - * Converts the log event to a JSON string. - * If the log event's message does not follow the expected format, it returns the formatted message directly. - * - * @param event the log event to format. - * @return A string representation of the log event in JSON format. - */ - @Override - public String toSerializable(LogEvent event) { - Map logEventMap = new HashMap<>(); - logEventMap.put("timestamp", event.getTimeMillis()); - logEventMap.put("message", event.getMessage().getFormattedMessage()); - logEventMap.put("domainName", DOMAIN_NAME); - logEventMap.put("clientId", CLIENT_ID); - - if (event.getMessage() instanceof OperationMessage && event.getMessage().getParameters().length == 1) { - logEventMap.put("StatusCode", event.getMessage().getParameters()[0]); - } - - Throwable throwable = event.getThrown(); - if (throwable != null) { - logEventMap.put("Exception", throwable.getClass().getName()); - logEventMap.put("ExceptionMessage", throwable.getMessage()); - } - - return convertToJson(logEventMap) + System.lineSeparator(); - } - - private String convertToJson(Map logEventMap) { - try { - return OBJECT_MAPPER.writeValueAsString(logEventMap); - } catch (JsonProcessingException e) { - // Logging this error using System.err to avoid recursion - System.err.println("Error serializing log event to JSON: " + e.getMessage()); - return "{\"Error\":\"Error serializing log event\"}"; - } - } -} \ No newline at end of file diff --git a/flint-core/src/main/java/org/opensearch/flint/core/logging/CustomLogging.java b/flint-core/src/main/java/org/opensearch/flint/core/logging/CustomLogging.java index de3879b1d..9f5e7e81e 100644 --- a/flint-core/src/main/java/org/opensearch/flint/core/logging/CustomLogging.java +++ b/flint-core/src/main/java/org/opensearch/flint/core/logging/CustomLogging.java @@ -1,36 +1,165 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + package org.opensearch.flint.core.logging; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.core.JsonProcessingException; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.Message; + +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.function.BiConsumer; /** - * CustomLogging class using the {@link CustomJsonLayout} for logging. + * CustomLogging provides a structured logging framework that supports various log levels + * and formats log messages as JSON. This new log format follows OTEL convention + * https://opentelemetry.io/docs/specs/semconv/general/logs/ */ public class CustomLogging { - // Define a static logger variable so that it references the custom logger private static final Logger logger = LogManager.getLogger(CustomLogging.class); + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + private static final String CLIENT_ID; + private static final String DOMAIN_NAME; + private static final String UNKNOWN = "UNKNOWN"; + + /** + * Default severity level follows https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-severitynumber + */ + private static final Map severityLevelMap = Map.of( + "TRACE", 1, + "DEBUG", 5, + "INFO", 9, + "WARN", 13, + "ERROR", 17, + "FATAL", 21 + ); + private static final BiConsumer defaultLogAction = logger::info; + private static final Map> logLevelActions = new HashMap<>(); + + static { + String[] parts = System.getenv().getOrDefault("FLINT_CLUSTER_NAME", UNKNOWN + ":" + UNKNOWN).split(":"); + CLIENT_ID = parts.length == 2 ? parts[0] : UNKNOWN; + DOMAIN_NAME = parts.length == 2 ? parts[1] : UNKNOWN; + + logLevelActions.put("DEBUG", logger::debug); + logLevelActions.put("INFO", logger::info); + logLevelActions.put("WARN", logger::warn); + logLevelActions.put("ERROR", logger::error); + logLevelActions.put("FATAL", logger::fatal); + } + + private static int getSeverityNumber(String level) { + return severityLevelMap.getOrDefault(level, 0); + } + + private static String convertToJson(Map logEventMap) { + try { + return OBJECT_MAPPER.writeValueAsString(logEventMap); + } catch (JsonProcessingException e) { + System.err.println("Error serializing log event to JSON: " + e.getMessage()); + return "{\"Error\":\"Error serializing log event\"}"; + } + } + + /** + * Constructs a log event map containing log details such as timestamp, severity, + * message body, and custom attributes including domainName and clientId. + * + * @param level The severity level of the log. + * @param content The main content of the log message. + * @param throwable An optional Throwable associated with log messages for error levels. + * @return A map representation of the log event. + */ + protected static Map constructLogEventMap(String level, Object content, Throwable throwable) { + if (content == null) { + throw new IllegalArgumentException("Log message must not be null"); + } + + Map logEventMap = new LinkedHashMap<>(); + Map body = new LinkedHashMap<>(); + constructMessageBody(content, body); + + Map attributes = new HashMap<>(); + attributes.put("domainName", DOMAIN_NAME); + attributes.put("clientId", CLIENT_ID); + + if (throwable != null) { + attributes.put("exception.type", throwable.getClass().getName()); + attributes.put("exception.message", throwable.getMessage()); + } + + logEventMap.put("timestamp", System.currentTimeMillis()); + logEventMap.put("severityText", level); + logEventMap.put("severityNumber", getSeverityNumber(level)); + logEventMap.put("body", body); + logEventMap.put("attributes", attributes); + + return logEventMap; + } + + private static void constructMessageBody(Object content, Map body) { + if (content instanceof Message) { + Message message = (Message) content; + body.put("message", message.getFormattedMessage()); + if (content instanceof OperationMessage && message.getParameters().length > 0) { + body.put("statusCode", message.getParameters()[0]); + } + } else { + body.put("message", content.toString()); + } + } + + /** + * Logs a message with the specified severity level, message content, and an optional throwable. + * + * @param level The severity level of the log. + * @param content The content of the log message. + * @param throwable An optional Throwable for logging errors or exceptions. + */ + private static void log(String level, Object content, Throwable throwable) { + Map logEventMap = constructLogEventMap(level, content, throwable); + String jsonMessage = convertToJson(logEventMap); + logLevelActions.getOrDefault(level, defaultLogAction).accept(jsonMessage, throwable); + } + + // Public logging methods for various severity levels. + + public static void logDebug(Object message) { + log("DEBUG", message, null); + } + + public static void logInfo(Object message) { + log("INFO", message, null); + } - public static void logDebug(String message) { - logger.debug(message); + public static void logWarning(Object message) { + log("WARN", message, null); } - public static void logInfo(String message) { - logger.info(message); + public static void logWarning(Object message, Throwable e) { + log("WARN", message, e); } - public static void logWarning(String message) { - logger.warn(message); + public static void logError(Object message) { + log("ERROR", message, null); } - public static void logWarning(String message, Throwable e) { - logger.warn(message, e); + public static void logError(Object message, Throwable throwable) { + log("ERROR", message, throwable); } - public static void logError(String message) { - logger.error(message); + public static void logFatal(Object message) { + log("FATAL", message, null); } - public static void logError(String message, Throwable throwable) { - logger.error(message, throwable); + public static void logFatal(Object message, Throwable throwable) { + log("FATAL", message, throwable); } -} \ No newline at end of file +} diff --git a/flint-core/src/main/java/org/opensearch/flint/core/metrics/reporter/DimensionedCloudWatchReporter.java b/flint-core/src/main/java/org/opensearch/flint/core/metrics/reporter/DimensionedCloudWatchReporter.java index e16eb0021..f4d456899 100644 --- a/flint-core/src/main/java/org/opensearch/flint/core/metrics/reporter/DimensionedCloudWatchReporter.java +++ b/flint-core/src/main/java/org/opensearch/flint/core/metrics/reporter/DimensionedCloudWatchReporter.java @@ -137,7 +137,6 @@ public void report(final SortedMap gauges, final SortedMap histograms, final SortedMap meters, final SortedMap timers) { - if (builder.withDryRun) { LOGGER.warn("** Reporter is running in 'DRY RUN' mode **"); } diff --git a/flint-core/src/test/java/org/opensearch/flint/core/logging/CustomJsonLayoutTest.java b/flint-core/src/test/java/org/opensearch/flint/core/logging/CustomJsonLayoutTest.java deleted file mode 100644 index c936677c0..000000000 --- a/flint-core/src/test/java/org/opensearch/flint/core/logging/CustomJsonLayoutTest.java +++ /dev/null @@ -1,90 +0,0 @@ -package org.opensearch.flint.core.logging; - -import org.apache.logging.log4j.core.LogEvent; -import org.apache.logging.log4j.message.Message; -import org.junit.Before; -import org.junit.Test; - -import java.lang.reflect.Field; -import java.nio.charset.StandardCharsets; -import java.util.Map; - -import static org.junit.Assert.*; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class CustomJsonLayoutTest { - private Map writeableEnvironmentVariables; - - @Before - public void setUp() throws NoSuchFieldException, IllegalAccessException { - // Setup writable environment variables - Class classOfMap = System.getenv().getClass(); - Field field = classOfMap.getDeclaredField("m"); - field.setAccessible(true); - writeableEnvironmentVariables = (Map) field.get(System.getenv()); - writeableEnvironmentVariables.put("FLINT_CLUSTER_NAME", "1234567890:testDomain"); - } - - private void cleanUp() { - // Clean up environment variables - writeableEnvironmentVariables.remove("FLINT_CLUSTER_NAME"); - } - - private LogEvent setupLogEvent(Class messageClass, String formattedMessage, Object... parameters) { - LogEvent event = mock(LogEvent.class); - when(event.getTimeMillis()).thenReturn(System.currentTimeMillis()); - - Message message = mock(messageClass); - when(message.getFormattedMessage()).thenReturn(formattedMessage); - if (parameters.length > 0) { - when(message.getParameters()).thenReturn(parameters); - } - when(event.getMessage()).thenReturn(message); - - Exception mockException = new Exception("Test exception message"); - when(event.getThrown()).thenReturn(mockException); - - return event; - } - - @Test - public void testSuccessfulLogPlainMessage() { - LogEvent event = setupLogEvent(Message.class, "Test message"); - - try { - CustomJsonLayout layout = CustomJsonLayout.createLayout(StandardCharsets.UTF_8); - - String result = layout.toSerializable(event); - assertNotNull(result); - assertTrue(result.contains("\"message\":\"Test message\"")); - assertTrue(result.contains("\"clientId\":\"1234567890\"")); - assertTrue(result.contains("\"domainName\":\"testDomain\"")); - assertFalse(result.contains("\"StatusCode\"")); - assertTrue(result.contains("\"Exception\":\"java.lang.Exception\"")); - assertTrue(result.contains("\"ExceptionMessage\":\"Test exception message\"")); - } finally { - cleanUp(); - } - } - - @Test - public void testSuccessfulLogOperationMessage() { - LogEvent event = setupLogEvent(OperationMessage.class, "Test message", 200); - - try { - CustomJsonLayout layout = CustomJsonLayout.createLayout(StandardCharsets.UTF_8); - - String result = layout.toSerializable(event); - assertNotNull(result); - assertTrue(result.contains("\"message\":\"Test message\"")); - assertTrue(result.contains("\"clientId\":\"1234567890\"")); - assertTrue(result.contains("\"domainName\":\"testDomain\"")); - assertTrue(result.contains("\"StatusCode\":200")); - assertTrue(result.contains("\"Exception\":\"java.lang.Exception\"")); - assertTrue(result.contains("\"ExceptionMessage\":\"Test exception message\"")); - } finally { - cleanUp(); - } - } -} diff --git a/flint-core/src/test/java/org/opensearch/flint/core/logging/CustomLoggingTest.java b/flint-core/src/test/java/org/opensearch/flint/core/logging/CustomLoggingTest.java new file mode 100644 index 000000000..e5cda05b8 --- /dev/null +++ b/flint-core/src/test/java/org/opensearch/flint/core/logging/CustomLoggingTest.java @@ -0,0 +1,77 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.flint.core.logging; + +import org.apache.logging.log4j.message.SimpleMessage; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.Test; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +@RunWith(Parameterized.class) +public class CustomLoggingTest { + private static final String testMsg = "Test message"; + + @Parameterized.Parameter(0) + public Object content; + + @Parameterized.Parameter(1) + public String expectedMessage; + + @Parameterized.Parameter(2) + public Object expectedStatusCode; + + @Parameterized.Parameter(3) + public String severityLevel; + + @Parameterized.Parameter(4) + public Throwable throwable; + + @Parameterized.Parameters(name = "{index}: Test with content={0}, expectedMessage={1}, expectedStatusCode={2}, severityLevel={3}, throwable={4}") + public static Collection data() { + return Arrays.asList(new Object[][]{ + {testMsg, testMsg, null, "INFO", null}, + {new SimpleMessage(testMsg), testMsg, null, "DEBUG", null}, + {new OperationMessage(testMsg, 403), testMsg, 403, "ERROR", new RuntimeException("Test Exception")}, + {testMsg, testMsg, null, "UNDEFINED_LEVEL", null}, // Test with an undefined severity level + {new SimpleMessage(testMsg), testMsg, null, "INFO", new Exception("New Exception")}, + {testMsg, testMsg, null, "WARN", null}, + {"", "", null, "INFO", null}, + {new SimpleMessage(testMsg), testMsg, null, "FATAL", new Error("Critical Error")}, + }); + } + + @Test + public void testConstructLogEventMap() { + Map logEventMap = CustomLogging.constructLogEventMap(severityLevel, content, throwable); + + assertEquals("Incorrect severity text", severityLevel, logEventMap.get("severityText")); + assertTrue("Timestamp should be present and greater than 0", (Long)logEventMap.get("timestamp") > 0); + assertNotNull("Severity number should not be null", logEventMap.get("severityNumber")); + + Map body = (Map) logEventMap.get("body"); + assertEquals("Expected message value not found", expectedMessage, body.get("message")); + if (expectedStatusCode != null) { + assertEquals("Expected status code not found", expectedStatusCode, body.get("statusCode")); + } else { + assertNull("Status code should be null", body.get("statusCode")); + } + + if (throwable != null) { + Map attributes = (Map) logEventMap.get("attributes"); + assertEquals("Exception type should match", throwable.getClass().getName(), attributes.get("exception.type")); + assertEquals("Exception message should match", throwable.getMessage(), attributes.get("exception.message")); + } + } +}