From c352cd600b7b1c46cdb7ddaaedc6b9f8578c6a03 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 12 Jul 2022 14:20:06 +0200 Subject: [PATCH 01/11] first draft of fallback MDC for Jul --- .../co/elastic/logging/EcsJsonSerializer.java | 2 +- .../co/elastic/logging/jul/EcsFormatter.java | 13 ++-- .../co/elastic/logging/jul/FallbackMdc.java | 70 +++++++++++++++++++ .../co/elastic/logging/jul/MdcSupplier.java | 21 +++--- .../logging/jul/JulFallbackMdcTest.java | 44 ++++++++++++ .../elastic/logging/log4j2/MdcSerializer.java | 2 + 6 files changed, 138 insertions(+), 14 deletions(-) create mode 100644 jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java create mode 100644 jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulFallbackMdcTest.java diff --git a/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java b/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java index dfb7411d..54625cc5 100644 --- a/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java +++ b/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java @@ -191,7 +191,7 @@ public static void serializeOrigin(StringBuilder builder, String fileName, Strin builder.append("},"); } - public static void serializeMDC(StringBuilder builder, Map properties) { + public static void serializeMDC(StringBuilder builder, Map properties) { // usages of this method to write data from MDC if (properties != null && !properties.isEmpty()) { for (Map.Entry entry : properties.entrySet()) { builder.append('\"'); 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 191642b3..d307319a 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 @@ -24,19 +24,19 @@ */ package co.elastic.logging.jul; +import co.elastic.logging.AdditionalField; +import co.elastic.logging.EcsJsonSerializer; + import java.util.Collections; import java.util.List; import java.util.logging.Formatter; import java.util.logging.LogManager; import java.util.logging.LogRecord; -import co.elastic.logging.AdditionalField; -import co.elastic.logging.EcsJsonSerializer; - public class EcsFormatter extends Formatter { private static final String UNKNOWN_FILE = ""; - private static final MdcSupplier mdcSupplier = MdcSupplier.Resolver.INSTANCE.resolve(); + private MdcSupplier mdcSupplier; private boolean stackTraceAsArray; private String serviceName; @@ -60,6 +60,7 @@ public EcsFormatter() { eventDataset = getProperty("co.elastic.logging.jul.EcsFormatter.eventDataset", null); eventDataset = EcsJsonSerializer.computeEventDataset(eventDataset, serviceName); setAdditionalFields(getProperty("co.elastic.logging.jul.EcsFormatter.additionalFields", null)); + mdcSupplier = MdcSupplier.Resolver.INSTANCE.resolve(); } @Override @@ -129,6 +130,10 @@ public void setAdditionalFields(List additionalFields) { this.additionalFields = additionalFields; } + void setMdcSupplier(MdcSupplier supplier) { + mdcSupplier = supplier; + } + private String getProperty(final String name, final String defaultValue) { String value = LogManager.getLogManager().getProperty(name); if (value == null) { diff --git a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java new file mode 100644 index 00000000..e124a474 --- /dev/null +++ b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java @@ -0,0 +1,70 @@ +/*- + * #%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; + +/** + * Fallback MDC implementation to be used when JUL is deployed without one + */ +public class FallbackMdc { + + public static final FallbackMdc INSTANCE = new FallbackMdc(); + + private final InheritableThreadLocal> tlm = new InheritableThreadLocal>() { + @Override + protected Map childValue(Map parentValue) { + if (parentValue == null || parentValue.isEmpty()) { + return Collections.emptyMap(); + } else { + return new HashMap(parentValue); + } + } + }; + + FallbackMdc() { + } + + public void put(String key, String value) { + getOrCreateMap().put(key, value); + } + + public Map getEntries() { + return tlm.get(); + } + + private Map getOrCreateMap() { + Map map = tlm.get(); + if (map == null || map.isEmpty()) { + map = new HashMap(); + tlm.set(map); + } + return map; + } + + +} diff --git a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/MdcSupplier.java b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/MdcSupplier.java index a6ee91c3..20a0adaa 100644 --- a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/MdcSupplier.java +++ b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/MdcSupplier.java @@ -43,16 +43,16 @@ MdcSupplier resolve() { // implementation. When no MDC bindings are available calls to MDC.put will be ignored by slf4j. // That is why we want to ensure that the StaticMDCBinder exists Class.forName("org.slf4j.impl.StaticMDCBinder"); - return (MdcSupplier) Class.forName("co.elastic.logging.jul.MdcSupplier$Available").getEnumConstants()[0]; - } catch (Exception e) { - return Unavailable.INSTANCE; - } catch (LinkageError e ) { - return Unavailable.INSTANCE; + return (MdcSupplier) Class.forName("co.elastic.logging.jul.MdcSupplier$Slf4j").getEnumConstants()[0]; + } catch (Exception ignore) { + } catch (LinkageError ignore) { } + return Fallback.INSTANCE; } } - enum Available implements MdcSupplier { + enum Slf4j implements MdcSupplier { + @SuppressWarnings("unused") // called through Resolver.resolve() INSTANCE; @Override @@ -65,15 +65,18 @@ public Map getMDC() { } } - enum Unavailable implements MdcSupplier { + /** + * Fallback to internal MDC + */ + enum Fallback implements MdcSupplier { INSTANCE; - Unavailable() { + Fallback() { } @Override public Map getMDC() { - return Collections.emptyMap(); + return FallbackMdc.INSTANCE.getEntries(); } } } diff --git a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulFallbackMdcTest.java b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulFallbackMdcTest.java new file mode 100644 index 00000000..cf4ace1e --- /dev/null +++ b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulFallbackMdcTest.java @@ -0,0 +1,44 @@ +/*- + * #%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; + +/** + * Tests the internal MDC delegation + */ +public class JulFallbackMdcTest extends JulLoggingTest { + + @Override + protected EcsFormatter createEcsFormatter() { + EcsFormatter formatter = super.createEcsFormatter(); + formatter.setMdcSupplier(MdcSupplier.Fallback.INSTANCE); + return formatter; + } + + @Override + public boolean putMdc(String key, String value) { + FallbackMdc.INSTANCE.put(key, value); + return true; + } +} diff --git a/log4j2-ecs-layout/src/main/java/co/elastic/logging/log4j2/MdcSerializer.java b/log4j2-ecs-layout/src/main/java/co/elastic/logging/log4j2/MdcSerializer.java index fdd1fba4..682d2b37 100644 --- a/log4j2-ecs-layout/src/main/java/co/elastic/logging/log4j2/MdcSerializer.java +++ b/log4j2-ecs-layout/src/main/java/co/elastic/logging/log4j2/MdcSerializer.java @@ -52,6 +52,8 @@ public static MdcSerializer resolve() { * Never reference directly in prod code so avoid linkage errors when TriConsumer or getContextData are not available */ enum UsingContextData implements MdcSerializer { + + @SuppressWarnings("unused") INSTANCE; private static final TriConsumer WRITE_MDC = new TriConsumer() { From dec653f7ee48d5581a00c07d3cb6cae668f2e1f9 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 12 Jul 2022 15:02:34 +0200 Subject: [PATCH 02/11] add tests + fix NPE --- .../co/elastic/logging/jul/FallbackMdc.java | 3 +- .../elastic/logging/jul/FallbackMdcTest.java | 59 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 jul-ecs-formatter/src/test/java/co/elastic/logging/jul/FallbackMdcTest.java diff --git a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java index e124a474..2a5a895f 100644 --- a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java +++ b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java @@ -54,7 +54,8 @@ public void put(String key, String value) { } public Map getEntries() { - return tlm.get(); + Map entries = tlm.get(); + return entries == null ? Collections.emptyMap(): entries; } private Map getOrCreateMap() { diff --git a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/FallbackMdcTest.java b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/FallbackMdcTest.java new file mode 100644 index 00000000..18dd388a --- /dev/null +++ b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/FallbackMdcTest.java @@ -0,0 +1,59 @@ +/*- + * #%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.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +public class FallbackMdcTest { + + private FallbackMdc mdc; + + @BeforeEach + void before() { + mdc = new FallbackMdc(); + } + + @Test + void emptyMdc() { + Map entries = mdc.getEntries(); + assertThat(entries).isEmpty(); + + assertThat(mdc.getEntries()).isSameAs(entries); + } + + @Test + void putSingleEntry() { + mdc.put("hello", "world"); + assertThat(mdc.getEntries()).containsEntry("hello", "world"); + + mdc.getEntries().clear(); + assertThat(mdc.getEntries()).isEmpty(); + } +} From d6b4087dae3031e7a4016469d044f3c151f9855b Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 12 Jul 2022 15:08:38 +0200 Subject: [PATCH 03/11] cleanup --- .../src/main/java/co/elastic/logging/EcsJsonSerializer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java b/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java index 54625cc5..dfb7411d 100644 --- a/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java +++ b/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java @@ -191,7 +191,7 @@ public static void serializeOrigin(StringBuilder builder, String fileName, Strin builder.append("},"); } - public static void serializeMDC(StringBuilder builder, Map properties) { // usages of this method to write data from MDC + public static void serializeMDC(StringBuilder builder, Map properties) { if (properties != null && !properties.isEmpty()) { for (Map.Entry entry : properties.entrySet()) { builder.append('\"'); From 820e19c84be837be4b428a704ba3edf379a7a562 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 25 Jul 2022 12:05:30 +0200 Subject: [PATCH 04/11] simplify and remove slf4j --- .../logging/AbstractEcsLoggingTest.java | 4 +- .../jboss/logmanager/JBossLogManagerTest.java | 2 +- jul-ecs-formatter/pom.xml | 12 --- .../co/elastic/logging/jul/EcsFormatter.java | 22 ++--- .../jul/{FallbackMdc.java => JulMdc.java} | 22 +++-- .../co/elastic/logging/jul/MdcSupplier.java | 82 ------------------- .../logging/jul/JulFallbackMdcTest.java | 44 ---------- .../elastic/logging/jul/JulLoggingTest.java | 15 ++-- .../{FallbackMdcTest.java => JulMdcTest.java} | 20 +++-- .../log4j/Log4jEcsLayoutIntegrationTest.java | 2 +- .../logging/log4j/Log4jEcsLayoutTest.java | 2 +- .../log4j2/AbstractLog4j2EcsLayoutTest.java | 2 +- 12 files changed, 54 insertions(+), 175 deletions(-) rename jul-ecs-formatter/src/main/java/co/elastic/logging/jul/{FallbackMdc.java => JulMdc.java} (77%) delete mode 100644 jul-ecs-formatter/src/main/java/co/elastic/logging/jul/MdcSupplier.java delete mode 100644 jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulFallbackMdcTest.java rename jul-ecs-formatter/src/test/java/co/elastic/logging/jul/{FallbackMdcTest.java => JulMdcTest.java} (82%) diff --git a/ecs-logging-core/src/test/java/co/elastic/logging/AbstractEcsLoggingTest.java b/ecs-logging-core/src/test/java/co/elastic/logging/AbstractEcsLoggingTest.java index bc83c4bf..68a87ee7 100644 --- a/ecs-logging-core/src/test/java/co/elastic/logging/AbstractEcsLoggingTest.java +++ b/ecs-logging-core/src/test/java/co/elastic/logging/AbstractEcsLoggingTest.java @@ -171,7 +171,7 @@ void testThreadContext() throws Exception { @Test void testThreadContextStack() throws Exception { - if (putNdc("foo")) { + if (putMdc("foo")) { debug("test"); assertThat(getAndValidateLastLogLine().get("tags").iterator().next().textValue()).isEqualTo("foo"); } @@ -223,7 +223,7 @@ public boolean putMdc(String key, String value) { return false; } - public boolean putNdc(String message) { + public boolean putMdc(String message) { return false; } diff --git a/jboss-logmanager-ecs-formatter/src/test/java/co/elastic/logging/jboss/logmanager/JBossLogManagerTest.java b/jboss-logmanager-ecs-formatter/src/test/java/co/elastic/logging/jboss/logmanager/JBossLogManagerTest.java index c74b0867..00748c84 100644 --- a/jboss-logmanager-ecs-formatter/src/test/java/co/elastic/logging/jboss/logmanager/JBossLogManagerTest.java +++ b/jboss-logmanager-ecs-formatter/src/test/java/co/elastic/logging/jboss/logmanager/JBossLogManagerTest.java @@ -55,7 +55,7 @@ public boolean putMdc(String key, String value) { } @Override - public boolean putNdc(String message) { + public boolean putMdc(String message) { NDC.push(message); return true; } diff --git a/jul-ecs-formatter/pom.xml b/jul-ecs-formatter/pom.xml index 93a88051..72679faf 100644 --- a/jul-ecs-formatter/pom.xml +++ b/jul-ecs-formatter/pom.xml @@ -27,17 +27,5 @@ test-jar test - - org.slf4j - slf4j-api - 1.7.30 - true - - - org.slf4j - slf4j-jdk14 - 1.7.30 - test - 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 d307319a..d1754361 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 @@ -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 @@ -36,8 +36,8 @@ public class EcsFormatter extends Formatter { private static final String UNKNOWN_FILE = ""; - private MdcSupplier mdcSupplier; - + private final JulMdc mdc; + private boolean stackTraceAsArray; private String serviceName; private String serviceVersion; @@ -52,15 +52,15 @@ public class EcsFormatter extends Formatter { */ public EcsFormatter() { serviceName = getProperty("co.elastic.logging.jul.EcsFormatter.serviceName", null); - serviceVersion= getProperty("co.elastic.logging.jul.EcsFormatter.serviceVersion", null); - serviceEnvironment= getProperty("co.elastic.logging.jul.EcsFormatter.serviceEnvironment", null); + serviceVersion = getProperty("co.elastic.logging.jul.EcsFormatter.serviceVersion", null); + serviceEnvironment = getProperty("co.elastic.logging.jul.EcsFormatter.serviceEnvironment", null); serviceNodeName = getProperty("co.elastic.logging.jul.EcsFormatter.serviceNodeName", null); includeOrigin = Boolean.parseBoolean(getProperty("co.elastic.logging.jul.EcsFormatter.includeOrigin", "false")); stackTraceAsArray = Boolean.parseBoolean(getProperty("co.elastic.logging.jul.EcsFormatter.stackTraceAsArray", "false")); eventDataset = getProperty("co.elastic.logging.jul.EcsFormatter.eventDataset", null); eventDataset = EcsJsonSerializer.computeEventDataset(eventDataset, serviceName); setAdditionalFields(getProperty("co.elastic.logging.jul.EcsFormatter.additionalFields", null)); - mdcSupplier = MdcSupplier.Resolver.INSTANCE.resolve(); + mdc = JulMdc.getInstance(); } @Override @@ -71,7 +71,7 @@ public String format(final LogRecord record) { EcsJsonSerializer.serializeFormattedMessage(builder, super.formatMessage(record)); EcsJsonSerializer.serializeEcsVersion(builder); EcsJsonSerializer.serializeAdditionalFields(builder, additionalFields); - EcsJsonSerializer.serializeMDC(builder, mdcSupplier.getMDC()); + EcsJsonSerializer.serializeMDC(builder, mdc.getEntries()); EcsJsonSerializer.serializeServiceName(builder, serviceName); EcsJsonSerializer.serializeServiceVersion(builder, serviceVersion); EcsJsonSerializer.serializeServiceEnvironment(builder, serviceEnvironment); @@ -117,7 +117,7 @@ public void setServiceNodeName(final String serviceNodeName) { public void setStackTraceAsArray(final boolean stackTraceAsArray) { this.stackTraceAsArray = stackTraceAsArray; } - + public void setEventDataset(String eventDataset) { this.eventDataset = eventDataset; } @@ -130,10 +130,6 @@ public void setAdditionalFields(List additionalFields) { this.additionalFields = additionalFields; } - void setMdcSupplier(MdcSupplier supplier) { - mdcSupplier = supplier; - } - private String getProperty(final String name, final String defaultValue) { String value = LogManager.getLogManager().getProperty(name); if (value == null) { diff --git a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/JulMdc.java similarity index 77% rename from jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java rename to jul-ecs-formatter/src/main/java/co/elastic/logging/jul/JulMdc.java index 2a5a895f..f609e1da 100644 --- a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/FallbackMdc.java +++ b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/JulMdc.java @@ -29,11 +29,16 @@ import java.util.Map; /** - * Fallback MDC implementation to be used when JUL is deployed without one + * MDC implementation for JUL as it does not have one */ -public class FallbackMdc { +public class JulMdc { - public static final FallbackMdc INSTANCE = new FallbackMdc(); + private static final JulMdc INSTANCE = new JulMdc(); + + // this method is used by APM agent in order to know if this MDC fallback is used or not + public static JulMdc getInstance() { + return INSTANCE; + } private final InheritableThreadLocal> tlm = new InheritableThreadLocal>() { @Override @@ -46,16 +51,23 @@ protected Map childValue(Map parentValue) { } }; - FallbackMdc() { + JulMdc() { } public void put(String key, String value) { getOrCreateMap().put(key, value); } + public void remove(String key) { + Map entries = tlm.get(); + if (entries != null) { + entries.remove(key); + } + } + public Map getEntries() { Map entries = tlm.get(); - return entries == null ? Collections.emptyMap(): entries; + return entries == null ? Collections.emptyMap() : entries; } private Map getOrCreateMap() { diff --git a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/MdcSupplier.java b/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/MdcSupplier.java deleted file mode 100644 index 20a0adaa..00000000 --- a/jul-ecs-formatter/src/main/java/co/elastic/logging/jul/MdcSupplier.java +++ /dev/null @@ -1,82 +0,0 @@ -/*- - * #%L - * Java ECS logging - * %% - * Copyright (C) 2019 - 2020 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.slf4j.MDC; - -import java.util.Collections; -import java.util.Map; - -public interface MdcSupplier { - Map getMDC(); - - enum Resolver { - INSTANCE; - - MdcSupplier resolve() { - try { - Class.forName("org.slf4j.MDC"); - - // The SLF4J API dependency does not contain an MDC binder by itself, the MDC binders come from an SLF4j - // implementation. When no MDC bindings are available calls to MDC.put will be ignored by slf4j. - // That is why we want to ensure that the StaticMDCBinder exists - Class.forName("org.slf4j.impl.StaticMDCBinder"); - return (MdcSupplier) Class.forName("co.elastic.logging.jul.MdcSupplier$Slf4j").getEnumConstants()[0]; - } catch (Exception ignore) { - } catch (LinkageError ignore) { - } - return Fallback.INSTANCE; - } - } - - enum Slf4j implements MdcSupplier { - @SuppressWarnings("unused") // called through Resolver.resolve() - INSTANCE; - - @Override - public Map getMDC() { - Map copyOfContextMap = MDC.getCopyOfContextMap(); - if (copyOfContextMap != null ) { - return copyOfContextMap; - } - return Collections.emptyMap(); - } - } - - /** - * Fallback to internal MDC - */ - enum Fallback implements MdcSupplier { - INSTANCE; - - Fallback() { - } - - @Override - public Map getMDC() { - return FallbackMdc.INSTANCE.getEntries(); - } - } -} diff --git a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulFallbackMdcTest.java b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulFallbackMdcTest.java deleted file mode 100644 index cf4ace1e..00000000 --- a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulFallbackMdcTest.java +++ /dev/null @@ -1,44 +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; - -/** - * Tests the internal MDC delegation - */ -public class JulFallbackMdcTest extends JulLoggingTest { - - @Override - protected EcsFormatter createEcsFormatter() { - EcsFormatter formatter = super.createEcsFormatter(); - formatter.setMdcSupplier(MdcSupplier.Fallback.INSTANCE); - return formatter; - } - - @Override - public boolean putMdc(String key, String value) { - FallbackMdc.INSTANCE.put(key, value); - return true; - } -} 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 36590b57..1f94962f 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 @@ -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 @@ -30,7 +30,6 @@ import com.fasterxml.jackson.databind.JsonNode; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.slf4j.MDC; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -71,9 +70,9 @@ public void publish(LogRecord record) { } private EcsFormatter formatter; - + private final Logger logger = Logger.getLogger(""); - + private final ByteArrayOutputStream out = new ByteArrayOutputStream(); private LogRecord record; @@ -85,7 +84,7 @@ public void debug(String message) { @Override public boolean putMdc(String key, String value) { - MDC.put(key, value); + JulMdc.getInstance().put(key, value); return true; } @@ -103,7 +102,7 @@ public void debug(String message, Object... logParams) { public void error(String message, Throwable t) { logger.log(Level.SEVERE, message, t); } - + @Override public JsonNode getLastLogLine() throws IOException { return objectMapper.readTree(out.toString()); @@ -153,7 +152,7 @@ void testAdditionalFieldsAsList() throws Exception { formatter.setAdditionalFields(List.of(new AdditionalField("key1", "value1"), new AdditionalField("key2", "value2"))); super.testAdditionalFields(); } - + private void clearHandlers() { for (Handler handler : logger.getHandlers()) { logger.removeHandler(handler); diff --git a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/FallbackMdcTest.java b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulMdcTest.java similarity index 82% rename from jul-ecs-formatter/src/test/java/co/elastic/logging/jul/FallbackMdcTest.java rename to jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulMdcTest.java index 18dd388a..a25598fa 100644 --- a/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/FallbackMdcTest.java +++ b/jul-ecs-formatter/src/test/java/co/elastic/logging/jul/JulMdcTest.java @@ -24,6 +24,7 @@ */ package co.elastic.logging.jul; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -31,13 +32,19 @@ import static org.assertj.core.api.Assertions.assertThat; -public class FallbackMdcTest { +public class JulMdcTest { - private FallbackMdc mdc; + private JulMdc mdc; @BeforeEach void before() { - mdc = new FallbackMdc(); + mdc = new JulMdc(); + } + + @AfterEach + void after() { + mdc.getEntries().clear(); + assertThat(mdc.getEntries()).isEmpty(); } @Test @@ -46,14 +53,17 @@ void emptyMdc() { assertThat(entries).isEmpty(); assertThat(mdc.getEntries()).isSameAs(entries); + + // should be a no-op + mdc.remove("missing"); } @Test - void putSingleEntry() { + void putRemoveSingleEntry() { mdc.put("hello", "world"); assertThat(mdc.getEntries()).containsEntry("hello", "world"); - mdc.getEntries().clear(); + mdc.remove("hello"); assertThat(mdc.getEntries()).isEmpty(); } } diff --git a/log4j-ecs-layout/src/test/java/co/elastic/logging/log4j/Log4jEcsLayoutIntegrationTest.java b/log4j-ecs-layout/src/test/java/co/elastic/logging/log4j/Log4jEcsLayoutIntegrationTest.java index 72dc5248..1847d34b 100644 --- a/log4j-ecs-layout/src/test/java/co/elastic/logging/log4j/Log4jEcsLayoutIntegrationTest.java +++ b/log4j-ecs-layout/src/test/java/co/elastic/logging/log4j/Log4jEcsLayoutIntegrationTest.java @@ -71,7 +71,7 @@ public boolean putMdc(String key, String value) { } @Override - public boolean putNdc(String message) { + public boolean putMdc(String message) { NDC.push(message); return true; } diff --git a/log4j-ecs-layout/src/test/java/co/elastic/logging/log4j/Log4jEcsLayoutTest.java b/log4j-ecs-layout/src/test/java/co/elastic/logging/log4j/Log4jEcsLayoutTest.java index cecc288c..55bfed7d 100644 --- a/log4j-ecs-layout/src/test/java/co/elastic/logging/log4j/Log4jEcsLayoutTest.java +++ b/log4j-ecs-layout/src/test/java/co/elastic/logging/log4j/Log4jEcsLayoutTest.java @@ -81,7 +81,7 @@ public boolean putMdc(String key, String value) { } @Override - public boolean putNdc(String message) { + public boolean putMdc(String message) { NDC.push(message); return true; } diff --git a/log4j2-ecs-layout/src/test/java/co/elastic/logging/log4j2/AbstractLog4j2EcsLayoutTest.java b/log4j2-ecs-layout/src/test/java/co/elastic/logging/log4j2/AbstractLog4j2EcsLayoutTest.java index 05937492..4036345b 100644 --- a/log4j2-ecs-layout/src/test/java/co/elastic/logging/log4j2/AbstractLog4j2EcsLayoutTest.java +++ b/log4j2-ecs-layout/src/test/java/co/elastic/logging/log4j2/AbstractLog4j2EcsLayoutTest.java @@ -120,7 +120,7 @@ public boolean putMdc(String key, String value) { } @Override - public boolean putNdc(String message) { + public boolean putMdc(String message) { ThreadContext.push(message); return true; } From b9413a936fefd0970de761efb0bf49f7d7292469 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 25 Jul 2022 17:28:06 +0200 Subject: [PATCH 05/11] make MDC static + test thread inheritance --- .../co/elastic/logging/jul/EcsFormatter.java | 4 +- .../java/co/elastic/logging/jul/JulMdc.java | 20 ++---- .../elastic/logging/jul/JulLoggingTest.java | 2 +- .../co/elastic/logging/jul/JulMdcTest.java | 63 ++++++++++++++----- 4 files changed, 53 insertions(+), 36 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 d1754361..c55a6235 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 @@ -36,7 +36,6 @@ public class EcsFormatter extends Formatter { private static final String UNKNOWN_FILE = ""; - private final JulMdc mdc; private boolean stackTraceAsArray; private String serviceName; @@ -60,7 +59,6 @@ public EcsFormatter() { eventDataset = getProperty("co.elastic.logging.jul.EcsFormatter.eventDataset", null); eventDataset = EcsJsonSerializer.computeEventDataset(eventDataset, serviceName); setAdditionalFields(getProperty("co.elastic.logging.jul.EcsFormatter.additionalFields", null)); - mdc = JulMdc.getInstance(); } @Override @@ -71,7 +69,7 @@ public String format(final LogRecord record) { EcsJsonSerializer.serializeFormattedMessage(builder, super.formatMessage(record)); EcsJsonSerializer.serializeEcsVersion(builder); EcsJsonSerializer.serializeAdditionalFields(builder, additionalFields); - EcsJsonSerializer.serializeMDC(builder, mdc.getEntries()); + EcsJsonSerializer.serializeMDC(builder, JulMdc.getEntries()); EcsJsonSerializer.serializeServiceName(builder, serviceName); EcsJsonSerializer.serializeServiceVersion(builder, serviceVersion); EcsJsonSerializer.serializeServiceEnvironment(builder, serviceEnvironment); 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 index f609e1da..cd44aa05 100644 --- 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 @@ -33,14 +33,7 @@ */ public class JulMdc { - private static final JulMdc INSTANCE = new JulMdc(); - - // this method is used by APM agent in order to know if this MDC fallback is used or not - public static JulMdc getInstance() { - return INSTANCE; - } - - private final InheritableThreadLocal> tlm = new InheritableThreadLocal>() { + private static final InheritableThreadLocal> tlm = new InheritableThreadLocal>() { @Override protected Map childValue(Map parentValue) { if (parentValue == null || parentValue.isEmpty()) { @@ -51,26 +44,24 @@ protected Map childValue(Map parentValue) { } }; - JulMdc() { - } - public void put(String key, String value) { + public static void put(String key, String value) { getOrCreateMap().put(key, value); } - public void remove(String key) { + public static void remove(String key) { Map entries = tlm.get(); if (entries != null) { entries.remove(key); } } - public Map getEntries() { + public static Map getEntries() { Map entries = tlm.get(); return entries == null ? Collections.emptyMap() : entries; } - private Map getOrCreateMap() { + private static Map getOrCreateMap() { Map map = tlm.get(); if (map == null || map.isEmpty()) { map = new HashMap(); @@ -79,5 +70,4 @@ private Map getOrCreateMap() { return map; } - } 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 1f94962f..52a8d83f 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 @@ -84,7 +84,7 @@ public void debug(String message) { @Override public boolean putMdc(String key, String value) { - JulMdc.getInstance().put(key, value); + JulMdc.put(key, value); return true; } 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 index a25598fa..afbde040 100644 --- 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 @@ -25,45 +25,74 @@ 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 java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import static org.assertj.core.api.Assertions.assertThat; public class JulMdcTest { - private JulMdc mdc; - - @BeforeEach - void before() { - mdc = new JulMdc(); - } - @AfterEach void after() { - mdc.getEntries().clear(); - assertThat(mdc.getEntries()).isEmpty(); + JulMdc.getEntries().clear(); + assertThat(JulMdc.getEntries()).isEmpty(); } @Test void emptyMdc() { - Map entries = mdc.getEntries(); + Map entries = JulMdc.getEntries(); assertThat(entries).isEmpty(); - assertThat(mdc.getEntries()).isSameAs(entries); + assertThat(JulMdc.getEntries()).isSameAs(entries); // should be a no-op - mdc.remove("missing"); + JulMdc.remove("missing"); } @Test void putRemoveSingleEntry() { - mdc.put("hello", "world"); - assertThat(mdc.getEntries()).containsEntry("hello", "world"); + JulMdc.put("hello", "world"); + assertThat(JulMdc.getEntries()).containsEntry("hello", "world"); + + JulMdc.remove("hello"); + assertThat(JulMdc.getEntries()).isEmpty(); + } + + @Test + void threadInheritance_singleValue() throws InterruptedException { + JulMdc.put("main", "main-value"); + assertThat(JulMdc.getEntries()).containsEntry("main", "main-value"); + + CountDownLatch childEnd = new CountDownLatch(1); + new Thread() { + @Override + public void run() { + assertThat(JulMdc.getEntries()).containsEntry("main", "main-value"); + childEnd.countDown(); + } + }.start(); + + childEnd.await(1, TimeUnit.SECONDS); + } + + @Test + void threadInheritance_empty() throws InterruptedException { + JulMdc.put("main", "main-value"); + JulMdc.remove("main"); + assertThat(JulMdc.getEntries()).isEmpty(); + + CountDownLatch childEnd = new CountDownLatch(1); + new Thread() { + @Override + public void run() { + assertThat(JulMdc.getEntries()).isEmpty(); + childEnd.countDown(); + } + }.start(); - mdc.remove("hello"); - assertThat(mdc.getEntries()).isEmpty(); + childEnd.await(1, TimeUnit.SECONDS); } } From 5b4688a8cb68cdbfcfbebc695037170664a71469 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 25 Jul 2022 17:41:41 +0200 Subject: [PATCH 06/11] minimize map size to most common size (3) --- .../src/main/java/co/elastic/logging/jul/JulMdc.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 index cd44aa05..ab1a5ac5 100644 --- 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 @@ -33,6 +33,11 @@ */ public class JulMdc { + /** + * This MDC is currently used with 3 key/values: trace,transaction and error IDs, thus + */ + private static final int INITIAL_CAPACITY = 4; + private static final InheritableThreadLocal> tlm = new InheritableThreadLocal>() { @Override protected Map childValue(Map parentValue) { @@ -64,7 +69,7 @@ public static Map getEntries() { private static Map getOrCreateMap() { Map map = tlm.get(); if (map == null || map.isEmpty()) { - map = new HashMap(); + map = new HashMap(INITIAL_CAPACITY); tlm.set(map); } return map; From abce98e78925262007e7b896544485ba0ac1296d Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 25 Jul 2022 17:43:06 +0200 Subject: [PATCH 07/11] fix typo --- .../src/main/java/co/elastic/logging/jul/JulMdc.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index ab1a5ac5..3de2d8ee 100644 --- 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 @@ -34,7 +34,7 @@ public class JulMdc { /** - * This MDC is currently used with 3 key/values: trace,transaction and error IDs, thus + * This MDC is currently used with 3 key/values: trace,transaction and error IDs. */ private static final int INITIAL_CAPACITY = 4; From a27e1f44cfe30a85f18e49161782c3248a674b39 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 25 Jul 2022 17:59:16 +0200 Subject: [PATCH 08/11] simplify to regular thread local --- .../java/co/elastic/logging/jul/JulMdc.java | 28 ++++----------- .../co/elastic/logging/jul/JulMdcTest.java | 36 ------------------- 2 files changed, 7 insertions(+), 57 deletions(-) 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 index 3de2d8ee..1caf4926 100644 --- 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 @@ -38,20 +38,15 @@ public class JulMdc { */ private static final int INITIAL_CAPACITY = 4; - private static final InheritableThreadLocal> tlm = new InheritableThreadLocal>() { - @Override - protected Map childValue(Map parentValue) { - if (parentValue == null || parentValue.isEmpty()) { - return Collections.emptyMap(); - } else { - return new HashMap(parentValue); - } - } - }; - + private static final ThreadLocal> tlm = new ThreadLocal>(); public static void put(String key, String value) { - getOrCreateMap().put(key, 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) { @@ -66,13 +61,4 @@ public static Map getEntries() { return entries == null ? Collections.emptyMap() : entries; } - private static Map getOrCreateMap() { - Map map = tlm.get(); - if (map == null || map.isEmpty()) { - map = new HashMap(INITIAL_CAPACITY); - tlm.set(map); - } - return map; - } - } 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 index afbde040..0406f2ff 100644 --- 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 @@ -28,8 +28,6 @@ import org.junit.jupiter.api.Test; import java.util.Map; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import static org.assertj.core.api.Assertions.assertThat; @@ -61,38 +59,4 @@ void putRemoveSingleEntry() { assertThat(JulMdc.getEntries()).isEmpty(); } - @Test - void threadInheritance_singleValue() throws InterruptedException { - JulMdc.put("main", "main-value"); - assertThat(JulMdc.getEntries()).containsEntry("main", "main-value"); - - CountDownLatch childEnd = new CountDownLatch(1); - new Thread() { - @Override - public void run() { - assertThat(JulMdc.getEntries()).containsEntry("main", "main-value"); - childEnd.countDown(); - } - }.start(); - - childEnd.await(1, TimeUnit.SECONDS); - } - - @Test - void threadInheritance_empty() throws InterruptedException { - JulMdc.put("main", "main-value"); - JulMdc.remove("main"); - assertThat(JulMdc.getEntries()).isEmpty(); - - CountDownLatch childEnd = new CountDownLatch(1); - new Thread() { - @Override - public void run() { - assertThat(JulMdc.getEntries()).isEmpty(); - childEnd.countDown(); - } - }.start(); - - childEnd.await(1, TimeUnit.SECONDS); - } } From d1fff3b1f849600f6863408e8b273e957b0ad51a Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 26 Jul 2022 15:44:22 +0200 Subject: [PATCH 09/11] Update comment for APM agent usage --- .../src/main/java/co/elastic/logging/jul/JulMdc.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index 1caf4926..3308952c 100644 --- 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 @@ -29,7 +29,8 @@ import java.util.Map; /** - * MDC implementation for JUL as it does not have one + * 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 { From ffbc4fdf855c07bb4f436d5c15bec1b2ab4ec2dc Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 26 Jul 2022 15:53:46 +0200 Subject: [PATCH 10/11] avoid test to be poluted by side effects --- .../src/test/java/co/elastic/logging/jul/JulMdcTest.java | 7 +++++++ 1 file changed, 7 insertions(+) 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 index 0406f2ff..31443c18 100644 --- 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 @@ -25,6 +25,7 @@ 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; @@ -33,6 +34,12 @@ public class JulMdcTest { + @BeforeEach + void before(){ + // prevently empty if any other test have left something + JulMdc.getEntries().clear(); + } + @AfterEach void after() { JulMdc.getEntries().clear(); From 68c75ee83cd24757aeb662e2fcf1b35cc044d2d8 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 1 Aug 2022 17:25:18 +0200 Subject: [PATCH 11/11] post-review fix + javadoc --- .../src/main/java/co/elastic/logging/jul/JulMdc.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 index 3308952c..4d2d3cb3 100644 --- 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 @@ -57,7 +57,13 @@ public static void remove(String key) { } } - public static Map getEntries() { + /** + * 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; }