search

kamon-io / Kamon

Distributed Tracing, Metrics and Context Propagation for applications running on the JVM
https://kamon.io
Other
1.41k stars 328 forks source link

kamon-pekko: find new way to instrument pinned dispatchers #1366

Open pjfanning opened 2 months ago

pjfanning commented 2 months ago

Relates to #1352

The existing ThreadPoolConfigCopyAdvice kamon-pekko doesn't work for Pekko 1.1 when Scala 2.13 is used. Pekko 1.1 has Scala 2 compiler inlining enabled. ThreadPoolConfig is a final case class and the copy function is derived by the Scala compiler. When a class is final, all its functions are final and can be inlined. Since the copy function is not explicitly declared, I haven't found a way to mark it as @noinline. I even tried adding the annotation to the case class.

  onType("org.apache.pekko.dispatch.ThreadPoolConfig")
    .mixin(classOf[HasDispatcherPrerequisites.Mixin])
    .advise(method("copy"), classOf[ThreadPoolConfigCopyAdvice])

Would it be feasible to try replacing this instrumentation point and concentrating on the PinnedDispatcher constructor instead? https://github.com/apache/pekko/blob/22a738511375baa23c5b84c87d44a065c0401e0e/actor/src/main/scala/org/apache/pekko/dispatch/PinnedDispatcher.scala#L28

fyi @hughsimpson

pjfanning commented 2 months ago

I guess where I went wrong is that I assumed you needed to do an override def if you wanted to replace the copy function but https://github.com/apache/pekko/pull/1489 appears to show that you don't need the override. The override causes compilation issues.

hughsimpson commented 2 months ago

@pjfanning Had just finished getting the test suite to pass when I saw this message 🤦 Indeed the solution I went for was to override the copy method, although the other solution seems to be instrumenting every copy callsite with @noinline which is... even less elegant, and anyway didn't seem to work for me. It would probably be better to try something else, as you say...