run tests for pekko 1.1
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 https://github.com/pjfanning/sbt-pekko-build/pull/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?
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...
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=truewhen 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.
Upstream changes in pekko are holding. I wonder if we should run some more automatic test on the nightlies to catch future changes earlier. I imagine pekko support is one of the bigger use cases of Kamon...
Upstream changes in pekko are holding. I wonder if we should run some more automatic test on the nightlies to catch future changes earlier. I imagine pekko support is one of the bigger use cases of Kamon...
No harm in testing against Pekko snapshots. The enabling of Scala 2 inlining was a big change and it would be rare for us to take on a large change like that.
Compiler might one day decide to inline something else we were depending on, and/or internals might change more. As you know I'd rather we had long term documented bindings, but I'm sure we both know how long that might take to materialise
Pushed a bumped pekko for pekko-http tests with failures
Working now with upstream changes
Hey @hughsimpson @pjfanning I'm trying to upgrade to Pekko HTTP 1.1.x but realised that some server metrics coming from Kamon are missing (for instance http.server.connection.lifetime.seconds.bucket) is there any plan to push forward this work? Happy to help :)
We need to do a Pekko-HTTP 1.1.1 or 1.2.0 release to get https://github.com/apache/pekko-http/pull/616 out there. @javimartinez would you time to test with a snapshot jar? https://pekko.apache.org/docs/pekko-http/current/contributing.html#snapshots 1.2.0-M0+33-e381127a-SNAPSHOT is the most recent one.
Sure @pjfanning
I tested that version as follow:
- Updated the pekkoHttpVersion in this branch with
1.2.0-M0+33-e381127a-SNAPSHOT - Ran kamon-pekko-http test suite (it passes)
- Published a version of Kamon locally
I can see that the metrics are back (at least the metric that I'm checking)
Is there a plan to do any of these releases soon?
P.D I'm gonna try to test it using HTTP 2
The ASF requires votes for releases and there is currently a vote for a different module. So I can't go around and make any promises about releases but I plan to start the discussion in a couple of days. it could still take a few weeks to do a release and that depends on whether other volunteers are available to assess and vote on release candidates.
Great, thank you @pjfanning. Hope we get the release soon :)