Support aws-otel-java-instrumentation agent with KMS 1.X SDK

[gautam-nutalapati]

Hi!

I am trying to use aws-otel-java-instrumentation java agent in my project and I see an issue shown below with KMS signing.
I am using KMS SDK 1.x and providing aws credentials via env vars.
I don't see this issue when I use the non-aws otel java agent and KMS SDK 1.x.

I created a sample project here to reproduce this. When using AWS KMS SDK 2.X, app does not throw the exception.
Is 1.X AWS KMS SDK supported by aws otel java agent? If it is supported, this is a valid issue.

Exception thrown when using AWS OTEL java agent with KMS AWS SDK 1.x:

[otel.javaagent 2022-01-25 17:27:16:006 -0600] [main] INFO io.opentelemetry.javaagent.tooling.VersionLogger - opentelemetry-javaagent - version: 1.10.0-aws

Exception in thread "main" java.lang.StringIndexOutOfBoundsException: begin 0, end -7, length 0
at java.base/java.lang.String.checkBoundsBeginEnd(String.java:3734)
at java.base/java.lang.String.substring(String.java:1903)
at io.opentelemetry.javaagent.shaded.instrumentation.awssdk.v1_11.AwsSdkExperimentalAttributesExtractor$1.computeValue(AwsSdkExperimentalAttributesExtractor.java:34)
at io.opentelemetry.javaagent.shaded.instrumentation.awssdk.v1_11.AwsSdkExperimentalAttributesExtractor$1.computeValue(AwsSdkExperimentalAttributesExtractor.java:30)
at java.base/java.lang.ClassValue.getFromHashMap(ClassValue.java:228)
at java.base/java.lang.ClassValue.getFromBackup(ClassValue.java:210)
at java.base/java.lang.ClassValue.get(ClassValue.java:116)
at io.opentelemetry.javaagent.shaded.instrumentation.awssdk.v1_11.AwsSdkExperimentalAttributesExtractor.extractOperationName(AwsSdkExperimentalAttributesExtractor.java:55)
at io.opentelemetry.javaagent.shaded.instrumentation.awssdk.v1_11.AwsSdkExperimentalAttributesExtractor.onStart(AwsSdkExperimentalAttributesExtractor.java:43)
at io.opentelemetry.javaagent.shaded.instrumentation.awssdk.v1_11.AwsSdkExperimentalAttributesExtractor.onStart(AwsSdkExperimentalAttributesExtractor.java:26)
at io.opentelemetry.javaagent.shaded.instrumentation.api.instrumenter.Instrumenter.start(Instrumenter.java:174)
at io.opentelemetry.javaagent.shaded.instrumentation.awssdk.v1_11.TracingRequestHandler.beforeRequest(TracingRequestHandler.java:41)
at io.opentelemetry.javaagent.instrumentation.awssdk.v1_11.TracingRequestHandler.beforeRequest(TracingRequestHandler.java:45)
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.runBeforeRequestHandlers(AmazonHttpClient.java:850)
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.doExecute(AmazonHttpClient.java:792)
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.executeWithTimer(AmazonHttpClient.java:779)
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.execute(AmazonHttpClient.java:753)
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.access$500(AmazonHttpClient.java:713)
at com.amazonaws.http.AmazonHttpClient$RequestExecutionBuilderImpl.execute(AmazonHttpClient.java:695)
at com.amazonaws.http.AmazonHttpClient.execute(AmazonHttpClient.java:559)
at com.amazonaws.http.AmazonHttpClient.execute(AmazonHttpClient.java:539)
at com.amazonaws.services.kms.AWSKMSClient.doInvoke(AWSKMSClient.java:7785)
at com.amazonaws.services.kms.AWSKMSClient.invoke(AWSKMSClient.java:7752)
at com.amazonaws.services.kms.AWSKMSClient.invoke(AWSKMSClient.java:7741)
at com.amazonaws.services.kms.AWSKMSClient.executeSign(AWSKMSClient.java:6631)
at com.amazonaws.services.kms.AWSKMSClient.sign(AWSKMSClient.java:6600)
at Main.main(Main.java:17)

[anuraaga]

Thanks @gautam-nutalapati - I have filed open-telemetry/opentelemetry-java-instrumentation#5229 upstream this does seem to be a bug in the instrumentation. The reason it is showing up in the aws otel agent and not upstream for you is that we default to -Dotel.instrumentation.aws-sdk.experimental-span-attributes=true in this distribution. As a workaround until that is fixed you can try to set that flag to false but may see less information in traces.

[anuraaga]

@gautam-nutalapati Thanks a lot for the repro app - I notice the issue is because of the {{ implicit subclassing pattern used here

https://github.com/gautam-nutalapati/test-aws-otel-agent/blob/main/src/main/java/Main.java#L10

It's not appropriate for the instrumentation to just crash for code that would otherwise work so I will work on fixing that, but does it work for you to use a more normal pattern calling setters on new SignRequest() without the subclassing?

[gautam-nutalapati]

@anuraaga Thank you for the fix in upstream repo! and also for pointing out the subclassing pattern!
Using setters has solved the issue for me.