From 6b13bcca631789b59c59bc5e6381ddf933d3b36d Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Thu, 28 Jan 2021 16:01:07 +0900 Subject: [PATCH] Use X-Ray propagator for aws sdk 1.1 instrumentation (#2117) * Use X-Ray propagator for aws sdk 1.1 instrumentation * Cleaner * Fix * Copy over doc --- .../instrumentation/api/tracer/HttpClientTracer.java | 2 +- .../aws-sdk/aws-sdk-1.11/javaagent/README.md | 12 ++++++++++++ .../javaagent/aws-sdk-1.11-javaagent.gradle | 5 +++++ .../awssdk/v1_11/AwsSdkClientTracer.java | 9 ++++++++- .../awssdk/v1_11/AwsSdkInstrumentationModule.java | 5 +++++ .../javaagent/src/test/groovy/Aws1ClientTest.groovy | 3 ++- .../groovy/Aws0ClientTest.groovy | 3 ++- 7 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 instrumentation/aws-sdk/aws-sdk-1.11/javaagent/README.md diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java index b95516ae81..78786971c2 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java @@ -77,7 +77,7 @@ public abstract class HttpClientTracer extends BaseT return context; } - private void inject(Context context, CARRIER carrier) { + protected void inject(Context context, CARRIER carrier) { Setter setter = getSetter(); if (setter == null) { throw new IllegalStateException( diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/README.md b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/README.md new file mode 100644 index 0000000000..3481c4d57a --- /dev/null +++ b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/README.md @@ -0,0 +1,12 @@ +# AWS Java SDK v1 Instrumentation + +Instrumentation for [AWS Java SDK v1](https://github.com/aws/aws-sdk-java). + +## Trace propagation + +The AWS SDK instrumentation currently only supports injecting the trace header into the request +using the [AWS Trace Header](https://docs.aws.amazon.com/xray/latest/devguide/xray-concepts.html#xray-concepts-tracingheader) format. +This format is the only format recognized by AWS managed services, and populating will allow +propagating the trace through them. If this does not fulfill your use case, perhaps because you are +using the same SDK with a different non-AWS managed service, let us know so we can provide +configuration for this behavior. diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/aws-sdk-1.11-javaagent.gradle b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/aws-sdk-1.11-javaagent.gradle index 629e8d2b79..c18993cfdc 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/aws-sdk-1.11-javaagent.gradle +++ b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/aws-sdk-1.11-javaagent.gradle @@ -45,6 +45,8 @@ configurations { } dependencies { + compileOnly deps.opentelemetryTraceProps + library group: 'com.amazonaws', name: 'aws-java-sdk-core', version: '1.11.0' // Include httpclient instrumentation for testing because it is a dependency for aws-sdk. @@ -56,6 +58,9 @@ dependencies { testLibrary group: 'com.amazonaws', name: 'aws-java-sdk-sqs', version: '1.11.106' testLibrary group: 'com.amazonaws', name: 'aws-java-sdk-dynamodb', version: '1.11.106' + // Make sure doesn't add HTTP headers + testInstrumentation project(':instrumentation:apache-httpclient:apache-httpclient-4.0:javaagent') + // needed for kinesis: testImplementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: versions.jackson diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/AwsSdkClientTracer.java b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/AwsSdkClientTracer.java index 92d89c20fb..1ec779be69 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/AwsSdkClientTracer.java +++ b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/AwsSdkClientTracer.java @@ -12,6 +12,7 @@ import com.amazonaws.Response; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapPropagator; +import io.opentelemetry.extension.trace.propagation.AwsXrayPropagator; import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer; import java.net.URI; import java.util.concurrent.ConcurrentHashMap; @@ -30,6 +31,11 @@ public class AwsSdkClientTracer extends HttpClientTracer, Request, public AwsSdkClientTracer() {} + @Override + protected void inject(Context context, Request request) { + AwsXrayPropagator.getInstance().inject(context, request, AwsSdkInjectAdapter.INSTANCE); + } + @Override protected String spanNameForRequest(Request request) { if (request == null) { @@ -112,7 +118,8 @@ public class AwsSdkClientTracer extends HttpClientTracer, Request, @Override protected TextMapPropagator.Setter> getSetter() { - return AwsSdkInjectAdapter.INSTANCE; + // We override injection and don't want to have the base class do it accidentally. + return null; } @Override diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/AwsSdkInstrumentationModule.java b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/AwsSdkInstrumentationModule.java index 0a0dbac6a5..69c1789302 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/AwsSdkInstrumentationModule.java +++ b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/AwsSdkInstrumentationModule.java @@ -20,6 +20,11 @@ public class AwsSdkInstrumentationModule extends InstrumentationModule { super("aws-sdk", "aws-sdk-1.11"); } + @Override + public String[] additionalHelperClassNames() { + return new String[] {"io.opentelemetry.extension.trace.propagation.AwsXrayPropagator"}; + } + @Override public List typeInstrumentations() { return asList( diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/groovy/Aws1ClientTest.groovy b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/groovy/Aws1ClientTest.groovy index a5f9f39265..95abf4c439 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/groovy/Aws1ClientTest.groovy +++ b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/groovy/Aws1ClientTest.groovy @@ -159,7 +159,8 @@ class Aws1ClientTest extends AgentTestRunner { } } } - server.lastRequest.headers.get("traceparent") != null + server.lastRequest.headers.get("X-Amzn-Trace-Id") != null + server.lastRequest.headers.get("traceparent") == null where: service | operation | method | path | handlerCount | client | call | additionalAttributes | body diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test_before_1_11_106/groovy/Aws0ClientTest.groovy b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test_before_1_11_106/groovy/Aws0ClientTest.groovy index 0939ae2a7f..f271b3cb9f 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test_before_1_11_106/groovy/Aws0ClientTest.groovy +++ b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test_before_1_11_106/groovy/Aws0ClientTest.groovy @@ -122,7 +122,8 @@ class Aws0ClientTest extends AgentTestRunner { } } } - server.lastRequest.headers.get("traceparent") != null + server.lastRequest.headers.get("X-Amzn-Trace-Id") != null + server.lastRequest.headers.get("traceparent") == null where: service | operation | method | path | handlerCount | client | additionalAttributes | call | body