Skip to content

Commit bcf7092

Browse files
xuzhiguangxuzhiguang
committed
fix(threadpool): avoid duplicate invoke spans
Match invokeAll and invokeAny only on ThreadPoolExecutor and use MethodInvocationContext to carry the invocation span lifecycle.
1 parent f388fcb commit bcf7092

7 files changed

Lines changed: 268 additions & 70 deletions

File tree

‎apm-sniffer/bootstrap-plugins/jdk-threadpool-plugin/src/main/java/org/apache/skywalking/apm/plugin/ThreadPoolInvokeMethodInterceptor.java‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,25 @@
2727
import org.apache.skywalking.apm.agent.core.context.ContextSnapshot;
2828
import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
2929
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
30-
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
31-
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
30+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.v2.InstanceMethodsAroundInterceptorV2;
31+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.v2.MethodInvocationContext;
3232
import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
3333
import org.apache.skywalking.apm.plugin.wrapper.SwCallableWrapper;
3434

35-
public class ThreadPoolInvokeMethodInterceptor implements InstanceMethodsAroundInterceptor {
35+
public class ThreadPoolInvokeMethodInterceptor implements InstanceMethodsAroundInterceptorV2 {
3636

3737
private static final String OPERATION_NAME_PREFIX = "ThreadPoolExecutor/";
3838

3939
@Override
4040
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
41-
MethodInterceptResult result) throws Throwable {
41+
MethodInvocationContext context) throws Throwable {
4242
if (!shouldEnhance(allArguments)) {
4343
return;
4444
}
4545

4646
AbstractSpan span = ContextManager.createLocalSpan(OPERATION_NAME_PREFIX + method.getName());
4747
span.setComponent(ComponentsDefine.JDK_THREADING);
48+
context.setContext(span);
4849

4950
ContextSnapshot contextSnapshot = ContextManager.capture();
5051
Collection<?> callables = (Collection<?>) allArguments[0];
@@ -57,18 +58,20 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
5758

5859
@Override
5960
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
60-
Object ret) throws Throwable {
61-
if (shouldEnhance(allArguments)) {
62-
ContextManager.stopSpan();
61+
Object ret, MethodInvocationContext context) throws Throwable {
62+
AbstractSpan span = (AbstractSpan) context.getContext();
63+
if (span != null) {
64+
ContextManager.stopSpan(span);
6365
}
6466
return ret;
6567
}
6668

6769
@Override
6870
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
69-
Class<?>[] argumentsTypes, Throwable t) {
70-
if (shouldEnhance(allArguments)) {
71-
ContextManager.activeSpan().log(t);
71+
Class<?>[] argumentsTypes, Throwable t, MethodInvocationContext context) {
72+
AbstractSpan span = (AbstractSpan) context.getContext();
73+
if (span != null) {
74+
span.log(t);
7275
}
7376
}
7477

‎apm-sniffer/bootstrap-plugins/jdk-threadpool-plugin/src/main/java/org/apache/skywalking/apm/plugin/define/ThreadPoolExecutorInstrumentation.java‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,10 @@ public class ThreadPoolExecutorInstrumentation extends ClassInstanceMethodsEnhan
3737

3838
private static final String INTERCEPT_SUBMIT_METHOD = "submit";
3939

40-
private static final String INTERCEPT_INVOKE_ALL_METHOD = "invokeAll";
41-
42-
private static final String INTERCEPT_INVOKE_ANY_METHOD = "invokeAny";
43-
4440
private static final String INTERCEPT_EXECUTE_METHOD_HANDLE = "org.apache.skywalking.apm.plugin.ThreadPoolExecuteMethodInterceptor";
4541

4642
private static final String INTERCEPT_SUBMIT_METHOD_HANDLE = "org.apache.skywalking.apm.plugin.ThreadPoolSubmitMethodInterceptor";
4743

48-
private static final String INTERCEPT_INVOKE_METHOD_HANDLE = "org.apache.skywalking.apm.plugin.ThreadPoolInvokeMethodInterceptor";
49-
5044
@Override
5145
public boolean isBootstrapInstrumentation() {
5246
return true;
@@ -92,23 +86,6 @@ public String getMethodsInterceptor() {
9286
return INTERCEPT_SUBMIT_METHOD_HANDLE;
9387
}
9488

95-
@Override
96-
public boolean isOverrideArgs() {
97-
return true;
98-
}
99-
},
100-
new InstanceMethodsInterceptPoint() {
101-
@Override
102-
public ElementMatcher<MethodDescription> getMethodsMatcher() {
103-
return ElementMatchers.named(INTERCEPT_INVOKE_ALL_METHOD)
104-
.or(ElementMatchers.named(INTERCEPT_INVOKE_ANY_METHOD));
105-
}
106-
107-
@Override
108-
public String getMethodsInterceptor() {
109-
return INTERCEPT_INVOKE_METHOD_HANDLE;
110-
}
111-
11289
@Override
11390
public boolean isOverrideArgs() {
11491
return true;

‎apm-sniffer/bootstrap-plugins/jdk-threadpool-plugin/src/main/java/org/apache/skywalking/apm/plugin/define/ThreadPoolExecutorInvokeInstrumentation.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.plugin.define;
20+
21+
import java.util.Collection;
22+
import java.util.concurrent.TimeUnit;
23+
import net.bytebuddy.description.method.MethodDescription;
24+
import net.bytebuddy.matcher.ElementMatcher;
25+
import net.bytebuddy.matcher.ElementMatchers;
26+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
27+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.v2.ClassInstanceMethodsEnhancePluginDefineV2;
28+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.v2.InstanceMethodsInterceptV2Point;
29+
import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
30+
import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
31+
32+
public class ThreadPoolExecutorInvokeInstrumentation extends ClassInstanceMethodsEnhancePluginDefineV2 {
33+
34+
private static final String ENHANCE_CLASS = "java.util.concurrent.ThreadPoolExecutor";
35+
36+
private static final String INTERCEPT_INVOKE_ALL_METHOD = "invokeAll";
37+
38+
private static final String INTERCEPT_INVOKE_ANY_METHOD = "invokeAny";
39+
40+
private static final String INTERCEPT_INVOKE_METHOD_HANDLE =
41+
"org.apache.skywalking.apm.plugin.ThreadPoolInvokeMethodInterceptor";
42+
43+
@Override
44+
public boolean isBootstrapInstrumentation() {
45+
return true;
46+
}
47+
48+
@Override
49+
protected ClassMatch enhanceClass() {
50+
return NameMatch.byName(ENHANCE_CLASS);
51+
}
52+
53+
@Override
54+
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
55+
return new ConstructorInterceptPoint[0];
56+
}
57+
58+
@Override
59+
public InstanceMethodsInterceptV2Point[] getInstanceMethodsInterceptV2Points() {
60+
return new InstanceMethodsInterceptV2Point[] {
61+
new InstanceMethodsInterceptV2Point() {
62+
@Override
63+
public ElementMatcher<MethodDescription> getMethodsMatcher() {
64+
return ElementMatchers.named(INTERCEPT_INVOKE_ALL_METHOD)
65+
.or(ElementMatchers.named(INTERCEPT_INVOKE_ANY_METHOD))
66+
.and(ElementMatchers.takesArguments(Collection.class)
67+
.or(ElementMatchers.takesArguments(
68+
Collection.class, long.class, TimeUnit.class)));
69+
}
70+
71+
@Override
72+
public String getMethodsInterceptorV2() {
73+
return INTERCEPT_INVOKE_METHOD_HANDLE;
74+
}
75+
76+
@Override
77+
public boolean isOverrideArgs() {
78+
return true;
79+
}
80+
}
81+
};
82+
}
83+
}

‎apm-sniffer/bootstrap-plugins/jdk-threadpool-plugin/src/main/resources/skywalking-plugin.def‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@
1515
# limitations under the License.
1616

1717
jdk-threadpool-plugin=org.apache.skywalking.apm.plugin.define.ThreadPoolExecutorInstrumentation
18+
jdk-threadpool-plugin=org.apache.skywalking.apm.plugin.define.ThreadPoolExecutorInvokeInstrumentation

0 commit comments

Comments
 (0)