You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/10/20 17:14:55 UTC
[GitHub] [skywalking-java] HScarb opened a new pull request #54: fix rocketmq message header properties garbled characters issue
HScarb opened a new pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54
<!--
⚠️ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
On this PR of rocketmq https://github.com/apache/rocketmq/pull/2961 add some new codes to delete the last `PROPERTY_SEPARATOR` of `properties`, however skywalking interceptor not expecting it.
https://github.com/apache/rocketmq/blob/fb8bc64e2a2551e76da24cb6af0fb3e5f986f8b3/common/src/main/java/org/apache/rocketmq/common/message/MessageDecoder.java#L442-L444
I fixing this issue by adding a `PROPERTY_SEPARATOR` before adding headers, and delete tailing `PROPERTY_SEPARATOR`.
### Fix https://github.com/apache/rocketmq/issues/3331
- [x] Add a unit test to verify that the fix works.
- [x] Explain briefly why the bug exists and how to fix it.
<!-- ==== 🔌 Remove this line WHEN AND ONLY WHEN you're adding a new plugin, follow the checklist 👇 ====
### Add an agent plugin to support <framework name>
- [ ] Add a test case for the new plugin, refer to [the doc](https://github.com/apache/skywalking-java/blob/main/docs/en/setup/service-agent/java-agent/Plugin-test.md)
- [ ] Add a component id in [the component-libraries.yml](https://github.com/apache/skywalking/blob/master/oap-server/server-starter/src/main/resources/component-libraries.yml)
- [ ] Add a logo in [the UI repo](https://github.com/apache/skywalking-rocketbot-ui/tree/master/src/views/components/topology/assets)
==== 🔌 Remove this line WHEN AND ONLY WHEN you're adding a new plugin, follow the checklist 👆 ==== -->
<!-- ==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👇 ====
### Improve the performance of <class or module or ...>
- [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking-java/blob/main/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
- [ ] The benchmark result.
```text
<Paste the benchmark results here>
```
- [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👆 ==== -->
<!-- ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👇 ====
### <Feature description>
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👆 ==== -->
- [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>.
- [x] Update the [`CHANGES` log](https://github.com/apache/skywalking-java/blob/main/CHANGES.md).
--
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
[GitHub] [skywalking-java] wu-sheng merged pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-949164155
> I added UTs for messageHeader of RocketMQ `4.9.x`
I have checked the UT, it seems good.
> and I'll add plugin integration tests later
So, I should wait for update in this pull request, right?
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-949096472
@liwei0423 Could you confirm whether this fixes your case?
I did a review, this looks good. But like I mentioned, RocketMQ plugin lacks plugin tests like Kafka and Pulsar, so, I can't 100% confirm this plugin and changed status works with all RocketMQ 4.x server and especially 4.9.
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-950030857
@HScarb New release should start recently, do you have time to finish this PR soon?
--
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
[GitHub] [skywalking-java] HScarb commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
HScarb commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-949161683
> @HScarb Could you confirm this works for `< 4.9` and `4.9.x`? Usually we accept new changes of plugin after plugin integration tests added.
I added UTs for messageHeader of RocketMQ `4.9.x`, and I'll add plugin integration tests later
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-950103595
At least we wouldn't do at weekend. That is @wankai123 call to start 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
[GitHub] [skywalking-java] HScarb commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
HScarb commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-950189518
> At least we wouldn't do at weekend. That is @wankai123 call to start this.
I have trouble running `run.sh` packaging test agent, so it may take longer time than expected.
Think it's better to submit rocketmq e2e test cases in another individual PR
And are there some detailed example about writing e2e test scenarios?
```shell
λ sh test\plugin\run.sh -f kafka-scenario
+++ dirname 'test\plugin\run.sh'
++ cd 'test\plugin'
++ pwd
+ home=/e/WS/Skywalking/skywalking-java/test/plugin
+ scenario_name=
+ force_build=off
+ cleanup=off
+ debug_mode=
+ mvnw=/e/WS/Skywalking/skywalking-java/test/plugin/../../mvnw
+ agent_home=/e/WS/Skywalking/skywalking-java/test/plugin/../../skywalking-agent
+ jacoco_home=/e/WS/Skywalking/skywalking-java/test/plugin/../jacoco
+ scenarios_home=/e/WS/Skywalking/skywalking-java/test/plugin/scenarios
+ num_of_testcases=
+ image_version=jdk8-1.0.0
+ jacoco_version=0.8.6
++ uname
+ os=MSYS_NT-10.0-19043
++ date +%s
+ start_stamp=1635007257
+ parse_commandline -f kafka-scenario
+ test 2 -gt 0
+ _key=-f
+ case "$_key" in
+ force_build=on
+ shift
+ test 1 -gt 0
+ _key=kafka-scenario
+ case "$_key" in
+ scenario_name=kafka-scenario
+ shift
+ test 0 -gt 0
+ [[ off == \o\n ]]
+ test -z kafka-scenario
+ [[ ! -d /e/WS/Skywalking/skywalking-java/test/plugin/../../skywalking-agent ]]
+ sed -i '' '/<sourceDirectory>scenarios\/kafka-scenario<\/sourceDirectory>/d' ./pom.xml
sed: can't read /<sourceDirectory>scenarios\/kafka-scenario<\/sourceDirectory>/d: No such file or directory
+ sed -i '/<sourceDirectory>scenarios\/kafka-scenario<\/sourceDirectory>/d' ./pom.xml
+ echo check code with the checkstyle-plugin
check code with the checkstyle-plugin
+ sed -i '' '/<\/sourceDirectories>/i\
<sourceDirectory>scenarios\/kafka-scenario<\/sourceDirectory>
' ./pom.xml
sed: can't read /<\/sourceDirectories>/i\
<sourceDirectory>scenarios\/kafka-scenario<\/sourceDirectory>
: No such file or directory
+ sed -i '/<\/sourceDirectories>/i <sourceDirectory>scenarios\/kafka-scenario<\/sourceDirectory>' ./pom.xml
+ [[ on == \o\n ]]
+ profile=
+ [[ jdk8-1.0.0 =~ jdk14- ]]
+ /e/WS/Skywalking/skywalking-java/test/plugin/../../mvnw --batch-mode -f /e/WS/Skywalking/skywalking-java/test/plugin/pom.xml clean package -DskipTests
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO]
[INFO] SkyWalking Plugins Tests [pom]
[INFO] SkyWalking Plugin Test Runner Helper [jar]
[INFO] SkyWalking Agent Test Tools [pom]
[INFO] SkyWalking Plugin Test Runner Images [pom]
[INFO] SkyWalking Tomcat Runner Image [jar]
[INFO] SkyWalking JVM Runner Image [jar]
[INFO]
[INFO] -----< org.apache.skywalking.plugin:apache-skywalking-test-plugin >-----
[INFO] Building SkyWalking Plugins Tests 1.0.0 [1/6]
[INFO] --------------------------------[ pom ]---------------------------------
[INFO]
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ apache-skywalking-test-plugin ---
[INFO] Deleting E:\WS\Skywalking\skywalking-java\test\plugin\target
[INFO]
[INFO] --- maven-checkstyle-plugin:3.1.1:check (validate) @ apache-skywalking-test-plugin ---
[INFO] Starting audit...
Audit done.
[INFO] You have 0 Checkstyle violations.
[INFO]
[INFO] -------------< org.apache.skywalking.plugin:runner-helper >-------------
[INFO] Building SkyWalking Plugin Test Runner Helper 1.0.0 [2/6]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ runner-helper ---
[INFO] Deleting E:\WS\Skywalking\skywalking-java\test\plugin\runner-helper\target
[INFO]
[INFO] --- maven-checkstyle-plugin:3.1.1:check (validate) @ runner-helper ---
[INFO] Starting audit...
Audit done.
[INFO] You have 0 Checkstyle violations.
[INFO]
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ runner-helper ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 5 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.8.1:compile (default-compile) @ runner-helper ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 15 source files to E:\WS\Skywalking\skywalking-java\test\plugin\runner-helper\target\classes
[INFO]
[INFO] --- maven-resources-plugin:2.6:testResources (default-testResources) @ runner-helper ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 3 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.8.1:testCompile (default-testCompile) @ runner-helper ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 3 source files to E:\WS\Skywalking\skywalking-java\test\plugin\runner-helper\target\test-classes
[INFO]
[INFO] --- maven-surefire-plugin:2.12.4:test (default-test) @ runner-helper ---
[INFO] Tests are skipped.
[INFO]
[INFO] --- maven-jar-plugin:2.4:jar (default-jar) @ runner-helper ---
[INFO] Building jar: E:\WS\Skywalking\skywalking-java\test\plugin\runner-helper\target\plugin-runner-helper.jar
[INFO]
[INFO] --- maven-shade-plugin:3.2.1:shade (default) @ runner-helper ---
[INFO] Including org.freemarker:freemarker:jar:2.3.28 in the shaded jar.
[INFO] Including com.google.guava:guava:jar:20.0 in the shaded jar.
[INFO] Including org.yaml:snakeyaml:jar:1.24 in the shaded jar.
[INFO] Including org.apache.logging.log4j:log4j-core:jar:2.8.1 in the shaded jar.
[INFO] Including org.apache.logging.log4j:log4j-api:jar:2.8.1 in the shaded jar.
[INFO]
[INFO] -----------< org.apache.skywalking.plugin:agent-test-tools >------------
[INFO] Building SkyWalking Agent Test Tools 1.0.0 [3/6]
[INFO] --------------------------------[ pom ]---------------------------------
[INFO]
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ agent-test-tools ---
[INFO] Deleting E:\WS\Skywalking\skywalking-java\test\plugin\agent-test-tools\target
[INFO]
[INFO] --- exec-maven-plugin:1.6.0:exec (default) @ agent-test-tools ---
Cloning into 'E:\WS\Skywalking\skywalking-java\test\plugin\agent-test-tools/target/agent-test-tools'...
fatal: reference is not a tree: 24270f8f1ee1cb9186ede5202ff1c4ae3d2d482a
curl: (3) Illegal characters found in URL
Error: Could not find or load main class org.apache.maven.wrapper.MavenWrapperMain
cp: cannot stat 'E:\WS\Skywalking\skywalking-java\test\plugin\agent-test-tools/target/agent-test-tools/dist/*': No such file or directory
[ERROR] Command execution failed.
org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
at org.apache.commons.exec.DefaultExecutor.executeInternal (DefaultExecutor.java:404)
at org.apache.commons.exec.DefaultExecutor.execute (DefaultExecutor.java:166)
at org.codehaus.mojo.exec.ExecMojo.executeCommandLine (ExecMojo.java:804)
at org.codehaus.mojo.exec.ExecMojo.executeCommandLine (ExecMojo.java:751)
at org.codehaus.mojo.exec.ExecMojo.execute (ExecMojo.java:313)
at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:137)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:210)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:81)
at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:56)
at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:128)
at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:305)
at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192)
at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105)
at org.apache.maven.cli.MavenCli.execute (MavenCli.java:956)
at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:288)
at org.apache.maven.cli.MavenCli.main (MavenCli.java:192)
at sun.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke (Method.java:498)
at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)
at sun.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke (Method.java:498)
at org.apache.maven.wrapper.BootstrapMainStarter.start (BootstrapMainStarter.java:39)
at org.apache.maven.wrapper.WrapperExecutor.execute (WrapperExecutor.java:122)
at org.apache.maven.wrapper.MavenWrapperMain.main (MavenWrapperMain.java:61)
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for SkyWalking Plugins Tests 1.0.0:
[INFO]
[INFO] SkyWalking Plugins Tests ........................... SUCCESS [ 2.692 s]
[INFO] SkyWalking Plugin Test Runner Helper ............... SUCCESS [ 4.431 s]
[INFO] SkyWalking Agent Test Tools ........................ FAILURE [ 1.824 s]
[INFO] SkyWalking Plugin Test Runner Images ............... SKIPPED
[INFO] SkyWalking Tomcat Runner Image ..................... SKIPPED
[INFO] SkyWalking JVM Runner Image ........................ SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 9.081 s
[INFO] Finished at: 2021-10-24T00:41:08+08:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.6.0:exec (default) on project agent-test-tools: Command execution failed. Process exited with an error: 1 (Exit value: 1) -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR] mvn <goals> -rf :agent-test-tools
```
--
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
[GitHub] [skywalking-java] vongosling commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
vongosling commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-949420632
@wu-sheng This is the compatibility issue we discussed earlier, and it's great to see that our guys can feedback to the upstream community and hopefully solve this problem.
--
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
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#discussion_r733180363
##########
File path: apm-sniffer/apm-sdk-plugin/rocketMQ-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/rocketMQ/v4/MessageSendInterceptor.java
##########
@@ -68,12 +68,20 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
while (next.hasNext()) {
next = next.next();
if (!StringUtil.isEmpty(next.getHeadValue())) {
+ if (properties.length() > 0 && properties.charAt(properties.length() - 1) != PROPERTY_SEPARATOR) {
+ // adapt for RocketMQ 4.9.x or later
+ properties.append(PROPERTY_SEPARATOR);
+ }
properties.append(next.getHeadKey());
properties.append(NAME_VALUE_SEPARATOR);
properties.append(next.getHeadValue());
properties.append(PROPERTY_SEPARATOR);
}
}
+ // remove trailing PROPERTY_SEPARATOR, which is unnecessary
+ if (properties.length() > 0 && properties.charAt(properties.length() - 1) == PROPERTY_SEPARATOR) {
+ properties.deleteCharAt(properties.length() - 1);
+ }
Review comment:
If this is required, why don't we remove `properties.append(PROPERTY_SEPARATOR);` in every time of adding key:value pair?
--
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
[GitHub] [skywalking-java] vongosling commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
vongosling commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-950337194
Yep, I think we found an improvement in the Windows platform.
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-948076933
You break the CI with format issue.
> [INFO] Starting audit...
Error: /home/runner/work/skywalking-java/skywalking-java/apm-sniffer/apm-sdk-plugin/rocketMQ-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/rocketMQ/v4/MessageSendInterceptorTest.java:188:33: 'typecast' is not followed by whitespace. [WhitespaceAfter]
Audit done.
[INFO] There is 1 error reported by Checkstyle 8.19 with /home/runner/work/skywalking-java/skywalking-java/apm-checkstyle/checkStyle.xml ruleset.
Error: src/test/java/org/apache/skywalking/apm/plugin/rocketMQ/v4/MessageSendInterceptorTest.java:[188,33] (whitespace) WhitespaceAfter: 'typecast' is not followed by whitespace.
There is style file in the root folder, please fix.
--
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
[GitHub] [skywalking-java] HScarb commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
HScarb commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-949412769
> So, I should wait for update in this pull request, right?
right
--
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
[GitHub] [skywalking-java] HScarb edited a comment on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
HScarb edited a comment on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-949412769
> So, I should wait for update in this pull request, right?
right
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-950340651
> Yep, I think we found an improvement in the Windows platform.
Sorry, AFAIK, all committers are using either Linux or MacOS. We don't have much experience on Windows. :(
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-949421281
> This is the compatibility issue we discussed earlier
I know. I just hope we could have a better testing for RocketMQ plugin.
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-950251663
> Cloning into 'E:\WS\Skywalking\skywalking-java\test\plugin\agent-test-tools/target/agent-test-tools'...
fatal: reference is not a tree: 24270f8f1ee1cb9186ede5202ff1c4ae3d2d482a
curl: (3) Illegal characters found in URL
https://github.com/apache/skywalking-java/blob/main/test/plugin/agent-test-tools/bin/fetch-code.sh This shell fetchs/clones the tool repo, it seems failing in your env.
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-948079164
And there is no plugin e2e testing for RocketMQ like Kafka, https://github.com/apache/skywalking-java/tree/main/test/plugin/scenarios/kafka-scenario.
Could you consider to add for RocketMQ 4.x? Please in there, the agent with plugins could be tested in real application, rather than UTs only.
This is the plugin test dev doc, https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/plugin-test/. Take a reference what Kafka and Pulsar provided, this should be not hard to follow.
--
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
[GitHub] [skywalking-java] HScarb commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
HScarb commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-950102261
> @HScarb New release should start recently, do you have time to finish this PR soon?
when will the new release start? I'll do it with in this weekend
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-949100153
@HScarb Could you confirm this works for `< 4.9` and `4.9.x`? Usually we accept new changes of plugin after plugin integration tests added.
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #54: fix rocketmq message header properties garbled characters issue
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #54:
URL: https://github.com/apache/skywalking-java/pull/54#issuecomment-950234370
We only tested this script in Linux and MacOS.
--
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