Extend spring.consume context through Spring Cloud AWS SQS error handlers#11257
Extend spring.consume context through Spring Cloud AWS SQS error handlers#11257
Conversation
ygree
added
type: bug
ygree
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a2c83769c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // which is not bundled in spring-messaging but is always present when Spring Kafka is. | ||
| extraDependency 'org.reactivestreams:reactive-streams:1.0.4' | ||
| // Spring Cloud AWS error-handler instrumentations reference ListenerExecutionFailedException. | ||
| extraDependency 'io.awspring.cloud:spring-cloud-aws-sqs:3.0.1' |
There was a problem hiding this comment.
In applications that use Spring Messaging/Spring Kafka but do not include spring-cloud-aws-sqs, adding this AWS dependency to the only spring-messaging muzzle directive makes the whole instrumentation safe only when AWS SQS is also present. The existing InvocableHandlerMethod/Kafka messaging instrumentation in this same module has no muzzleDirective() override, and the project docs note module directives are checked against all instrumentations by default, so non-AWS Spring Messaging apps can lose their existing spring.consume instrumentation instead of only skipping the new AWS-specific instrumenters. Split the AWS instrumenters behind their own directive/module or override muzzleDirective() so the base Spring Messaging instrumentation remains loadable without AWS.
Useful? React with 👍 / 👎.
| if (null != error) { | ||
| DECORATE.onError(span, error); | ||
| if (SpringMessageErrorHandlerHelper.isInAwsListenerInvocation()) { | ||
| SpringMessageErrorHandlerHelper.capturePendingContinuation(span); | ||
| } |
There was a problem hiding this comment.
When an @SqsListener returns a CompletableFuture/CompletionStage that completes exceptionally after invoke returns, @Advice.Thrown error is null here, so no pending continuation is captured. Spring Cloud AWS wraps that later async failure in MessageListenerExecutionStage rather than through createListenerException, so the error handler still runs without the spring.consume context for the normal async-failure path; the added test only covers a method that throws before returning its future. Capture/attach the continuation for failed async results as well as synchronous throws.
Useful? React with 👍 / 👎.
1 participant
What Does This Do
Extend spring.consume context through Spring Cloud AWS SQS error handlers
Motivation
Spring Cloud AWS SQS error handler missing trace context
Additional Notes
Ref: #11234
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.