From c7fdd4e40f6f298ec9050a5052e88c2fc98efa9e Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 27 Jan 2022 16:52:35 +0100 Subject: [PATCH 1/2] Add TraceIdentifierMapAdapter for optimizing Log4j2 correlation --- .../log/shader/TraceIdentifierMapAdapter.java | 158 ++++++++++++++++++ .../shader/TraceIdentifierMapAdapterTest.java | 59 +++++++ 2 files changed, 217 insertions(+) create mode 100644 apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapter.java create mode 100644 apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapterTest.java diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapter.java b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapter.java new file mode 100644 index 0000000000..3a18d9798e --- /dev/null +++ b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapter.java @@ -0,0 +1,158 @@ +package co.elastic.apm.agent.log.shader; + +import co.elastic.apm.agent.impl.GlobalTracer; +import co.elastic.apm.agent.impl.Tracer; +import co.elastic.apm.agent.impl.transaction.Span; +import co.elastic.apm.agent.impl.transaction.Transaction; + +import javax.annotation.Nullable; +import java.util.AbstractMap; +import java.util.AbstractSet; +import java.util.Arrays; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Set; +import java.util.concurrent.Callable; + +public class TraceIdentifierMapAdapter extends AbstractMap { + + private static final TraceIdentifierMapAdapter INSTANCE = new TraceIdentifierMapAdapter(); + + private static final Set> ENTRY_SET = new TraceIdentifierEntrySet(); + private static final List ALL_KEYS = Arrays.asList("trace.id", "transaction.id", "span.id"); + private static final Tracer tracer = GlobalTracer.get(); + private static final List> ENTRIES = Arrays.asList( + new LazyEntry("trace.id", new Callable() { + @Override + @Nullable + public String call() { + Transaction transaction = tracer.currentTransaction(); + if (transaction == null) { + return null; + } + return transaction.getTraceContext().getTraceId().toString(); + } + }), + new LazyEntry("transaction.id", new Callable() { + @Override + @Nullable + public String call() { + Transaction transaction = tracer.currentTransaction(); + if (transaction == null) { + return null; + } + return transaction.getTraceContext().getId().toString(); + } + }), + new LazyEntry("span.id", new Callable() { + @Override + @Nullable + public String call() { + Span span = tracer.getActiveSpan(); + if (span == null) { + return null; + } + return span.getTraceContext().getId().toString(); + } + }) + ); + + public static Map get() { + return INSTANCE; + } + + private TraceIdentifierMapAdapter() { + } + + @Override + public Set> entrySet() { + return ENTRY_SET; + } + + public Iterable allKeys() { + return ALL_KEYS; + } + + private static class TraceIdentifierEntrySet extends AbstractSet> { + + @Override + public int size() { + int size = 0; + for (Entry ignored : this) { + size++; + } + return size; + } + + @Override + public Iterator> iterator() { + return new Iterator>() { + private int i = 0; + @Nullable + private Entry next = findNext(); + + @Override + public boolean hasNext() { + return next != null; + } + + @Override + public Entry next() { + if (next != null) { + try { + return next; + } finally { + next = findNext(); + } + } else { + throw new NoSuchElementException(); + } + } + + @Nullable + private Entry findNext() { + Entry next = null; + while (next == null && i < ENTRIES.size()) { + next = ENTRIES.get(i++); + if (next.getValue() == null) { + next = null; + } + } + return next; + } + }; + } + + } + + private static class LazyEntry implements Entry { + private final String key; + private final Callable valueSupplier; + + private LazyEntry(String key, Callable valueSupplier) { + this.key = key; + this.valueSupplier = valueSupplier; + } + + @Override + public String getKey() { + return this.key; + } + + @Override + public String getValue() { + try { + return this.valueSupplier.call(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + @Override + public String setValue(String value) { + throw new UnsupportedOperationException(); + } + } +} diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapterTest.java b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapterTest.java new file mode 100644 index 0000000000..664920dfb7 --- /dev/null +++ b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapterTest.java @@ -0,0 +1,59 @@ +package co.elastic.apm.agent.log.shader; + +import co.elastic.apm.agent.MockTracer; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.impl.GlobalTracer; +import co.elastic.apm.agent.impl.Scope; +import co.elastic.apm.agent.impl.transaction.Span; +import co.elastic.apm.agent.impl.transaction.Transaction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + + +class TraceIdentifierMapAdapterTest { + + private ElasticApmTracer tracer = MockTracer.createRealTracer(); + + @BeforeEach + void setUp() { + GlobalTracer.init(tracer); + } + + @AfterEach + void tearDown() { + tracer.stop(); + GlobalTracer.setNoop(); + } + + @Test + void testNoContext() { + assertThat(TraceIdentifierMapAdapter.get()).isEmpty(); + } + + @Test + void testTransactionContext() { + Transaction transaction = tracer.startRootTransaction(null); + try (Scope scope = transaction.activateInScope()) { + assertThat(TraceIdentifierMapAdapter.get()).containsOnlyKeys("trace.id", "transaction.id"); + } finally { + transaction.end(); + } + assertThat(TraceIdentifierMapAdapter.get()).isEmpty(); + } + + @Test + void testSpanContext() { + Transaction transaction = tracer.startRootTransaction(null); + Span span = transaction.createSpan(); + try (Scope scope = span.activateInScope()) { + assertThat(TraceIdentifierMapAdapter.get()).containsOnlyKeys("trace.id", "transaction.id", "span.id"); + } finally { + span.end(); + } + transaction.end(); + assertThat(TraceIdentifierMapAdapter.get()).isEmpty(); + } +} From c4174a356406e537cc5458360e372cf1beb836e1 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Fri, 28 Jan 2022 09:28:24 +0100 Subject: [PATCH 2/2] Avoid instrumenting direct logger methods --- .../elastic/apm/agent/bci/IndyBootstrap.java | 15 +++++- .../shader/AbstractLogCorrelationHelper.java | 26 ++++++---- ...AbstractLogCorrelationInstrumentation.java | 52 ------------------- ...pter.java => CorrelationIdMapAdapter.java} | 50 ++++++++++++------ ....java => CorrelationIdMapAdapterTest.java} | 30 ++++++++--- .../shader/LogShadingInstrumentationTest.java | 10 ++-- ...Log4j1TraceCorrelationInstrumentation.java | 30 +++++------ .../log4j2/Log4j2LogCorrelationHelper.java | 11 ++-- ...Log4j2TraceCorrelationInstrumentation.java | 27 +++++----- ...ogbackTraceCorrelationInstrumentation.java | 18 ++++--- 10 files changed, 135 insertions(+), 134 deletions(-) delete mode 100644 apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/AbstractLogCorrelationInstrumentation.java rename apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/{TraceIdentifierMapAdapter.java => CorrelationIdMapAdapter.java} (73%) rename apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/{TraceIdentifierMapAdapterTest.java => CorrelationIdMapAdapterTest.java} (52%) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/IndyBootstrap.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/IndyBootstrap.java index a1dbe47967..b2f9f2c512 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/IndyBootstrap.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/IndyBootstrap.java @@ -22,13 +22,14 @@ import co.elastic.apm.agent.bci.classloading.IndyPluginClassLoader; import co.elastic.apm.agent.bci.classloading.LookupExposer; import co.elastic.apm.agent.common.JvmRuntimeInfo; +import co.elastic.apm.agent.sdk.logging.Logger; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; +import co.elastic.apm.agent.sdk.state.CallDepth; import co.elastic.apm.agent.sdk.state.GlobalState; import co.elastic.apm.agent.util.PackageScanner; import net.bytebuddy.asm.Advice; import net.bytebuddy.dynamic.ClassFileLocator; import net.bytebuddy.dynamic.loading.ClassInjector; -import co.elastic.apm.agent.sdk.logging.Logger; -import co.elastic.apm.agent.sdk.logging.LoggerFactory; import org.stagemonitor.configuration.ConfigurationOptionProvider; import org.stagemonitor.util.IOUtils; @@ -211,6 +212,8 @@ public class IndyBootstrap { @Nullable static Method indyBootstrapMethod; + private static final CallDepth callDepth = CallDepth.get(IndyBootstrap.class); + public static Method getIndyBootstrapMethod(final Logger logger) { if (indyBootstrapMethod != null) { return indyBootstrapMethod; @@ -356,6 +359,12 @@ public static ConstantCallSite bootstrap(MethodHandles.Lookup lookup, MethodType adviceMethodType, Object... args) { try { + if (callDepth.isNestedCallAndIncrement()) { + // avoid re-entrancy and stack overflow errors + // may happen when bootstrapping an instrumentation that also gets triggered during the bootstrap + // for example, adding correlation ids to the thread context when executing logger.debug + return null; + } String adviceClassName = (String) args[0]; int enter = (Integer) args[1]; Class instrumentedType = (Class) args[2]; @@ -411,6 +420,8 @@ public static ConstantCallSite bootstrap(MethodHandles.Lookup lookup, // must not be a static field as it would initialize logging before it's ready LoggerFactory.getLogger(IndyBootstrap.class).error(e.getMessage(), e); return null; + } finally { + callDepth.decrement(); } } diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/AbstractLogCorrelationHelper.java b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/AbstractLogCorrelationHelper.java index 32ebee2715..c4ebe4929d 100644 --- a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/AbstractLogCorrelationHelper.java +++ b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/AbstractLogCorrelationHelper.java @@ -22,14 +22,12 @@ import co.elastic.apm.agent.sdk.state.CallDepth; import javax.annotation.Nullable; +import java.util.Map; public abstract class AbstractLogCorrelationHelper { private static final CallDepth callDepth = CallDepth.get(AbstractLogCorrelationHelper.class); - public static final String TRACE_ID_MDC_KEY = "trace.id"; - public static final String TRANSACTION_ID_MDC_KEY = "transaction.id"; - /** * Adds the active transaction's ID and trace ID to the MDC in the outmost logging API call * @param activeTransaction the currently active transaction, or {@code null} if there is no such @@ -39,8 +37,9 @@ public boolean beforeLoggingApiCall(@Nullable Transaction activeTransaction) { if (callDepth.isNestedCallAndIncrement() || activeTransaction == null) { return false; } - addToMdc(TRACE_ID_MDC_KEY, activeTransaction.getTraceContext().getTraceId().toString()); - addToMdc(TRANSACTION_ID_MDC_KEY, activeTransaction.getTraceContext().getTransactionId().toString()); + addToMdc(CorrelationIdMapAdapter.TRACE_ID, activeTransaction.getTraceContext().getTraceId().toString()); + addToMdc(CorrelationIdMapAdapter.TRANSACTION_ID, activeTransaction.getTraceContext().getTransactionId().toString()); + addToMdc(CorrelationIdMapAdapter.get()); return true; } @@ -50,12 +49,21 @@ public boolean beforeLoggingApiCall(@Nullable Transaction activeTransaction) { */ public void afterLoggingApi(boolean added) { if (callDepth.isNestedCallAndDecrement() && added) { - removeFromMdc(TRACE_ID_MDC_KEY); - removeFromMdc(TRANSACTION_ID_MDC_KEY); + removeFromMdc(CorrelationIdMapAdapter.TRACE_ID); + removeFromMdc(CorrelationIdMapAdapter.TRANSACTION_ID); + removeFromMdc(CorrelationIdMapAdapter.allKeys()); } } - protected abstract void addToMdc(String key, String value); + protected void addToMdc(String key, String value) { + } + + protected void addToMdc(Map correlationIds) { + } - protected abstract void removeFromMdc(String key); + protected void removeFromMdc(String key) { + } + + protected void removeFromMdc(Iterable correlationIdKeys) { + } } diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/AbstractLogCorrelationInstrumentation.java b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/AbstractLogCorrelationInstrumentation.java deleted file mode 100644 index 3fa926f1d9..0000000000 --- a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/AbstractLogCorrelationInstrumentation.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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. - */ -package co.elastic.apm.agent.log.shader; - - -import co.elastic.apm.agent.bci.TracerAwareInstrumentation; -import co.elastic.apm.agent.bci.bytebuddy.CustomElementMatchers; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.matcher.ElementMatcher; - -import static net.bytebuddy.matcher.ElementMatchers.isBootstrapClassLoader; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.not; - -public abstract class AbstractLogCorrelationInstrumentation extends TracerAwareInstrumentation { - - @Override - public ElementMatcher.Junction getClassLoaderMatcher() { - return not(isBootstrapClassLoader()).and(not(CustomElementMatchers.isAgentClassLoader())); - } - - @Override - public ElementMatcher getMethodMatcher() { - return named("trace").or( - named("debug").or( - named("info").or( - named("warn").or( - named("error").or( - named("log") - ) - ) - ) - ) - ); - } -} diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapter.java b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/CorrelationIdMapAdapter.java similarity index 73% rename from apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapter.java rename to apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/CorrelationIdMapAdapter.java index 3a18d9798e..7eaee5fae0 100644 --- a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapter.java +++ b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/main/java/co/elastic/apm/agent/log/shader/CorrelationIdMapAdapter.java @@ -1,8 +1,25 @@ +/* + * 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. + */ package co.elastic.apm.agent.log.shader; import co.elastic.apm.agent.impl.GlobalTracer; import co.elastic.apm.agent.impl.Tracer; -import co.elastic.apm.agent.impl.transaction.Span; import co.elastic.apm.agent.impl.transaction.Transaction; import javax.annotation.Nullable; @@ -16,15 +33,18 @@ import java.util.Set; import java.util.concurrent.Callable; -public class TraceIdentifierMapAdapter extends AbstractMap { +public class CorrelationIdMapAdapter extends AbstractMap { - private static final TraceIdentifierMapAdapter INSTANCE = new TraceIdentifierMapAdapter(); + public static final String TRACE_ID = "trace.id"; + public static final String TRANSACTION_ID = "transaction.id"; + public static final String SPAN_ID = "span.id"; + private static final CorrelationIdMapAdapter INSTANCE = new CorrelationIdMapAdapter(); private static final Set> ENTRY_SET = new TraceIdentifierEntrySet(); - private static final List ALL_KEYS = Arrays.asList("trace.id", "transaction.id", "span.id"); + private static final List ALL_KEYS = Arrays.asList(TRACE_ID, TRANSACTION_ID/*, SPAN_ID*/); private static final Tracer tracer = GlobalTracer.get(); - private static final List> ENTRIES = Arrays.asList( - new LazyEntry("trace.id", new Callable() { + private static final List> ENTRIES = Arrays.>asList( + new LazyEntry(TRACE_ID, new Callable() { @Override @Nullable public String call() { @@ -35,7 +55,7 @@ public String call() { return transaction.getTraceContext().getTraceId().toString(); } }), - new LazyEntry("transaction.id", new Callable() { + new LazyEntry(TRANSACTION_ID, new Callable() { @Override @Nullable public String call() { @@ -45,8 +65,8 @@ public String call() { } return transaction.getTraceContext().getId().toString(); } - }), - new LazyEntry("span.id", new Callable() { + })/*, + new LazyEntry(SPAN_ID, new Callable() { @Override @Nullable public String call() { @@ -56,14 +76,18 @@ public String call() { } return span.getTraceContext().getId().toString(); } - }) + })*/ ); public static Map get() { return INSTANCE; } - private TraceIdentifierMapAdapter() { + public static Iterable allKeys() { + return ALL_KEYS; + } + + private CorrelationIdMapAdapter() { } @Override @@ -71,10 +95,6 @@ public Set> entrySet() { return ENTRY_SET; } - public Iterable allKeys() { - return ALL_KEYS; - } - private static class TraceIdentifierEntrySet extends AbstractSet> { @Override diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapterTest.java b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/CorrelationIdMapAdapterTest.java similarity index 52% rename from apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapterTest.java rename to apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/CorrelationIdMapAdapterTest.java index 664920dfb7..b174652e94 100644 --- a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/TraceIdentifierMapAdapterTest.java +++ b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/CorrelationIdMapAdapterTest.java @@ -1,3 +1,21 @@ +/* + * 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. + */ package co.elastic.apm.agent.log.shader; import co.elastic.apm.agent.MockTracer; @@ -13,7 +31,7 @@ import static org.assertj.core.api.Assertions.assertThat; -class TraceIdentifierMapAdapterTest { +class CorrelationIdMapAdapterTest { private ElasticApmTracer tracer = MockTracer.createRealTracer(); @@ -30,18 +48,18 @@ void tearDown() { @Test void testNoContext() { - assertThat(TraceIdentifierMapAdapter.get()).isEmpty(); + assertThat(CorrelationIdMapAdapter.get()).isEmpty(); } @Test void testTransactionContext() { Transaction transaction = tracer.startRootTransaction(null); try (Scope scope = transaction.activateInScope()) { - assertThat(TraceIdentifierMapAdapter.get()).containsOnlyKeys("trace.id", "transaction.id"); + assertThat(CorrelationIdMapAdapter.get()).containsOnlyKeys("trace.id", "transaction.id"); } finally { transaction.end(); } - assertThat(TraceIdentifierMapAdapter.get()).isEmpty(); + assertThat(CorrelationIdMapAdapter.get()).isEmpty(); } @Test @@ -49,11 +67,11 @@ void testSpanContext() { Transaction transaction = tracer.startRootTransaction(null); Span span = transaction.createSpan(); try (Scope scope = span.activateInScope()) { - assertThat(TraceIdentifierMapAdapter.get()).containsOnlyKeys("trace.id", "transaction.id", "span.id"); + assertThat(CorrelationIdMapAdapter.get()).containsOnlyKeys("trace.id", "transaction.id"); } finally { span.end(); } transaction.end(); - assertThat(TraceIdentifierMapAdapter.get()).isEmpty(); + assertThat(CorrelationIdMapAdapter.get()).isEmpty(); } } diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/LogShadingInstrumentationTest.java b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/LogShadingInstrumentationTest.java index ed7a4f09a7..c90dac1006 100644 --- a/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/LogShadingInstrumentationTest.java +++ b/apm-agent-plugins/apm-log-shader-plugin/apm-log-shader-plugin-common/src/test/java/co/elastic/apm/agent/log/shader/LogShadingInstrumentationTest.java @@ -45,8 +45,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static co.elastic.apm.agent.log.shader.AbstractLogCorrelationHelper.TRACE_ID_MDC_KEY; -import static co.elastic.apm.agent.log.shader.AbstractLogCorrelationHelper.TRANSACTION_ID_MDC_KEY; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; @@ -286,8 +284,8 @@ private void verifyEcsLogLine(JsonNode ecsLogLineTree) { assertThat(ecsLogLineTree.get("event.dataset").textValue()).isEqualTo(serviceName + ".FILE"); assertThat(ecsLogLineTree.get("service.version").textValue()).isEqualTo("v42"); assertThat(ecsLogLineTree.get("some.field").textValue()).isEqualTo("some-value"); - assertThat(ecsLogLineTree.get(TRACE_ID_MDC_KEY).textValue()).isEqualTo(transaction.getTraceContext().getTraceId().toString()); - assertThat(ecsLogLineTree.get(TRANSACTION_ID_MDC_KEY).textValue()).isEqualTo(transaction.getTraceContext().getTransactionId().toString()); + assertThat(ecsLogLineTree.get("trace.id").textValue()).isEqualTo(transaction.getTraceContext().getTraceId().toString()); + assertThat(ecsLogLineTree.get("transaction.id").textValue()).isEqualTo(transaction.getTraceContext().getTransactionId().toString()); } private ArrayList readShadeLogFile() throws IOException { @@ -325,8 +323,8 @@ private void verifyEcsFormat(String[] splitRawLogLine, JsonNode ecsLogLineTree) assertThat(ecsLogLineTree.get("event.dataset").textValue()).isEqualTo(serviceName + ".FILE"); assertThat(ecsLogLineTree.get("service.version").textValue()).isEqualTo("v42"); assertThat(ecsLogLineTree.get("some.field").textValue()).isEqualTo("some-value"); - assertThat(ecsLogLineTree.get(TRACE_ID_MDC_KEY).textValue()).isEqualTo(transaction.getTraceContext().getTraceId().toString()); - assertThat(ecsLogLineTree.get(TRANSACTION_ID_MDC_KEY).textValue()).isEqualTo(transaction.getTraceContext().getTransactionId().toString()); + assertThat(ecsLogLineTree.get("trace.id").textValue()).isEqualTo(transaction.getTraceContext().getTraceId().toString()); + assertThat(ecsLogLineTree.get("transaction.id").textValue()).isEqualTo(transaction.getTraceContext().getTransactionId().toString()); } /** diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-log4j1-plugin/src/main/java/co/elastic/apm/agent/log4j1/Log4j1TraceCorrelationInstrumentation.java b/apm-agent-plugins/apm-log-shader-plugin/apm-log4j1-plugin/src/main/java/co/elastic/apm/agent/log4j1/Log4j1TraceCorrelationInstrumentation.java index ffdf7ee76b..7db5f5525d 100644 --- a/apm-agent-plugins/apm-log-shader-plugin/apm-log4j1-plugin/src/main/java/co/elastic/apm/agent/log4j1/Log4j1TraceCorrelationInstrumentation.java +++ b/apm-agent-plugins/apm-log-shader-plugin/apm-log4j1-plugin/src/main/java/co/elastic/apm/agent/log4j1/Log4j1TraceCorrelationInstrumentation.java @@ -18,46 +18,40 @@ */ package co.elastic.apm.agent.log4j1; -import co.elastic.apm.agent.log.shader.AbstractLogCorrelationInstrumentation; +import co.elastic.apm.agent.bci.TracerAwareInstrumentation; import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +import org.apache.log4j.spi.LoggingEvent; import java.util.Collection; import java.util.Collections; -import static co.elastic.apm.agent.bci.bytebuddy.CustomElementMatchers.classLoaderCanLoadClass; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; -import static net.bytebuddy.matcher.ElementMatchers.nameContains; import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; -public class Log4j1TraceCorrelationInstrumentation extends AbstractLogCorrelationInstrumentation { +/** + * Instruments {@link org.apache.log4j.Category#callAppenders(LoggingEvent)} + */ +public class Log4j1TraceCorrelationInstrumentation extends TracerAwareInstrumentation { @Override public Collection getInstrumentationGroupNames() { return Collections.singleton("log4j1-correlation"); } - @Override - public ElementMatcher.Junction getClassLoaderMatcher() { - return super.getClassLoaderMatcher().and(classLoaderCanLoadClass("org.apache.log4j.Category")); - } - - @Override - public ElementMatcher getTypeMatcherPreFilter() { - return named("org.apache.log4j.Category").or(nameContains("Logger")); - } - @Override public ElementMatcher getTypeMatcher() { - return hasSuperType(named("org.apache.log4j.Category")); + return named("org.apache.log4j.Category"); } @Override public ElementMatcher getMethodMatcher() { - return named("fatal").or(super.getMethodMatcher()); + return named("callAppenders") + .and(takesArguments(1)) + .and(takesArgument(0, named("org.apache.log4j.spi.LoggingEvent"))); } public static class AdviceClass { diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/Log4j2LogCorrelationHelper.java b/apm-agent-plugins/apm-log-shader-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/Log4j2LogCorrelationHelper.java index b5e93d027a..fafc95a0ac 100644 --- a/apm-agent-plugins/apm-log-shader-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/Log4j2LogCorrelationHelper.java +++ b/apm-agent-plugins/apm-log-shader-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/Log4j2LogCorrelationHelper.java @@ -21,14 +21,17 @@ import co.elastic.apm.agent.log.shader.AbstractLogCorrelationHelper; import org.apache.logging.log4j.ThreadContext; +import java.util.Map; + public class Log4j2LogCorrelationHelper extends AbstractLogCorrelationHelper { + @Override - protected void addToMdc(String key, String value) { - ThreadContext.put(key, value); + protected void addToMdc(Map correlationIds) { + ThreadContext.putAll(correlationIds); } @Override - protected void removeFromMdc(String key) { - ThreadContext.remove(key); + protected void removeFromMdc(Iterable correlationIdKeys) { + ThreadContext.removeAll(correlationIdKeys); } } diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/Log4j2TraceCorrelationInstrumentation.java b/apm-agent-plugins/apm-log-shader-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/Log4j2TraceCorrelationInstrumentation.java index 980a945ccd..f49bdbad85 100644 --- a/apm-agent-plugins/apm-log-shader-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/Log4j2TraceCorrelationInstrumentation.java +++ b/apm-agent-plugins/apm-log-shader-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/Log4j2TraceCorrelationInstrumentation.java @@ -18,7 +18,7 @@ */ package co.elastic.apm.agent.log4j2; -import co.elastic.apm.agent.log.shader.AbstractLogCorrelationInstrumentation; +import co.elastic.apm.agent.bci.TracerAwareInstrumentation; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; @@ -28,39 +28,36 @@ import java.util.Collection; import java.util.Collections; -import static co.elastic.apm.agent.bci.bytebuddy.CustomElementMatchers.classLoaderCanLoadClass; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; -import static net.bytebuddy.matcher.ElementMatchers.nameContains; +import static net.bytebuddy.matcher.ElementMatchers.nameEndsWith; import static net.bytebuddy.matcher.ElementMatchers.named; -public class Log4j2TraceCorrelationInstrumentation extends AbstractLogCorrelationInstrumentation { +/** + * Instruments {@link org.apache.logging.log4j.core.impl.LogEventFactory#createEvent} + */ +public class Log4j2TraceCorrelationInstrumentation extends TracerAwareInstrumentation { @Override public Collection getInstrumentationGroupNames() { return Collections.singleton("log4j2-correlation"); } - @Override - public ElementMatcher.Junction getClassLoaderMatcher() { - return super.getClassLoaderMatcher().and(classLoaderCanLoadClass("org.apache.logging.log4j.Logger")); - } - @Override public ElementMatcher getTypeMatcherPreFilter() { - return nameContains("Logger"); + return nameEndsWith("LogEventFactory"); } @Override public ElementMatcher getTypeMatcher() { - return hasSuperType(named("org.apache.logging.log4j.Logger")); + return hasSuperType( + named("org.apache.logging.log4j.core.impl.LogEventFactory") + .or(named("org.apache.logging.log4j.core.impl.LocationAwareLogEventFactory")) + ); } @Override public ElementMatcher getMethodMatcher() { - return named("fatal").or( - named("catching").or( - super.getMethodMatcher()) - ); + return named("createEvent"); } public static class AdviceClass { diff --git a/apm-agent-plugins/apm-log-shader-plugin/apm-logback-plugin/apm-logback-plugin-impl/src/main/java/co/elastic/apm/agent/logback/LogbackTraceCorrelationInstrumentation.java b/apm-agent-plugins/apm-log-shader-plugin/apm-logback-plugin/apm-logback-plugin-impl/src/main/java/co/elastic/apm/agent/logback/LogbackTraceCorrelationInstrumentation.java index 790db0e37e..fc23655d93 100644 --- a/apm-agent-plugins/apm-log-shader-plugin/apm-logback-plugin/apm-logback-plugin-impl/src/main/java/co/elastic/apm/agent/logback/LogbackTraceCorrelationInstrumentation.java +++ b/apm-agent-plugins/apm-log-shader-plugin/apm-logback-plugin/apm-logback-plugin-impl/src/main/java/co/elastic/apm/agent/logback/LogbackTraceCorrelationInstrumentation.java @@ -18,18 +18,22 @@ */ package co.elastic.apm.agent.logback; -import co.elastic.apm.agent.log.shader.AbstractLogCorrelationInstrumentation; +import ch.qos.logback.classic.spi.ILoggingEvent; +import co.elastic.apm.agent.bci.TracerAwareInstrumentation; import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import java.util.Collection; import java.util.Collections; -import static co.elastic.apm.agent.bci.bytebuddy.CustomElementMatchers.classLoaderCanLoadClass; import static net.bytebuddy.matcher.ElementMatchers.named; -public class LogbackTraceCorrelationInstrumentation extends AbstractLogCorrelationInstrumentation { +/** + * Instruments {@link ch.qos.logback.classic.Logger#callAppenders(ILoggingEvent)} + */ +public class LogbackTraceCorrelationInstrumentation extends TracerAwareInstrumentation { @Override public Collection getInstrumentationGroupNames() { @@ -37,13 +41,13 @@ public Collection getInstrumentationGroupNames() { } @Override - public ElementMatcher.Junction getClassLoaderMatcher() { - return super.getClassLoaderMatcher().and(classLoaderCanLoadClass("ch.qos.logback.classic.Logger")); + public ElementMatcher getTypeMatcher() { + return named("ch.qos.logback.classic.Logger"); } @Override - public ElementMatcher getTypeMatcher() { - return named("ch.qos.logback.classic.Logger"); + public ElementMatcher getMethodMatcher() { + return named("callAppenders"); } public static class AdviceClass {