-
Notifications
You must be signed in to change notification settings - Fork 11.5k
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
[RIP-18]Add opentracing support for Apache RocketMQ #1525
Conversation
|
||
@Override | ||
public void sendMessageBefore(SendMessageContext context) { | ||
span = tracer.buildSpan("sendMessage").asChildOf(rootSpan).start(); |
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.
Could we extract concrete buildspan parameter to a provider extension while not this constant inject?
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.
Consider withTag method
<dependency> | ||
<groupId>io.jaegertracing</groupId> | ||
<artifactId>jaeger-client</artifactId> | ||
<version>0.32.0</version> | ||
</dependency> | ||
<dependency> |
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.
It would be better to write some unit test before writing use case examples. In addition, would you like to analyze your imported dependency and its children dependencies license are compatible with ASL 2.0?
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.
ok,I will add some unit tests ,and the dependency of jaeger-client is compatible with ASL 2.0.
Already done in #2860 |
What is the purpose of the change
add the support of opentracing
Brief changelog
Add the trace hook to sendMessage and consumeMessage
Add the Constructor to DefaultMQProducer and DefaultMQPushConsumer
Add examples of tracing producer and consumer by jaeger
Verifying this change
https://shimo.im/docs/gDhdTrrHjjQwycXv/
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.