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 for observing suspending functions with @Observed annotation #4827

Open
ilya40umov opened this issue Mar 4, 2024 · 2 comments
Open

Comments

@ilya40umov
Copy link

Problem Statement

@Observed annotation, which is powered by ObservedAspect, does not seem to be able to correctly handle suspending methods. The aspect is able to invoke the suspending call, but the newly created observation does not become "current" within the suspending method.

Rationale
The issue #4754 is covering additions to the Observation API, so that it can be used to observe a suspending block of code, however, a lot of Spring users may prefer to use the annotation @Observed instead.

Additional context

Seems like Spring Framework does support AOP on suspending code since some time ago: spring-projects/spring-framework#22462. E.g. AopUtils has additional logic for dealing with invoking a suspending function.

Also, looking at this comment has made me think that ObservedAspect could actually get a mono/flux from calling ProceedingJoinPoint.proceed() and then put the newly created observation into the Reactor context via ObservationThreadLocalAccessor.KEY.

Raised initially in issues/4754#issuecomment-1977193040 by @ArvindEnvoy

@ilya40umov
Copy link
Author

My curiously got better of me and I implemented a super hacky solution for this problem here: ObservedSuspendingAspect

This aspect is working with a custom annotation for now that can be added on a suspending method (but obviously @Observed annotation itself should be used in the final solution).

As I didn't find any easy way to manipulate coroutine context in an aspect, I had to get the old context from the continuation argument, tweak it a bit and then put a new, delegating continuation into the proceed()'s arguments. I'm not sure if this is even a valid use of Coroutines API 😄

@ilya40umov
Copy link
Author

Assuming that the hacky solution I linked above would be okey, there is still a question of how to add this extra code to ObservedAspect. Especially, given that it lives in the micrometer-observation module, which does not really have anything to do with Kotlin.

What I think could work in this case is the following:

  • Add another constructor to ObservedAspect which would take an extra optional parameter, which would represent a strategy for handling calls on suspending methods. An interface for this strategy would need to be declared in micrometer-observation module. (This could be potentially made a bit more generic to work with a list of strategies, each one having an extra method like "applies", so that the first matching strategy would be then used by the aspect).
  • The actual implementation of this strategy could then be added to micrometer-core, as it has a package for various Kotlin-specific APIs.
  • ObservationAutoConfiguration.java in Spring Boot would then be able to use something like KotlinDetector.java or just a condition on one of Kotlin/coroutines class, to call this new constructor with extra strategies provived. This would make sure that this behavior is only added for Kotlin projects that have coroutines library on the classpath.

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

No branches or pull requests

1 participant