Add instrumentation for spring-cloud-aws SqsListener annotation #12314
Conversation
...avaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/aws/AwsSqsTest.java
Outdated
Show resolved
Hide resolved
instrumentation/spring/spring-cloud-aws-3.0/javaagent/build.gradle.kts
Outdated
Show resolved
Hide resolved
| * This class is internal and is hence not for public use. Its APIs are unstable and can change at | ||
| * any time. | ||
| */ | ||
| public final class AwsSdkInstrumenterFactory { |
There was a problem hiding this comment.
Sorry, I don't understand here very much. Could you explain more about adjustment of visibility here? BTW, if necessary, add same comment on https://github.com/laurit/opentelemetry-java-instrumentation/blob/645e3e3c8658c9231c7327d035a4773743236dc1/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/SqsAdviceBridge.java#L8
There was a problem hiding this comment.
This class is used by AwsSdkTelemetry that is in a different package. This comment is enforced by tooling on public classes in internal package. It is not needed on SqsAdviceBridge as it is not in internal package.
| @Override | ||
| public void transform(TypeTransformer transformer) { | ||
| transformer.applyAdviceToMethod( | ||
| named("onMessage").and(takesArgument(0, named("org.springframework.messaging.Message"))), |
There was a problem hiding this comment.
https://github.com/awspring/spring-cloud-aws/blob/0882fb17899a8bd448014603f366afdefe6e6fc7/spring-cloud-aws-sqs/src/main/java/io/awspring/cloud/sqs/listener/adapter/MessagingMessageListenerAdapter.java#L45
for it, don't we need to support? If yes, is it necessary to comment here?
There was a problem hiding this comment.
added a TODO. I skipped it because I wasn't quite sure how to handle it.
|
|
||
| @BeforeAll | ||
| static void setUp() { | ||
| sqs = SQSRestServerBuilder.withPort(0).withInterface("localhost").start(); |
There was a problem hiding this comment.
any concern about port conflicts here?
| sqs = SQSRestServerBuilder.withPort(0).withInterface("localhost").start(); | |
| int sqsPort = PortUtils.findOpenPort(); | |
| sqs = SQSRestServerBuilder.withPort(sqsPort).withInterface("localhost").start(); |
There was a problem hiding this comment.
Port 0 usually means any port, os will pick whatever port is free. This is really the best option. The algorithm in PortUtils.findOpenPort mostly attempts to guard that tests running in parallel wouldn't try to use the same port, it is not able guarantee that some other application won't use the port it has picked.
Resolves #12267
Resolves #4788
Hopefully also resolves #9575