Skip to content

Commit

Permalink
Closes #780 - Added opt-out for MDC injection of individual libraries (
Browse files Browse the repository at this point in the history
  • Loading branch information
Jonas Kunz authored Jul 3, 2020
1 parent a43f4be commit d658c3d
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ public class TraceIdMDCInjectionSettings {

/**
* Specifies whether log correlation shall be performed or not.
* If enabled, the currently active traceID will be published to the MDC of all log libraries.
* If enabled, the currently active traceID will be published to the MDC of all log libraries,
* if they have not been explicitly disabled.
*/
private boolean enabled;

Expand All @@ -20,4 +21,19 @@ public class TraceIdMDCInjectionSettings {
*/
@NotBlank
private String key;

/**
* Log4J2 injection will only take place, if this field and {@link #enabled} are true.
*/
private boolean log4j2Enabled;

/**
* Log4J1 injection will only take place, if this field and {@link #enabled} are true.
*/
private boolean log4j1Enabled;

/**
* Slf4j injection will only take place, if this field and {@link #enabled} are true.
*/
private boolean slf4jEnabled;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ inspectit:
enabled: false
# the key which is used to store the trace id in the MDC
key: "traceid"
# Opt-out option for slf4J
slf4j-enabled: true
# Opt-out option for Log4J1
log4j1-enabled: true
# Opt-out option for Log4J2
log4j2-enabled: true
trace-id-auto-injection:
# whether the trace id should automatically injected into log statements
enabled: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@

import com.google.common.annotations.VisibleForTesting;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Component;
import rocks.inspectit.ocelot.bootstrap.instrumentation.DoNotInstrumentMarker;
import rocks.inspectit.ocelot.config.model.tracing.TraceIdMDCInjectionSettings;
import rocks.inspectit.ocelot.core.AgentImpl;
import rocks.inspectit.ocelot.core.config.InspectitConfigChangedEvent;
import rocks.inspectit.ocelot.core.config.InspectitEnvironment;
import rocks.inspectit.ocelot.core.instrumentation.correlation.log.adapters.Log4J1MDCAdapter;
import rocks.inspectit.ocelot.core.instrumentation.correlation.log.adapters.Log4J2MDCAdapter;
import rocks.inspectit.ocelot.core.instrumentation.correlation.log.adapters.MDCAdapter;
Expand All @@ -14,6 +19,7 @@
import javax.annotation.PostConstruct;
import java.util.*;
import java.util.function.Function;
import java.util.stream.Collectors;

/**
* Provides access to all MDCs on the classpath through a single interface.
Expand All @@ -27,6 +33,7 @@ public class MDCAccess implements IClassDiscoveryListener {
* Non-throwing closeable.
*/
public interface Undo extends AutoCloseable {

@Override
void close();

Expand All @@ -37,6 +44,9 @@ public interface Undo extends AutoCloseable {
};
}

@Autowired
private InspectitEnvironment inspectitEnv;

/**
* Maps class names of MDC classes to their corresponding adapter factory methods.
*/
Expand All @@ -46,7 +56,14 @@ public interface Undo extends AutoCloseable {
* Holds references to all MDC Adapters for all classes found on the classpath.
*/
@VisibleForTesting
WeakHashMap<Class<?>, MDCAdapter> activeAdapters = new WeakHashMap<>();
WeakHashMap<Class<?>, MDCAdapter> availableAdapters = new WeakHashMap<>();

/**
* Holds references to all MDC Adapters which are enabled.
* The adapters are weakly referenced because their livetime is controlled by {@link #availableAdapters}.
*/
@VisibleForTesting
Set<MDCAdapter> enabledAdapters = Collections.emptySet();

/**
* Registers all implemented log api MDC adapters.
Expand All @@ -64,11 +81,12 @@ void registerAdapters() {
*
* @param key the key under which the given value shall be put into all MDCs
* @param value the value to insert
*
* @return A function for undoing the change in all MDCs (Restoring any previously set value).
*/
public Undo put(String key, String value) {
List<Undo> undos = new ArrayList<>();
for (MDCAdapter adapter : activeAdapters.values()) {
for (MDCAdapter adapter : enabledAdapters) {
undos.add(adapter.set(key, value));
}
return () -> {
Expand All @@ -80,7 +98,7 @@ public Undo put(String key, String value) {
}

@Override
public void onNewClassesDiscovered(Set<Class<?>> newClasses) {
public synchronized void onNewClassesDiscovered(Set<Class<?>> newClasses) {
newClasses.stream()
.filter(clazz -> mdcAdapterBuilders.containsKey(clazz.getName()))
.filter(clazz -> clazz.getClassLoader() != AgentImpl.INSPECTIT_CLASS_LOADER)
Expand All @@ -89,10 +107,21 @@ public void onNewClassesDiscovered(Set<Class<?>> newClasses) {
try {
log.debug("Found MDC implementation for log correlation: {}", clazz.getName());
MDCAdapter adapter = mdcAdapterBuilders.get(clazz.getName()).apply(clazz);
activeAdapters.put(clazz, adapter);
availableAdapters.put(clazz, adapter);
updateEnabledAdaptersSet();
} catch (Throwable t) {
log.error("Error creating log-correlation MDC adapter for class {}", clazz.getName(), t);
}
});
}

@EventListener(InspectitConfigChangedEvent.class)
@VisibleForTesting
synchronized void updateEnabledAdaptersSet() {
TraceIdMDCInjectionSettings settings =
inspectitEnv.getCurrentConfig().getTracing().getLogCorrelation().getTraceIdMdcInjection();
enabledAdapters = availableAdapters.values().stream()
.filter(adapter -> adapter.isEnabledForConfig(settings))
.collect(Collectors.toCollection(() -> Collections.newSetFromMap(new WeakHashMap<>())));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rocks.inspectit.ocelot.core.instrumentation.correlation.log.adapters;

import lombok.extern.slf4j.Slf4j;
import rocks.inspectit.ocelot.config.model.tracing.TraceIdMDCInjectionSettings;
import rocks.inspectit.ocelot.core.utils.WeakMethodReference;

/**
Expand All @@ -22,6 +23,7 @@ private Log4J1MDCAdapter(WeakMethodReference put, WeakMethodReference get, WeakM
* Creates an Adapater given a org.apache.log4j.MDC class.
*
* @param mdcClazz the org.apache.log4j.MDC class
*
* @return and adapter for setting values on the given MDC
*/
public static Log4J1MDCAdapter get(Class<?> mdcClazz) {
Expand All @@ -34,4 +36,9 @@ public static Log4J1MDCAdapter get(Class<?> mdcClazz) {
throw new IllegalArgumentException("MDC class did not contain expected methods", e);
}
}

@Override
public boolean isEnabledForConfig(TraceIdMDCInjectionSettings settings) {
return settings.isLog4j1Enabled() && settings.isEnabled();
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rocks.inspectit.ocelot.core.instrumentation.correlation.log.adapters;

import lombok.extern.slf4j.Slf4j;
import rocks.inspectit.ocelot.config.model.tracing.TraceIdMDCInjectionSettings;
import rocks.inspectit.ocelot.core.utils.WeakMethodReference;

/**
Expand All @@ -22,6 +23,7 @@ private Log4J2MDCAdapter(WeakMethodReference put, WeakMethodReference get, WeakM
* Creates an Adapater given a ThreadContext class.
*
* @param mdcClazz the org.apache.logging.log4j.ThreadContext class
*
* @return and adapter for setting values on the given thread context.
*/
public static Log4J2MDCAdapter get(Class<?> mdcClazz) {
Expand All @@ -34,4 +36,9 @@ public static Log4J2MDCAdapter get(Class<?> mdcClazz) {
throw new IllegalArgumentException("ThreadContext class did not contain expected methods", e);
}
}

@Override
public boolean isEnabledForConfig(TraceIdMDCInjectionSettings settings) {
return settings.isLog4j2Enabled() && settings.isEnabled();
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package rocks.inspectit.ocelot.core.instrumentation.correlation.log.adapters;

import rocks.inspectit.ocelot.config.model.tracing.TraceIdMDCInjectionSettings;
import rocks.inspectit.ocelot.core.instrumentation.correlation.log.MDCAccess;

/**
Expand All @@ -13,7 +14,17 @@ public interface MDCAdapter {
*
* @param key the MDC key to use
* @param value the value to place in the MDC, null if the value for the key should be erased
*
* @return a {@link Undo} which reverts the change performed by this method call.
*/
MDCAccess.Undo set(String key, String value);

/**
* Checks if this Adapter is enabled based on the given configuration.
*
* @param settings the currently active configuration
*
* @return true if enabled, false otherwise
*/
boolean isEnabledForConfig(TraceIdMDCInjectionSettings settings);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rocks.inspectit.ocelot.core.instrumentation.correlation.log.adapters;

import lombok.extern.slf4j.Slf4j;
import rocks.inspectit.ocelot.config.model.tracing.TraceIdMDCInjectionSettings;
import rocks.inspectit.ocelot.core.utils.WeakMethodReference;

/**
Expand All @@ -23,6 +24,7 @@ private Slf4jMDCAdapter(WeakMethodReference put, WeakMethodReference get, WeakMe
* Creates an adapter for a given org.slf4j.MDC class.
*
* @param mdcClazz reference to the org.slf4j.MDC class
*
* @return an adapter for the given class
*/
public static Slf4jMDCAdapter get(Class<?> mdcClazz) {
Expand All @@ -35,4 +37,9 @@ public static Slf4jMDCAdapter get(Class<?> mdcClazz) {
throw new IllegalArgumentException("MDC class did not contain expected methods", e);
}
}

@Override
public boolean isEnabledForConfig(TraceIdMDCInjectionSettings settings) {
return settings.isSlf4jEnabled() && settings.isEnabled();
}
}
Original file line number Diff line number Diff line change
@@ -1,27 +1,35 @@
package rocks.inspectit.ocelot.core.instrumentation.correlation.log;

import com.google.common.collect.ImmutableSet;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.stubbing.Answer;
import rocks.inspectit.ocelot.core.config.InspectitEnvironment;
import rocks.inspectit.ocelot.core.instrumentation.correlation.log.adapters.MDCAdapter;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doReturn;

@ExtendWith(MockitoExtension.class)
public class MDCAccessTest {

@Mock(answer = Answers.RETURNS_MOCKS)
InspectitEnvironment inspectitEnv;

@InjectMocks
MDCAccess access;

Expand All @@ -36,17 +44,15 @@ public class MDCAccessTest {

@BeforeEach
void reset() {
access.activeAdapters.clear();
access.availableAdapters.clear();
}

@Nested
class Put {

@Test
void verifyUndoOrderReversed() {
access.activeAdapters.put(Long.class, adapterA);
access.activeAdapters.put(Double.class, adapterB);
access.activeAdapters.put(Byte.class, adapterC);
access.enabledAdapters = ImmutableSet.of(adapterA, adapterB, adapterC);

List<Object> setOrder = new ArrayList<>();
List<Object> undoOrder = new ArrayList<>();
Expand Down Expand Up @@ -75,4 +81,24 @@ void verifyUndoOrderReversed() {
}

}

@Nested
class UpdateEnabledAdapters {

@Test
void verifyUndoOrderReversed() {
access.availableAdapters.put(Byte.class, adapterA);
access.availableAdapters.put(Short.class, adapterB);
access.availableAdapters.put(Integer.class, adapterC);

doReturn(false).when(adapterA).isEnabledForConfig(any());
doReturn(true).when(adapterB).isEnabledForConfig(any());
doReturn(true).when(adapterC).isEnabledForConfig(any());

access.updateEnabledAdaptersSet();

assertThat(access.enabledAdapters).containsExactlyInAnyOrder(adapterB, adapterC);
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import rocks.inspectit.ocelot.config.model.tracing.TraceIdMDCInjectionSettings;
import rocks.inspectit.ocelot.core.utils.WeakMethodReference;

import java.util.HashMap;
Expand Down Expand Up @@ -42,6 +43,10 @@ void setup() throws Exception {
WeakMethodReference get = WeakMethodReference.create(DummyMDC.class, "get", String.class);
WeakMethodReference remove = WeakMethodReference.create(DummyMDC.class, "remove", String.class);
adapter = new AbstractStaticMapMDCAdapter(put, get, remove) {
@Override
public boolean isEnabledForConfig(TraceIdMDCInjectionSettings settings) {
return true;
}
};
}

Expand All @@ -66,7 +71,6 @@ void ensureValuesRemoved() {
assertThat(DummyMDC.contents.containsKey("myKey")).isFalse();
}


@Test
void ensureUndoPreservesPreviousValue() {
DummyMDC.put("myKey", "someValue");
Expand Down
15 changes: 14 additions & 1 deletion inspectit-ocelot-documentation/docs/tracing/log-correlation.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ As a result, log messages, generated within an exported trace, will be prefixed

You can change the key under which the trace-id is placed in the MDC using the property `inspectit.tracing.log-correlation.trace-id-mdc-injection.key`.

By default, the trace-id will be inserted into all MDCs. If required, you can selectively exclude the supported libraries using the following flags:
```yaml
inspectit:
tracing:
log-correlation:
trace-id-mdc-injection:
slf4j-enabled: true # Set to "false" to disable slf4J-Support
log4j1-enabled: true # Set to "false" to disable Log4J Version 1 Support
log4j2-enabled: true # Set to "false" to disable Log4J Version 2 Support
```



## Automatical Trace ID Injection

:::warning Experimental Feature
Expand All @@ -60,7 +73,7 @@ Currently, the following logging APIs and Facades are supported:
* `Log4J Version 1`
* `Log4J Version 2`

The automatical trace ID injection is disabled by default. You can enable it using the following configuration snippet:
The automatic trace ID injection is disabled by default. You can enable it using the following configuration snippet:
```yaml
inspectit:
tracing:
Expand Down

0 comments on commit d658c3d

Please sign in to comment.