GH-4306: Stop listener observations for filtered records#4330
Conversation
2c99531 to
74bc1cb
Compare
artembilan
left a comment
There was a problem hiding this comment.
This is very elegant solution and I see how you have thought it through.
Please, add your name to the @author list of all the affected classes.
Thank you!
| /** | ||
| * Set the {@link ObservationRegistry} to stop observations for filtered records. | ||
| * @param observationRegistry the observation registry. | ||
| * @since 4.0.3 |
There was a problem hiding this comment.
Please, refine the version for real upcoming release - 4.0.4: https://github.com/spring-projects/spring-kafka/milestone/289
There was a problem hiding this comment.
I’ve added my name to the author list in all affected classes and updated the since tag to 4.0.4.
Thanks for the review. 😄 👍
Signed-off-by: Jinhui Kim <xzdfor@naver.com>
74bc1cb to
dbbee11
Compare
|
@BDARJH , thank you for the contribution; looking forward for more! One note: consider to do a good commit messages instead of posting everything to the PR description. And another note: no need in squashing commits on contribution. Just couple remarks how to make your contribution process much easier. |
Fixes #4306
Problem
When observation is enabled for record listeners, the container starts an observation per record.
For adapter-aware listeners, the container expects the delegate adapter to stop it.
With
FilteringMessageListenerAdapter, filtered records do not call delegateonMessage().In that path, the observation could remain active and leak.
Changes
ObservationRegistrysupport toFilteringMessageListenerAdapter.KafkaMessageListenerContainerobservation setup to propagateObservationRegistrythrough delegating filtering listener chains.
Tests
verifyFilteredRecordStopsObservationverifyFilteredRetryableRecordStopsObservationMicrometerMetricsTestspasses.Notes
The fix is intentionally scoped to the filtered-record short-circuit path to avoid changing
the existing async/reply observation lifecycle behavior.