-
Notifications
You must be signed in to change notification settings - Fork 679
Add support gRPC 1.59.x and 1.70.x server interceptor trace #764
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
Changes from 5 commits
c54f287
fe5cd51
0c88fa9
7288e2e
89206ee
956c9a5
4551e27
cd4bf58
5a0b960
fa3ff02
0f3dc3c
0653a43
9677d03
29a1f0a
303ffd4
611071a
3b5f02f
bd35702
22b38a6
3e9fd79
797d850
d0ed0a7
5dd664e
a6b609f
13fa3d4
0b3f5a0
1975779
fafe941
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,17 +24,13 @@ | |
| import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; | ||
| import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine; | ||
| import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; | ||
| import org.apache.skywalking.apm.agent.core.plugin.match.MultiClassNameMatch; | ||
|
|
||
| import static net.bytebuddy.matcher.ElementMatchers.named; | ||
| import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments; | ||
| import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName; | ||
|
|
||
| public class AbstractServerImplBuilderInstrumentation extends ClassInstanceMethodsEnhancePluginDefine { | ||
|
|
||
| public static final String ENHANCE_CLASS = "io.grpc.internal.AbstractServerImplBuilder"; | ||
| public static final String ENHANCE_METHOD = "build"; | ||
| public static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.grpc.v1.server.AbstractServerImplBuilderInterceptor"; | ||
|
|
||
| @Override | ||
| public ConstructorInterceptPoint[] getConstructorsInterceptPoints() { | ||
| return new ConstructorInterceptPoint[0]; | ||
|
|
@@ -46,12 +42,12 @@ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() { | |
| new InstanceMethodsInterceptPoint() { | ||
| @Override | ||
| public ElementMatcher<MethodDescription> getMethodsMatcher() { | ||
| return named(ENHANCE_METHOD).and(takesNoArguments()); | ||
| return named("build").and(takesNoArguments()); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you change this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just keep this class using local variables
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see the point of changing this. Please follow original style.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| } | ||
|
|
||
| @Override | ||
| public String getMethodsInterceptor() { | ||
| return INTERCEPT_CLASS; | ||
| return "org.apache.skywalking.apm.plugin.grpc.v1.server.AbstractServerImplBuilderInterceptor"; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -64,6 +60,9 @@ public boolean isOverrideArgs() { | |
|
|
||
| @Override | ||
| protected ClassMatch enhanceClass() { | ||
| return byName(ENHANCE_CLASS); | ||
| return MultiClassNameMatch.byMultiClassMatch( | ||
| "io.grpc.internal.AbstractServerImplBuilder", | ||
| "io.grpc.internal.ServerImplBuilder", | ||
| "io.grpc.internal.ForwardingServerBuilder"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these for different versions? If so, please add comments to the codes. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,7 @@ | |
| - graphql-9.x | ||
| - graphql-12.x-15.x | ||
| - graphql-16plus | ||
| - grpc-1.x | ||
| - grpc-1.x/1.60.x/1.70.x | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this? You don't change the metadata of the plugin. |
||
| - gson-2.8.x | ||
| - guava-cache | ||
| - h2-1.x | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,7 +62,7 @@ metrics based on the tracing data. | |
| * [Dubbox](https://github.com/dangdangdotcom/dubbox) 2.8.4 | ||
| * [Apache Dubbo](https://github.com/apache/dubbo) 2.7.x -> 3.x | ||
| * [Motan](https://github.com/weibocom/motan) 0.2.x -> 1.1.0 | ||
| * [gRPC](https://github.com/grpc/grpc-java) 1.x | ||
| * [gRPC](https://github.com/grpc/grpc-java) 1.x, 1.60.x, 1.70.x | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1.x includes 1.60 and 1.70
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keeping 1. x ?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a doc, this is something defined in the plugin.def.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even with new test codes, this doesn't get affected as you don't change plugin.def config file. |
||
| * [Apache ServiceComb Java Chassis](https://github.com/apache/servicecomb-java-chassis) 1.x, 2.x | ||
| * [SOFARPC](https://github.com/alipay/sofa-rpc) 5.4.0 | ||
| * [Armeria](https://github.com/line/armeria) 0.63.0 -> 1.22.0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| #!/bin/bash | ||
| # | ||
| # Licensed to the Apache Software Foundation (ASF) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| home="$(cd "$(dirname $0)"; pwd)" | ||
|
|
||
| java -jar ${agent_opts} ${home}/../libs/grpc-1.60.x-1.70.x-scenario.jar & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about client? Is it still working by old codes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Client, grpc has not been changed to the class that needs to be enhanced in our bytecode, so there is no significant impact
Only the classes we enhanced on the server side have been removed, so everything needs to be followed up