Skip to content

Commit 159e1e6

Browse files
tjquinnotjquinno
committed
Review comments. Reorg to maintain only one map of suppliers
1 parent 63f9f79 commit 159e1e6

File tree

11 files changed

+184
-20
lines changed

11 files changed

+184
-20
lines changed

‎logging/common/src/main/java/io/helidon/logging/common/HelidonMdc.java‎

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2022 Oracle and/or its affiliates.
2+
* Copyright (c) 2020, 2025 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -15,22 +15,33 @@
1515
*/
1616
package io.helidon.logging.common;
1717

18+
import java.util.HashMap;
1819
import java.util.List;
19-
import java.util.Objects;
20+
import java.util.Map;
2021
import java.util.Optional;
2122
import java.util.ServiceLoader;
23+
import java.util.function.Supplier;
2224

2325
import io.helidon.common.HelidonServiceLoader;
2426
import io.helidon.logging.common.spi.MdcProvider;
2527

2628
/**
2729
* Helidon MDC delegates values across all of the supported logging frameworks on the classpath.
30+
* <p>
31+
* Helidon permits adding MDC entries using {@code Supplier<String>} values as well as direct {@code String} values.
32+
* Although some logging implementations provide their own context maps (for example {@code ThreadContext} in Log4J and
33+
* {@code MDC} in SLF4J), they map MDC keys to {@code String} values, not to arbitrary objects that would accommodate
34+
* {@code Supplier<String>}. Therefore, Helidon manages its own map of key/supplier pairs and resolves all lookups using that map.
35+
* <p>
36+
* Helidon also propagates key/string pair assignments to the logging implementations' context maps.
2837
*/
2938
public class HelidonMdc {
3039

3140
private static final List<MdcProvider> MDC_PROVIDERS = HelidonServiceLoader
3241
.builder(ServiceLoader.load(MdcProvider.class)).build().asList();
3342

43+
private static final ThreadLocal<Map<String, Supplier<String>>> SUPPLIERS = ThreadLocal.withInitial(HashMap::new);
44+
3445
private HelidonMdc() {
3546
throw new UnsupportedOperationException("This class cannot be instantiated");
3647
}
@@ -42,22 +53,36 @@ private HelidonMdc() {
4253
* @param value entry value
4354
*/
4455
public static void set(String key, String value) {
56+
SUPPLIERS.get().put(key, value::toString);
4557
MDC_PROVIDERS.forEach(provider -> provider.put(key, value));
4658
}
4759

60+
/**
61+
* Propagate the value supplier to all {@link MdcProvider} instances registered.
62+
*
63+
* @param key entry key
64+
* @param valueSupplier supplier of the entry value
65+
*/
66+
public static void set(String key, Supplier<String> valueSupplier) {
67+
SUPPLIERS.get().put(key, valueSupplier);
68+
MDC_PROVIDERS.forEach(provider -> provider.put(key, valueSupplier.get()));
69+
}
70+
4871
/**
4972
* Remove value with the specific key from all of the instances of {@link MdcProvider}.
5073
*
5174
* @param key key
5275
*/
5376
public static void remove(String key) {
77+
SUPPLIERS.get().remove(key);
5478
MDC_PROVIDERS.forEach(provider -> provider.remove(key));
5579
}
5680

5781
/**
5882
* Remove all of the entries bound to the current thread from the instances of {@link MdcProvider}.
5983
*/
6084
public static void clear() {
85+
SUPPLIERS.get().clear();
6186
MDC_PROVIDERS.forEach(MdcProvider::clear);
6287
}
6388

@@ -68,10 +93,20 @@ public static void clear() {
6893
* @return found value bound to key
6994
*/
7095
public static Optional<String> get(String key) {
71-
return MDC_PROVIDERS.stream()
72-
.map(provider -> provider.get(key))
73-
.filter(Objects::nonNull)
74-
.findFirst();
96+
return SUPPLIERS.get().containsKey(key) ? Optional.of(SUPPLIERS.get().get(key).get()) : Optional.empty();
97+
}
98+
99+
static Map<String, Supplier<String>> suppliers() {
100+
return new HashMap<>(SUPPLIERS.get());
101+
}
102+
103+
static void suppliers(Map<String, Supplier<String>> suppliers) {
104+
SUPPLIERS.get().clear();
105+
SUPPLIERS.get().putAll(suppliers);
106+
}
107+
108+
static void clearSuppliers() {
109+
SUPPLIERS.get().clear();
75110
}
76111

77112
}

‎logging/jul/src/test/java/io/helidon/logging/jul/JulMdcTest.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2024 Oracle and/or its affiliates.
2+
* Copyright (c) 2020, 2025 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -45,6 +45,8 @@ public class JulMdcTest {
4545

4646
private static final String TEST_KEY = "test";
4747
private static final String TEST_VALUE = "value";
48+
private static final String TEST_SUPPLIED_KEY = "supplied_test";
49+
private static final String TEST_SUPPLIED_VALUE = "supplied_value";
4850

4951
@Test
5052
public void testMdc() {
@@ -54,19 +56,22 @@ public void testMdc() {
5456
LogConfig.initClass();
5557
OUTPUT_STREAM.reset();
5658
HelidonMdc.set(TEST_KEY, TEST_VALUE);
59+
HelidonMdc.set(TEST_SUPPLIED_KEY, () -> TEST_SUPPLIED_VALUE);
5760
String message = "This is test logging message";
5861
String thread = Thread.currentThread().toString();
5962
String logMessage = logMessage(message);
60-
assertThat(logMessage, endsWith(thread + ": " + message + " " + TEST_VALUE + System.lineSeparator()));
63+
assertThat(logMessage, endsWith(thread + ": " + message + " " + TEST_VALUE + " " + TEST_SUPPLIED_VALUE + System.lineSeparator()));
6164

6265
HelidonMdc.remove(TEST_KEY);
66+
HelidonMdc.remove(TEST_SUPPLIED_KEY);
6367
logMessage = logMessage(message);
64-
assertThat(logMessage, endsWith(thread + ": " + message + " " + System.lineSeparator()));
68+
assertThat(logMessage, endsWith(thread + ": " + message + " " + " " + System.lineSeparator()));
6569

6670
HelidonMdc.set(TEST_KEY, TEST_VALUE);
71+
HelidonMdc.set(TEST_SUPPLIED_KEY, () -> TEST_SUPPLIED_VALUE);
6772
HelidonMdc.clear();
6873
logMessage = logMessage(message);
69-
assertThat(logMessage, endsWith(thread + ": " + message + " " + System.lineSeparator()));
74+
assertThat(logMessage, endsWith(thread + ": " + message + " " + " " + System.lineSeparator()));
7075
} finally {
7176
System.setOut(original);
7277
}
@@ -84,13 +89,14 @@ private String logMessage(String message) {
8489
@Test
8590
public void testThreadPropagationWithContext() {
8691
HelidonMdc.set(TEST_KEY, TEST_VALUE);
92+
HelidonMdc.set(TEST_SUPPLIED_KEY, () -> TEST_SUPPLIED_VALUE);
8793
Context context = Context.create();
8894
ExecutorService executor = Contexts.wrap(Executors.newFixedThreadPool(1));
8995

9096
Contexts.runInContext(context, () -> {
9197
try {
9298
String value = executor.submit(new TestCallable()).get();
93-
assertThat(value, is(TEST_VALUE));
99+
assertThat(value, is(TEST_VALUE + "/" + TEST_SUPPLIED_VALUE));
94100
} catch (Exception e) {
95101
throw new ExecutorException("failed to execute", e);
96102
}
@@ -100,11 +106,12 @@ public void testThreadPropagationWithContext() {
100106
@Test
101107
public void testThreadPropagationWithoutContext() {
102108
HelidonMdc.set(TEST_KEY, TEST_VALUE);
109+
HelidonMdc.set(TEST_SUPPLIED_KEY, () -> TEST_SUPPLIED_VALUE);
103110
ExecutorService executor = Contexts.wrap(Executors.newFixedThreadPool(1));
104111

105112
try {
106113
String value = executor.submit(new TestCallable()).get();
107-
assertThat(value, is(TEST_VALUE));
114+
assertThat(value, is(TEST_VALUE + "/" + TEST_SUPPLIED_VALUE));
108115
} catch (Exception e) {
109116
throw new ExecutorException("failed to execute", e);
110117
}
@@ -114,7 +121,7 @@ private static final class TestCallable implements Callable<String> {
114121

115122
@Override
116123
public String call() {
117-
return JulMdc.get(TEST_KEY);
124+
return JulMdc.get(TEST_KEY) + "/" + JulMdc.get(TEST_SUPPLIED_KEY);
118125
}
119126
}
120127

‎logging/jul/src/test/resources/logging.properties‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#
2-
# Copyright (c) 2020 Oracle and/or its affiliates.
2+
# Copyright (c) 2020, 2025 Oracle and/or its affiliates.
33
#
44
# Licensed under the Apache License, Version 2.0 (the "License");
55
# you may not use this file except in compliance with the License.
@@ -18,7 +18,7 @@
1818
handlers=io.helidon.logging.jul.HelidonConsoleHandler
1919

2020
# HelidonConsoleHandler uses a SimpleFormatter subclass that replaces "!thread!" with the current thread
21-
java.util.logging.SimpleFormatter.format=%1$tY.%1$tm.%1$td %1$tH:%1$tM:%1$tS %4$s %3$s !thread!: %5$s%6$s %X{test}%n
21+
java.util.logging.SimpleFormatter.format=%1$tY.%1$tm.%1$td %1$tH:%1$tM:%1$tS %4$s %3$s !thread!: %5$s%6$s %X{test} %X{supplied_test}%n
2222

2323
# Global logging level. Can be overridden by specific loggers
2424
.level=INFO

‎tracing/provider-tests/pom.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@
5050
<groupId>io.helidon.common.testing</groupId>
5151
<artifactId>helidon-common-testing-junit5</artifactId>
5252
</dependency>
53+
<dependency>
54+
<groupId>io.helidon.logging</groupId>
55+
<artifactId>helidon-logging-jul</artifactId>
56+
</dependency>
5357
<dependency>
5458
<groupId>org.junit.jupiter</groupId>
5559
<artifactId>junit-jupiter-api</artifactId>

‎tracing/provider-tests/src/main/java/io/helidon/tracing/providers/tests/TestMdc.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* Copyright (c) 2025 Oracle and/or its affiliates.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.helidon.tracing.providers.tests;
18+
19+
import java.io.ByteArrayInputStream;
20+
import java.io.IOException;
21+
import java.nio.charset.StandardCharsets;
22+
import java.util.logging.Level;
23+
import java.util.logging.LogManager;
24+
import java.util.logging.Logger;
25+
26+
import io.helidon.common.testing.junit5.InMemoryLoggingHandler;
27+
import io.helidon.logging.jul.HelidonFormatter;
28+
import io.helidon.tracing.Scope;
29+
import io.helidon.tracing.Span;
30+
import io.helidon.tracing.Tracer;
31+
32+
import org.junit.jupiter.api.BeforeAll;
33+
import org.junit.jupiter.api.Test;
34+
35+
import static org.hamcrest.MatcherAssert.assertThat;
36+
import static org.hamcrest.Matchers.containsString;
37+
38+
class TestMdc {
39+
40+
@BeforeAll
41+
static void beforeAll() throws IOException {
42+
String loggingConfig = "# HelidonConsoleHandler uses a SimpleFormatter subclass that replaces \"!thread!\" with the current thread\n"
43+
+ "java.util.logging.SimpleFormatter.format=%1$tY.%1$tm.%1$td %1$tH:%1$tM:%1$tS %4$s %3$s !thread!: %5$s%6$s trace_id %X{trace_id}%n\n";
44+
45+
LogManager.getLogManager().readConfiguration(new ByteArrayInputStream(loggingConfig.getBytes(StandardCharsets.UTF_8)));
46+
}
47+
48+
@Test
49+
void testTraceId() {
50+
51+
Logger logger = Logger.getLogger(TestMdc.class.getName());
52+
HelidonFormatter helidonFormatter = new HelidonFormatter();
53+
try (InMemoryLoggingHandler loggingHandler = InMemoryLoggingHandler.create(logger)) {
54+
loggingHandler.setFormatter(helidonFormatter);
55+
Span span = Tracer.global().spanBuilder("logging-test-span").start();
56+
String expectedTraceId = span.context().traceId();
57+
String formattedMessage;
58+
try (Scope ignored = span.activate()) {
59+
logger.log(Level.INFO, "Test log message");
60+
formattedMessage = helidonFormatter.format(loggingHandler.logRecords().getFirst());
61+
}
62+
63+
assertThat("MDC-processed log message",
64+
formattedMessage,
65+
containsString("trace_id " + expectedTraceId));
66+
}
67+
68+
}
69+
}

‎tracing/provider-tests/src/main/java/module-info.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2024 Oracle and/or its affiliates.
2+
* Copyright (c) 2024, 2025 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@
2020
module io.helidon.tracing.provider.tests {
2121

2222
requires java.logging;
23+
requires io.helidon.logging.jul;
2324
requires io.helidon.tracing;
2425
requires io.helidon.common.context;
2526
requires io.helidon.common.testing.junit5;

‎tracing/providers/opentelemetry/pom.xml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@
117117
<configuration>
118118
<excludes>
119119
<exclude>**/TestGlobalTracerAssignment.java</exclude>
120+
<exclude>**/TestMdc.java</exclude>
120121
</excludes>
121122
</configuration>
122123
</execution>
@@ -131,6 +132,17 @@
131132
</includes>
132133
</configuration>
133134
</execution>
135+
<execution>
136+
<id>mdc-test</id>
137+
<goals>
138+
<goal>test</goal>
139+
</goals>
140+
<configuration>
141+
<includes>
142+
<include>**/TestMdc.java</include>
143+
</includes>
144+
</configuration>
145+
</execution>
134146
</executions>
135147
</plugin>
136148
</plugins>

‎tracing/providers/opentracing/pom.xml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,19 @@
141141
</configurationParameters>
142142
</properties>
143143
</configuration>
144+
<executions>
145+
<execution>
146+
<id>mdc-test</id>
147+
<goals>
148+
<goal>test</goal>
149+
</goals>
150+
<configuration>
151+
<includes>
152+
<include>**/TestMdc.java</include>
153+
</includes>
154+
</configuration>
155+
</execution>
156+
</executions>
144157
</plugin>
145158
</plugins>
146159
</build>

‎tracing/providers/zipkin/pom.xml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@
154154
<configuration>
155155
<excludes>
156156
<exclude>io.helidon.tracing.providers.tests.TestTracerAndSpanPropagation.java</exclude>
157+
<exclude>**/TestMdc.java</exclude>
157158
</excludes>
158159
</configuration>
159160
</execution>
@@ -168,6 +169,17 @@
168169
</includes>
169170
</configuration>
170171
</execution>
172+
<execution>
173+
<id>mdc-test</id>
174+
<goals>
175+
<goal>test</goal>
176+
</goals>
177+
<configuration>
178+
<includes>
179+
<include>**/TestMdc.java</include>
180+
</includes>
181+
</configuration>
182+
</execution>
171183
</executions>
172184
</plugin>
173185
</plugins>

0 commit comments

Comments
 (0)