-
Notifications
You must be signed in to change notification settings - Fork 11.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ISSUE #2860] support opentracing for rocketmq client #2861
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2861 +/- ##
=============================================
+ Coverage 46.34% 46.47% +0.12%
- Complexity 4384 4403 +19
=============================================
Files 549 552 +3
Lines 36399 36500 +101
Branches 4819 4828 +9
=============================================
+ Hits 16868 16962 +94
+ Misses 17423 17413 -10
- Partials 2108 2125 +17 Continue to review full report at Codecov.
|
<groupId>io.opentracing</groupId> | ||
<artifactId>opentracing-api</artifactId> | ||
<version>0.33.0</version> | ||
<scope>provided</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch
<dependency> | ||
<groupId>io.jaegertracing</groupId> | ||
<artifactId>jaeger-core</artifactId> | ||
<version>1.6.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wu-sheng I know we had rocketmq support for skywalking a long time ago, and I would like to see if it would work perfectly if we switched to skywalking. I'm looking forward to seeing more substantial integration between the two communities:-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for reaching me here. SkyWalking supports RocketMQ through our plugin system, so, we don't need to rely on the OpenTracing APIs layer, also don't ask the RocketMQ community to change the source code.
Also, as a member of OpenTracing, I have to say, OpenTracing is no longer active anymore. The members are either quit or join OpenTelemetry. OpenTracing and OpenCensus have been merged. SkyWalking community already doesn't recommend users to choose OpenTracing-SkyWalking bridge.
RocketMQ 3.x and 4.x plugins are available here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very helpful. OpenTelemetry is a very promising direction. Until it's fully integrated, the community will prefer two more open approaches :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If people really ask about OpenTracing + SkyWalking, I am thinking no one will, just tell them use our agent. All things are going to be out of the box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the non-intrusive bytebuddy instrument version. rocketMQ-3.x-plugin should be obsolete, we could push forward the 4.x, although it only integrate a older incubator client version :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The good thing is, SkyWalking has a version identification feature, so, it only works if your user is still there. And no harm if there isn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to implement tracing support for skywalking inside rocketmq, without using interceptors, if you are interested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yuz10 I would recommend you take a look how SkyWalking's OT bridge works, you may change your mind after you read codes :) We are not working like Jaeger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And OT community is bad of back-compatible, so 0.33 is not working with skywalking OT bridge. It was 0.31 if I recommend it correctly. And the crazy thing is, 0.33 and 0.31 have a big gap. Who knows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice reminder~ nowadays, its dependency is provided scope. I would like to keep a close eye on this field.
* [ISSUE #1233] Fix CVE-2011-1473 * fix Multiple instances in the same application share MQClientInstance * [ISSUE #2748] Fix deleteSubscriptionGroup not remove consumer offset * [ISSUE #2745] Changed the support time of the request/reply feature to 4.6.0. Co-authored-by: von gosling <vongosling@apache.org> * [ISSUE #2729] Replace with Math.min method call * [ISSUE #2801]Fix NamesrvAddr connot set in Producer * [ISSUE 2800] optimize: the spelling of topicSynFlag Co-authored-by: ph3636 <tianxingguang@kanzhun.com> * [ISSUE #2803] Fix the endpoint cannot get instanceId without http (#2804) * fix the endpoint cannot get instanceId without http * fix the endpoint cannot get instanceId without http * add unit test * add unit test * add unit test Co-authored-by: panzhi33 <wb-pz502261@alibaba-inc.com> * fix messageArrivingListener NPE * [ISSUE #2538]Optimize log output when message trace saving fails * [ISSUE #2811] Fix the wrong topic was consumed in the DefaultMessageStoreTest test program * [ISSUE #2821] Overriding the ServiceThread#shutdown in HAClient class * [ISSUE #2805] remove redundant package imports * [ISSUE #2833] Support trace for TranscationProducer (#2834) * [ISSUE #2732] Fix message loss problem when rebalance with LitePullConsumer (#2832) * [ISSUE #2732] Fix message loss problem when rebalance with LitePullConsumer * Fix message loss problem when rebalance with LitePullConsumer, update 2 * [ISSUE #2846]fix -E might not port to other systems * fix some nonconformity after checkstyle * Support OpenTracing(#2861) * [ISSUE #2872] remove log files created by integration test when mvn clean * [ISSUE #2872] move log files created by integration test to target dir * Change log level to debug: "Half offset {} has been committed/rolled back" * Fix unit test stability Bump mockito-core to 3.10.0, remove powermock dependency, suppress useless logging * [ISSUE #2898] Resolve rocketmq-example project failed during checkstyle execution (#2899) Co-authored-by: SSpirits <shadowyspirits@outlook.com> Co-authored-by: panzhi33 <wb-pz502261@alibaba-inc.com> Co-authored-by: panzhi <panzhi33@qq.com> Co-authored-by: ArronHuang <41609451+ArronHuang@users.noreply.github.com> Co-authored-by: von gosling <vongosling@apache.org> Co-authored-by: drgnchan <40224023+drgnchan@users.noreply.github.com> Co-authored-by: zhangjidi2016 <zhangjidi@cmss.chinamobile.com> Co-authored-by: ph3636 <38041490+ph3636@users.noreply.github.com> Co-authored-by: ph3636 <tianxingguang@kanzhun.com> Co-authored-by: BurningCN <1015773611@qq.com> Co-authored-by: francis lee <francislee.cn@outlook.com> Co-authored-by: 灼华 <43363120+BurningCN@users.noreply.github.com> Co-authored-by: yuz10 <845238369@qq.com> Co-authored-by: huangli <areyouok@gmail.com> Co-authored-by: chenrl <raymond2366@outlook.com> Co-authored-by: ayanamist <ayanamist@gmail.com> Co-authored-by: zhangjidi2016 <1017543663@qq.com>
Make sure set the target branch to
develop
[ISSUE #2860] support opentracing for rocketmq client
support opentracing for rocketmq client
Follow this checklist to help us incorporate your contribution quickly and easily. Notice,
it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR
.[ISSUE #123] Fix UnknownException when host config not exist
. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle
to make sure basic checks pass. Runmvn clean install -DskipITs
to make sure unit-test pass. Runmvn clean test-compile failsafe:integration-test
to make sure integration-test pass.