Skip to content

Switching to use Span concurrent map in Scala plugin #2323

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 8, 2021
Merged

Switching to use Span concurrent map in Scala plugin #2323

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions ...rent-plugin/src/main/java/co/elastic/apm/agent/scalaconcurrent/FutureInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
package co.elastic.apm.agent.scalaconcurrent;

import co.elastic.apm.agent.bci.TracerAwareInstrumentation;
import co.elastic.apm.agent.collections.WeakConcurrentProviderImpl;
import co.elastic.apm.agent.impl.transaction.AbstractSpan;
import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent;
import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
Expand All @@ -39,8 +39,7 @@
public abstract class FutureInstrumentation extends TracerAwareInstrumentation {

@SuppressWarnings("WeakerAccess")
public static final WeakMap<Object, AbstractSpan<?>> promisesToContext =
WeakConcurrent.buildMap();
public static final WeakMap<Object, AbstractSpan<?>> promisesToContext = WeakConcurrentProviderImpl.createWeakSpanMap();

@Nonnull
@Override
Expand All @@ -66,9 +65,6 @@ public static void onExit(@Advice.This Object thiz) {
final AbstractSpan<?> context = tracer.getActive();
if (context != null) {
promisesToContext.put(thiz, context);
// this span might be ended before the Promise$Transformation#run method starts
// we have to avoid that this span gets recycled, even in the above mentioned case
context.incrementReferences();
}
}
}
Expand All @@ -91,12 +87,13 @@ public static class AdviceClass {
@Nullable
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
public static Object onEnter(@Advice.This Object thiz) {
AbstractSpan<?> context = promisesToContext.remove(thiz);
// We cannot remove yet, as this may decrement the ref count of the span to 0 if it has already ended,
// thus causing it to be recycled just before we activate it on the current thread. So we first get().
AbstractSpan<?> context = promisesToContext.get(thiz);
if (context != null) {
context.activate();
// decrements the reference we incremented to avoid that the parent context gets recycled before the promise is run
// because we have activated it, we can be sure it doesn't get recycled until we deactivate in the OnMethodExit advice
context.decrementReferences();
// Now it's safe to remove, as ref count is at least 2
promisesToContext.remove(thiz);
}
return context;
}
Expand Down