Skip to content
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

fix(aws-sdk): use RPC attributes from spec #5166

Merged
merged 8 commits into from Jan 19, 2022
Merged

fix(aws-sdk): use RPC attributes from spec #5166

merged 8 commits into from Jan 19, 2022

Conversation

blumamir
Copy link
Member

Fix #5159

For aws-sdk-1.11 and aws-sdk-2.2 instrumentations:

  • Rename aws.service to rpc.service according to the spec.
  • Rename aws.operation to rpc.method according to the spec.
  • Add missing attribute from spec: rpc.system
  • The above rpc attributes are always published, unlike the aws.* attributes which are set only when otel.instrumentation.aws-sdk.experimental-span-attributes=true
  • Renamed AwsSdkAttributesExtractor.java to AwsSdkHttpAttributesExtractor.java, to make it consistent with the other extractors in the module. This extractor is extracting http client attributes only, and not aws-sdk domain attributes.
  • Fixed the tests to use the new attributes and removed the old non-spec-compliant names

This is my first PR in java, so please take a good look.

@blumamir blumamir requested a review from a team as a code owner January 18, 2022 09:43
@blumamir
Copy link
Member Author

A test seems to fail in CI (PR build / test (15, openj9) (pull_request) Failing after 29m — test (15, openj9)), but I don't think it's related to my changes:

 Task :instrumentation:spring:spring-rmi-4.0:javaagent:test

SpringRmiTest > Client call creates spans FAILED
    java.rmi.ConnectException: Connection refused to host: 10.1.0.130; nested exception is: 
    	java.net.ConnectException: Connection timed out
        at java.rmi@15.0.2/sun.rmi.transport.tcp.TCPEndpoint.newSocket(TCPEndpoint.java:623)
        at java.rmi@15.0.2/sun.rmi.transport.tcp.TCPChannel.createConnection(TCPChannel.java:209)
        at java.rmi@15.0.2/sun.rmi.transport.tcp.TCPChannel.newConnection(TCPChannel.java:196)
        at java.rmi@15.0.2/sun.rmi.server.UnicastRef.invoke(UnicastRef.java:132)
        at java.rmi@15.0.2/java.rmi.server.RemoteObjectInvocationHandler.invokeRemoteMethod(RemoteObjectInvocationHandler.java:217)
        at java.rmi@15.0.2/java.rmi.server.RemoteObjectInvocationHandler.invoke(RemoteObjectInvocationHandler.java:162)
        at app//org.springframework.remoting.rmi.RmiClientInterceptor.doInvoke(RmiClientInterceptor.java:399)
        at app//org.springframework.remoting.rmi.RmiClientInterceptor.doInvoke(RmiClientInterceptor.java:345)
        at app//org.springframework.remoting.rmi.RmiClientInterceptor.invoke(RmiClientInterceptor.java:260)
        at app//org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
        at app//org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:207)
        at SpringRmiTest.Client call creates spans_closure1(SpringRmiTest.groovy:72)
        at SpringRmiTest.Client call creates spans_closure1(SpringRmiTest.groovy)
        at app//groovy.lang.Closure.call(Closure.java:412)
        at app//io.opentelemetry.instrumentation.testing.TestInstrumenters.runWithInstrumenter(TestInstrumenters.java:84)
        at app//io.opentelemetry.instrumentation.testing.TestInstrumenters.runWithSpan(TestInstrumenters.java:44)
        at app//io.opentelemetry.instrumentation.testing.InstrumentationTestRunner.runWithSpan(InstrumentationTestRunner.java:119)
        at app//io.opentelemetry.instrumentation.test.InstrumentationSpecification.runWithSpan(InstrumentationSpecification.groovy:120)
        at SpringRmiTest.Client call creates spans(SpringRmiTest.groovy:72)

        Caused by:
        java.net.ConnectException: Connection timed out
            at java.base@15.0.2/sun.nio.ch.Net.connect(Net.java:574)
            at java.base@15.0.2/sun.nio.ch.Net.connect(Net.java:563)
            at java.base@15.0.2/sun.nio.ch.NioSocketImpl.connect(NioSocketImpl.java:588)
            at java.base@15.0.2/java.net.SocksSocketImpl.connect(SocksSocketImpl.java:333)
            at java.base@15.0.2/java.net.Socket.connect(Socket.java:648)
            at java.base@15.0.2/java.net.Socket.connect(Socket.java:597)
            at java.base@15.0.2/java.net.Socket.<init>(Socket.java:520)
            at java.base@15.0.2/java.net.Socket.<init>(Socket.java:294)
            at java.rmi@15.0.2/sun.rmi.transport.tcp.TCPDirectSocketFactory.createSocket(TCPDirectSocketFactory.java:40)
            at java.rmi@15.0.2/sun.rmi.transport.tcp.TCPEndpoint.newSocket(TCPEndpoint.java:617)
            ... 18 more

Is it just flacky test? should I ignore it?

@mateuszrzeszutek
Copy link
Member

Is it just flacky test? should I ignore it?

Yes, that's just a flaky test that's been introduced very recently - don't worry about it. I'll rerun the checks on your branch just in case.

Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

blumamir and others added 2 commits January 19, 2022 12:39
…opentelemetry/instrumentation/awssdk/v1_11/AwsSdkRpcAttributesExtractor.java

Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>
…pentelemetry/instrumentation/awssdk/v2_2/AwsSdkRpcAttributesExtractor.java

Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>
@trask trask merged commit e28235f into open-telemetry:main Jan 19, 2022
RashmiRam pushed a commit to RashmiRam/opentelemetry-auto-instr-java that referenced this pull request May 23, 2022
* style: rename AwsSdkAttributes to AwsSdkHttpAttributes

* feat(aws-sdk-1.11): use rpc attributes from spec

* feat(aws-sdk-2.2): use rpc attributes from spec

* fix: update apache-camel test with aws-sdk rpc attributes

* chore: lint fix

* Update instrumentation/aws-sdk/aws-sdk-1.11/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/AwsSdkRpcAttributesExtractor.java

Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>

* Update instrumentation/aws-sdk/aws-sdk-2.2/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AwsSdkRpcAttributesExtractor.java

Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>

Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws-sdk instrumentation is missing the RPC attributes from specification
5 participants