Implement Kotlin Coroutines Plugin #1964
Conversation
github-actions
bot
added
agent-java
community
❕ Build Aborted
Expand to view the summary
Build stats
Trends 🧪Steps errors
Expand to view the steps failures
|
|
Do I need to change project.version manually or here is automatic version bump? |
|
@felixbarny should I add you to this PR? or how can it be reviewed? |
|
can we get this one implemented soon? |
@thecloudgeek but generally it works, you can copy the source code and build your own plugin |
I have Also, I am getting different trace_id per new worker. I don't see any ways to correlate these coroutines logs of same transaction. |
| private CoroutineConstructorAdvice() { | ||
| } | ||
|
|
||
| @Advice.OnMethodEnter(suppress = Throwable.class) |
There was a problem hiding this comment.
Here I think we also need to include inline=false like: @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
There was a problem hiding this comment.
@bastrich No, doesn't fix. I see major problem here. @jerryjpark has same problem also.
I see the main problem is reflection for java version 11. Reflection is disabled in java version 11.
- | java.lang.NoSuchFieldException: span
-- | --
| Oct 11, 2021 @ 08:45:25.689 | - | at java.base/java.lang.Class.getDeclaredField(Unknown Source)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.AbstractCoroutine.onCompleted(AbstractCoroutine.kt:82)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.AbstractCoroutine.onCompletionInternal(AbstractCoroutine.kt:104)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.JobSupport.tryFinalizeSimpleState(JobSupport.kt:294)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.JobSupport.tryMakeCompleting(JobSupport.kt:853)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.JobSupport.makeCompletingOnce$kotlinx_coroutines_core(JobSupport.kt:825)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:209)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.CoroutineScopeKt.coroutineScope(CoroutineScope.kt:194)
| Oct 11, 2021 @ 08:45:25.689 | - | at ..service.$suspendImpl(ABCService.kt:319)
| Oct 11, 2021 @ 08:45:25.689 | - | at ..abc.mockServic(ABCService.kt:1)
| Oct 11, 2021 @ 08:45:25.689 | - | at model$mock$2$mock$1.invokeSuspend(ABC.kt:95)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
| Oct 11, 2021 @ 08:45:25.689 | - | at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
There was a problem hiding this comment.
I think we have to write code without reflection like:
public abstract class KotlinCoroutineInstrumentation extends TracerAwareInstrumentation {
private static final Logger log = LoggerFactory.getLogger(KotlinCoroutineInstrumentation.class);
public static final WeakConcurrentMap<Object, AbstractSpan<?>> promisesToContext =
new WeakConcurrentMap.WithInlinedExpunction<>();
@Nonnull
@Override
public Collection<String> getInstrumentationGroupNames() {
return Arrays.asList("kotlin-coroutine", "experimental");
}
public static class ConstructorInstrumentation extends KotlinCoroutineInstrumentation {
@Override
public ElementMatcher<? super TypeDescription> getTypeMatcher() {
return named("kotlinx.coroutines.AbstractCoroutine");
}
@Override
public ElementMatcher<? super MethodDescription> getMethodMatcher() {
return isConstructor();
}
public static class AdviceClass {
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
public static void onEnter(@Advice.This Object thiz) {
log.info("what is thiz" + thiz);
log.info("What has promisesToContext" + promisesToContext);
final AbstractSpan<?> context = tracer.getActive();
if (context != null) {
promisesToContext.put(thiz, context);
context.incrementReferences();
context.setNonDiscardable();
}
}
}
}
..................
I am new to kotlin world. Also, we are using graphql heavily which is also new. @bastrich Could you have some idea this way? I definitely needs time to understand the depth of kotlin's coroutines first.
|
Is there anything that needs to be configured like JVM configs for this AOP code to work? I'm getting below error when I launch the application from CLI. It is currently working for me only when I run the application in IntelliJ IDEA. |
|
@jerryjpark how do you build and add an elastic apm dependency? which kotlin version? |
|
@bastrich thanks for your reply. I don't have access to my computer now and I won't for couple more days. I should've shared with you more of the stack trace. The underlying reason (which may or may not be apparent in the stack trace I shared above) is because the Coroutine class the complaining classloader was looking for was not available to the classloader. So, yes it could have to do with the particular version of the class binary the claasloader was looking for. So, I matched the (provided scope) Kotlin dependencies for your plugin to the version of Kotlin used in our project (1.5.0) and rebuilt the plugin to rule out version mismatch as an issue. Still, I cannot say for sure that it is not a version mismatch issue until I debug the Gradle package part. We are using Gradle and Gradle plugin to package our application with all its dependencies. We are clearly providing the correct version of the Kotlin dependencies that your plugin expects as runtime deps(at least per the Gradle project def). Once I return to work, I plan to inspect the fat jar file to begin with to make sure the runtime Kotlin dependencies make to where they should be. Then, I will debug the class loaders if there can an issue for the Kotlin dependency to be found by the relevant classloader that throws CNFE. |
|
Hi! We just realized that we haven't looked into this issue in a while. We're sorry! We're labeling this issue as |
|
Hi! This issue has been stale for a while and we're going to close it as part of our cleanup procedure. We appreciate your contribution and would like to apologize if we have not been able to review it, due to the current heavy load of the team. Feel free to re-open this issue if you think it should stay open. Thank you for your contribution! |
|
Is this dead? |
|
Does anyone have a plan or picked this up recently? It seems the PR is out-of-date for a while. My team would like to use this to implement a new service instead of doing a workaround like the existing one. Although we're absolute novices, we may consider making some changes to this. Currently, I successfully converted the code that uses reflection to non-reflection, but it's very limited and still breaks the app for some cases, e.g.,
|
Implemented #1963
Here I build CoroutineInterceptor and CoroutineWrapper classes manually with ByteBuddy and in the local scope of the advice method. Otherwise, if I build them in another static context or just write like simple Java/Kotlin classes, I get ClassDefNotFoundError for Kotlin classes when integrating with traced applications. I don't know why, but building classes with ByteBuddy in the local scope of advice function resolved that issue. And I wrote an integration test for such case.