From 0152632dbe381ec41a1d65dda2dfc79a7aacac18 Mon Sep 17 00:00:00 2001 From: Louis Chu Date: Thu, 29 Feb 2024 12:14:49 -0800 Subject: [PATCH 1/3] Refactor flint log format Signed-off-by: Louis Chu --- .../flint/core/logging/CustomJsonLayout.java | 83 +++++++++++++++++++ .../flint/core/logging/OperationMessage.java | 58 +++++++++++++ .../HttpStatusCodeResultPredicate.java | 1 - .../core/logging/CustomJsonLayoutTest.java | 64 ++++++++++++++ .../metrics/reporter/DimensionUtilsTest.java | 1 - 5 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 flint-core/src/main/java/org/opensearch/flint/core/logging/CustomJsonLayout.java create mode 100644 flint-core/src/main/java/org/opensearch/flint/core/logging/OperationMessage.java create mode 100644 flint-core/src/test/java/org/opensearch/flint/core/logging/CustomJsonLayoutTest.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 new file mode 100644 index 000000000..98715c2c2 --- /dev/null +++ b/flint-core/src/main/java/org/opensearch/flint/core/logging/CustomJsonLayout.java @@ -0,0 +1,83 @@ +/* + * 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; + +@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); + } + + @PluginFactory + public static CustomJsonLayout createLayout(@PluginAttribute(value = "charset", defaultString = "UTF-8") Charset charset) { + return new CustomJsonLayout(charset); + } + + @Override + public String toSerializable(LogEvent event) { + if (!(event.getMessage() instanceof OperationMessage)) { + return event.getMessage().getFormattedMessage() + System.lineSeparator(); + } + + 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().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/OperationMessage.java b/flint-core/src/main/java/org/opensearch/flint/core/logging/OperationMessage.java new file mode 100644 index 000000000..7ded763fb --- /dev/null +++ b/flint-core/src/main/java/org/opensearch/flint/core/logging/OperationMessage.java @@ -0,0 +1,58 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.flint.core.logging; + +import org.apache.logging.log4j.message.Message; + +/** + * Represents an operation message with optional status code for logging purposes. + */ +public final class OperationMessage implements Message { + private final String message; + private final Integer statusCode; + + /** + * Constructs an OperationMessage without a status code. + * + * @param message The message content. + */ + public OperationMessage(String message) { + this(message, null); + } + + /** + * Constructs an OperationMessage with a message and an optional status code. + * + * @param message The message content. + * @param statusCode An optional status code, can be null. + */ + public OperationMessage(String message, Integer statusCode) { + this.message = message; + this.statusCode = statusCode; + } + + @Override + public String getFormattedMessage() { + return message; + } + + @Override + public String getFormat() { + return message; + } + + @Override + public Object[] getParameters() { + // Always return an array, even if empty, to avoid null checks on the consuming side + return statusCode != null ? new Object[]{statusCode} : new Object[]{}; + } + + @Override + public Throwable getThrowable() { + // This implementation does not support Throwable directly + return null; + } +} diff --git a/flint-core/src/main/scala/org/opensearch/flint/core/http/handler/HttpStatusCodeResultPredicate.java b/flint-core/src/main/scala/org/opensearch/flint/core/http/handler/HttpStatusCodeResultPredicate.java index fa82e3655..ee2a23e3f 100644 --- a/flint-core/src/main/scala/org/opensearch/flint/core/http/handler/HttpStatusCodeResultPredicate.java +++ b/flint-core/src/main/scala/org/opensearch/flint/core/http/handler/HttpStatusCodeResultPredicate.java @@ -18,7 +18,6 @@ * @param result type (supposed to be HttpResponse for OS client) */ public class HttpStatusCodeResultPredicate implements CheckedPredicate { - private static final Logger LOG = Logger.getLogger(HttpStatusCodeResultPredicate.class.getName()); /** 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 new file mode 100644 index 000000000..271f00a06 --- /dev/null +++ b/flint-core/src/test/java/org/opensearch/flint/core/logging/CustomJsonLayoutTest.java @@ -0,0 +1,64 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.flint.core.logging; + +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.message.Message; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + + +public class CustomJsonLayoutTest { + @Test + public void testSuccessfulLogPlainMessage() { + LogEvent event = mock(LogEvent.class); + when(event.getMessage()).thenReturn(mock(Message.class)); + when(event.getMessage().getFormattedMessage()).thenReturn("Test message"); + + CustomJsonLayout layout = CustomJsonLayout.createLayout(StandardCharsets.UTF_8); + String result = layout.toSerializable(event); + assertNotNull(result); + assertEquals(result, "Test message" + System.lineSeparator()); + } + + @Test + public void testSuccessfulLogOperationMessage() throws NoSuchFieldException, IllegalAccessException { + Class classOfMap = System.getenv().getClass(); + Field field = classOfMap.getDeclaredField("m"); + field.setAccessible(true); + Map writeableEnvironmentVariables = (Map)field.get(System.getenv()); + writeableEnvironmentVariables.put("FLINT_CLUSTER_NAME", "1234567890:testDomain"); + + LogEvent event = mock(LogEvent.class); + when(event.getTimeMillis()).thenReturn(System.currentTimeMillis()); + when(event.getMessage()).thenReturn(mock(OperationMessage.class)); + when(event.getMessage().getFormattedMessage()).thenReturn("Test message"); + when(event.getMessage().getParameters()).thenReturn(new Object[] {new Integer(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")); + } finally { + // since system environment is shared by other tests. Make sure to remove them before exiting. + writeableEnvironmentVariables.remove("FLINT_CLUSTER_NAME"); + } + } +} diff --git a/flint-core/src/test/java/org/opensearch/flint/core/metrics/reporter/DimensionUtilsTest.java b/flint-core/src/test/java/org/opensearch/flint/core/metrics/reporter/DimensionUtilsTest.java index 2178c3c22..55f03a14b 100644 --- a/flint-core/src/test/java/org/opensearch/flint/core/metrics/reporter/DimensionUtilsTest.java +++ b/flint-core/src/test/java/org/opensearch/flint/core/metrics/reporter/DimensionUtilsTest.java @@ -7,7 +7,6 @@ import org.apache.spark.SparkConf; import org.apache.spark.SparkEnv; -import org.apache.spark.metrics.source.FlintMetricSource; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; From b9b945e3261d79852b29b2c44d28695f69f7b32c Mon Sep 17 00:00:00 2001 From: Louis Chu Date: Fri, 1 Mar 2024 08:54:08 -0800 Subject: [PATCH 2/3] Add java docs and update ut Signed-off-by: Louis Chu --- .../flint/core/logging/CustomJsonLayout.java | 26 ++++-- .../flint/core/logging/CustomLogging.java | 36 ++++++++ .../HttpStatusCodeResultPredicate.java | 1 + .../core/logging/CustomJsonLayoutTest.java | 82 ++++++++++++------- 4 files changed, 112 insertions(+), 33 deletions(-) create mode 100644 flint-core/src/main/java/org/opensearch/flint/core/logging/CustomLogging.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 index 98715c2c2..40e72c99f 100644 --- 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 @@ -18,6 +18,13 @@ 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(); @@ -41,24 +48,33 @@ 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) { - if (!(event.getMessage() instanceof OperationMessage)) { - return event.getMessage().getFormattedMessage() + System.lineSeparator(); - } - 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().getParameters().length == 1) { + if (event.getMessage() instanceof OperationMessage && event.getMessage().getParameters().length == 1) { logEventMap.put("StatusCode", event.getMessage().getParameters()[0]); } 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 new file mode 100644 index 000000000..de3879b1d --- /dev/null +++ b/flint-core/src/main/java/org/opensearch/flint/core/logging/CustomLogging.java @@ -0,0 +1,36 @@ +package org.opensearch.flint.core.logging; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +/** + * CustomLogging class using the {@link CustomJsonLayout} for logging. + */ +public class CustomLogging { + // Define a static logger variable so that it references the custom logger + private static final Logger logger = LogManager.getLogger(CustomLogging.class); + + public static void logDebug(String message) { + logger.debug(message); + } + + public static void logInfo(String message) { + logger.info(message); + } + + public static void logWarning(String message) { + logger.warn(message); + } + + public static void logWarning(String message, Throwable e) { + logger.warn(message, e); + } + + public static void logError(String message) { + logger.error(message); + } + + public static void logError(String message, Throwable throwable) { + logger.error(message, throwable); + } +} \ No newline at end of file diff --git a/flint-core/src/main/scala/org/opensearch/flint/core/http/handler/HttpStatusCodeResultPredicate.java b/flint-core/src/main/scala/org/opensearch/flint/core/http/handler/HttpStatusCodeResultPredicate.java index ee2a23e3f..fa82e3655 100644 --- a/flint-core/src/main/scala/org/opensearch/flint/core/http/handler/HttpStatusCodeResultPredicate.java +++ b/flint-core/src/main/scala/org/opensearch/flint/core/http/handler/HttpStatusCodeResultPredicate.java @@ -18,6 +18,7 @@ * @param result type (supposed to be HttpResponse for OS client) */ public class HttpStatusCodeResultPredicate implements CheckedPredicate { + private static final Logger LOG = Logger.getLogger(HttpStatusCodeResultPredicate.class.getName()); /** 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 index 271f00a06..c936677c0 100644 --- 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 @@ -1,51 +1,76 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.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.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; - public class CustomJsonLayoutTest { - @Test - public void testSuccessfulLogPlainMessage() { - LogEvent event = mock(LogEvent.class); - when(event.getMessage()).thenReturn(mock(Message.class)); - when(event.getMessage().getFormattedMessage()).thenReturn("Test message"); + private Map writeableEnvironmentVariables; - CustomJsonLayout layout = CustomJsonLayout.createLayout(StandardCharsets.UTF_8); - String result = layout.toSerializable(event); - assertNotNull(result); - assertEquals(result, "Test message" + System.lineSeparator()); - } - - @Test - public void testSuccessfulLogOperationMessage() throws NoSuchFieldException, IllegalAccessException { + @Before + public void setUp() throws NoSuchFieldException, IllegalAccessException { + // Setup writable environment variables Class classOfMap = System.getenv().getClass(); Field field = classOfMap.getDeclaredField("m"); field.setAccessible(true); - Map writeableEnvironmentVariables = (Map)field.get(System.getenv()); + 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()); - when(event.getMessage()).thenReturn(mock(OperationMessage.class)); - when(event.getMessage().getFormattedMessage()).thenReturn("Test message"); - when(event.getMessage().getParameters()).thenReturn(new Object[] {new Integer(200)}); + + 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); @@ -56,9 +81,10 @@ public void testSuccessfulLogOperationMessage() throws NoSuchFieldException, Ill 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 { - // since system environment is shared by other tests. Make sure to remove them before exiting. - writeableEnvironmentVariables.remove("FLINT_CLUSTER_NAME"); + cleanUp(); } } } From d37ada50131ada1d01bf046a6e56b24d1607bee7 Mon Sep 17 00:00:00 2001 From: Louis Chu Date: Thu, 7 Mar 2024 16:03:33 -0800 Subject: [PATCH 3/3] 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..8908e763b 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 LinkedHashMap<>(); + 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")); + } + } +}