From 07189c62126dc9ac66b2256afe06f5238f7dbe62 Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Wed, 23 Apr 2025 16:17:56 +0200 Subject: [PATCH 01/10] Ignore logging of reserved keys when passed as StructuredArgument. --- .../json/resolver/PowertoolsResolver.java | 17 ++++++++-- .../PowertoolsResolverArgumentsTest.java | 19 +++++++++-- .../internal/handler/PowertoolsArguments.java | 34 +++++++++++++++---- .../powertools/logging/logback/JsonUtils.java | 20 +++++++++-- .../internal/LambdaJsonEncoderTest.java | 10 ++++++ .../internal/handler/PowertoolsArguments.java | 34 +++++++++++++++---- .../logging/argument/ArrayArgument.java | 7 ++++ .../logging/argument/JsonArgument.java | 7 ++++ .../logging/argument/KeyValueArgument.java | 7 ++++ .../logging/argument/MapArgument.java | 22 +++++++++++- .../logging/argument/StructuredArgument.java | 16 +++++++-- .../argument/StructuredArgumentsTest.java | 21 +++++++++++- 12 files changed, 191 insertions(+), 23 deletions(-) diff --git a/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java b/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java index c98da7833..11d8bc9e4 100644 --- a/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java +++ b/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java @@ -27,7 +27,9 @@ import java.io.IOException; import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.logging.log4j.core.LogEvent; @@ -45,6 +47,9 @@ * to be able to recognize powertools fields in the LambdaJsonLayout.json file. */ final class PowertoolsResolver implements EventResolver { + private static final Set RESERVED_LOG_KEYS = Stream + .concat(PowertoolsLoggedFields.stringValues().stream(), List.of("message", "level", "timestamp").stream()) + .collect(Collectors.toSet()); private static final EventResolver COLD_START_RESOLVER = new EventResolver() { @Override @@ -191,9 +196,17 @@ public void resolve(LogEvent logEvent, JsonWriter jsonWriter) { Object[] arguments = logEvent.getMessage().getParameters(); if (arguments != null) { stream(arguments).filter(StructuredArgument.class::isInstance).forEach(argument -> { - serializer.writeRaw(','); try { - ((StructuredArgument) argument).writeTo(serializer); + final StructuredArgument structArg = (StructuredArgument) argument; + final Iterable logKeys = structArg.keys(); + // If any of the logKeys are a reserved key we are going to ignore the argument + for (final String logKey : logKeys) { + if (RESERVED_LOG_KEYS.contains(logKey)) { + return; + } + } + serializer.writeRaw(','); + structArg.writeTo(serializer); } catch (IOException e) { System.err.printf("Failed to encode log event, error: %s.%n", e.getMessage()); } diff --git a/powertools-logging/powertools-logging-log4j/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolverArgumentsTest.java b/powertools-logging/powertools-logging-log4j/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolverArgumentsTest.java index 24014a759..4af8a24c5 100644 --- a/powertools-logging/powertools-logging-log4j/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolverArgumentsTest.java +++ b/powertools-logging/powertools-logging-log4j/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolverArgumentsTest.java @@ -35,6 +35,8 @@ import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.slf4j.MDC; + +import software.amazon.lambda.powertools.logging.internal.PowertoolsLoggedFields; import software.amazon.lambda.powertools.logging.internal.handler.PowertoolsArguments; @Order(2) @@ -83,9 +85,14 @@ void shouldLogArgumentsAsJsonWhenUsingRawJson() { // THEN File logFile = new File("target/logfile.json"); assertThat(contentOf(logFile)) - .contains("\"input\":{\"awsRegion\":\"eu-west-1\",\"body\":\"plop\",\"eventSource\":\"eb\",\"messageAttributes\":{\"keyAttribute\":{\"stringListValues\":[\"val1\",\"val2\",\"val3\"]}},\"messageId\":\"1212abcd\"}") + .contains( + "\"input\":{\"awsRegion\":\"eu-west-1\",\"body\":\"plop\",\"eventSource\":\"eb\",\"messageAttributes\":{\"keyAttribute\":{\"stringListValues\":[\"val1\",\"val2\",\"val3\"]}},\"messageId\":\"1212abcd\"}") .contains("\"message\":\"1212abcd\"") .contains("\"message\":\"Message body = plop and id = \\\"1212abcd\\\"\""); + // Reserved keys should be ignored + PowertoolsLoggedFields.stringValues().stream().forEach(reservedKey -> { + assertThat(contentOf(logFile)).doesNotContain("\"" + reservedKey + "\":\"shouldBeIgnored\""); + }); } @Test @@ -107,9 +114,15 @@ void shouldLogArgumentsAsJsonWhenUsingKeyValue() { // THEN File logFile = new File("target/logfile.json"); assertThat(contentOf(logFile)) - .contains("\"input\":{\"awsRegion\":\"eu-west-1\",\"body\":\"plop\",\"eventSource\":\"eb\",\"messageAttributes\":{\"keyAttribute\":{\"stringListValues\":[\"val1\",\"val2\",\"val3\"]}},\"messageId\":\"1212abcd\"}") + .contains( + "\"input\":{\"awsRegion\":\"eu-west-1\",\"body\":\"plop\",\"eventSource\":\"eb\",\"messageAttributes\":{\"keyAttribute\":{\"stringListValues\":[\"val1\",\"val2\",\"val3\"]}},\"messageId\":\"1212abcd\"}") .contains("\"message\":\"1212abcd\"") .contains("\"message\":\"Message body = plop and id = \\\"1212abcd\\\"\""); + + // Reserved keys should be ignored + PowertoolsLoggedFields.stringValues().stream().forEach(reservedKey -> { + assertThat(contentOf(logFile)).doesNotContain("\"" + reservedKey + "\":\"shouldBeIgnored\""); + }); } private void setupContext() { @@ -119,4 +132,4 @@ private void setupContext() { when(context.getMemoryLimitInMB()).thenReturn(10); when(context.getAwsRequestId()).thenReturn("RequestId"); } -} \ No newline at end of file +} diff --git a/powertools-logging/powertools-logging-log4j/src/test/java/software/amazon/lambda/powertools/logging/internal/handler/PowertoolsArguments.java b/powertools-logging/powertools-logging-log4j/src/test/java/software/amazon/lambda/powertools/logging/internal/handler/PowertoolsArguments.java index 387074590..1fc235ff7 100644 --- a/powertools-logging/powertools-logging-log4j/src/test/java/software/amazon/lambda/powertools/logging/internal/handler/PowertoolsArguments.java +++ b/powertools-logging/powertools-logging-log4j/src/test/java/software/amazon/lambda/powertools/logging/internal/handler/PowertoolsArguments.java @@ -16,15 +16,21 @@ import static software.amazon.lambda.powertools.logging.internal.PowertoolsLoggedFields.CORRELATION_ID; +import java.util.HashMap; +import java.util.Map; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; + import com.amazonaws.services.lambda.runtime.Context; import com.amazonaws.services.lambda.runtime.RequestHandler; import com.amazonaws.services.lambda.runtime.events.SQSEvent; import com.fasterxml.jackson.core.JsonProcessingException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.slf4j.MDC; + import software.amazon.lambda.powertools.logging.Logging; import software.amazon.lambda.powertools.logging.argument.StructuredArguments; +import software.amazon.lambda.powertools.logging.internal.PowertoolsLoggedFields; import software.amazon.lambda.powertools.utilities.JsonConfig; public class PowertoolsArguments implements RequestHandler { @@ -41,11 +47,27 @@ public String handleRequest(SQSEvent.SQSMessage input, Context context) { try { MDC.put(CORRELATION_ID.getName(), input.getMessageId()); if (argumentFormat == ArgumentFormat.JSON) { - LOG.debug("SQS Event", StructuredArguments.json("input", - JsonConfig.get().getObjectMapper().writeValueAsString(input))); + LOG.debug("SQS Event", + StructuredArguments.json("input", + JsonConfig.get().getObjectMapper().writeValueAsString(input)), + // function_name is a reserved key by PowertoolsLoggedFields and should be omitted + StructuredArguments.entry("function_name", "shouldBeIgnored")); } else { - LOG.debug("SQS Event", StructuredArguments.entry("input", input)); + LOG.debug("SQS Event", + StructuredArguments.entry("input", input), + // function_name is a reserved key by PowertoolsLoggedFields and should be omitted + StructuredArguments.entry("function_name", "shouldBeIgnored")); + } + + // Attempt logging all reserved keys, the values should not be overwritten by "shouldBeIgnored" + final Map reservedKeysMap = new HashMap<>(); + for (String field : PowertoolsLoggedFields.stringValues()) { + reservedKeysMap.put(field, "shouldBeIgnored"); } + reservedKeysMap.put("message", "shouldBeIgnored"); + reservedKeysMap.put("level", "shouldBeIgnored"); + reservedKeysMap.put("timestamp", "shouldBeIgnored"); + LOG.debug("Reserved keys", StructuredArguments.entries(reservedKeysMap)); LOG.debug("{}", input.getMessageId()); LOG.warn("Message body = {} and id = \"{}\"", input.getBody(), input.getMessageId()); } catch (JsonProcessingException e) { diff --git a/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java b/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java index b98a8eada..97312df8a 100644 --- a/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java +++ b/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java @@ -19,9 +19,14 @@ import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Date; +import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TimeZone; import java.util.TreeMap; +import java.util.stream.Collectors; +import java.util.stream.Stream; + import software.amazon.lambda.powertools.logging.argument.StructuredArgument; import software.amazon.lambda.powertools.logging.internal.JsonSerializer; import software.amazon.lambda.powertools.logging.internal.PowertoolsLoggedFields; @@ -30,13 +35,16 @@ * Json tools to serialize common fields */ final class JsonUtils { + private static final Set RESERVED_LOG_KEYS = Stream + .concat(PowertoolsLoggedFields.stringValues().stream(), List.of("message", "level", "timestamp").stream()) + .collect(Collectors.toSet()); private JsonUtils() { // static utils } static void serializeTimestamp(JsonSerializer generator, long timestamp, String timestampFormat, - String timestampFormatTimezoneId, String timestampAttributeName) { + String timestampFormatTimezoneId, String timestampAttributeName) { String formattedTimestamp; if (timestampFormat == null || timestamp < 0) { formattedTimestamp = String.valueOf(timestamp); @@ -77,8 +85,16 @@ static void serializeArguments(ILoggingEvent event, JsonSerializer serializer) t if (arguments != null) { for (Object argument : arguments) { if (argument instanceof StructuredArgument) { + final StructuredArgument structArg = (StructuredArgument) argument; + final Iterable logKeys = structArg.keys(); + // If any of the logKeys are a reserved key we are going to ignore the argument + for (final String logKey : logKeys) { + if (RESERVED_LOG_KEYS.contains(logKey)) { + return; + } + } serializer.writeRaw(','); - ((StructuredArgument) argument).writeTo(serializer); + structArg.writeTo(serializer); } } } diff --git a/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java b/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java index 638857cb3..f7d77eff9 100644 --- a/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java +++ b/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java @@ -29,6 +29,8 @@ import com.amazonaws.services.lambda.runtime.Context; import com.amazonaws.services.lambda.runtime.events.SQSEvent; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.util.JSONPObject; + import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; @@ -126,6 +128,10 @@ void shouldLogArgumentsAsJsonWhenUsingRawJson() { .contains("\"input\":{\"awsRegion\":\"eu-west-1\",\"body\":\"plop\",\"eventSource\":\"eb\",\"messageAttributes\":{\"keyAttribute\":{\"stringListValues\":[\"val1\",\"val2\",\"val3\"]}},\"messageId\":\"1212abcd\"}") .contains("\"message\":\"1212abcd\"") .contains("\"message\":\"Message body = plop and id = \"1212abcd\"\""); + // Reserved keys should be ignored + PowertoolsLoggedFields.stringValues().stream().forEach(reservedKey -> { + assertThat(contentOf(logFile)).doesNotContain("\"" + reservedKey + "\":\"shouldBeIgnored\""); + }); } @Test @@ -150,6 +156,10 @@ void shouldLogArgumentsAsJsonWhenUsingKeyValue() { .contains("\"input\":{\"awsRegion\":\"eu-west-1\",\"body\":\"plop\",\"eventSource\":\"eb\",\"messageAttributes\":{\"keyAttribute\":{\"stringListValues\":[\"val1\",\"val2\",\"val3\"]}},\"messageId\":\"1212abcd\"}") .contains("\"message\":\"1212abcd\"") .contains("\"message\":\"Message body = plop and id = \"1212abcd\"\""); + // Reserved keys should be ignored + PowertoolsLoggedFields.stringValues().stream().forEach(reservedKey -> { + assertThat(contentOf(logFile)).doesNotContain("\"" + reservedKey + "\":\"shouldBeIgnored\""); + }); } private final LoggingEvent loggingEvent = new LoggingEvent("fqcn", logger, Level.INFO, "message", null, null); diff --git a/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/handler/PowertoolsArguments.java b/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/handler/PowertoolsArguments.java index 387074590..1fc235ff7 100644 --- a/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/handler/PowertoolsArguments.java +++ b/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/handler/PowertoolsArguments.java @@ -16,15 +16,21 @@ import static software.amazon.lambda.powertools.logging.internal.PowertoolsLoggedFields.CORRELATION_ID; +import java.util.HashMap; +import java.util.Map; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; + import com.amazonaws.services.lambda.runtime.Context; import com.amazonaws.services.lambda.runtime.RequestHandler; import com.amazonaws.services.lambda.runtime.events.SQSEvent; import com.fasterxml.jackson.core.JsonProcessingException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.slf4j.MDC; + import software.amazon.lambda.powertools.logging.Logging; import software.amazon.lambda.powertools.logging.argument.StructuredArguments; +import software.amazon.lambda.powertools.logging.internal.PowertoolsLoggedFields; import software.amazon.lambda.powertools.utilities.JsonConfig; public class PowertoolsArguments implements RequestHandler { @@ -41,11 +47,27 @@ public String handleRequest(SQSEvent.SQSMessage input, Context context) { try { MDC.put(CORRELATION_ID.getName(), input.getMessageId()); if (argumentFormat == ArgumentFormat.JSON) { - LOG.debug("SQS Event", StructuredArguments.json("input", - JsonConfig.get().getObjectMapper().writeValueAsString(input))); + LOG.debug("SQS Event", + StructuredArguments.json("input", + JsonConfig.get().getObjectMapper().writeValueAsString(input)), + // function_name is a reserved key by PowertoolsLoggedFields and should be omitted + StructuredArguments.entry("function_name", "shouldBeIgnored")); } else { - LOG.debug("SQS Event", StructuredArguments.entry("input", input)); + LOG.debug("SQS Event", + StructuredArguments.entry("input", input), + // function_name is a reserved key by PowertoolsLoggedFields and should be omitted + StructuredArguments.entry("function_name", "shouldBeIgnored")); + } + + // Attempt logging all reserved keys, the values should not be overwritten by "shouldBeIgnored" + final Map reservedKeysMap = new HashMap<>(); + for (String field : PowertoolsLoggedFields.stringValues()) { + reservedKeysMap.put(field, "shouldBeIgnored"); } + reservedKeysMap.put("message", "shouldBeIgnored"); + reservedKeysMap.put("level", "shouldBeIgnored"); + reservedKeysMap.put("timestamp", "shouldBeIgnored"); + LOG.debug("Reserved keys", StructuredArguments.entries(reservedKeysMap)); LOG.debug("{}", input.getMessageId()); LOG.warn("Message body = {} and id = \"{}\"", input.getBody(), input.getMessageId()); } catch (JsonProcessingException e) { diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java index 28b29146e..a85e277ed 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java @@ -15,6 +15,8 @@ package software.amazon.lambda.powertools.logging.argument; import java.util.Objects; +import java.util.Set; + import software.amazon.lambda.powertools.logging.internal.JsonSerializer; /** @@ -40,4 +42,9 @@ public void writeTo(JsonSerializer serializer) { public String toString() { return key + "=" + StructuredArguments.toString(values); } + + @Override + public Iterable keys() { + return Set.of(key); + } } diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java index e14f23788..0fafbc516 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java @@ -15,6 +15,8 @@ package software.amazon.lambda.powertools.logging.argument; import java.util.Objects; +import java.util.Set; + import software.amazon.lambda.powertools.logging.internal.JsonSerializer; /** @@ -39,4 +41,9 @@ public void writeTo(JsonSerializer serializer) { public String toString() { return key + "=" + rawJson; } + + @Override + public Iterable keys() { + return Set.of(key); + } } diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java index 569667419..fee4f36dc 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java @@ -15,6 +15,8 @@ package software.amazon.lambda.powertools.logging.argument; import java.util.Objects; +import java.util.Set; + import software.amazon.lambda.powertools.logging.internal.JsonSerializer; /** @@ -39,6 +41,11 @@ public String toString() { return key + "=" + StructuredArguments.toString(value); } + @Override + public Iterable keys() { + return Set.of(getKey()); + } + public String getKey() { return key; } diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java index 9a06ea095..af82905cb 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java @@ -15,8 +15,12 @@ package software.amazon.lambda.powertools.logging.argument; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; +import java.util.stream.Collectors; + import software.amazon.lambda.powertools.logging.internal.JsonSerializer; +import java.util.Set; /** * See {@link StructuredArguments#entries(Map)} @@ -35,8 +39,13 @@ public MapArgument(Map map) { @Override public void writeTo(JsonSerializer serializer) { if (map != null) { - for (Map.Entry entry : map.entrySet()) { + for (Iterator> entries = map.entrySet().iterator(); entries.hasNext();) { + Map.Entry entry = entries.next(); serializer.writeObjectField(String.valueOf(entry.getKey()), entry.getValue()); + // If the map has more than one entry, we need to print a (comma) separator to avoid breaking the JSON + if (entries.hasNext()) { + serializer.writeSeparator(); + } } } } @@ -45,4 +54,15 @@ public void writeTo(JsonSerializer serializer) { public String toString() { return String.valueOf(map); } + + @Override + public Iterable keys() { + if (map == null) { + return Set.of(); + } + + // Object::toString is used to convert the key to a string, as per the behavior of Map.toString() in case the + // key is not already a String. + return map.keySet().stream().map(Object::toString).collect(Collectors.toSet()); + } } diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java index 21fea068d..4383a413c 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java @@ -15,7 +15,9 @@ package software.amazon.lambda.powertools.logging.argument; import java.io.IOException; + import org.slf4j.Logger; + import software.amazon.lambda.powertools.logging.internal.JsonSerializer; /** @@ -26,8 +28,10 @@ public interface StructuredArgument { /** * Writes the data associated with this argument to the given {@link JsonSerializer}. * - * @param serializer the {@link JsonSerializer} to produce JSON content - * @throws IOException if an I/O error occurs + * @param serializer + * the {@link JsonSerializer} to produce JSON content + * @throws IOException + * if an I/O error occurs */ void writeTo(JsonSerializer serializer) throws IOException; @@ -42,4 +46,12 @@ public interface StructuredArgument { */ String toString(); + /** + * Returns the root-level log keys associated with this argument. If the argument has only one key, this method + * will return a single-element iterable. + * + * @return the keys associated with this argument + */ + Iterable keys(); + } diff --git a/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java b/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java index 64200e640..505fde992 100644 --- a/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java +++ b/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java @@ -49,6 +49,7 @@ void keyValueArgument() throws IOException { // THEN assertThat(sb.toString()).hasToString("\"basket\":{\"products\":[{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45},{\"id\":98,\"name\":\"Playstation 5\",\"price\":499.99}]}"); assertThat(argument.toString()).hasToString("basket=Basket{products=[Product{id=42, name='Nintendo DS', price=299.45}, Product{id=98, name='Playstation 5', price=499.99}]}"); + assertThat(argument.keys()).containsExactlyInAnyOrder("basket"); } @Test @@ -64,11 +65,27 @@ void mapArgument() throws IOException { // THEN assertThat(sb.toString()) - .contains("\"nds\":{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45}") + .contains("\"nds\":{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45},") .contains("\"ps5\":{\"id\":98,\"name\":\"Playstation 5\",\"price\":499.99}"); assertThat(argument.toString()) .contains("nds=Product{id=42, name='Nintendo DS', price=299.45}") .contains("ps5=Product{id=98, name='Playstation 5', price=499.99}"); + assertThat(argument.keys()).containsExactlyInAnyOrder("nds", "ps5"); + } + + @Test + void emptyMapArgument() throws IOException { + // GIVEN + Map catalog = new HashMap<>(); + + // WHEN + StructuredArgument argument = StructuredArguments.entries(catalog); + argument.writeTo(serializer); + + // THEN + assertThat(sb.toString()).isEmpty(); + assertThat(argument.toString()).hasToString("{}"); + assertThat(argument.keys()).isEmpty(); } @Test @@ -86,6 +103,7 @@ void arrayArgument() throws IOException { // THEN assertThat(sb.toString()).contains("\"products\":[{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45},{\"id\":98,\"name\":\"Playstation 5\",\"price\":499.99}]"); assertThat(argument.toString()).contains("products=[Product{id=42, name='Nintendo DS', price=299.45}, Product{id=98, name='Playstation 5', price=499.99}]"); + assertThat(argument.keys()).containsExactlyInAnyOrder("products"); } @Test @@ -100,6 +118,7 @@ void jsonArgument() throws IOException { // THEN assertThat(sb.toString()).contains("\"product\":{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45}"); assertThat(argument.toString()).contains("product={\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45}"); + assertThat(argument.keys()).containsExactlyInAnyOrder("product"); } } From 70459e515e5d4dfd9c1521b87923219f9063d171 Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Wed, 23 Apr 2025 16:28:16 +0200 Subject: [PATCH 02/10] Add error to reserved log keys. --- .../layout/template/json/resolver/PowertoolsResolver.java | 3 ++- .../amazon/lambda/powertools/logging/logback/JsonUtils.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java b/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java index 11d8bc9e4..cfaa23443 100644 --- a/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java +++ b/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java @@ -48,7 +48,8 @@ */ final class PowertoolsResolver implements EventResolver { private static final Set RESERVED_LOG_KEYS = Stream - .concat(PowertoolsLoggedFields.stringValues().stream(), List.of("message", "level", "timestamp").stream()) + .concat(PowertoolsLoggedFields.stringValues().stream(), + List.of("message", "level", "timestamp", "error").stream()) .collect(Collectors.toSet()); private static final EventResolver COLD_START_RESOLVER = new EventResolver() { diff --git a/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java b/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java index 97312df8a..ea97d3404 100644 --- a/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java +++ b/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java @@ -36,7 +36,8 @@ */ final class JsonUtils { private static final Set RESERVED_LOG_KEYS = Stream - .concat(PowertoolsLoggedFields.stringValues().stream(), List.of("message", "level", "timestamp").stream()) + .concat(PowertoolsLoggedFields.stringValues().stream(), + List.of("message", "level", "timestamp", "error").stream()) .collect(Collectors.toSet()); private JsonUtils() { From f7848860080c7c96dc7f53cbb723435b1748c678 Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Wed, 23 Apr 2025 16:28:37 +0200 Subject: [PATCH 03/10] Update docs with a warning about logging of reserved keys when using structured arguments. --- docs/core/logging.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/core/logging.md b/docs/core/logging.md index e3b0ebdb3..67d5d42d3 100644 --- a/docs/core/logging.md +++ b/docs/core/logging.md @@ -640,6 +640,9 @@ To append additional keys in your logs, you can use the `StructuredArguments` cl } ``` +???+ warning "Warning" + If the key name of your structured argument matches any of the [standard structured keys](#standard-structured-keys) or any of the [additional structured keys](#additional-structured-keys) the whole argument will be ignored. This is to protect you from accidentally overwriting reserved keys such as the log level or Lambda context information. + **Using MDC** Mapped Diagnostic Context (MDC) is essentially a Key-Value store. It is supported by the [SLF4J API](https://www.slf4j.org/manual.html#mdc){target="_blank"}, From a5e6b0933545f82c10a01994687eeb4f2019cd69 Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Wed, 23 Apr 2025 16:45:32 +0200 Subject: [PATCH 04/10] Update docs with warning in MDC section as well. --- docs/core/logging.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/core/logging.md b/docs/core/logging.md index 67d5d42d3..2c6d9dc64 100644 --- a/docs/core/logging.md +++ b/docs/core/logging.md @@ -640,7 +640,7 @@ To append additional keys in your logs, you can use the `StructuredArguments` cl } ``` -???+ warning "Warning" +???+ warning "Do not use arguments with reserved keys" If the key name of your structured argument matches any of the [standard structured keys](#standard-structured-keys) or any of the [additional structured keys](#additional-structured-keys) the whole argument will be ignored. This is to protect you from accidentally overwriting reserved keys such as the log level or Lambda context information. **Using MDC** @@ -653,6 +653,9 @@ Mapped Diagnostic Context (MDC) is essentially a Key-Value store. It is supporte ???+ warning "Custom keys stored in the MDC are persisted across warm invocations" Always set additional keys as part of your handler method to ensure they have the latest value, or explicitly clear them with [`clearState=true`](#clearing-state). +???+ warning "Do not add reserved keys to MDC" + Avoid adding any of the keys listed in [standard structured keys](#standard-structured-keys) and [additional structured keys](#additional-structured-keys) to your MDC. This may cause unindented behavior and will overwrite the context set by Powertools. Unlike with StructuredArguments, Powertools will **not** ignore reserved keys set via MDC. + ### Removing additional keys From 84e9557316529eaf26209c377da9627dd38f914f Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Thu, 24 Apr 2025 10:05:08 +0200 Subject: [PATCH 05/10] Update product name in documentation. --- docs/core/logging.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/core/logging.md b/docs/core/logging.md index 2c6d9dc64..5a6264615 100644 --- a/docs/core/logging.md +++ b/docs/core/logging.md @@ -654,7 +654,7 @@ Mapped Diagnostic Context (MDC) is essentially a Key-Value store. It is supporte Always set additional keys as part of your handler method to ensure they have the latest value, or explicitly clear them with [`clearState=true`](#clearing-state). ???+ warning "Do not add reserved keys to MDC" - Avoid adding any of the keys listed in [standard structured keys](#standard-structured-keys) and [additional structured keys](#additional-structured-keys) to your MDC. This may cause unindented behavior and will overwrite the context set by Powertools. Unlike with StructuredArguments, Powertools will **not** ignore reserved keys set via MDC. + Avoid adding any of the keys listed in [standard structured keys](#standard-structured-keys) and [additional structured keys](#additional-structured-keys) to your MDC. This may cause unindented behavior and will overwrite the context set by the Logger. Unlike with StructuredArguments, the Logger will **not** ignore reserved keys set via MDC. ### Removing additional keys From 2657657b9a663d8e4cbd219901c771b0b0c8b7bc Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Thu, 24 Apr 2025 10:49:30 +0200 Subject: [PATCH 06/10] Make StructuredArgument implementations package private. They should only be instantiated by factory StructuredArguments. --- .../lambda/powertools/logging/argument/ArrayArgument.java | 2 +- .../amazon/lambda/powertools/logging/argument/JsonArgument.java | 2 +- .../lambda/powertools/logging/argument/KeyValueArgument.java | 2 +- .../amazon/lambda/powertools/logging/argument/MapArgument.java | 2 +- .../lambda/powertools/logging/argument/StructuredArgument.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java index a85e277ed..4c9a45b6f 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java @@ -22,7 +22,7 @@ /** * See {@link StructuredArguments#array(String, Object...)} */ -public class ArrayArgument implements StructuredArgument { +class ArrayArgument implements StructuredArgument { private final String key; private final Object[] values; diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java index 0fafbc516..04667ca81 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java @@ -22,7 +22,7 @@ /** * See {@link StructuredArguments#json(String, String)} */ -public class JsonArgument implements StructuredArgument { +class JsonArgument implements StructuredArgument { private final String key; private final String rawJson; diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java index fee4f36dc..34c5d65c8 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java @@ -22,7 +22,7 @@ /** * See {@link StructuredArguments#entry(String, Object)} */ -public class KeyValueArgument implements StructuredArgument { +class KeyValueArgument implements StructuredArgument { private final String key; private final Object value; diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java index af82905cb..35b851c40 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java @@ -25,7 +25,7 @@ /** * See {@link StructuredArguments#entries(Map)} */ -public class MapArgument implements StructuredArgument { +class MapArgument implements StructuredArgument { private final Map map; public MapArgument(Map map) { diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java index 4383a413c..ddfe038d0 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java @@ -24,7 +24,7 @@ * A wrapper for an argument passed to a log method (e.g. {@link Logger#info(String, Object...)}) * that adds data to the JSON event. */ -public interface StructuredArgument { +interface StructuredArgument { /** * Writes the data associated with this argument to the given {@link JsonSerializer}. * From 58bb26d6f769741054c452b62ca2f361fcf3cf39 Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Thu, 24 Apr 2025 11:11:45 +0200 Subject: [PATCH 07/10] Make StructuredArgument interface public and remove package private dependency in LambdaLoggingAspectTest. --- .../logging/argument/StructuredArgument.java | 2 +- .../internal/LambdaLoggingAspectTest.java | 33 ++++++++----------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java index ddfe038d0..4383a413c 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java @@ -24,7 +24,7 @@ * A wrapper for an argument passed to a log method (e.g. {@link Logger#info(String, Object...)}) * that adds data to the JSON event. */ -interface StructuredArgument { +public interface StructuredArgument { /** * Writes the data associated with this argument to the given {@link JsonSerializer}. * diff --git a/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspectTest.java b/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspectTest.java index 557c6c893..4825d89f3 100644 --- a/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspectTest.java +++ b/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspectTest.java @@ -72,7 +72,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import software.amazon.lambda.powertools.common.internal.LambdaHandlerProcessor; -import software.amazon.lambda.powertools.logging.argument.KeyValueArgument; +import software.amazon.lambda.powertools.logging.argument.StructuredArgument; import software.amazon.lambda.powertools.logging.handlers.PowertoolsLogAlbCorrelationId; import software.amazon.lambda.powertools.logging.handlers.PowertoolsLogApiGatewayHttpApiCorrelationId; import software.amazon.lambda.powertools.logging.handlers.PowertoolsLogApiGatewayRestApiCorrelationId; @@ -466,9 +466,8 @@ void shouldLogEventForHandlerWithLogEventAnnotation() { // THEN TestLogger logger = (TestLogger) PowertoolsLogEvent.getLogger(); assertThat(logger.getArguments()).hasSize(1); - KeyValueArgument argument = (KeyValueArgument) logger.getArguments()[0]; - assertThat(argument.getKey()).isEqualTo("event"); - assertThat(argument.getValue()).isEqualTo(listOfOneElement); + StructuredArgument argument = (StructuredArgument) logger.getArguments()[0]; + assertThat(argument.toString()).hasToString("event=" + listOfOneElement.toString()); } @Test @@ -490,9 +489,9 @@ void shouldLogEventForHandlerWhenEnvVariableSetToTrue() { TestLogger logger = (TestLogger) ((PowertoolsLogEventEnvVar)requestHandler).getLogger(); try { assertThat(logger.getArguments()).hasSize(1); - KeyValueArgument argument = (KeyValueArgument) logger.getArguments()[0]; - assertThat(argument.getKey()).isEqualTo("event"); - assertThat(argument.getValue()).isEqualTo(message); + StructuredArgument argument = (StructuredArgument) logger.getArguments()[0]; + // assertThat(argument.getKey()).isEqualTo("event"); + // assertThat(argument.getValue()).isEqualTo(message); } finally { LoggingConstants.POWERTOOLS_LOG_EVENT = false; if (logger != null){ @@ -532,9 +531,8 @@ void shouldLogEventForStreamHandler() throws IOException { TestLogger logger = (TestLogger) PowertoolsLogEventForStream.getLogger(); try { assertThat(logger.getArguments()).hasSize(1); - KeyValueArgument argument = (KeyValueArgument) logger.getArguments()[0]; - assertThat(argument.getKey()).isEqualTo("event"); - assertThat(argument.getValue()).isEqualTo("{\"key\":\"value\"}"); + StructuredArgument argument = (StructuredArgument) logger.getArguments()[0]; + assertThat(argument.toString()).hasToString("event={\"key\":\"value\"}"); } finally { logger.clearArguments(); } @@ -552,9 +550,8 @@ void shouldLogResponseForHandlerWithLogResponseAnnotation() { TestLogger logger = (TestLogger) PowertoolsLogResponse.getLogger(); try { assertThat(logger.getArguments()).hasSize(1); - KeyValueArgument argument = (KeyValueArgument) logger.getArguments()[0]; - assertThat(argument.getKey()).isEqualTo("response"); - assertThat(argument.getValue()).isEqualTo("Hola mundo"); + StructuredArgument argument = (StructuredArgument) logger.getArguments()[0]; + assertThat(argument.toString()).hasToString("response=Hola mundo"); } finally { logger.clearArguments(); } @@ -574,9 +571,8 @@ void shouldLogResponseForHandlerWhenEnvVariableSetToTrue() { TestLogger logger = (TestLogger) PowertoolsLogEnabled.getLogger(); try { assertThat(logger.getArguments()).hasSize(1); - KeyValueArgument argument = (KeyValueArgument) logger.getArguments()[0]; - assertThat(argument.getKey()).isEqualTo("response"); - assertThat(argument.getValue()).isEqualTo("Bonjour le monde"); + StructuredArgument argument = (StructuredArgument) logger.getArguments()[0]; + assertThat(argument.toString()).hasToString("response=Bonjour le monde"); } finally { LoggingConstants.POWERTOOLS_LOG_RESPONSE = false; logger.clearArguments(); @@ -600,9 +596,8 @@ void shouldLogResponseForStreamHandler() throws IOException { TestLogger logger = (TestLogger) PowertoolsLogResponseForStream.getLogger(); try { assertThat(logger.getArguments()).hasSize(1); - KeyValueArgument argument = (KeyValueArgument) logger.getArguments()[0]; - assertThat(argument.getKey()).isEqualTo("response"); - assertThat(argument.getValue()).isEqualTo(input); + StructuredArgument argument = (StructuredArgument) logger.getArguments()[0]; + assertThat(argument.toString()).hasToString("response=" + input); } finally { logger.clearArguments(); } From 9f16c465c77b8f2e18e5f129d0c5e33e87285edb Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Thu, 24 Apr 2025 12:33:34 +0200 Subject: [PATCH 08/10] Move reserved key logic from logging implementation to StructuredArguments. --- docs/core/logging.md | 2 +- .../json/resolver/PowertoolsResolver.java | 43 ++++------- .../PowertoolsResolverArgumentsTest.java | 6 ++ .../powertools/logging/logback/JsonUtils.java | 18 +---- .../internal/LambdaJsonEncoderTest.java | 6 ++ .../logging/argument/StructuredArguments.java | 74 +++++++++++++++++-- .../argument/StructuredArgumentsTest.java | 49 ++++++++++-- .../internal/LambdaLoggingAspectTest.java | 7 +- 8 files changed, 145 insertions(+), 60 deletions(-) diff --git a/docs/core/logging.md b/docs/core/logging.md index 5a6264615..9fa6922b0 100644 --- a/docs/core/logging.md +++ b/docs/core/logging.md @@ -641,7 +641,7 @@ To append additional keys in your logs, you can use the `StructuredArguments` cl ``` ???+ warning "Do not use arguments with reserved keys" - If the key name of your structured argument matches any of the [standard structured keys](#standard-structured-keys) or any of the [additional structured keys](#additional-structured-keys) the whole argument will be ignored. This is to protect you from accidentally overwriting reserved keys such as the log level or Lambda context information. + If the key name of your structured argument matches any of the [standard structured keys](#standard-structured-keys) or any of the [additional structured keys](#additional-structured-keys) the key will be ignored. This is to protect you from accidentally overwriting reserved keys such as the log level or Lambda context information. **Using MDC** diff --git a/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java b/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java index cfaa23443..8ada50f49 100644 --- a/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java +++ b/powertools-logging/powertools-logging-log4j/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolver.java @@ -27,9 +27,7 @@ import java.io.IOException; import java.util.Collections; -import java.util.List; import java.util.Map; -import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.logging.log4j.core.LogEvent; @@ -47,10 +45,6 @@ * to be able to recognize powertools fields in the LambdaJsonLayout.json file. */ final class PowertoolsResolver implements EventResolver { - private static final Set RESERVED_LOG_KEYS = Stream - .concat(PowertoolsLoggedFields.stringValues().stream(), - List.of("message", "level", "timestamp", "error").stream()) - .collect(Collectors.toSet()); private static final EventResolver COLD_START_RESOLVER = new EventResolver() { @Override @@ -198,16 +192,8 @@ public void resolve(LogEvent logEvent, JsonWriter jsonWriter) { if (arguments != null) { stream(arguments).filter(StructuredArgument.class::isInstance).forEach(argument -> { try { - final StructuredArgument structArg = (StructuredArgument) argument; - final Iterable logKeys = structArg.keys(); - // If any of the logKeys are a reserved key we are going to ignore the argument - for (final String logKey : logKeys) { - if (RESERVED_LOG_KEYS.contains(logKey)) { - return; - } - } serializer.writeRaw(','); - structArg.writeTo(serializer); + ((StructuredArgument) argument).writeTo(serializer); } catch (IOException e) { System.err.printf("Failed to encode log event, error: %s.%n", e.getMessage()); } @@ -218,19 +204,20 @@ public void resolve(LogEvent logEvent, JsonWriter jsonWriter) { private final EventResolver internalResolver; - private static final Map eventResolverMap = Collections.unmodifiableMap(Stream.of(new Object[][] { - { SERVICE.getName(), SERVICE_RESOLVER }, - { FUNCTION_NAME.getName(), FUNCTION_NAME_RESOLVER }, - { FUNCTION_VERSION.getName(), FUNCTION_VERSION_RESOLVER }, - { FUNCTION_ARN.getName(), FUNCTION_ARN_RESOLVER }, - { FUNCTION_MEMORY_SIZE.getName(), FUNCTION_MEMORY_RESOLVER }, - { FUNCTION_REQUEST_ID.getName(), FUNCTION_REQ_RESOLVER }, - { FUNCTION_COLD_START.getName(), COLD_START_RESOLVER }, - { FUNCTION_TRACE_ID.getName(), XRAY_TRACE_RESOLVER }, - { SAMPLING_RATE.getName(), SAMPLING_RATE_RESOLVER }, - { "region", REGION_RESOLVER }, - { "account_id", ACCOUNT_ID_RESOLVER } - }).collect(Collectors.toMap(data -> (String) data[0], data -> (EventResolver) data[1]))); + private static final Map eventResolverMap = Collections + .unmodifiableMap(Stream.of(new Object[][] { + { SERVICE.getName(), SERVICE_RESOLVER }, + { FUNCTION_NAME.getName(), FUNCTION_NAME_RESOLVER }, + { FUNCTION_VERSION.getName(), FUNCTION_VERSION_RESOLVER }, + { FUNCTION_ARN.getName(), FUNCTION_ARN_RESOLVER }, + { FUNCTION_MEMORY_SIZE.getName(), FUNCTION_MEMORY_RESOLVER }, + { FUNCTION_REQUEST_ID.getName(), FUNCTION_REQ_RESOLVER }, + { FUNCTION_COLD_START.getName(), COLD_START_RESOLVER }, + { FUNCTION_TRACE_ID.getName(), XRAY_TRACE_RESOLVER }, + { SAMPLING_RATE.getName(), SAMPLING_RATE_RESOLVER }, + { "region", REGION_RESOLVER }, + { "account_id", ACCOUNT_ID_RESOLVER } + }).collect(Collectors.toMap(data -> (String) data[0], data -> (EventResolver) data[1]))); PowertoolsResolver(final TemplateResolverConfig config) { diff --git a/powertools-logging/powertools-logging-log4j/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolverArgumentsTest.java b/powertools-logging/powertools-logging-log4j/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolverArgumentsTest.java index 4af8a24c5..463ad043d 100644 --- a/powertools-logging/powertools-logging-log4j/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolverArgumentsTest.java +++ b/powertools-logging/powertools-logging-log4j/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/PowertoolsResolverArgumentsTest.java @@ -92,6 +92,9 @@ void shouldLogArgumentsAsJsonWhenUsingRawJson() { // Reserved keys should be ignored PowertoolsLoggedFields.stringValues().stream().forEach(reservedKey -> { assertThat(contentOf(logFile)).doesNotContain("\"" + reservedKey + "\":\"shouldBeIgnored\""); + assertThat(contentOf(logFile)).contains( + "\"message\":\"Attempted to use reserved key '" + reservedKey + + "' in structured argument. This key will be ignored.\""); }); } @@ -122,6 +125,9 @@ void shouldLogArgumentsAsJsonWhenUsingKeyValue() { // Reserved keys should be ignored PowertoolsLoggedFields.stringValues().stream().forEach(reservedKey -> { assertThat(contentOf(logFile)).doesNotContain("\"" + reservedKey + "\":\"shouldBeIgnored\""); + assertThat(contentOf(logFile)).contains( + "\"message\":\"Attempted to use reserved key '" + reservedKey + + "' in structured argument. This key will be ignored.\""); }); } diff --git a/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java b/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java index ea97d3404..67d6b268d 100644 --- a/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java +++ b/powertools-logging/powertools-logging-logback/src/main/java/software/amazon/lambda/powertools/logging/logback/JsonUtils.java @@ -19,13 +19,9 @@ import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Date; -import java.util.List; import java.util.Map; -import java.util.Set; import java.util.TimeZone; import java.util.TreeMap; -import java.util.stream.Collectors; -import java.util.stream.Stream; import software.amazon.lambda.powertools.logging.argument.StructuredArgument; import software.amazon.lambda.powertools.logging.internal.JsonSerializer; @@ -35,10 +31,6 @@ * Json tools to serialize common fields */ final class JsonUtils { - private static final Set RESERVED_LOG_KEYS = Stream - .concat(PowertoolsLoggedFields.stringValues().stream(), - List.of("message", "level", "timestamp", "error").stream()) - .collect(Collectors.toSet()); private JsonUtils() { // static utils @@ -86,16 +78,8 @@ static void serializeArguments(ILoggingEvent event, JsonSerializer serializer) t if (arguments != null) { for (Object argument : arguments) { if (argument instanceof StructuredArgument) { - final StructuredArgument structArg = (StructuredArgument) argument; - final Iterable logKeys = structArg.keys(); - // If any of the logKeys are a reserved key we are going to ignore the argument - for (final String logKey : logKeys) { - if (RESERVED_LOG_KEYS.contains(logKey)) { - return; - } - } serializer.writeRaw(','); - structArg.writeTo(serializer); + ((StructuredArgument) argument).writeTo(serializer); } } } diff --git a/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java b/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java index f7d77eff9..9b6fb8d1c 100644 --- a/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java +++ b/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java @@ -131,6 +131,9 @@ void shouldLogArgumentsAsJsonWhenUsingRawJson() { // Reserved keys should be ignored PowertoolsLoggedFields.stringValues().stream().forEach(reservedKey -> { assertThat(contentOf(logFile)).doesNotContain("\"" + reservedKey + "\":\"shouldBeIgnored\""); + assertThat(contentOf(logFile)).contains( + "\"message\":\"Attempted to use reserved key '" + reservedKey + + "' in structured argument. This key will be ignored.\""); }); } @@ -159,6 +162,9 @@ void shouldLogArgumentsAsJsonWhenUsingKeyValue() { // Reserved keys should be ignored PowertoolsLoggedFields.stringValues().stream().forEach(reservedKey -> { assertThat(contentOf(logFile)).doesNotContain("\"" + reservedKey + "\":\"shouldBeIgnored\""); + assertThat(contentOf(logFile)).contains( + "\"message\":\"Attempted to use reserved key '" + reservedKey + + "' in structured argument. This key will be ignored.\""); }); } diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArguments.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArguments.java index 8a75b3118..f56a42ea3 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArguments.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArguments.java @@ -15,7 +15,16 @@ package software.amazon.lambda.powertools.logging.argument; import java.util.Arrays; +import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import software.amazon.lambda.powertools.logging.internal.PowertoolsLoggedFields; /** * Factory for creating {@link StructuredArgument}s. @@ -23,54 +32,109 @@ */ public class StructuredArguments { + private static final Logger LOGGER = LoggerFactory.getLogger(StructuredArguments.class); + + /** + * Set of reserved keys that should not be used in structured arguments. + * When a reserved key is used, the method will return null. + */ + private static final Set RESERVED_KEYS = Stream + .concat(PowertoolsLoggedFields.stringValues().stream(), + List.of("message", "level", "timestamp", "error").stream()) + .collect(Collectors.toSet()); + private StructuredArguments() { // nothing to do, use static methods only } + /** + * Checks if the provided key is a reserved key. + * If the key is reserved, logs a warning message. + * + * @param key the key to check + * @return true if the key is reserved, false otherwise + */ + private static boolean isReservedKey(String key) { + if (key != null && RESERVED_KEYS.contains(key)) { + LOGGER.warn( + "Attempted to use reserved key '{}' in structured argument. This key will be ignored.", key); + return true; + } + return false; + } + /** * Adds "key": "value" to the JSON structure and "key=value" to the formatted message. + * Returns null if the key is a reserved key. * * @param key the field name * @param value the value associated with the key (can be any kind of object) - * @return a {@link StructuredArgument} populated with the data + * @return a {@link StructuredArgument} populated with the data, or null if key is reserved */ public static StructuredArgument entry(String key, Object value) { + if (isReservedKey(key)) { + return null; + } return new KeyValueArgument(key, value); } /** * Adds a "key": "value" to the JSON structure for each entry in the map * and {@code map.toString()} to the formatted message. + * If the map contains any reserved keys, those entries will be filtered out. * * @param map {@link Map} holding the key/value pairs - * @return a {@link MapArgument} populated with the data + * @return a {@link MapArgument} populated with the data, with reserved keys filtered out */ public static StructuredArgument entries(Map map) { - return new MapArgument(map); + if (map == null) { + return null; + } + + // Create a new map without reserved keys + Map filteredMap = new java.util.HashMap<>(); + for (Map.Entry entry : map.entrySet()) { + if (entry.getKey() != null) { + String keyStr = String.valueOf(entry.getKey()); + if (!isReservedKey(keyStr)) { + filteredMap.put(entry.getKey(), entry.getValue()); + } + } + } + + return new MapArgument(filteredMap); } /** * Adds a field to the JSON structure with key as the key and where value * is a JSON array of objects AND a string version of the array to the formatted message: * {@code "key": [value, value]} + * Returns null if the key is a reserved key. * * @param key the field name * @param values elements of the array associated with the key - * @return an {@link ArrayArgument} populated with the data + * @return an {@link ArrayArgument} populated with the data, or null if key is reserved */ public static StructuredArgument array(String key, Object... values) { + if (isReservedKey(key)) { + return null; + } return new ArrayArgument(key, values); } /** * Adds the {@code rawJson} to the JSON structure and * the {@code rawJson} to the formatted message. + * Returns null if the key is a reserved key. * * @param key the field name * @param rawJson the raw JSON String - * @return a {@link JsonArgument} populated with the data + * @return a {@link JsonArgument} populated with the data, or null if key is reserved */ public static StructuredArgument json(String key, String rawJson) { + if (isReservedKey(key)) { + return null; + } return new JsonArgument(key, rawJson); } diff --git a/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java b/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java index 505fde992..5fda73d3b 100644 --- a/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java +++ b/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java @@ -15,12 +15,15 @@ package software.amazon.lambda.powertools.logging.argument; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertNull; import java.io.IOException; import java.util.HashMap; import java.util.Map; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; + import software.amazon.lambda.powertools.logging.internal.JsonSerializer; import software.amazon.lambda.powertools.logging.model.Basket; import software.amazon.lambda.powertools.logging.model.Product; @@ -47,8 +50,10 @@ void keyValueArgument() throws IOException { argument.writeTo(serializer); // THEN - assertThat(sb.toString()).hasToString("\"basket\":{\"products\":[{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45},{\"id\":98,\"name\":\"Playstation 5\",\"price\":499.99}]}"); - assertThat(argument.toString()).hasToString("basket=Basket{products=[Product{id=42, name='Nintendo DS', price=299.45}, Product{id=98, name='Playstation 5', price=499.99}]}"); + assertThat(sb.toString()).hasToString( + "\"basket\":{\"products\":[{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45},{\"id\":98,\"name\":\"Playstation 5\",\"price\":499.99}]}"); + assertThat(argument.toString()).hasToString( + "basket=Basket{products=[Product{id=42, name='Nintendo DS', price=299.45}, Product{id=98, name='Playstation 5', price=499.99}]}"); assertThat(argument.keys()).containsExactlyInAnyOrder("basket"); } @@ -79,6 +84,7 @@ void emptyMapArgument() throws IOException { Map catalog = new HashMap<>(); // WHEN + assertNull(StructuredArguments.entries(null)); StructuredArgument argument = StructuredArguments.entries(catalog); argument.writeTo(serializer); @@ -91,7 +97,7 @@ void emptyMapArgument() throws IOException { @Test void arrayArgument() throws IOException { // GIVEN - Product[] products = new Product[]{ + Product[] products = new Product[] { new Product(42, "Nintendo DS", 299.45), new Product(98, "Playstation 5", 499.99) }; @@ -101,8 +107,10 @@ void arrayArgument() throws IOException { argument.writeTo(serializer); // THEN - assertThat(sb.toString()).contains("\"products\":[{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45},{\"id\":98,\"name\":\"Playstation 5\",\"price\":499.99}]"); - assertThat(argument.toString()).contains("products=[Product{id=42, name='Nintendo DS', price=299.45}, Product{id=98, name='Playstation 5', price=499.99}]"); + assertThat(sb.toString()).contains( + "\"products\":[{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45},{\"id\":98,\"name\":\"Playstation 5\",\"price\":499.99}]"); + assertThat(argument.toString()).contains( + "products=[Product{id=42, name='Nintendo DS', price=299.45}, Product{id=98, name='Playstation 5', price=499.99}]"); assertThat(argument.keys()).containsExactlyInAnyOrder("products"); } @@ -121,4 +129,35 @@ void jsonArgument() throws IOException { assertThat(argument.keys()).containsExactlyInAnyOrder("product"); } + @Test + void reservedKeywordArgumentIgnored() throws IOException { + // GIVEN + Basket basket = new Basket(); + basket.add(new Product(42, "Nintendo DS", 299.45)); + basket.add(new Product(98, "Playstation 5", 499.99)); + Product[] products = new Product[] { + new Product(42, "Nintendo DS", 299.45), + new Product(98, "Playstation 5", 499.99) + }; + String rawJson = "{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45}"; + Map catalog = new HashMap<>(); + catalog.put("nds", new Product(42, "Nintendo DS", 299.45)); + catalog.put("message", new Product(98, "Playstation 5", 499.99)); + + // THEN + assertNull(StructuredArguments.entry("message", basket)); + assertNull(StructuredArguments.array("message", products)); + assertNull(StructuredArguments.json("message", rawJson)); + + StructuredArgument mapArg = StructuredArguments.entries(catalog); + mapArg.writeTo(serializer); + assertThat(sb.toString()) + .contains("\"nds\":{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45}"); + assertThat(sb.toString()).doesNotContain("message"); + assertThat(mapArg.toString()) + .contains("nds=Product{id=42, name='Nintendo DS', price=299.45}"); + assertThat(mapArg.toString()).doesNotContain("message"); + assertThat(mapArg.keys()).containsExactlyInAnyOrder("nds"); + } + } diff --git a/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspectTest.java b/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspectTest.java index 4825d89f3..aba4664fe 100644 --- a/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspectTest.java +++ b/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspectTest.java @@ -486,15 +486,14 @@ void shouldLogEventForHandlerWhenEnvVariableSetToTrue() { requestHandler.handleRequest(message, context); // THEN - TestLogger logger = (TestLogger) ((PowertoolsLogEventEnvVar)requestHandler).getLogger(); + TestLogger logger = (TestLogger) ((PowertoolsLogEventEnvVar) requestHandler).getLogger(); try { assertThat(logger.getArguments()).hasSize(1); StructuredArgument argument = (StructuredArgument) logger.getArguments()[0]; - // assertThat(argument.getKey()).isEqualTo("event"); - // assertThat(argument.getValue()).isEqualTo(message); + assertThat(argument.toString()).hasToString("event={messageId: 1234abcd,awsRegion: eu-west-1,body: body,}"); } finally { LoggingConstants.POWERTOOLS_LOG_EVENT = false; - if (logger != null){ + if (logger != null) { logger.clearArguments(); } } From ab9c44a1555b3c82c95c5b5e93f01baf1c1907d7 Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Thu, 24 Apr 2025 12:39:37 +0200 Subject: [PATCH 09/10] Remove .keys() method again from StructuredArgument interface. It is no longer needed now. --- .../powertools/logging/argument/ArrayArgument.java | 5 ----- .../powertools/logging/argument/JsonArgument.java | 5 ----- .../logging/argument/KeyValueArgument.java | 6 ------ .../powertools/logging/argument/MapArgument.java | 12 ------------ .../logging/argument/StructuredArgument.java | 8 -------- .../logging/argument/StructuredArgumentsTest.java | 6 ------ 6 files changed, 42 deletions(-) diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java index 4c9a45b6f..cbedbdc0f 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/ArrayArgument.java @@ -15,7 +15,6 @@ package software.amazon.lambda.powertools.logging.argument; import java.util.Objects; -import java.util.Set; import software.amazon.lambda.powertools.logging.internal.JsonSerializer; @@ -43,8 +42,4 @@ public String toString() { return key + "=" + StructuredArguments.toString(values); } - @Override - public Iterable keys() { - return Set.of(key); - } } diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java index 04667ca81..debea18c5 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/JsonArgument.java @@ -15,7 +15,6 @@ package software.amazon.lambda.powertools.logging.argument; import java.util.Objects; -import java.util.Set; import software.amazon.lambda.powertools.logging.internal.JsonSerializer; @@ -42,8 +41,4 @@ public String toString() { return key + "=" + rawJson; } - @Override - public Iterable keys() { - return Set.of(key); - } } diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java index 34c5d65c8..54648d99e 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/KeyValueArgument.java @@ -15,7 +15,6 @@ package software.amazon.lambda.powertools.logging.argument; import java.util.Objects; -import java.util.Set; import software.amazon.lambda.powertools.logging.internal.JsonSerializer; @@ -41,11 +40,6 @@ public String toString() { return key + "=" + StructuredArguments.toString(value); } - @Override - public Iterable keys() { - return Set.of(getKey()); - } - public String getKey() { return key; } diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java index 35b851c40..1155fa93b 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/MapArgument.java @@ -17,10 +17,8 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; -import java.util.stream.Collectors; import software.amazon.lambda.powertools.logging.internal.JsonSerializer; -import java.util.Set; /** * See {@link StructuredArguments#entries(Map)} @@ -55,14 +53,4 @@ public String toString() { return String.valueOf(map); } - @Override - public Iterable keys() { - if (map == null) { - return Set.of(); - } - - // Object::toString is used to convert the key to a string, as per the behavior of Map.toString() in case the - // key is not already a String. - return map.keySet().stream().map(Object::toString).collect(Collectors.toSet()); - } } diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java index 4383a413c..00cc651eb 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/argument/StructuredArgument.java @@ -46,12 +46,4 @@ public interface StructuredArgument { */ String toString(); - /** - * Returns the root-level log keys associated with this argument. If the argument has only one key, this method - * will return a single-element iterable. - * - * @return the keys associated with this argument - */ - Iterable keys(); - } diff --git a/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java b/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java index 5fda73d3b..b478ac0f0 100644 --- a/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java +++ b/powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/argument/StructuredArgumentsTest.java @@ -54,7 +54,6 @@ void keyValueArgument() throws IOException { "\"basket\":{\"products\":[{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45},{\"id\":98,\"name\":\"Playstation 5\",\"price\":499.99}]}"); assertThat(argument.toString()).hasToString( "basket=Basket{products=[Product{id=42, name='Nintendo DS', price=299.45}, Product{id=98, name='Playstation 5', price=499.99}]}"); - assertThat(argument.keys()).containsExactlyInAnyOrder("basket"); } @Test @@ -75,7 +74,6 @@ void mapArgument() throws IOException { assertThat(argument.toString()) .contains("nds=Product{id=42, name='Nintendo DS', price=299.45}") .contains("ps5=Product{id=98, name='Playstation 5', price=499.99}"); - assertThat(argument.keys()).containsExactlyInAnyOrder("nds", "ps5"); } @Test @@ -91,7 +89,6 @@ void emptyMapArgument() throws IOException { // THEN assertThat(sb.toString()).isEmpty(); assertThat(argument.toString()).hasToString("{}"); - assertThat(argument.keys()).isEmpty(); } @Test @@ -111,7 +108,6 @@ void arrayArgument() throws IOException { "\"products\":[{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45},{\"id\":98,\"name\":\"Playstation 5\",\"price\":499.99}]"); assertThat(argument.toString()).contains( "products=[Product{id=42, name='Nintendo DS', price=299.45}, Product{id=98, name='Playstation 5', price=499.99}]"); - assertThat(argument.keys()).containsExactlyInAnyOrder("products"); } @Test @@ -126,7 +122,6 @@ void jsonArgument() throws IOException { // THEN assertThat(sb.toString()).contains("\"product\":{\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45}"); assertThat(argument.toString()).contains("product={\"id\":42,\"name\":\"Nintendo DS\",\"price\":299.45}"); - assertThat(argument.keys()).containsExactlyInAnyOrder("product"); } @Test @@ -157,7 +152,6 @@ void reservedKeywordArgumentIgnored() throws IOException { assertThat(mapArg.toString()) .contains("nds=Product{id=42, name='Nintendo DS', price=299.45}"); assertThat(mapArg.toString()).doesNotContain("message"); - assertThat(mapArg.keys()).containsExactlyInAnyOrder("nds"); } } From 66cad55ffcb0b1ea0c9a317576c0a51b9e947aab Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Thu, 24 Apr 2025 12:52:52 +0200 Subject: [PATCH 10/10] Update documentation warnings regarding logging of reserved keys. --- docs/core/logging.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/core/logging.md b/docs/core/logging.md index 9fa6922b0..1160f62ff 100644 --- a/docs/core/logging.md +++ b/docs/core/logging.md @@ -354,6 +354,9 @@ Your logs will always include the following keys in your structured logging: | **xray_trace_id** | String | "1-5759e988-bd862e3fe1be46a994272793" | X-Ray Trace ID when [Tracing is enabled](https://docs.aws.amazon.com/lambda/latest/dg/services-xray.html){target="_blank"} | | **error** | Map | `{ "name": "InvalidAmountException", "message": "Amount must be superior to 0", "stack": "at..." }` | Eventual exception (e.g. when doing `logger.error("Error", new InvalidAmountException("Amount must be superior to 0"));`) | +???+ note + If you emit a log message with a key that matches one of the [standard structured keys](#standard-structured-keys) or one of the [additional structured keys](#additional-structured-keys), the Logger will log a warning message and ignore the key. + ## Additional structured keys ### Logging Lambda context information @@ -640,8 +643,8 @@ To append additional keys in your logs, you can use the `StructuredArguments` cl } ``` -???+ warning "Do not use arguments with reserved keys" - If the key name of your structured argument matches any of the [standard structured keys](#standard-structured-keys) or any of the [additional structured keys](#additional-structured-keys) the key will be ignored. This is to protect you from accidentally overwriting reserved keys such as the log level or Lambda context information. +???+ warning "Do not use reserved keys in `StructuredArguments`" + If the key name of your structured argument matches any of the [standard structured keys](#standard-structured-keys) or any of the [additional structured keys](#additional-structured-keys), the Logger will log a warning message and ignore the key. This is to protect you from accidentally overwriting reserved keys such as the log level or Lambda context information. **Using MDC** @@ -654,7 +657,7 @@ Mapped Diagnostic Context (MDC) is essentially a Key-Value store. It is supporte Always set additional keys as part of your handler method to ensure they have the latest value, or explicitly clear them with [`clearState=true`](#clearing-state). ???+ warning "Do not add reserved keys to MDC" - Avoid adding any of the keys listed in [standard structured keys](#standard-structured-keys) and [additional structured keys](#additional-structured-keys) to your MDC. This may cause unindented behavior and will overwrite the context set by the Logger. Unlike with StructuredArguments, the Logger will **not** ignore reserved keys set via MDC. + Avoid adding any of the keys listed in [standard structured keys](#standard-structured-keys) and [additional structured keys](#additional-structured-keys) to your MDC. This may cause unindented behavior and will overwrite the context set by the Logger. Unlike with `StructuredArguments`, the Logger will **not** ignore reserved keys set via MDC. ### Removing additional keys