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

Support Akka persistence in otel extension #296

Closed
lgajowy opened this issue Feb 7, 2022 · 3 comments · Fixed by #311
Closed

Support Akka persistence in otel extension #296

lgajowy opened this issue Feb 7, 2022 · 3 comments · Fixed by #311
Assignees
Milestone

Comments

@lgajowy
Copy link
Contributor

lgajowy commented Feb 7, 2022

Similar to #295. The task is to enable akka persistence metrics in our otel extension. It can be done similarly to: #291.

NOTE: please enable -Dotel.javaagent.debug=true when running with the otel agent as there might be some errors (given my initial investigation).

@lgajowy lgajowy self-assigned this Feb 7, 2022
@lgajowy lgajowy added the WIP Work in progress label Feb 7, 2022
@adamgadomski adamgadomski modified the milestone: Release 0.5.2 Feb 8, 2022
@lgajowy
Copy link
Contributor Author

lgajowy commented Feb 8, 2022

I'm getting

Cannot call super (or default) method for public void akka.persistence.typed.internal.Running.onWriteSuccess(akka.actor.typed.scaladsl.ActorContext,akka.persistence.PersistentRepr)

afaik this happens because the open telemetry agent uses disableClassFormatChanges(). That forbids changes on the instrumented class file (Running in this case).

Indeed, we are using intercept() that tries to replace the existing implementation. From intercept() method docs:

If a method is declared by the instrumented type and the type builder creates a subclass or redefinition, any preexisting method is replaced by the given implementation

Other than that, it seems that using the intercept api is not a good idea in general - the creator of ByteBuddy claims that it's not supported on some jvms: raphw/byte-buddy#110 (comment). So it seems to be a good call from the open telemetry team to use disableClassFormatChanges().

I will now investigate if the same instrumentation can be done with the Advice api that seems to be working smoothly for akka http and is the generally favoured way to go.

@worekleszczy
Copy link
Contributor

@lgajowy Moving to Advice api is definitely a good move 🙂

I just want to add that AFAIK for us limitation of intercept api is not really relevant - in the linked issue there was a requirement for retransformations - this is in fact protected by JVM, so after a class is loaded it's not possible to add any methods or field (both public and private). My guess is that intercept api moved current implementation to a private method for it to be accessible but Advice just rewrite the body of the method with additional code.

One worry that I have and it's a big one is what exactly disableClassFormatChanges - if it disables all changes even for first time load we're in trouble because without it we're not going to be able to instrument akka.actor 🙄 .

I'll do some testing to see if that's really the case.

@lgajowy
Copy link
Contributor Author

lgajowy commented Feb 11, 2022

I'll do some testing to see if that's really the case.

Sure, thanks! Alternatively, wait a few days - I was going to move the Akka actor instrumentations right after I merge the persistence PR. So I will definitely have the above-mentioned problems if that's really the case.

@lgajowy lgajowy removed the WIP Work in progress label Feb 11, 2022
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 a pull request may close this issue.

3 participants