Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add instrumentation for rocketmq #2263
Add instrumentation for rocketmq #2263
Changes from 25 commits
535b95b
0d1fbed
cb059ab
2f2cfcf
d3fd46b
bce0921
924b2e3
fb41154
451d3a5
24d1d4a
fd9f3a4
06ea7bc
f40b086
bad5c16
73bbefb
06175b2
fb3dc3d
a7f764c
66617dc
4094f4a
558acf7
310c268
c98b828
1ea63af
449b8e2
55aa9f2
a92c9b3
b5ba2ca
0887762
48de0f7
ad8cbc7
f28eee6
81c3720
1c2c71f
0ed11a0
c32bc40
460092a
0489ef9
963e012
00aa1fa
5419081
8419db8
aa2c7b5
8c606f6
4a00e83
11aa520
9ffde5d
6ad79eb
24de0ca
3aac1a5
b4eb7c3
58801ed
398246b
5b45a6a
f873db7
c0c4c9d
28f30fc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
What if there are >1 messages? The rest won't be traced at all?
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.
When msgs. size( )>1, do we create one span or several span?
1.We create several span,but their "SemanticAttributes" vaules are the same.
2. Create one span, whose span name is marked that this span processes multiple data, for example : test-topic process size(or batch) 10 .
I think the second one is better. What's your opinion?
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.
Hmm, I think that messages may have different attributes, like different message id or queue offset.
Maybe we can do it the same way AWS SQS lambda handler instrumentation does in this scenario:
This way each message will get its own span containing a link to propagated producer span, and the whole consumer processing will be wrapped in a single span. WDYT? @anuraaga
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.
Oh, and you can check out
TracingSqsEventHandler
,TracingSqsMessageHandler
andAwsLambdaMessageTracer
for an example of this.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.
This situation is also described in the spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/messaging.md#batch-receiving
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.
In this particular case if the propagation is turned off you shouldn't add a link - it'll end up pointing to the
parentContext
anyway.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 filed this spec issue we may be able to handle this in the SDK too open-telemetry/opentelemetry-specification#1492
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.
Let's leave this code as is, next SDK will handle invalid links for us