You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "huicunjun (via GitHub)" <gi...@apache.org> on 2025/07/25 15:50:54 UTC
[PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
huicunjun opened a new pull request, #764:
URL: https://github.com/apache/skywalking-java/pull/764
Add support gRPC 1.60.x and 1.70.x server interceptor trace
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2232859043
##########
test/plugin/scenarios/grpc-1.60.x-1.70.x-scenario/support-version.list:
##########
@@ -0,0 +1,23 @@
+# 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.
+
+# INTERNAL: HTTP/2 error code: INTERNAL_ERROR Received Goaway occur in test cases 1.0.0 to 1.5.0
+# So these versions were not included in support-version.list. if you know what caused it, please help us.
+
+# Contains only the last version number of each minor version
+
+1.73.0
+1.63.1
Review Comment:
I saw the old grpc cases ended in 1.25. What is the status of 1.25 -> 1.,60?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126432860
I think we don't have that in today's test framework. What do you propose? If you want to enhance that part, we could discuss in a new discussion about this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121359037
All things have been documented, https://skywalking.apache.org/docs/skywalking-java/next/en/setup/service-agent/java-agent/plugin-test/#local-test-and-pull-request-to-the-upstream
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121607580
Can you execute the build action again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3125770452
> Can you build it? I rewrote the test scenario
try again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.59.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #764:
URL: https://github.com/apache/skywalking-java/pull/764
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121331216
If that changed, yes, you need new test codes.
Also, you need to add the case into GHA control file, otherwise, nothing is going to run in CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121356811
The GHA control file is here, .github/workflows
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2231604521
##########
apm-sniffer/apm-sdk-plugin/grpc-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/grpc/v1/define/AbstractServerImplBuilderInstrumentation.java:
##########
@@ -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());
Review Comment:
Why do you change this?
##########
apm-sniffer/apm-sdk-plugin/grpc-1.x-plugin/pom.xml:
##########
@@ -31,10 +31,11 @@
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
- <grpc-all.version>1.6.0</grpc-all.version>
+ <grpc-all.version>1.73.0</grpc-all.version>
Review Comment:
Can't see the point of changing this.
If you just need to intercept new classes(just class name), and everything else works, you should update grpc test scenarios(plugin test).
We could verify your new declared versionz.
Also, docs(supported framework) and changes(change log) should be updated accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121608588
> Can you execute the build action again
exec bash ./test/plugin/run.sh -f grpc-1.60.x-1.70.x-scenario
result
<img width="1984" height="564" alt="image" src="https://github.com/user-attachments/assets/85526bf9-aeed-431b-8948-331dc051b27b" />
Does this mean passing the test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2232570788
##########
apm-sniffer/apm-sdk-plugin/grpc-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/grpc/v1/define/AbstractServerImplBuilderInstrumentation.java:
##########
@@ -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());
Review Comment:
I can't see the point of changing this. Please follow original style.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2233015518
##########
test/plugin/scenarios/grpc-1.60.x-1.70.x-scenario/support-version.list:
##########
@@ -0,0 +1,23 @@
+# 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.
+
+# INTERNAL: HTTP/2 error code: INTERNAL_ERROR Received Goaway occur in test cases 1.0.0 to 1.5.0
+# So these versions were not included in support-version.list. if you know what caused it, please help us.
+
+# Contains only the last version number of each minor version
+
+1.73.0
+1.63.1
Review Comment:
This is the Grpc change record, which states that,
[https://github.com/grpc/grpc-java/commit/050ae18e2a53019f5d199427c91e7d519686fc05](https://github.com/grpc/grpc-java/commit/050ae18e2a53019f5d199427c91e7d519686fc05)
AbstractServerImplBuilder is available in versions before<=1.58.1,
That's our old grpc scenario
Can support up to version<=1.58.1
The newly added interception points are now adapted to 1.59.0->1.73.0
I believe there won't be any major changes to grpc in the future,
To be frank, we have covered grpc 1. x
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2233016731
##########
CHANGES.md:
##########
@@ -16,6 +16,7 @@ Release Notes.
* Fix RabbitMQ Consumer could not receive handleCancelOk callback.
* Support for tracking in lettuce versions 6.5.x and above.
* Upgrade byte-buddy version to 1.17.6.
+* Support gRPC 1.60.x and 1.70.x server interceptor trace
Review Comment:
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126063541
> Your new tests failed with Shengyu cases. Please recheck.
Shengyu cases Passed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2232572353
##########
docs/en/setup/service-agent/java-agent/Plugin-list.md:
##########
@@ -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
Review Comment:
Why this? You don't change the metadata of the plugin.
##########
docs/en/setup/service-agent/java-agent/Supported-list.md:
##########
@@ -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
Review Comment:
1.x includes 1.60 and 1.70
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2234586769
##########
apm-sniffer/apm-sdk-plugin/grpc-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/grpc/v1/server/AbstractServerImplBuilderInterceptor.java:
##########
@@ -32,12 +34,23 @@
public class AbstractServerImplBuilderInterceptor implements InstanceMethodsAroundInterceptor {
@Override
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
- MethodInterceptResult result) {
+ MethodInterceptResult result) throws Throwable {
if (objInst.getSkyWalkingDynamicField() == null) {
ServerBuilder<?> builder = (ServerBuilder) objInst;
- ServerInterceptor interceptor = new ServerInterceptor();
- builder.intercept(interceptor);
- objInst.setSkyWalkingDynamicField(interceptor);
+ Field field = findField(builder.getClass());
+ if (field != null) {
+ field.setAccessible(true);
+ List<?> interceptors = (List<?>) field.get(builder);
+ boolean hasCustomInterceptor = interceptors.stream()
+ .anyMatch(i -> i.getClass() == ServerInterceptor.class);
Review Comment:
Yes, this is a one-time check during server initialization. The performance overhead is negligible in this context.
I don't rule out the possibility of dynamically building GRPC server scenarios, so I'll add caching instead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3125744457
Can you build it? I rewrote the test scenario
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126444247
> I think we don't have that in today's test framework. What do you propose? If you want to enhance that part, we could discuss in a new discussion about this.
Ok
Do you need me to provide more testing scenarios for the GRPC version?
Or rather, we have already completed the testing scenario for this PR,
How about I submit the PR separately to supplement the testing scenarios for the other missing ones?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126711535
> One last thing, is the PR title and change log still accurate? You are only adding support for 1.60 and 1.70?
change to grpc-1.59.x-1.70.x-scenario
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3124418139
Your new tests failed with Shengyu cases. Please recheck.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3124888815
> Your new tests failed with Shengyu cases. Please recheck.
Okay, I'll take a look today
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2232858972
##########
CHANGES.md:
##########
@@ -16,6 +16,7 @@ Release Notes.
* Fix RabbitMQ Consumer could not receive handleCancelOk callback.
* Support for tracking in lettuce versions 6.5.x and above.
* Upgrade byte-buddy version to 1.17.6.
+* Support gRPC 1.60.x and 1.70.x server interceptor trace
Review Comment:
How about client? Is it still working by old codes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3122029317
I am not sure abouy shenyiu caae yet. Maybe we need to ask Apache.
Senfyu about this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2234464701
##########
apm-sniffer/apm-sdk-plugin/grpc-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/grpc/v1/server/AbstractServerImplBuilderInterceptor.java:
##########
@@ -32,12 +34,23 @@
public class AbstractServerImplBuilderInterceptor implements InstanceMethodsAroundInterceptor {
@Override
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
- MethodInterceptResult result) {
+ MethodInterceptResult result) throws Throwable {
if (objInst.getSkyWalkingDynamicField() == null) {
ServerBuilder<?> builder = (ServerBuilder) objInst;
- ServerInterceptor interceptor = new ServerInterceptor();
- builder.intercept(interceptor);
- objInst.setSkyWalkingDynamicField(interceptor);
+ Field field = findField(builder.getClass());
+ if (field != null) {
+ field.setAccessible(true);
+ List<?> interceptors = (List<?>) field.get(builder);
+ boolean hasCustomInterceptor = interceptors.stream()
+ .anyMatch(i -> i.getClass() == ServerInterceptor.class);
Review Comment:
Is this a one-time operation? Because reflection is not that quick.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2232222774
##########
apm-sniffer/apm-sdk-plugin/grpc-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/grpc/v1/define/AbstractServerImplBuilderInstrumentation.java:
##########
@@ -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());
Review Comment:
Just keep this class using local variables
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3123788066
> I am not sure abouy shenyiu caae yet. Maybe we need to ask Apache. Senfyu about this.
As Shenyu plugin has a kind of gRPC proxy feature, I am not sure how it gets affected. Meanwhile, another PR is running okay, ref to https://github.com/apache/skywalking-java/pull/762.
From the CI logs, I can see a lot of gRPC NAT errors got caught in the expected data. You could see that in the scenario test logs.
Here is Shenyu issue page, if you need helps, may be better to reach them? https://github.com/apache/shenyu/issues
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121302217
> I don't think you run the test scenarios locally.
>
> * If the changes are just targeting new class, you should just need to update existing scenarios versions.
> * Please read plugin test scenarios and run the existing scenarios locally, which should cover old versions and add new versions there.
> * New scenarios codes should only be considered if the grpc changed the way of business, which seem not.
I am unable to add new test scenarios to the existing ones because these new classes are only available in grpc 1.60. x and 1.70. x,
The testing scenario can only rely on one grpc version
I have kept all the old version test scenarios
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2232578444
##########
docs/en/setup/service-agent/java-agent/Supported-list.md:
##########
@@ -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
Review Comment:
keeping 1. x ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121338052
> GHA control file,
where is the GHA control file
grpc-1.60.x-1.70.x-scenario this test case , Can we keep it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121357724
> As codes are different, you need new cases for 1.60 and 1.70.
done, grpc-1.60.x-1.70.x-scenario
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126053239
try again too ,
If it still doesn't work, you may need to split the version span more finely and write a separate case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126052284
> > Can you build it? I rewrote the test scenario
>
> try again ðŸ˜
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2232234169
##########
apm-sniffer/apm-sdk-plugin/grpc-1.x-plugin/pom.xml:
##########
@@ -31,10 +31,11 @@
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
- <grpc-all.version>1.6.0</grpc-all.version>
+ <grpc-all.version>1.73.0</grpc-all.version>
Review Comment:
Yes, the new version of grpc has added new implementations and removed old ones, rendering all facets invalid
Not upgrading the grpc pom version does not affect
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2232587759
##########
docs/en/setup/service-agent/java-agent/Supported-list.md:
##########
@@ -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
Review Comment:
Even with new test codes, this doesn't get affected as you don't change plugin.def config file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121355368
As codes are different, you need new cases for 1.60 and 1.70.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun closed pull request #764: Add support gRPC 1.60.x and 1.70.x server interceptor trace
URL: https://github.com/apache/skywalking-java/pull/764
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126426566
Write multiple copies of the same test scenario,

Besides this ${test. framework. version}
Is there a way to provide a version method for dynamically replacing other dependencies?

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126568799
> different
1.7.1
1.6.1 -Dtest.a=666
1.6.1,-Dtest.a=666
Separate by space or comma
Modify run.sh and handle this format,
What do you think? If possible, you can bring up another PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2233084692
##########
test/plugin/scenarios/grpc-1.60.x-1.70.x-scenario/support-version.list:
##########
@@ -0,0 +1,23 @@
+# 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.
+
+# INTERNAL: HTTP/2 error code: INTERNAL_ERROR Received Goaway occur in test cases 1.0.0 to 1.5.0
+# So these versions were not included in support-version.list. if you know what caused it, please help us.
+
+# Contains only the last version number of each minor version
+
+1.73.0
+1.63.1
Review Comment:
> Please add some versions in between to the test scenarios. And adjust your change logs to support newer grpc(until 1.70).
This is not an easy task, as different versions from 1.25.0 to 1.70.0 now rely on different protoc Maven plugins, and The dependencies of the GRPC code generated by compilation also vary

Just changing the grpc version is not enough to compile successfully,

This may require creating different testing scenarios, and some older versions of GRPC cannot be started on Mac Apple chips. Currently, GRPC Scenario cannot compile proto files on my computer, and my computer cannot start this case

My idea is that in grpc-1.60.x-1.70.x-scenario,
Support version. list only provides the grpc version for cases that are compatible with this set of grpc APIs

Those who do not support this testing scenario also need to submit a PR to supplement the testing scenarios left by GRPC,

In fact, agents can be enhanced to proxy without affecting core functions, only the testing scenarios are not covered
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121328772
> > > I don't think you run the test scenarios locally.
> >
> >
> > >
> >
> >
> > > * If the changes are just targeting new class, you should just need to update existing scenarios versions.
> >
> >
> > > * Please read plugin test scenarios and run the existing scenarios locally, which should cover old versions and add new versions there.
> >
> >
> > > * New scenarios codes should only be considered if the grpc changed the way of business, which seem not.
> >
> >
> > I am unable to add new test scenarios to the existing ones because these new classes are only available in grpc 1.60. x and 1.70. x,
> > The testing scenario can only rely on one grpc version
> > I have kept all the old version test scenarios
>
> The test framework is going to build your demo app codes version by version. You just need to add new versions, unless demo codes have a new way to write. AFAIK, grpc doesn't break.
old version use this api usePlaintext(true)
`
ManagedChannelBuilder.forAddress(grpcProviderHost, grpcProviderPort).usePlaintext(true).build();
`
new version >1.60.x use usePlaintext()
`
ManagedChannelBuilder.forAddress(grpcProviderHost, grpcProviderPort).usePlaintext().build();
`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2232583963
##########
apm-sniffer/apm-sdk-plugin/grpc-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/grpc/v1/define/AbstractServerImplBuilderInstrumentation.java:
##########
@@ -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());
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126706277
One last thing, is the PR title and change log still accurate? You are only adding support for 1.60 and 1.70?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126491037
> About 1.26 to 1.59, could you share what is the issue? Generally the current status is good. I just want to get fully understanding.
There's no big problem, it just needs to be confirmed that the grpc version corresponds to the supported version of protoc and needs to be compared.
Otherwise, it may fail to compile or prompt at runtime that certain classes are indeed present.
I looked for it, but there is very little information available. I asked AI and the information provided was not very accurate
I always look at the release date of grpc java, estimate their support range, and adjust them one by one to see which versions can be supported (which is quite time-consuming)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126468537
About 1.26 to 1.59, could you share what is the issue?
Generally the current status is good. I just want to get fully understanding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2233015518
##########
test/plugin/scenarios/grpc-1.60.x-1.70.x-scenario/support-version.list:
##########
@@ -0,0 +1,23 @@
+# 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.
+
+# INTERNAL: HTTP/2 error code: INTERNAL_ERROR Received Goaway occur in test cases 1.0.0 to 1.5.0
+# So these versions were not included in support-version.list. if you know what caused it, please help us.
+
+# Contains only the last version number of each minor version
+
+1.73.0
+1.63.1
Review Comment:
This is the Grpc change record, which states that,
[https://github.com/grpc/grpc-java/commit/050ae18e2a53019f5d199427c91e7d519686fc05](url)
AbstractServerImplBuilder is available in versions before<=1.58.1,
That's our old grpc scenario
Can support up to version<=1.58.1
The newly added interception points are now adapted to 1.59.0->1.73.0
I believe there won't be any major changes to grpc in the future,
To be frank, we have covered grpc 1. x
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3122013785
ci error output ：
`
[ERROR] Scenario[shenyu-2.4.x-grpc-scenario-2.4.3] failed!
/home/runner/work/skywalking-java/skywalking-java/test/plugin/workspace/shenyu-2.4.x-grpc-scenario/2.4.3/scenario.sh: line 57: /home/runner/work/skywalking-java/skywalking-java/test/plugin/workspace/shenyu-2.4.x-grpc-scenario/2.4.3/logs/container.log: No such file or directory
Error: Process completed with exit code 1.
`
shenyu-2.4.x-grpc-scenario
this case no modify
Why can't it execute normally
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2233086560
##########
test/plugin/scenarios/grpc-1.60.x-1.70.x-scenario/support-version.list:
##########
@@ -0,0 +1,23 @@
+# 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.
+
+# INTERNAL: HTTP/2 error code: INTERNAL_ERROR Received Goaway occur in test cases 1.0.0 to 1.5.0
+# So these versions were not included in support-version.list. if you know what caused it, please help us.
+
+# Contains only the last version number of each minor version
+
+1.73.0
+1.63.1
Review Comment:
Perhaps this part of the testing scenario needs to be covered later
It should be analyzing their API differences and dependency differences, and defining several different testing scenarios
it may be that
grpc-1.30.x-1.40.x-scenario,
grpc-1.40.x-1.50.x-scenario,
grpc-1.50.x-1.60.x-scenario,
grpc-1.60.x-1.70.x-scenario,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121362076
> The GHA control file is here, .github/workflows
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2233601081
##########
apm-sniffer/apm-sdk-plugin/grpc-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/grpc/v1/define/AbstractServerImplBuilderInstrumentation.java:
##########
@@ -64,6 +63,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");
Review Comment:
Are these for different versions? If so, please add comments to the codes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3123771465
`
<protocArtifact>com.google.protobuf:protoc:3.23.4:exe:${os.detected.classifier}
</protocArtifact>
`
this protobuf version Cannot be dynamically changed
in this case project grpc-1.50.x-1.70.x-scenario
Can only cover up to 1.50 -> 1.73.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121325091
> > > I don't think you run the test scenarios locally.
> >
> >
> > >
> >
> >
> > > * If the changes are just targeting new class, you should just need to update existing scenarios versions.
> >
> >
> > > * Please read plugin test scenarios and run the existing scenarios locally, which should cover old versions and add new versions there.
> >
> >
> > > * New scenarios codes should only be considered if the grpc changed the way of business, which seem not.
> >
> >
> > I am unable to add new test scenarios to the existing ones because these new classes are only available in grpc 1.60. x and 1.70. x,
> > The testing scenario can only rely on one grpc version
> > I have kept all the old version test scenarios
>
> The test framework is going to build your demo app codes version by version. You just need to add new versions, unless demo codes have a new way to write. AFAIK, grpc doesn't break.
old version
`
ManagedChannelBuilder.forAddress(grpcProviderHost, grpcProviderPort).usePlaintext(true).build();
`
> 1.60.x use usePlaintext()
`
ManagedChannelBuilder.forAddress(grpcProviderHost, grpcProviderPort).usePlaintext().build();
`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3121317367
> > I don't think you run the test scenarios locally.
>
> >
>
> > * If the changes are just targeting new class, you should just need to update existing scenarios versions.
>
> > * Please read plugin test scenarios and run the existing scenarios locally, which should cover old versions and add new versions there.
>
> > * New scenarios codes should only be considered if the grpc changed the way of business, which seem not.
>
>
>
> I am unable to add new test scenarios to the existing ones because these new classes are only available in grpc 1.60. x and 1.70. x,
>
> The testing scenario can only rely on one grpc version
>
> I have kept all the old version test scenarios
>
>
>
>
The test framework is going to build your demo app codes version by version.
You just need to add new versions, unless demo codes have a new way to write. AFAIK, grpc doesn't break.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2232578923
##########
docs/en/setup/service-agent/java-agent/Supported-list.md:
##########
@@ -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
Review Comment:
This is not a doc, this is something defined in the plugin.def.
As commented, please follow plugin dev docs and plugin test docs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#discussion_r2233027135
##########
test/plugin/scenarios/grpc-1.60.x-1.70.x-scenario/support-version.list:
##########
@@ -0,0 +1,23 @@
+# 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.
+
+# INTERNAL: HTTP/2 error code: INTERNAL_ERROR Received Goaway occur in test cases 1.0.0 to 1.5.0
+# So these versions were not included in support-version.list. if you know what caused it, please help us.
+
+# Contains only the last version number of each minor version
+
+1.73.0
+1.63.1
Review Comment:
Please add some versions in between to the test scenarios.
And adjust your change logs to support newer grpc(until 1.70).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126702546
I think it is fine to merge. Those several old releases should not be a big problem at all.
We could skip tests for them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support gRPC 1.60.x and 1.70.x server interceptor trace [skywalking-java]
Posted by "huicunjun (via GitHub)" <gi...@apache.org>.
huicunjun commented on PR #764:
URL: https://github.com/apache/skywalking-java/pull/764#issuecomment-3126499327
> About 1.26 to 1.59, could you share what is the issue? Generally the current status is good. I just want to get fully understanding.
The currently approved cases are
<=1.25.0
1.30.0 -> 1.39.0
1.59.0 -> 1.70.x
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org