Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

run tests for pekko 1.1 #1362

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

run tests for pekko 1.1 #1362

wants to merge 1 commit into from

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Sep 16, 2024

Adding pekko 1.1 to tests to catch the regressions.

This fails various tests -- especially for scala 2.13, although a couple for 2.12 as well -- because of pekko's decision to enable inlining in the build process (cf pjfanning/sbt-pekko-build#2). I have run the tests with a locally-published version of pekko where I've set -Dpekko.no.inline=true and everything passes. Workarounds are, as yet, unclear... @pjfanning anything you'd be able to suggest?

@hughsimpson
Copy link
Contributor Author

It's easier to see what's going wrong by looking at scala 2.12, since that's failing fewer tests. We instrument the message buffer nodes with:

  onType("org.apache.pekko.util.MessageBuffer$Node")
    .mixin(classOf[HasContext.Mixin])
    .advise(isConstructor, classOf[CaptureCurrentContextOnExit])
    .advise(method("apply"), classOf[InvokeWithCapturedContext])

which adds a context store to each message buffer node and instantiates it from the environment context on construction. This part works. What doesn't work is the .advise(method("apply"), classOf[InvokeWithCapturedContext]) step; presumably because the function call is inlined, we don't get a chance to apply our advice, so when the node is executed, the stored context is not propagated into the function call. This issue is even more prevalent in scala 2.13, but entirely missing from scala 3.

It may be the case that we can simply only support pekko 1.1 instrumentation in scala 3, although that's certainly not the ideal outcome...

@pjfanning
Copy link
Contributor

So far, there are no great solutions. I have seen some similar issues with https://github.com/pjfanning/micrometer-pekko (itself a fork of Kamon from years ago). I decompiled a few Pekko classes for Scala 2.13 and the necessary methods for the instrumentation still seemed to be in the classes and in the call traces but still the instrumentation missed certain calls in practise.

Scala 2.13 users, in particular, and maybe Scala 2.12 users who really need Kamon can consider

  • sticking with Pekko 1.0 - there are not massive changes in Pekko 1.1
  • compiling Pekko 1.1 for themselves - as @hughsimpson mentioned, this requires -Dpekko.no.inline=true when you run the Pekko sbt build.

There are a few other projects out there that offer Pekko metrics that don't rely on instrumenting the classes at runtime but they require code changes to your code base to uptake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants