From 89bbb049e3e03631be7102a8dedac7d0e6e415b3 Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Thu, 3 Feb 2022 13:37:08 +0100 Subject: [PATCH 1/5] Added `Transaction#setApplicationClassLoader`to public api --- .../co/elastic/apm/api/NoopTransaction.java | 6 ++++ .../java/co/elastic/apm/api/Transaction.java | 12 ++++++++ .../co/elastic/apm/api/TransactionImpl.java | 6 ++++ .../agent/impl/transaction/TraceContext.java | 2 +- .../pluginapi/TransactionInstrumentation.java | 29 +++++++++++++++++++ ...ic.apm.agent.sdk.ElasticApmInstrumentation | 1 + .../TransactionInstrumentationTest.java | 22 ++++++++++++++ docs/public-api.asciidoc | 9 ++++++ 8 files changed, 86 insertions(+), 1 deletion(-) diff --git a/apm-agent-api/src/main/java/co/elastic/apm/api/NoopTransaction.java b/apm-agent-api/src/main/java/co/elastic/apm/api/NoopTransaction.java index fe66caf0da..9869cbe31c 100644 --- a/apm-agent-api/src/main/java/co/elastic/apm/api/NoopTransaction.java +++ b/apm-agent-api/src/main/java/co/elastic/apm/api/NoopTransaction.java @@ -226,4 +226,10 @@ public Span setDestinationAddress(@Nullable String address, int port) { public Span setDestinationService(@Nullable String resource) { return this; } + + @Nonnull + @Override + public Transaction setApplicationClassLoader(@Nullable ClassLoader classLoader) { + return this; + } } diff --git a/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java b/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java index a4a4f1d4ee..f8902509f9 100644 --- a/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java +++ b/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java @@ -331,4 +331,16 @@ public interface Transaction extends Span { @Override Scope activate(); + /** + * Sets the class loader of the application that started the transaction. + *

+ * The class loader is used to determine the service name and version of the transaction. If this method is called + * after child spans are already created, they may be associated with the wrong service name and version. + *

+ * + * @param classLoader the class loader of the application that started the transaction + * @return this + */ + @Nonnull + Transaction setApplicationClassLoader(ClassLoader classLoader); } diff --git a/apm-agent-api/src/main/java/co/elastic/apm/api/TransactionImpl.java b/apm-agent-api/src/main/java/co/elastic/apm/api/TransactionImpl.java index da0ce5bfe5..5cfe35cd62 100644 --- a/apm-agent-api/src/main/java/co/elastic/apm/api/TransactionImpl.java +++ b/apm-agent-api/src/main/java/co/elastic/apm/api/TransactionImpl.java @@ -186,4 +186,10 @@ public Transaction setDestinationAddress(@Nullable String address, int port) { public Transaction setDestinationService(@Nullable String resource) { throw new UnsupportedOperationException(); } + + @Nonnull + public Transaction setApplicationClassLoader(ClassLoader classLoader) { + // co.elastic.apm.agent.pluginapi.TransactionInstrumentation.SetClassLoaderInstrumentation + return this; + } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java index 455e6272b6..8d0be03982 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java @@ -732,7 +732,7 @@ public int hashCode() { return Objects.hash(traceId, id, parentId, flags); } - void setApplicationClassLoader(@Nullable ClassLoader classLoader) { + public void setApplicationClassLoader(@Nullable ClassLoader classLoader) { if (ClassLoaderUtils.isBootstrapClassLoader(classLoader) || ClassLoaderUtils.isAgentClassLoader(classLoader)) { return; } diff --git a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java index d96638fabd..318c5c3e7e 100644 --- a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java +++ b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java @@ -18,6 +18,9 @@ */ package co.elastic.apm.agent.pluginapi; +import co.elastic.apm.agent.configuration.ServiceInfo; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.impl.GlobalTracer; import co.elastic.apm.agent.impl.transaction.Id; import co.elastic.apm.agent.impl.transaction.TraceContext; import co.elastic.apm.agent.impl.transaction.Transaction; @@ -147,4 +150,30 @@ public static void addCustomContext(@Advice.FieldValue(value = "span", typing = } } } + + public static class SetClassLoaderInstrumentation extends TransactionInstrumentation { + public SetClassLoaderInstrumentation() { + super(named("setApplicationClassLoader")); + } + + public static class AdviceClass { + + private static final ElasticApmTracer tracer = GlobalTracer.getTracerImpl(); + + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + public static void setClassLoader(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object transaction, + @Advice.Argument(0) ClassLoader classLoader) { + if (transaction instanceof Transaction) { + TraceContext traceContext = ((Transaction) transaction).getTraceContext(); + traceContext.setApplicationClassLoader(classLoader); + + ServiceInfo serviceInfo = tracer.getServiceInfo(classLoader); + if (serviceInfo != null) { + traceContext.setServiceName(serviceInfo.getServiceName()); + traceContext.setServiceVersion(serviceInfo.getServiceVersion()); + } + } + } + } + } } diff --git a/apm-agent-plugins/apm-api-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation b/apm-agent-plugins/apm-api-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation index 161e9bceca..031ba28588 100644 --- a/apm-agent-plugins/apm-api-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation +++ b/apm-agent-plugins/apm-api-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation @@ -7,6 +7,7 @@ co.elastic.apm.agent.pluginapi.TransactionInstrumentation$SetFrameworkNameInstru co.elastic.apm.agent.pluginapi.TransactionInstrumentation$SetUserInstrumentation co.elastic.apm.agent.pluginapi.TransactionInstrumentation$EnsureParentIdInstrumentation co.elastic.apm.agent.pluginapi.TransactionInstrumentation$SetResultInstrumentation +co.elastic.apm.agent.pluginapi.TransactionInstrumentation$SetClassLoaderInstrumentation co.elastic.apm.agent.pluginapi.TransactionInstrumentation$AddCustomContextInstrumentation co.elastic.apm.agent.pluginapi.AbstractSpanInstrumentation$SetNameInstrumentation co.elastic.apm.agent.pluginapi.AbstractSpanInstrumentation$SetTypeInstrumentation diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java index cd84e97cba..e3263b959e 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java @@ -19,10 +19,12 @@ package co.elastic.apm.agent.pluginapi; import co.elastic.apm.AbstractApiTest; +import co.elastic.apm.agent.configuration.ServiceInfo; import co.elastic.apm.agent.impl.TracerInternalApiUtils; import co.elastic.apm.api.AbstractSpanImplAccessor; import co.elastic.apm.api.ElasticApm; import co.elastic.apm.api.Outcome; +import co.elastic.apm.api.Scope; import co.elastic.apm.api.Span; import co.elastic.apm.api.Transaction; import org.apache.commons.lang3.RandomStringUtils; @@ -32,6 +34,8 @@ import org.junit.jupiter.params.provider.MethodSource; import javax.annotation.Nullable; +import java.net.URL; +import java.net.URLClassLoader; import java.security.SecureRandom; import static org.assertj.core.api.Assertions.assertThat; @@ -296,6 +300,24 @@ void setOutcome_success() { testSetOutcome(Outcome.SUCCESS); } + @Test + void setClassLoader() { + ClassLoader classLoader = new URLClassLoader(new URL[0]); + + tracer.overrideServiceInfoForClassLoader(classLoader, ServiceInfo.of("My Service", "My Version")); + + transaction.setApplicationClassLoader(classLoader); + try (Scope scope = transaction.activate()) { + assertThat(tracer.getActive().getTraceContext().getApplicationClassLoader()).isSameAs(classLoader); + } finally { + endTransaction(); + } + + co.elastic.apm.agent.impl.transaction.Transaction reportedTransaction = reporter.getFirstTransaction(); + assertThat(reportedTransaction.getTraceContext().getServiceName()).isEqualTo("My Service"); + assertThat(reportedTransaction.getTraceContext().getServiceVersion()).isEqualTo("My Version"); + } + private void testSetOutcome(Outcome outcome) { // set it first to a different value than the expected one Outcome[] values = Outcome.values(); diff --git a/docs/public-api.asciidoc b/docs/public-api.asciidoc index 2861f1c11e..7a290c2288 100644 --- a/docs/public-api.asciidoc +++ b/docs/public-api.asciidoc @@ -551,6 +551,15 @@ use `UNKNOWN` when the outcome can't be properly known. Outcome is used to compute error rates between services, using `UNKNOWN` will not alter those rates. The value set through API will have higher priority over the value that might be set by auto-instrumentation. +[float] +[[api-transaction-set-class-loader]] +==== `Transaction setApplicationClassLoader(ClassLoader classLoader)` +Sets the class loader of the application that started the transaction. + +* `classLoader`` the class loader of the application that started the transaction + +NOTE: The class loader is used to determine the service name and version of the transaction. If this method is called after +child spans are already created, they may be associated with the wrong service name and version. [float] [[api-transaction-set-start-timestamp]] From dc6380509777fd0e6c81e5de66ec72c8ecfed5d7 Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Thu, 3 Feb 2022 13:48:45 +0100 Subject: [PATCH 2/5] Added #2444 to the changelog --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index dc6350bfdb..369090ebef 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -43,6 +43,7 @@ endif::[] * Added support for instrumenting Struts 2 static resource requests - {pull}1949[#1949] * Added support for Java/Jakarta WebSocket ServerEndpoint - {pull}2281[#2281] * Added support for setting the service name on Log4j2's EcsLayout - {pull}2296[#2296] +* Added support for setting the application class loader for a transaction via the public api - {pull}2444[#2444] [float] ===== Bug fixes From ed4781e240a5e0c660a8233453890b2d4aed7451 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 3 Feb 2022 14:39:49 +0100 Subject: [PATCH 3/5] Auto-set service info when setting class loader in TraceContext --- .../apm/agent/impl/ElasticApmTracer.java | 19 +++++-------------- .../agent/impl/transaction/TraceContext.java | 10 ++++++++-- .../pluginapi/TransactionInstrumentation.java | 11 ----------- docs/public-api.asciidoc | 2 +- 4 files changed, 14 insertions(+), 28 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java index e1d6557bb5..df41dc3cc1 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java @@ -173,7 +173,7 @@ public Transaction startRootTransaction(Sampler sampler, long epochMicros, @Null Transaction transaction = null; if (isRunning()) { transaction = createTransaction().start(TraceContext.asRoot(), null, epochMicros, sampler, initiatingClassLoader); - afterTransactionStart(initiatingClassLoader, transaction); + afterTransactionStart(transaction); } return transaction; } @@ -198,7 +198,7 @@ public Transaction startChildTransaction(@Nullable C headerCarrier, TextHead if (isRunning()) { transaction = createTransaction().start(TraceContext.getFromTraceContextTextHeaders(), headerCarrier, textHeadersGetter, epochMicros, sampler, initiatingClassLoader); - afterTransactionStart(initiatingClassLoader, transaction); + afterTransactionStart(transaction); } return transaction; } @@ -217,12 +217,12 @@ public Transaction startChildTransaction(@Nullable C headerCarrier, BinaryHe if (isRunning()) { transaction = createTransaction().start(TraceContext.getFromTraceContextBinaryHeaders(), headerCarrier, binaryHeadersGetter, epochMicros, sampler, initiatingClassLoader); - afterTransactionStart(initiatingClassLoader, transaction); + afterTransactionStart(transaction); } return transaction; } - private void afterTransactionStart(@Nullable ClassLoader initiatingClassLoader, Transaction transaction) { + private void afterTransactionStart(Transaction transaction) { if (logger.isDebugEnabled()) { logger.debug("startTransaction {}", transaction); if (logger.isTraceEnabled()) { @@ -230,11 +230,6 @@ private void afterTransactionStart(@Nullable ClassLoader initiatingClassLoader, new RuntimeException("this exception is just used to record where the transaction has been started from")); } } - final ServiceInfo serviceInfo = getServiceInfo(initiatingClassLoader); - if (serviceInfo != null) { - transaction.getTraceContext().setServiceName(serviceInfo.getServiceName()); - transaction.getTraceContext().setServiceVersion(serviceInfo.getServiceVersion()); - } } public Transaction noopTransaction() { @@ -343,11 +338,7 @@ private ErrorCapture captureException(long epochMicros, @Nullable Throwable e, @ parent.setNonDiscardable(); } else { error.getTraceContext().getId().setToRandomValue(); - ServiceInfo serviceInfo = getServiceInfo(initiatingClassLoader); - if (serviceInfo != null) { - error.getTraceContext().setServiceName(serviceInfo.getServiceName()); - error.getTraceContext().setServiceVersion(serviceInfo.getServiceVersion()); - } + error.getTraceContext().setApplicationClassLoader(initiatingClassLoader); } return error; } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java index 8d0be03982..29352392dd 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java @@ -19,17 +19,18 @@ package co.elastic.apm.agent.impl.transaction; import co.elastic.apm.agent.configuration.CoreConfiguration; +import co.elastic.apm.agent.configuration.ServiceInfo; import co.elastic.apm.agent.impl.ElasticApmTracer; import co.elastic.apm.agent.impl.Tracer; import co.elastic.apm.agent.impl.sampling.Sampler; import co.elastic.apm.agent.objectpool.Recyclable; +import co.elastic.apm.agent.sdk.logging.Logger; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent; import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap; import co.elastic.apm.agent.util.ByteUtils; import co.elastic.apm.agent.util.ClassLoaderUtils; import co.elastic.apm.agent.util.HexUtils; -import co.elastic.apm.agent.sdk.logging.Logger; -import co.elastic.apm.agent.sdk.logging.LoggerFactory; import javax.annotation.Nullable; import java.lang.ref.WeakReference; @@ -742,6 +743,11 @@ public void setApplicationClassLoader(@Nullable ClassLoader classLoader) { classLoaderWeakReferenceCache.putIfAbsent(classLoader, local); } applicationClassLoader = local; + final ServiceInfo serviceInfo = tracer.getServiceInfo(classLoader); + if (serviceInfo != null) { + setServiceName(serviceInfo.getServiceName()); + setServiceVersion(serviceInfo.getServiceVersion()); + } } @Nullable diff --git a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java index 318c5c3e7e..6c5bae4839 100644 --- a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java +++ b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java @@ -18,9 +18,6 @@ */ package co.elastic.apm.agent.pluginapi; -import co.elastic.apm.agent.configuration.ServiceInfo; -import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.impl.GlobalTracer; import co.elastic.apm.agent.impl.transaction.Id; import co.elastic.apm.agent.impl.transaction.TraceContext; import co.elastic.apm.agent.impl.transaction.Transaction; @@ -158,20 +155,12 @@ public SetClassLoaderInstrumentation() { public static class AdviceClass { - private static final ElasticApmTracer tracer = GlobalTracer.getTracerImpl(); - @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) public static void setClassLoader(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object transaction, @Advice.Argument(0) ClassLoader classLoader) { if (transaction instanceof Transaction) { TraceContext traceContext = ((Transaction) transaction).getTraceContext(); traceContext.setApplicationClassLoader(classLoader); - - ServiceInfo serviceInfo = tracer.getServiceInfo(classLoader); - if (serviceInfo != null) { - traceContext.setServiceName(serviceInfo.getServiceName()); - traceContext.setServiceVersion(serviceInfo.getServiceVersion()); - } } } } diff --git a/docs/public-api.asciidoc b/docs/public-api.asciidoc index 7a290c2288..db6b8ba952 100644 --- a/docs/public-api.asciidoc +++ b/docs/public-api.asciidoc @@ -556,7 +556,7 @@ The value set through API will have higher priority over the value that might be ==== `Transaction setApplicationClassLoader(ClassLoader classLoader)` Sets the class loader of the application that started the transaction. -* `classLoader`` the class loader of the application that started the transaction +* `classLoader` the class loader of the application that started the transaction NOTE: The class loader is used to determine the service name and version of the transaction. If this method is called after child spans are already created, they may be associated with the wrong service name and version. From 2422a89138e7ec1f83a77b3f5312b1959ee898da Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 3 Feb 2022 15:40:15 +0100 Subject: [PATCH 4/5] Fix Javadoc --- apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java b/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java index f8902509f9..f929f26067 100644 --- a/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java +++ b/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java @@ -333,7 +333,7 @@ public interface Transaction extends Span { /** * Sets the class loader of the application that started the transaction. - *

+ *

* The class loader is used to determine the service name and version of the transaction. If this method is called * after child spans are already created, they may be associated with the wrong service name and version. *

From 71051a98a50c66bfc7909d07bf997bfbb40c4780 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 3 Feb 2022 17:20:45 +0100 Subject: [PATCH 5/5] Fix InitServiceNameInstrumentationTest --- .../elastic/apm/agent/impl/transaction/TraceContext.java | 2 +- .../co/elastic/apm/agent/mdc/MdcActivationListener.java | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java index 29352392dd..4f5b98a9c1 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java @@ -734,7 +734,7 @@ public int hashCode() { } public void setApplicationClassLoader(@Nullable ClassLoader classLoader) { - if (ClassLoaderUtils.isBootstrapClassLoader(classLoader) || ClassLoaderUtils.isAgentClassLoader(classLoader)) { + if (ClassLoaderUtils.isBootstrapClassLoader(classLoader)) { return; } WeakReference local = classLoaderWeakReferenceCache.get(classLoader); diff --git a/apm-agent-plugins/apm-log-correlation-plugin/src/main/java/co/elastic/apm/agent/mdc/MdcActivationListener.java b/apm-agent-plugins/apm-log-correlation-plugin/src/main/java/co/elastic/apm/agent/mdc/MdcActivationListener.java index 769e94a800..7e3786f8c7 100644 --- a/apm-agent-plugins/apm-log-correlation-plugin/src/main/java/co/elastic/apm/agent/mdc/MdcActivationListener.java +++ b/apm-agent-plugins/apm-log-correlation-plugin/src/main/java/co/elastic/apm/agent/mdc/MdcActivationListener.java @@ -27,6 +27,7 @@ import co.elastic.apm.agent.logging.LoggingConfiguration; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; +import co.elastic.apm.agent.util.ClassLoaderUtils; import javax.annotation.Nullable; import java.lang.invoke.MethodHandle; @@ -227,10 +228,10 @@ public void after(TraceContext deactivatedContext, boolean isError) throws Throw */ private ClassLoader getApplicationClassLoader(TraceContext context) { ClassLoader applicationClassLoader = context.getApplicationClassLoader(); - if (applicationClassLoader != null) { - return applicationClassLoader; - } else { + if (applicationClassLoader == null || ClassLoaderUtils.isAgentClassLoader(applicationClassLoader)) { return getFallbackClassLoader(); + } else { + return applicationClassLoader; } } private ClassLoader getFallbackClassLoader() {