From 4fbc7ad276eb6866db06c3fedf02e492d2d0aae0 Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Wed, 22 Dec 2021 20:38:00 +0100 Subject: [PATCH 1/2] Added support for setting the application class loader when starting a transaction via the public api --- .../java/co/elastic/apm/api/ElasticApm.java | 50 +++++++++++++++++-- .../ElasticApmApiInstrumentation.java | 15 +++--- .../api/ElasticApmApiInstrumentationTest.java | 48 +++++++++++++++++- docs/public-api.asciidoc | 26 ++++++++++ 4 files changed, 127 insertions(+), 12 deletions(-) diff --git a/apm-agent-api/src/main/java/co/elastic/apm/api/ElasticApm.java b/apm-agent-api/src/main/java/co/elastic/apm/api/ElasticApm.java index 640299fcac..a67ab22b33 100644 --- a/apm-agent-api/src/main/java/co/elastic/apm/api/ElasticApm.java +++ b/apm-agent-api/src/main/java/co/elastic/apm/api/ElasticApm.java @@ -74,11 +74,23 @@ public class ElasticApm { */ @Nonnull public static Transaction startTransaction() { - Object transaction = doStartTransaction(); + return startTransaction(null); + } + + /** + * Similar to {@link ElasticApm#startTransaction()} but with a specific application class loader. + * + * @param classLoader the application class loader + * @return the started transaction. + * @since 1.29.0 + */ + @Nonnull + public static Transaction startTransaction(@Nullable ClassLoader classLoader) { + Object transaction = doStartTransaction(classLoader); return transaction != null ? new TransactionImpl(transaction) : NoopTransaction.INSTANCE; } - private static Object doStartTransaction() { + private static Object doStartTransaction(ClassLoader classLoader) { // co.elastic.apm.api.ElasticApmInstrumentation.StartTransactionInstrumentation.doStartTransaction return null; } @@ -117,7 +129,20 @@ private static Object doStartTransaction() { */ @Nonnull public static Transaction startTransactionWithRemoteParent(final HeaderExtractor headerExtractor) { - return startTransactionWithRemoteParent(headerExtractor, null); + return startTransactionWithRemoteParent(headerExtractor, null, null); + } + + /** + * Similar to {@link ElasticApm#startTransactionWithRemoteParent(HeaderExtractor)} but with a specific application class loader. + * + * @param headerExtractor a function which receives a header name and returns the fist header with that name + * @param classLoader the application class loader + * @return the started transaction + * @since 1.29.0 + */ + @Nonnull + public static Transaction startTransactionWithRemoteParent(final HeaderExtractor headerExtractor, @Nullable ClassLoader classLoader) { + return startTransactionWithRemoteParent(headerExtractor, null, classLoader); } /** @@ -156,13 +181,28 @@ public static Transaction startTransactionWithRemoteParent(final HeaderExtractor */ @Nonnull public static Transaction startTransactionWithRemoteParent(HeaderExtractor headerExtractor, HeadersExtractor headersExtractor) { + return startTransactionWithRemoteParent(headerExtractor, headersExtractor, null); + } + + /** + * Similar to {@link ElasticApm#startTransactionWithRemoteParent(HeaderExtractor, HeadersExtractor)} but with a specific application class loader. + * + * @param headerExtractor a function which receives a header name and returns the fist header with that name + * @param headersExtractor a function which receives a header name and returns all headers with that name + * @param classLoader the application class loader + * @return the started transaction + * @since 1.29.0 + */ + @Nonnull + public static Transaction startTransactionWithRemoteParent(HeaderExtractor headerExtractor, HeadersExtractor headersExtractor, @Nullable ClassLoader classLoader) { Object transaction = doStartTransactionWithRemoteParentFunction(ApiMethodHandles.GET_FIRST_HEADER, headerExtractor, - ApiMethodHandles.GET_ALL_HEADERS, headersExtractor); + ApiMethodHandles.GET_ALL_HEADERS, headersExtractor, classLoader); return transaction != null ? new TransactionImpl(transaction) : NoopTransaction.INSTANCE; } private static Object doStartTransactionWithRemoteParentFunction(MethodHandle getFirstHeader, HeaderExtractor headerExtractor, - MethodHandle getAllHeaders, HeadersExtractor headersExtractor) { + MethodHandle getAllHeaders, HeadersExtractor headersExtractor, + ClassLoader classLoader) { // co.elastic.apm.agent.pluginapi.ElasticApmApiInstrumentation.StartTransactionWithRemoteParentInstrumentation return null; } diff --git a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/ElasticApmApiInstrumentation.java b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/ElasticApmApiInstrumentation.java index 471ae0ff02..da0b547965 100644 --- a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/ElasticApmApiInstrumentation.java +++ b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/ElasticApmApiInstrumentation.java @@ -60,8 +60,8 @@ public static class AdviceClass { @Nullable @Advice.AssignReturned.ToReturned @Advice.OnMethodExit(suppress = Throwable.class, inline = false) - public static Object doStartTransaction(@Advice.Origin Class clazz) { - Transaction transaction = tracer.startRootTransaction(clazz.getClassLoader()); + public static Object doStartTransaction(@Advice.Origin Class clazz, @Advice.Argument(value = 0, optional = true) @Nullable ClassLoader classLoader) { + Transaction transaction = tracer.startRootTransaction(classLoader != null ? classLoader : clazz.getClassLoader()); if (transaction != null) { transaction.setFrameworkName(Utils.FRAMEWORK_NAME); } @@ -85,16 +85,19 @@ public static Object doStartTransaction(@Advice.Origin Class clazz, @Advice.Argument(0) MethodHandle getFirstHeader, @Advice.Argument(1) @Nullable Object headerExtractor, @Advice.Argument(2) MethodHandle getAllHeaders, - @Advice.Argument(3) @Nullable Object headersExtractor) { + @Advice.Argument(3) @Nullable Object headersExtractor, + @Advice.Argument(value = 4, optional = true) @Nullable ClassLoader classLoader) { + ClassLoader initiatingClassLoader = classLoader != null ? classLoader : clazz.getClassLoader(); + Transaction transaction = null; if (headersExtractor != null) { HeadersExtractorBridge headersExtractorBridge = HeadersExtractorBridge.get(getFirstHeader, getAllHeaders); - transaction = tracer.startChildTransaction(HeadersExtractorBridge.Extractor.of(headerExtractor, headersExtractor), headersExtractorBridge, clazz.getClassLoader()); + transaction = tracer.startChildTransaction(HeadersExtractorBridge.Extractor.of(headerExtractor, headersExtractor), headersExtractorBridge, initiatingClassLoader); } else if (headerExtractor != null) { HeaderExtractorBridge headersExtractorBridge = HeaderExtractorBridge.get(getFirstHeader); - transaction = tracer.startChildTransaction(headerExtractor, headersExtractorBridge, clazz.getClassLoader()); + transaction = tracer.startChildTransaction(headerExtractor, headersExtractorBridge, initiatingClassLoader); } else { - transaction = tracer.startRootTransaction(clazz.getClassLoader()); + transaction = tracer.startRootTransaction(initiatingClassLoader); } if (transaction != null) { transaction.setFrameworkName(Utils.FRAMEWORK_NAME); diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java index 214b0642de..d8f146251d 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java @@ -23,6 +23,8 @@ import co.elastic.apm.agent.impl.TextHeaderMapAccessor; import co.elastic.apm.agent.impl.TracerInternalApiUtils; import co.elastic.apm.agent.impl.transaction.AbstractSpan; +import co.elastic.apm.agent.impl.transaction.TraceContext; +import co.elastic.apm.agent.pluginapi.ElasticApmApiInstrumentation; import org.junit.jupiter.api.Test; import java.util.Collections; @@ -33,6 +35,9 @@ class ElasticApmApiInstrumentationTest extends AbstractApiTest { + private static ClassLoader cl = new ClassLoader("Test", ElasticApmApiInstrumentation.class.getClassLoader()) { + }; + @Test void testCreateTransaction() { Transaction transaction = ElasticApm.startTransaction(); @@ -41,6 +46,15 @@ void testCreateTransaction() { transaction.end(); } + @Test + void testCreateTransactionWithApplicationClassLoader() { + Transaction transaction = ElasticApm.startTransaction(cl); + assertThat(transaction).isNotSameAs(NoopTransaction.INSTANCE); + assertApplicationClassLoader(transaction, cl); + assertThat(ElasticApm.currentTransaction()).isSameAs(NoopTransaction.INSTANCE); + transaction.end(); + } + @Test void testNoCurrentTransaction() { assertThat(ElasticApm.currentTransaction()).isSameAs(NoopTransaction.INSTANCE); @@ -270,6 +284,19 @@ void testTransactionWithRemoteParentFunction() { parent.end(); } + @Test + void testTransactionWithRemoteParentFunctionAndApplicationClassLoader() { + AbstractSpan parent = tracer.startRootTransaction(null); + assertThat(parent).isNotNull(); + Map headerMap = new HashMap<>(); + parent.propagateTraceContext(headerMap, TextHeaderMapAccessor.INSTANCE); + Transaction transaction = ElasticApm.startTransactionWithRemoteParent(headerMap::get, cl); + assertApplicationClassLoader(transaction, cl); + transaction.end(); + assertThat(reporter.getFirstTransaction().isChildOf(parent)).isTrue(); + parent.end(); + } + @Test void testTransactionWithRemoteParentFunctions() { AbstractSpan parent = tracer.startRootTransaction(null); @@ -292,6 +319,19 @@ void testTransactionWithRemoteParentHeaders() { parent.end(); } + @Test + void testTransactionWithRemoteParentHeadersAndApplicationClassLoader() { + AbstractSpan parent = tracer.startRootTransaction(null); + assertThat(parent).isNotNull(); + Map headerMap = new HashMap<>(); + parent.propagateTraceContext(headerMap, TextHeaderMapAccessor.INSTANCE); + Transaction transaction = ElasticApm.startTransactionWithRemoteParent(null, key -> Collections.singletonList(headerMap.get(key)), cl); + assertApplicationClassLoader(transaction, cl); + transaction.end(); + assertThat(reporter.getFirstTransaction().isChildOf(parent)).isTrue(); + parent.end(); + } + @Test void testTransactionWithRemoteParentNullFunction() { ElasticApm.startTransactionWithRemoteParent(null).end(); @@ -300,7 +340,7 @@ void testTransactionWithRemoteParentNullFunction() { @Test void testTransactionWithRemoteParentNullFunctions() { - ElasticApm.startTransactionWithRemoteParent(null, null).end(); + ElasticApm.startTransactionWithRemoteParent(null, (HeadersExtractor) null).end(); assertThat(reporter.getFirstTransaction().getTraceContext().isRoot()).isTrue(); } @@ -344,4 +384,10 @@ void testFrameworkNameWithStartTransactionWithRemoteParent() { ElasticApm.startTransactionWithRemoteParent(null).end(); assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("API"); } + + private static void assertApplicationClassLoader(Transaction transaction, ClassLoader applicationClassLoader) { + TransactionImpl transactionImpl = (TransactionImpl) transaction; + TraceContext traceContext = ((co.elastic.apm.agent.impl.transaction.Transaction) transactionImpl.span).getTraceContext(); + assertThat(traceContext.getApplicationClassLoader()).isEqualTo(applicationClassLoader); + } } diff --git a/docs/public-api.asciidoc b/docs/public-api.asciidoc index 2861f1c11e..968e82e133 100644 --- a/docs/public-api.asciidoc +++ b/docs/public-api.asciidoc @@ -126,6 +126,13 @@ NOTE: Transactions created via this method can not be retrieved by calling <>. See <> on how to achieve that. +[float] +[[api-start-transaction-classloader]] +==== `Transaction startTransaction(ClassLoader)` added[1.29.0] + +Similar to <> but with a specific application class loader. + +* `classLoader`: the application class loader [float] [[api-start-transaction-with-remote-parent-header]] @@ -158,6 +165,15 @@ public Response onIncomingRequest(Request request) throws Exception { NOTE: If the protocol supports multi-value headers, use <> +[float] +[[api-start-transaction-with-remote-parent-header-classloader]] +==== `Transaction startTransaction(HeaderExtractor, ClassLoader)` added[1.29.0] + +Similar to <> but with a specific application class loader. + +* `headerExtractor`: a functional interface which receives a header name and returns the first header with that name +* `classLoader`: the application class loader + [float] [[api-start-transaction-with-remote-parent-headers]] ==== `Transaction startTransactionWithRemoteParent(HeaderExtractor, HeadersExtractor)` added[1.3.0] @@ -192,6 +208,16 @@ public Response onIncomingRequest(Request request) throws Exception { NOTE: If the protocol does not support multi-value headers, use <> +[float] +[[api-start-transaction-with-remote-parent-headers-classloader]] +==== `Transaction startTransaction(HeaderExtractor, HeadersExtractor, ClassLoader)` added[1.29.0] + +Similar to <> but with a specific application class loader. + +* `headerExtractor`: a functional interface which receives a header name and returns the first header with that name +* `headersExtractor`: a functional interface which receives a header name and returns all headers with that name +* `classLoader`: the application class loader + //---------------------------- [float] [[api-annotation]] From e977b118a6ade21049204d6b941ee1d89e889549 Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Wed, 22 Dec 2021 21:11:03 +0100 Subject: [PATCH 2/2] Updated changelog --- CHANGELOG.asciidoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 61e50225b0..16f567b36d 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -23,6 +23,10 @@ endif::[] [[release-notes-1.28.4]] ==== 1.28.4 - YYYY/MM/DD +[float] +===== Features +* Added support for setting the application class loader when starting a transaction via the public api - {pull}2369[#2369] + [float] ===== Bug fixes