From c23b063c0f5464ab875fd30016d7e5ff1c870315 Mon Sep 17 00:00:00 2001 From: jdoherty07 Date: Wed, 26 Apr 2023 10:51:57 +0100 Subject: [PATCH 1/3] read tracing info from system properties --- .../AwsXrayEnvSpanLinksExtractor.java | 34 +++++++++--- .../AwsXrayEnvSpanLinksExtractorTest.java | 53 ++++++++++++++++--- 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java b/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java index 342f65b50ab2..65304a35fc74 100644 --- a/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java +++ b/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java @@ -16,6 +16,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.SpanLinksExtractor; import io.opentelemetry.instrumentation.awslambdacore.v1_0.AwsLambdaRequest; import java.util.Collections; +import java.util.HashMap; import java.util.Locale; import java.util.Map; @@ -26,7 +27,7 @@ final class AwsXrayEnvSpanLinksExtractor implements SpanLinksExtractor { private static final String AWS_TRACE_HEADER_ENV_KEY = "_X_AMZN_TRACE_ID"; - + private static final String AWS_TRACE_HEADER_PROP = "com.amazonaws.xray.traceHeader"; // lower-case map getter used for extraction private static final String AWS_TRACE_HEADER_PROPAGATOR_KEY = "x-amzn-trace-id"; @@ -42,22 +43,39 @@ public void extract( } public static void extract(SpanLinksBuilder spanLinks) { - String parentTraceHeader = System.getenv(AWS_TRACE_HEADER_ENV_KEY); - if (parentTraceHeader == null || parentTraceHeader.isEmpty()) { + Map contextMap = getSystemPropertyOrEnvironmentVariable(); + if (contextMap.isEmpty()) { return; } Context xrayContext = - AwsXrayPropagator.getInstance() - .extract( - Context.root(), - Collections.singletonMap(AWS_TRACE_HEADER_PROPAGATOR_KEY, parentTraceHeader), - MapGetter.INSTANCE); + AwsXrayPropagator.getInstance().extract(Context.root(), contextMap, MapGetter.INSTANCE); SpanContext envVarSpanCtx = Span.fromContext(xrayContext).getSpanContext(); if (envVarSpanCtx.isValid()) { spanLinks.addLink(envVarSpanCtx, LINK_ATTRIBUTES); } } + public static Map getSystemPropertyOrEnvironmentVariable() { + String parentTraceEnvKeyEnvironment = System.getenv(AWS_TRACE_HEADER_ENV_KEY); + String parentTraceHeaderProperty = System.getProperty(AWS_TRACE_HEADER_PROP); + boolean isEnvVariableEmptyOrNull = isEmptyOrNull(parentTraceEnvKeyEnvironment); + boolean isSystemPropertyEmptyOrNull = isEmptyOrNull(parentTraceHeaderProperty); + + Map contextMap = new HashMap<>(); + if (!isSystemPropertyEmptyOrNull) { + contextMap.put(AWS_TRACE_HEADER_PROPAGATOR_KEY, parentTraceHeaderProperty); + return contextMap; + } else if (!isEnvVariableEmptyOrNull) { + contextMap.put(AWS_TRACE_HEADER_PROPAGATOR_KEY, parentTraceEnvKeyEnvironment); + return contextMap; + } + return Collections.emptyMap(); + } + + public static boolean isEmptyOrNull(String value) { + return value == null || value.isEmpty(); + } + private enum MapGetter implements TextMapGetter> { INSTANCE; diff --git a/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/test/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractorTest.java b/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/test/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractorTest.java index 061f59cdf95b..509bfbd05eaf 100644 --- a/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/test/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractorTest.java +++ b/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/test/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractorTest.java @@ -21,6 +21,7 @@ import uk.org.webcompere.systemstubs.environment.EnvironmentVariables; import uk.org.webcompere.systemstubs.jupiter.SystemStub; import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension; +import uk.org.webcompere.systemstubs.properties.SystemProperties; /** * This class is internal and is hence not for public use. Its APIs are unstable and can change at @@ -32,19 +33,42 @@ class AwsXrayEnvSpanLinksExtractorTest { Attributes.of(AttributeKey.stringKey("source"), "x-ray-env"); @SystemStub final EnvironmentVariables environmentVariables = new EnvironmentVariables(); + @SystemStub final SystemProperties systemProperties = new SystemProperties(); @Test - void shouldIgnoreIfEnvVarEmpty() { + void shouldIgnoreIfEnvVarAndSystemPropertyEmpty() { // given SpanLinksBuilder spanLinksBuilder = mock(SpanLinksBuilder.class); environmentVariables.set("_X_AMZN_TRACE_ID", ""); - + systemProperties.set("com.amazonaws.xray.traceHeader", ""); // when AwsXrayEnvSpanLinksExtractor.extract(spanLinksBuilder); // then verifyNoInteractions(spanLinksBuilder); } + @Test + void shouldLinkAwsParentHeaderAndChooseSystemPropertyIfValidAndNotSampled() { + // given + SpanLinksBuilder spanLinksBuilder = mock(SpanLinksBuilder.class); + environmentVariables.set( + "_X_AMZN_TRACE_ID", + "Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=0000000000000456;Sampled=0"); + systemProperties.set( + "com.amazonaws.xray.traceHeader", + "Root=1-8a3c60f7-d188f8fa79d48a391a778fa7;Parent=0000000000000789;Sampled=0"); + // when + AwsXrayEnvSpanLinksExtractor.extract(spanLinksBuilder); + // then + ArgumentCaptor captor = ArgumentCaptor.forClass(SpanContext.class); + verify(spanLinksBuilder).addLink(captor.capture(), eq(EXPECTED_LINK_ATTRIBUTES)); + SpanContext spanContext = captor.getValue(); + assertThat(spanContext.isValid()).isTrue(); + assertThat(spanContext.isSampled()).isFalse(); + assertThat(spanContext.getSpanId()).isEqualTo("0000000000000789"); + assertThat(spanContext.getTraceId()).isEqualTo("8a3c60f7d188f8fa79d48a391a778fa7"); + } + @Test void shouldLinkAwsParentHeaderIfValidAndNotSampled() { // given @@ -52,7 +76,25 @@ void shouldLinkAwsParentHeaderIfValidAndNotSampled() { environmentVariables.set( "_X_AMZN_TRACE_ID", "Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=0000000000000456;Sampled=0"); + // when + AwsXrayEnvSpanLinksExtractor.extract(spanLinksBuilder); + // then + ArgumentCaptor captor = ArgumentCaptor.forClass(SpanContext.class); + verify(spanLinksBuilder).addLink(captor.capture(), eq(EXPECTED_LINK_ATTRIBUTES)); + SpanContext spanContext = captor.getValue(); + assertThat(spanContext.isValid()).isTrue(); + assertThat(spanContext.isSampled()).isFalse(); + assertThat(spanContext.getSpanId()).isEqualTo("0000000000000456"); + assertThat(spanContext.getTraceId()).isEqualTo("8a3c60f7d188f8fa79d48a391a778fa6"); + } + @Test + void shouldLinkAwsParentHeaderIfValidAndNotSampledSystemProperty() { + // given + SpanLinksBuilder spanLinksBuilder = mock(SpanLinksBuilder.class); + systemProperties.set( + "com.amazonaws.xray.traceHeader", + "Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=0000000000000456;Sampled=0"); // when AwsXrayEnvSpanLinksExtractor.extract(spanLinksBuilder); // then @@ -66,13 +108,12 @@ void shouldLinkAwsParentHeaderIfValidAndNotSampled() { } @Test - void shouldLinkAwsParentHeaderIfValidAndSampled() { + void shouldLinkAwsParentHeaderIfValidAndSampledSystemProperty() { // given SpanLinksBuilder spanLinksBuilder = mock(SpanLinksBuilder.class); - environmentVariables.set( - "_X_AMZN_TRACE_ID", + systemProperties.set( + "com.amazonaws.xray.traceHeader", "Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=0000000000000456;Sampled=1"); - // when AwsXrayEnvSpanLinksExtractor.extract(spanLinksBuilder); // then From de83bcaf977dac0f80314e42c70164b61d058691 Mon Sep 17 00:00:00 2001 From: jdoherty07 Date: Wed, 26 Apr 2023 21:14:28 +0100 Subject: [PATCH 2/3] updated implementation following review --- .../AwsXrayEnvSpanLinksExtractor.java | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java b/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java index 65304a35fc74..b158f46be17f 100644 --- a/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java +++ b/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java @@ -16,7 +16,6 @@ import io.opentelemetry.instrumentation.api.instrumenter.SpanLinksExtractor; import io.opentelemetry.instrumentation.awslambdacore.v1_0.AwsLambdaRequest; import java.util.Collections; -import java.util.HashMap; import java.util.Locale; import java.util.Map; @@ -55,21 +54,14 @@ public static void extract(SpanLinksBuilder spanLinks) { } } - public static Map getSystemPropertyOrEnvironmentVariable() { - String parentTraceEnvKeyEnvironment = System.getenv(AWS_TRACE_HEADER_ENV_KEY); - String parentTraceHeaderProperty = System.getProperty(AWS_TRACE_HEADER_PROP); - boolean isEnvVariableEmptyOrNull = isEmptyOrNull(parentTraceEnvKeyEnvironment); - boolean isSystemPropertyEmptyOrNull = isEmptyOrNull(parentTraceHeaderProperty); - - Map contextMap = new HashMap<>(); - if (!isSystemPropertyEmptyOrNull) { - contextMap.put(AWS_TRACE_HEADER_PROPAGATOR_KEY, parentTraceHeaderProperty); - return contextMap; - } else if (!isEnvVariableEmptyOrNull) { - contextMap.put(AWS_TRACE_HEADER_PROPAGATOR_KEY, parentTraceEnvKeyEnvironment); - return contextMap; + private static Map getSystemPropertyOrEnvironmentVariable() { + String traceHeader = System.getProperty(AWS_TRACE_HEADER_PROP); + if (isEmptyOrNull(traceHeader)) { + traceHeader = System.getenv(AWS_TRACE_HEADER_ENV_KEY); } - return Collections.emptyMap(); + return isEmptyOrNull(traceHeader) + ? Collections.emptyMap() + : Collections.singletonMap(AWS_TRACE_HEADER_PROPAGATOR_KEY, traceHeader); } public static boolean isEmptyOrNull(String value) { From 59ccab28dec3035a4f808d38f0758b957b34bd11 Mon Sep 17 00:00:00 2001 From: jdoherty07 Date: Wed, 3 May 2023 10:54:58 +0100 Subject: [PATCH 3/3] updated method name per PR comment from Alexander --- .../v1_0/internal/AwsXrayEnvSpanLinksExtractor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java b/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java index b158f46be17f..c88cf20c917d 100644 --- a/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java +++ b/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/AwsXrayEnvSpanLinksExtractor.java @@ -42,7 +42,7 @@ public void extract( } public static void extract(SpanLinksBuilder spanLinks) { - Map contextMap = getSystemPropertyOrEnvironmentVariable(); + Map contextMap = getTraceHeaderMap(); if (contextMap.isEmpty()) { return; } @@ -54,7 +54,7 @@ public static void extract(SpanLinksBuilder spanLinks) { } } - private static Map getSystemPropertyOrEnvironmentVariable() { + private static Map getTraceHeaderMap() { String traceHeader = System.getProperty(AWS_TRACE_HEADER_PROP); if (isEmptyOrNull(traceHeader)) { traceHeader = System.getenv(AWS_TRACE_HEADER_ENV_KEY); @@ -64,7 +64,7 @@ private static Map getSystemPropertyOrEnvironmentVariable() { : Collections.singletonMap(AWS_TRACE_HEADER_PROPAGATOR_KEY, traceHeader); } - public static boolean isEmptyOrNull(String value) { + private static boolean isEmptyOrNull(String value) { return value == null || value.isEmpty(); }