wu-sheng commented on PR #796:
URL: https://github.com/apache/skywalking-java/pull/796#issuecomment-4030797297
1. The 2.x test scenario uses spring-boot-starter-amqp versions
2.0.0.RELEASE through 2.4.0 — these are all EOL Spring Boot 2 versions. Given
that 2.x is no longer maintained. We may be better to remove that?
2. Thread name check in RabbitMQConsumerInterceptor is fragile and unreliable
(rabbitmq-plugin/.../RabbitMQConsumerInterceptor.java)
```
if
(Thread.currentThread().getName().toLowerCase().contains("springframework")) {
return;
}
```
This is the biggest problem in the PR. Relying on thread names to
determine behavior is extremely brittle:
- Thread names are not a contract and can be customized by users or
framework versions.
- If a non-Spring application happens to name threads with
"springframework", the original RabbitMQ plugin silently breaks.
- If Spring changes thread naming conventions, both plugins break
(duplicate spans or no spans).
- This couples the existing rabbitmq-plugin to Spring-specific knowledge,
violating separation of concerns.
3. Consider adopting plugin v2 APIs.
4. Supported-list.d should be updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
