Scala/Slick AsyncExecutor instrumentation

[dboreham]

Hello, does the agent as-is automatically instrument applications using Slick? It has been reported to me that it doesn't (parent span is lost after the context switch to a thread in Slick's AsyncExecutor), however I see some Slick-related code, and a test in the agent: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/scala-fork-join-2.8/javaagent/src/slickTest/scala/SlickUtils.scala which suggests that perhaps it is expected to work. I wondered if perhaps the agent's Scala support was developed in the past when Slick used the stock Scala ForkJoinPool and things had rotted in the meantime.

Anyway, it'd be great to hear any thoughts, pointers, etc on this before I dive in. Thanks.

[trask]

hey @dboreham! the scala-fork-join-2.8 module is just targeted at the legacy Scala ForkJoinPool, which was deprecated in 2.12 IIUC

if Slick has its own ExecutorService implementation, you may be able to just add that to

opentelemetry-java-instrumentation/instrumentation/executors/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/javaconcurrent/AbstractExecutorInstrumentation.java

Lines 57 to 101 in 21f968a

	String[] includeExecutors = {
	"akka.actor.ActorSystemImpl$$anon$1",
	"akka.dispatch.BalancingDispatcher",
	"akka.dispatch.Dispatcher",
	"akka.dispatch.Dispatcher$LazyExecutorServiceDelegate",
	"akka.dispatch.ExecutionContexts$sameThreadExecutionContext$",
	"akka.dispatch.forkjoin.ForkJoinPool",
	"akka.dispatch.ForkJoinExecutorConfigurator$AkkaForkJoinPool",
	"akka.dispatch.MessageDispatcher",
	"akka.dispatch.PinnedDispatcher",
	"com.google.common.util.concurrent.AbstractListeningExecutorService",
	"com.google.common.util.concurrent.MoreExecutors$ListeningDecorator",
	"com.google.common.util.concurrent.MoreExecutors$ScheduledListeningDecorator",
	"io.netty.channel.epoll.EpollEventLoop",
	"io.netty.channel.epoll.EpollEventLoopGroup",
	"io.netty.channel.MultithreadEventLoopGroup",
	"io.netty.channel.nio.NioEventLoop",
	"io.netty.channel.nio.NioEventLoopGroup",
	"io.netty.channel.SingleThreadEventLoop",
	"io.netty.util.concurrent.AbstractEventExecutor",
	"io.netty.util.concurrent.AbstractEventExecutorGroup",
	"io.netty.util.concurrent.AbstractScheduledEventExecutor",
	"io.netty.util.concurrent.DefaultEventExecutor",
	"io.netty.util.concurrent.DefaultEventExecutorGroup",
	"io.netty.util.concurrent.GlobalEventExecutor",
	"io.netty.util.concurrent.MultithreadEventExecutorGroup",
	"io.netty.util.concurrent.SingleThreadEventExecutor",
	"java.util.concurrent.AbstractExecutorService",
	"java.util.concurrent.CompletableFuture$ThreadPerTaskExecutor",
	"java.util.concurrent.Executors$DelegatedExecutorService",
	"java.util.concurrent.Executors$FinalizableDelegatedExecutorService",
	"java.util.concurrent.ForkJoinPool",
	"java.util.concurrent.ScheduledThreadPoolExecutor",
	"java.util.concurrent.ThreadPoolExecutor",
	"org.apache.tomcat.util.threads.ThreadPoolExecutor",
	"org.eclipse.jetty.util.thread.QueuedThreadPool", // dispatch() covered in the jetty module
	"org.eclipse.jetty.util.thread.ReservedThreadExecutor",
	"org.glassfish.grizzly.threadpool.GrizzlyExecutorService",
	"org.jboss.threads.EnhancedQueueExecutor",
	"play.api.libs.streams.Execution$trampoline$",
	"play.shaded.ahc.io.netty.util.concurrent.ThreadPerTaskExecutor",
	"scala.concurrent.forkjoin.ForkJoinPool",
	"scala.concurrent.Future$InternalCallbackExecutor$",
	"scala.concurrent.impl.ExecutionContextImpl",
	};

in this case, adding a test with testLibrary Slick will make sure CI always runs against both that base version of slick and the "latest" version of Slick, which will ensure it doesn't rot again, e.g.

opentelemetry-java-instrumentation/instrumentation/cdi-testing/build.gradle.kts

Lines 6 to 8 in 21f968a

	testLibrary("org.jboss.weld:weld-core:2.3.0.Final")
	testLibrary("org.jboss.weld.se:weld-se:2.3.0.Final")
	testLibrary("org.jboss.weld.se:weld-se-core:2.3.0.Final")

if not, you may need to implement something similar to the ExecutorService instrumentation but for Slick's specific classes

in this case, check out https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/contributing/writing-instrumentation.md (and https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/contributing/writing-instrumentation-module.md)

[dboreham]

Ok thanks. I wondered about adding the executor to that list. But Slick actually just uses the stock java.util.concurrent.ThreadPoolExecutor now, which prompts the question: why doesn't it work already? I will investigate and report back...