-
Notifications
You must be signed in to change notification settings - Fork 786
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
aws-sdk-2.2.: Support injection into SQS.SendMessageBatch message attributes #8798
aws-sdk-2.2.: Support injection into SQS.SendMessageBatch message attributes #8798
Conversation
456f50a
to
8b48a0b
Compare
Rebased & updated PR description. |
b695eee
to
0b4c6b8
Compare
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.
LGTM 👍
...c/main/groovy/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2SqsTracingTest.groovy
Outdated
Show resolved
Hide resolved
...c/main/groovy/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2SqsTracingTest.groovy
Show resolved
Hide resolved
trace(1, xrayInjectionEnabled ? 4 : 3) { | ||
span(0) { | ||
name "Sqs.SendMessageBatch" | ||
kind CLIENT // TODO: Probably this should be producer, but that would be a breaking change |
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 general I think it'd make sense for SQS to implement the messaging semconv; but that's probably a quite large project, and we might just defer it until the messaging semconv stability.
…/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2SqsTracingTest.groovy Co-authored-by: Mateusz Rzeszutek <mrzeszutek@splunk.com>
@Oberon00 can you resolve the conflicts? thx |
Continuing after #8775, this PR contains both more refactorings and the SQS SendMessageBatch injection support. If requested, I can move the refactorings (all commits before 456f50a, which at the time of PR creation is the last commit) to a separate PR.