From fb9dbc9ece96870f63dacc8a88adb9c2916a7ff5 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Wed, 3 Aug 2022 18:06:32 +0200 Subject: [PATCH 1/3] allow sub-classes to alter mdc lookup --- .../java/co/elastic/logging/jul/EcsFormatter.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java index c55a623..0b191cf 100644 --- a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java +++ b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java @@ -29,6 +29,7 @@ import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.logging.Formatter; import java.util.logging.LogManager; import java.util.logging.LogRecord; @@ -69,7 +70,7 @@ public String format(final LogRecord record) { EcsJsonSerializer.serializeFormattedMessage(builder, super.formatMessage(record)); EcsJsonSerializer.serializeEcsVersion(builder); EcsJsonSerializer.serializeAdditionalFields(builder, additionalFields); - EcsJsonSerializer.serializeMDC(builder, JulMdc.getEntries()); + EcsJsonSerializer.serializeMDC(builder, getMdcEntries()); EcsJsonSerializer.serializeServiceName(builder, serviceName); EcsJsonSerializer.serializeServiceVersion(builder, serviceVersion); EcsJsonSerializer.serializeServiceEnvironment(builder, serviceEnvironment); @@ -92,6 +93,15 @@ public String format(final LogRecord record) { return builder.toString(); } + /** + * Allow sub-classes to override MDC resolution. + * + * @return MDC entries + */ + protected Map getMdcEntries() { + return JulMdc.getEntries(); + } + public void setIncludeOrigin(final boolean includeOrigin) { this.includeOrigin = includeOrigin; } From 3b348f10afeae921106074f7fe5f10101e4d7ea1 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 8 Aug 2022 14:27:41 +0200 Subject: [PATCH 2/3] simplify and remove MDC impl --- .../co/elastic/logging/jul/EcsFormatter.java | 6 +- .../java/co/elastic/logging/jul/JulMdc.java | 71 ------------------- .../elastic/logging/jul/EcsFormatterTest.java | 4 +- .../elastic/logging/jul/JulLoggingTest.java | 6 -- .../co/elastic/logging/jul/JulMdcTest.java | 69 ------------------ 5 files changed, 5 insertions(+), 151 deletions(-) delete mode 100644 jul-ecs-formatter/src/main/java/co/elastic/logging/jul/JulMdc.java delete mode 100644 jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulMdcTest.java diff --git a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java index 0b191cf..375421a 100644 --- a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java +++ b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java @@ -94,12 +94,12 @@ public String format(final LogRecord record) { } /** - * Allow sub-classes to override MDC resolution. + * Used by APM agent to provide MDC for JUL through instrumentation * * @return MDC entries */ - protected Map getMdcEntries() { - return JulMdc.getEntries(); + private Map getMdcEntries() { + return Collections.emptyMap(); } public void setIncludeOrigin(final boolean includeOrigin) { diff --git a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/JulMdc.java b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/JulMdc.java deleted file mode 100644 index 4d2d3cb..0000000 --- a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/JulMdc.java +++ /dev/null @@ -1,71 +0,0 @@ -/*- - * #%L - * Java ECS logging - * %% - * Copyright (C) 2019 - 2022 Elastic and contributors - * %% - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * #L% - */ -package co.elastic.logging.jul; - -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; - -/** - * MDC implementation for JUL as it does not have one. - * This MDC will be used by the APM agent for logs correlation. - */ -public class JulMdc { - - /** - * This MDC is currently used with 3 key/values: trace,transaction and error IDs. - */ - private static final int INITIAL_CAPACITY = 4; - - private static final ThreadLocal> tlm = new ThreadLocal>(); - - public static void put(String key, String value) { - Map map = tlm.get(); - if (map == null) { - map = new HashMap(INITIAL_CAPACITY); - tlm.set(map); - } - map.put(key, value); - } - - public static void remove(String key) { - Map entries = tlm.get(); - if (entries != null) { - entries.remove(key); - } - } - - /** - * Get the MDC entries, the returned map should not escape the current thread as the map implementation is not - * thread-safe and thus concurrent modification is not supported. - * - * @return map of MDC entries - */ - static Map getEntries() { - Map entries = tlm.get(); - return entries == null ? Collections.emptyMap() : entries; - } - -} diff --git a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/EcsFormatterTest.java b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/EcsFormatterTest.java index 464a1f6..6de1333 100644 --- a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/EcsFormatterTest.java +++ b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/EcsFormatterTest.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY diff --git a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulLoggingTest.java b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulLoggingTest.java index 52a8d83..7dcd4a9 100644 --- a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulLoggingTest.java +++ b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulLoggingTest.java @@ -82,12 +82,6 @@ public void debug(String message) { logger.log(Level.FINE, message); } - @Override - public boolean putMdc(String key, String value) { - JulMdc.put(key, value); - return true; - } - @Override public ParameterizedLogSupport getParameterizedLogSettings() { return ParameterizedLogSupport.NUMBER_AND_BRACKETS; diff --git a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulMdcTest.java b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulMdcTest.java deleted file mode 100644 index 31443c1..0000000 --- a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulMdcTest.java +++ /dev/null @@ -1,69 +0,0 @@ -/*- - * #%L - * Java ECS logging - * %% - * Copyright (C) 2019 - 2022 Elastic and contributors - * %% - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * #L% - */ -package co.elastic.logging.jul; - -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import java.util.Map; - -import static org.assertj.core.api.Assertions.assertThat; - -public class JulMdcTest { - - @BeforeEach - void before(){ - // prevently empty if any other test have left something - JulMdc.getEntries().clear(); - } - - @AfterEach - void after() { - JulMdc.getEntries().clear(); - assertThat(JulMdc.getEntries()).isEmpty(); - } - - @Test - void emptyMdc() { - Map entries = JulMdc.getEntries(); - assertThat(entries).isEmpty(); - - assertThat(JulMdc.getEntries()).isSameAs(entries); - - // should be a no-op - JulMdc.remove("missing"); - } - - @Test - void putRemoveSingleEntry() { - JulMdc.put("hello", "world"); - assertThat(JulMdc.getEntries()).containsEntry("hello", "world"); - - JulMdc.remove("hello"); - assertThat(JulMdc.getEntries()).isEmpty(); - } - -} From 22ea928fbd49061197ebc9a998ee57d549e8e0cc Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 8 Aug 2022 16:11:26 +0200 Subject: [PATCH 3/3] keep method protected for testing --- .../co/elastic/logging/jul/EcsFormatter.java | 5 +- .../elastic/logging/jul/EcsFormatterTest.java | 49 ++++++++++++++++--- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java index 375421a..0851a1c 100644 --- a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java +++ b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/EcsFormatter.java @@ -94,11 +94,12 @@ public String format(final LogRecord record) { } /** - * Used by APM agent to provide MDC for JUL through instrumentation + * Used by APM agent to provide MDC for JUL through instrumentation and to allow testing without an actual MDC + * implementation. * * @return MDC entries */ - private Map getMdcEntries() { + protected Map getMdcEntries() { return Collections.emptyMap(); } diff --git a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/EcsFormatterTest.java b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/EcsFormatterTest.java index 6de1333..573114d 100644 --- a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/EcsFormatterTest.java +++ b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/EcsFormatterTest.java @@ -24,12 +24,15 @@ */ package co.elastic.logging.jul; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.time.Instant; +import java.util.HashMap; +import java.util.Map; import java.util.logging.Level; import java.util.logging.LogRecord; @@ -40,7 +43,7 @@ public class EcsFormatterTest { private final EcsFormatter formatter = new EcsFormatter(); private final LogRecord record = new LogRecord(Level.INFO, "Example Message"); - private final ObjectMapper objectMapper = new ObjectMapper(); + private static final ObjectMapper objectMapper = new ObjectMapper(); @BeforeEach void setUp() { @@ -57,13 +60,13 @@ public void testFormatWithIncludeOriginFlag() throws Exception { final String result = formatter.format(record); - assertThat(objectMapper.readTree(result).at("/log/origin/file/name").textValue()).isEqualTo("ExampleClass.java"); - assertThat(objectMapper.readTree(result).at("/log/origin/function").textValue()).isEqualTo("exampleMethod"); + assertThat(parseJson(result).at("/log/origin/file/name").textValue()).isEqualTo("ExampleClass.java"); + assertThat(parseJson(result).at("/log/origin/function").textValue()).isEqualTo("exampleMethod"); } @Test public void testFormatWithoutIncludeOriginFlag() throws Exception { - final JsonNode result = objectMapper.readTree(formatter.format(record)); + final JsonNode result = parseJson(formatter.format(record)); assertThat(result.get("log.origin")).isNull(); } @@ -71,7 +74,7 @@ public void testFormatWithoutIncludeOriginFlag() throws Exception { public void testFormatWithoutLoggerName() throws Exception { record.setLoggerName(null); - final JsonNode result = objectMapper.readTree(formatter.format(record)); + final JsonNode result = parseJson(formatter.format(record)); assertThat(result.get("log.logger")).isNull(); } @@ -80,7 +83,7 @@ public void testFormatWithoutLoggerName() throws Exception { public void testFormatWithEmptyLoggerName() throws Exception { record.setLoggerName(""); - final JsonNode result = objectMapper.readTree(formatter.format(record)); + final JsonNode result = parseJson(formatter.format(record)); assertThat(result.get("log.logger").textValue()).isEmpty(); } @@ -90,7 +93,7 @@ public void testFormatWithInnerClassName() throws Exception { formatter.setIncludeOrigin(true); record.setSourceClassName("test.ExampleClass$InnerClass"); - JsonNode result = objectMapper.readTree(formatter.format(record)); + JsonNode result = parseJson(formatter.format(record)); assertThat(result.at("/log/origin/file/name").textValue()).isEqualTo("ExampleClass.java"); assertThat(result.at("/log/origin/function").textValue()).isEqualTo("exampleMethod"); } @@ -100,9 +103,39 @@ public void testFormatWithInvalidClassName() throws Exception { formatter.setIncludeOrigin(true); record.setSourceClassName("$test.ExampleClass"); - JsonNode result = objectMapper.readTree(formatter.format(record)); + JsonNode result = parseJson(formatter.format(record)); assertThat(result.at("/log/origin/file/name").textValue()).isEqualTo(""); assertThat(result.at("/log/origin/function").textValue()).isEqualTo("exampleMethod"); } + @Test + void testMdcSerialization_singleEntry() { + Map mdc = new HashMap<>(); + TestMdcEcsFormatter mdcFormatter = new TestMdcEcsFormatter(mdc); + mdc.put("mdc.key", "value"); + JsonNode result = parseJson(mdcFormatter.format(record)); + assertThat(result.get("mdc.key").textValue()).isEqualTo("value"); + } + + private static JsonNode parseJson(String formatter) { + try { + return objectMapper.readTree(formatter); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + private static class TestMdcEcsFormatter extends EcsFormatter { + private final Map mdc; + + public TestMdcEcsFormatter(Map mdc) { + this.mdc = mdc; + } + + @Override + protected Map getMdcEntries() { + return mdc; + } + } + }