Skip to content

Commit 96fd1d7

Browse files
adamleantechMateusz Rzeszutek
adamleantech
and
Mateusz Rzeszutek
authored
Add Baggage to logback MDC controlled by flag (#7892)
The intention here is to allow users of the java agent to set a VM flag in order to be able to add values in the current Baggage context to MDC for logback. It seemed unwise to turn this on by default - if the application is configured to print all MDC contents (as it often the case with JSON output) then baggage would be logged out by default which may either bloat the logs or result in sensitive data being exposed unitentionally. Addresses #1391 and #6708 Note that this is my first contribution to this repo, I've done my best to follow the existing approaches to things like testing but would appreciate any feedback. --------- Co-authored-by: Mateusz Rzeszutek <[email protected]>
1 parent bb1bc5d commit 96fd1d7

File tree

12 files changed

+280
-52
lines changed

12 files changed

+280
-52
lines changed

‎instrumentation/logback/logback-mdc-1.0/javaagent/build.gradle.kts‎

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,42 @@ muzzle {
1010
}
1111
}
1212

13+
testing {
14+
suites {
15+
val addBaggageTest by registering(JvmTestSuite::class) {
16+
targets {
17+
all {
18+
testTask.configure {
19+
jvmArgs("-Dotel.instrumentation.logback-mdc.add-baggage=true")
20+
}
21+
}
22+
}
23+
}
24+
25+
withType(JvmTestSuite::class) {
26+
dependencies {
27+
if (findProperty("testLatestDeps") as Boolean) {
28+
implementation("ch.qos.logback:logback-classic:+")
29+
} else {
30+
implementation("ch.qos.logback:logback-classic") {
31+
version {
32+
strictly("1.0.0")
33+
}
34+
}
35+
implementation("org.slf4j:slf4j-api") {
36+
version {
37+
strictly("1.6.4")
38+
}
39+
}
40+
}
41+
42+
implementation(project(":instrumentation:logback:logback-mdc-1.0:testing"))
43+
implementation(project(":instrumentation:logback:logback-mdc-1.0:javaagent"))
44+
}
45+
}
46+
}
47+
}
48+
1349
dependencies {
1450
implementation(project(":instrumentation:logback:logback-mdc-1.0:library"))
1551

@@ -24,21 +60,10 @@ dependencies {
2460
strictly("1.6.4")
2561
}
2662
}
63+
}
2764

28-
if (findProperty("testLatestDeps") as Boolean) {
29-
testImplementation("ch.qos.logback:logback-classic:+")
30-
} else {
31-
testImplementation("ch.qos.logback:logback-classic") {
32-
version {
33-
strictly("1.0.0")
34-
}
35-
}
36-
testImplementation("org.slf4j:slf4j-api") {
37-
version {
38-
strictly("1.6.4")
39-
}
40-
}
65+
tasks {
66+
named("check") {
67+
dependsOn(testing.suites)
4168
}
42-
43-
testImplementation(project(":instrumentation:logback:logback-mdc-1.0:testing"))
4469
}

‎instrumentation/logback/logback-mdc-1.0/javaagent/src/addBaggageTest/groovy/io/opentelemetry/javaagent/instrumentation/logback/v1_0/LogbackWithBaggageTest.groovy‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.instrumentation.logback.v1_0
7+
8+
import io.opentelemetry.instrumentation.logback.mdc.v1_0.AbstractLogbackWithBaggageTest
9+
import io.opentelemetry.instrumentation.test.AgentTestTrait
10+
11+
class LogbackWithBaggageTest extends AbstractLogbackWithBaggageTest implements AgentTestTrait {
12+
}

‎instrumentation/logback/logback-mdc-1.0/javaagent/src/addBaggageTest/resources/logback.xml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<configuration>
3+
<appender name="LIST" class="ch.qos.logback.core.read.ListAppender" />
4+
5+
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
6+
<encoder>
7+
<pattern>%coloredLevel %logger{15} - %message%n%xException{10}</pattern>
8+
</encoder>
9+
</appender>
10+
11+
<logger name="test">
12+
<level value="info" />
13+
<appender-ref ref="LIST" />
14+
</logger>
15+
16+
<root level="debug">
17+
<appender-ref ref="STDOUT" />
18+
</root>
19+
</configuration>

‎instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LogbackSingletons.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.instrumentation.logback.mdc.v1_0;
7+
8+
import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil;
9+
10+
public final class LogbackSingletons {
11+
private static final boolean ADD_BAGGAGE =
12+
ConfigPropertiesUtil.getBoolean("otel.instrumentation.logback-mdc.add-baggage", false);
13+
14+
public static boolean addBaggage() {
15+
return ADD_BAGGAGE;
16+
}
17+
18+
private LogbackSingletons() {}
19+
}

‎instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggingEventInstrumentation.java‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
1818

1919
import ch.qos.logback.classic.spi.ILoggingEvent;
20+
import io.opentelemetry.api.baggage.Baggage;
21+
import io.opentelemetry.api.baggage.BaggageEntry;
2022
import io.opentelemetry.api.trace.SpanContext;
2123
import io.opentelemetry.context.Context;
2224
import io.opentelemetry.instrumentation.api.util.VirtualField;
@@ -54,11 +56,11 @@ public void transform(TypeTransformer transformer) {
5456

5557
@SuppressWarnings("unused")
5658
public static class GetMdcAdvice {
57-
5859
@Advice.OnMethodExit(suppress = Throwable.class)
5960
public static void onExit(
6061
@Advice.This ILoggingEvent event,
6162
@Advice.Return(typing = Typing.DYNAMIC, readOnly = false) Map<String, String> contextData) {
63+
6264
if (contextData != null && contextData.containsKey(TRACE_ID)) {
6365
// Assume already instrumented event if traceId is present.
6466
return;
@@ -69,15 +71,28 @@ public static void onExit(
6971
return;
7072
}
7173

74+
Map<String, String> spanContextData = new HashMap<>();
75+
7276
SpanContext spanContext = Java8BytecodeBridge.spanFromContext(context).getSpanContext();
73-
if (!spanContext.isValid()) {
74-
return;
77+
78+
if (spanContext.isValid()) {
79+
spanContextData.put(TRACE_ID, spanContext.getTraceId());
80+
spanContextData.put(SPAN_ID, spanContext.getSpanId());
81+
spanContextData.put(TRACE_FLAGS, spanContext.getTraceFlags().asHex());
7582
}
7683

77-
Map<String, String> spanContextData = new HashMap<>();
78-
spanContextData.put(TRACE_ID, spanContext.getTraceId());
79-
spanContextData.put(SPAN_ID, spanContext.getSpanId());
80-
spanContextData.put(TRACE_FLAGS, spanContext.getTraceFlags().asHex());
84+
if (LogbackSingletons.addBaggage()) {
85+
Baggage baggage = Java8BytecodeBridge.baggageFromContext(context);
86+
87+
// using a lambda here does not play nicely with instrumentation bytecode process
88+
// (Java 6 related errors are observed) so relying on for loop instead
89+
for (Map.Entry<String, BaggageEntry> entry : baggage.asMap().entrySet()) {
90+
spanContextData.put(
91+
entry.getKey(),
92+
// prefix all baggage values to avoid clashes with existing context
93+
"baggage." + entry.getValue().getValue());
94+
}
95+
}
8196

8297
if (contextData == null) {
8398
contextData = spanContextData;

‎instrumentation/logback/logback-mdc-1.0/library/build.gradle.kts‎

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,44 @@ plugins {
22
id("otel.library-instrumentation")
33
}
44

5+
testing {
6+
suites {
7+
val addBaggageTest by registering(JvmTestSuite::class) {
8+
targets {
9+
all {
10+
testTask.configure {
11+
jvmArgs("-Dotel.instrumentation.logback-mdc.add-baggage=true")
12+
}
13+
}
14+
}
15+
}
16+
17+
withType(JvmTestSuite::class) {
18+
dependencies {
19+
if (findProperty("testLatestDeps") as Boolean) {
20+
implementation("ch.qos.logback:logback-classic:+")
21+
} else {
22+
implementation("ch.qos.logback:logback-classic") {
23+
version {
24+
strictly("1.0.0")
25+
}
26+
}
27+
implementation("org.slf4j:slf4j-api") {
28+
version {
29+
strictly("1.6.4")
30+
}
31+
}
32+
}
33+
34+
implementation(project(":instrumentation:logback:logback-mdc-1.0:testing"))
35+
implementation(project(":instrumentation:logback:logback-mdc-1.0:library"))
36+
}
37+
}
38+
}
39+
}
40+
541
dependencies {
42+
643
// pin the version strictly to avoid overriding by dependencyManagement versions
744
compileOnly("ch.qos.logback:logback-classic") {
845
version {
@@ -14,21 +51,10 @@ dependencies {
1451
strictly("1.6.4")
1552
}
1653
}
54+
}
1755

18-
if (findProperty("testLatestDeps") as Boolean) {
19-
testImplementation("ch.qos.logback:logback-classic:+")
20-
} else {
21-
testImplementation("ch.qos.logback:logback-classic") {
22-
version {
23-
strictly("1.0.0")
24-
}
25-
}
26-
testImplementation("org.slf4j:slf4j-api") {
27-
version {
28-
strictly("1.6.4")
29-
}
30-
}
56+
tasks {
57+
named("check") {
58+
dependsOn(testing.suites)
3159
}
32-
33-
testImplementation(project(":instrumentation:logback:logback-mdc-1.0:testing"))
3460
}

‎instrumentation/logback/logback-mdc-1.0/library/src/addBaggageTest/groovy/io/opentelemetry/instrumentation/logback/mdc/v1_0/LogbackWithBaggageTest.groovy‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.instrumentation.logback.mdc.v1_0
7+
8+
9+
import io.opentelemetry.instrumentation.test.LibraryTestTrait
10+
11+
class LogbackWithBaggageTest extends AbstractLogbackWithBaggageTest implements LibraryTestTrait {
12+
}

‎instrumentation/logback/logback-mdc-1.0/library/src/addBaggageTest/resources/logback.xml‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Copyright The OpenTelemetry Authors
4+
~
5+
~ Licensed under the Apache License, Version 2.0 (the "License");
6+
~ you may not use this file except in compliance with the License.
7+
~ You may obtain a copy of the License at
8+
~
9+
~ http://www.apache.org/licenses/LICENSE-2.0
10+
~
11+
~ Unless required by applicable law or agreed to in writing, software
12+
~ distributed under the License is distributed on an "AS IS" BASIS,
13+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
~ See the License for the specific language governing permissions and
15+
~ limitations under the License.
16+
-->
17+
18+
<configuration>
19+
<appender name="LIST" class="ch.qos.logback.core.read.ListAppender"/>
20+
21+
<appender name="OTEL"
22+
class="io.opentelemetry.instrumentation.logback.mdc.v1_0.OpenTelemetryAppender">
23+
<addBaggage>true</addBaggage>
24+
<appender-ref ref="LIST"/>
25+
</appender>
26+
27+
<logger name="test">
28+
<level value="info"/>
29+
<appender-ref ref="OTEL"/>
30+
</logger>
31+
</configuration>

‎instrumentation/logback/logback-mdc-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/mdc/v1_0/OpenTelemetryAppender.java‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
import ch.qos.logback.core.UnsynchronizedAppenderBase;
1616
import ch.qos.logback.core.spi.AppenderAttachable;
1717
import ch.qos.logback.core.spi.AppenderAttachableImpl;
18+
import io.opentelemetry.api.baggage.Baggage;
1819
import io.opentelemetry.api.trace.Span;
1920
import io.opentelemetry.api.trace.SpanContext;
21+
import io.opentelemetry.context.Context;
2022
import io.opentelemetry.instrumentation.logback.mdc.v1_0.internal.UnionMap;
2123
import java.lang.reflect.Proxy;
2224
import java.util.HashMap;
@@ -25,26 +27,48 @@
2527

2628
public class OpenTelemetryAppender extends UnsynchronizedAppenderBase<ILoggingEvent>
2729
implements AppenderAttachable<ILoggingEvent> {
30+
private volatile boolean addBaggage;
2831

2932
private final AppenderAttachableImpl<ILoggingEvent> aai = new AppenderAttachableImpl<>();
3033

31-
public static ILoggingEvent wrapEvent(ILoggingEvent event) {
32-
Span currentSpan = Span.current();
33-
if (!currentSpan.getSpanContext().isValid()) {
34-
return event;
35-
}
34+
/**
35+
* When set to true this will enable addition of all baggage entries to MDC. This can be done by
36+
* adding the following to the logback.xml config for this appender. {@code
37+
* <addBaggage>true</addBaggage>}
38+
*
39+
* @param addBaggage True if baggage should be added to MDC
40+
*/
41+
public void setAddBaggage(boolean addBaggage) {
42+
this.addBaggage = addBaggage;
43+
}
3644

45+
public ILoggingEvent wrapEvent(ILoggingEvent event) {
3746
Map<String, String> eventContext = event.getMDCPropertyMap();
3847
if (eventContext != null && eventContext.containsKey(TRACE_ID)) {
3948
// Assume already instrumented event if traceId is present.
4049
return event;
4150
}
4251

4352
Map<String, String> contextData = new HashMap<>();
44-
SpanContext spanContext = currentSpan.getSpanContext();
45-
contextData.put(TRACE_ID, spanContext.getTraceId());
46-
contextData.put(SPAN_ID, spanContext.getSpanId());
47-
contextData.put(TRACE_FLAGS, spanContext.getTraceFlags().asHex());
53+
Context context = Context.current();
54+
Span currentSpan = Span.fromContext(context);
55+
56+
if (currentSpan.getSpanContext().isValid()) {
57+
SpanContext spanContext = currentSpan.getSpanContext();
58+
contextData.put(TRACE_ID, spanContext.getTraceId());
59+
contextData.put(SPAN_ID, spanContext.getSpanId());
60+
contextData.put(TRACE_FLAGS, spanContext.getTraceFlags().asHex());
61+
}
62+
63+
if (addBaggage) {
64+
Baggage baggage = Baggage.fromContext(context);
65+
baggage.forEach(
66+
(key, value) ->
67+
contextData.put(
68+
key,
69+
// prefix all baggage values to avoid clashes with existing context
70+
"baggage." + value.getValue()));
71+
}
4872

4973
if (eventContext == null) {
5074
eventContext = contextData;

0 commit comments

Comments
 (0)