Skip to content

Commit

Permalink
[Bug fixes] Lambda - duplicate lambda spans + appsignals from unsampl…
Browse files Browse the repository at this point in the history
…ed spans (#1000)

### Issue 1 - Duplicate lambda root spans
The instrumentation produces 2 lambda root spans upon each invocation of
the function. This is a known issue in OTel:
open-telemetry/opentelemetry-java-instrumentation#7808

#### Fix
- Otel has fixed it in [this
PR](open-telemetry/opentelemetry-java-instrumentation#10736)
but the change wasn't ported to the 1.x versions. So I took the diff
from the upstream commit and made it into a patch that we apply when
building the ADOT Java Lambda Layer for v1.33.x.

### Issue 2 - Unsampled spans do not produce Application Signals metrics
On lambda environment, we export 100% of the spans to X-Ray to ensure we
are able to provide 100% Application Signals metrics. However, currently
only the sampled spans show up on the "Services" page and the unsampled
spans do not.

#### Fix
- Upon comparing the sampled vs unsampled spans, I noticed that the
unsampled spans are missing the attributes like `aws.local.service` and
`aws.local.operation` which are required to generate Application Signals
metrics.
- The fix is to wrap the `OtlpUdpSpanExporter` instance for unsampled
spans with the `AwsMetricAttributesSpanExporter` so that the exported
spans have the desired attributes.

#### Testing
- After creating a layer with the fix, I set the `OTEL_TRACES_SAMPLER`
to `always_off`. Then I invoked the function once.
- The metrics appeared on the Application Signals console.
- See the following screenshots
  
<img width="1722" alt="image"
src="https://github.com/user-attachments/assets/fa10e09f-ae24-4ab3-989f-838aacfb7e50"
/>

<img width="1722" alt="image"
src="https://github.com/user-attachments/assets/d777304f-2cd9-4348-8e29-74f4a8b6b917"
/>


*Description of changes:*


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
  • Loading branch information
srprash authored Jan 15, 2025
1 parent a7d3e00 commit c8bce79
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,27 @@ private SdkTracerProviderBuilder customizeTracerProviderBuilder(

// If running on Lambda, we just need to export 100% spans and skip generating any Application
// Signals metrics.
if (isLambdaEnvironment()) {
if (isLambdaEnvironment()
&& System.getenv(OTEL_EXPORTER_OTLP_TRACES_ENDPOINT_CONFIG) == null) {
String tracesEndpoint =
Optional.ofNullable(System.getenv(AWS_XRAY_DAEMON_ADDRESS_CONFIG))
.orElse(DEFAULT_UDP_ENDPOINT);
SpanExporter spanExporter =
new OtlpUdpSpanExporterBuilder()
.setPayloadSampleDecision(TracePayloadSampleDecision.UNSAMPLED)
.setEndpoint(tracesEndpoint)
.build();

// Wrap the udp exporter with the AwsMetricsAttributesSpanExporter to add Application
// Signals attributes to unsampled spans too
SpanExporter appSignalsSpanExporter =
AwsMetricAttributesSpanExporterBuilder.create(
spanExporter, ResourceHolder.getResource())
.build();

tracerProviderBuilder.addSpanProcessor(
AwsUnsampledOnlySpanProcessorBuilder.create()
.setSpanExporter(appSignalsSpanExporter)
.setMaxExportBatchSize(LAMBDA_SPAN_EXPORT_BATCH_SIZE)
.build());
return tracerProviderBuilder;
Expand Down
88 changes: 87 additions & 1 deletion lambda-layer/patches/opentelemetry-java-instrumentation.patch
Original file line number Diff line number Diff line change
Expand Up @@ -642,4 +642,90 @@ index cc1414c0bf..db8a59b046 100644
+val alphaVersion = "1.33.6-adot-lambda1-alpha"

allprojects {
if (findProperty("otel.stable") != "true") {
if (findProperty("otel.stable") != "true") {
diff --git a/instrumentation/aws-lambda/aws-lambda-core-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awslambdacore/v1_0/AwsLambdaRequestHandlerInstrumentation.java b/instrumentation/aws-lambda/aws-lambda-core-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awslambdacore/v1_0/AwsLambdaRequestHandlerInstrumentation.java
index 2332f24a8f..0743cdea75 100644
--- a/instrumentation/aws-lambda/aws-lambda-core-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awslambdacore/v1_0/AwsLambdaRequestHandlerInstrumentation.java
+++ b/instrumentation/aws-lambda/aws-lambda-core-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awslambdacore/v1_0/AwsLambdaRequestHandlerInstrumentation.java
@@ -10,7 +10,9 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.
import static io.opentelemetry.javaagent.instrumentation.awslambdacore.v1_0.AwsLambdaInstrumentationHelper.functionInstrumenter;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
+import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.not;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;

import com.amazonaws.services.lambda.runtime.Context;
@@ -35,7 +37,8 @@ public class AwsLambdaRequestHandlerInstrumentation implements TypeInstrumentati

@Override
public ElementMatcher<TypeDescription> typeMatcher() {
- return implementsInterface(named("com.amazonaws.services.lambda.runtime.RequestHandler"));
+ return implementsInterface(named("com.amazonaws.services.lambda.runtime.RequestHandler"))
+ .and(not(nameStartsWith("com.amazonaws.services.lambda.runtime.api.client")));
}

@Override
diff --git a/instrumentation/aws-lambda/aws-lambda-core-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/awslambdacore/v1_0/AwsLambdaTest.java b/instrumentation/aws-lambda/aws-lambda-core-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/awslambdacore/v1_0/AwsLambdaTest.java
index af939fcb6d..8b8398950a 100644
--- a/instrumentation/aws-lambda/aws-lambda-core-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/awslambdacore/v1_0/AwsLambdaTest.java
+++ b/instrumentation/aws-lambda/aws-lambda-core-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/awslambdacore/v1_0/AwsLambdaTest.java
@@ -5,14 +5,19 @@

package io.opentelemetry.javaagent.instrumentation.awslambdacore.v1_0;

+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
import static org.assertj.core.api.Assertions.assertThat;

import com.amazonaws.services.lambda.runtime.Context;
import com.amazonaws.services.lambda.runtime.RequestHandler;
+import com.amazonaws.services.lambda.runtime.api.client.AwsLambdaInternalRequestHandler;
+import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.instrumentation.awslambdacore.v1_0.AbstractAwsLambdaTest;
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import io.opentelemetry.semconv.SemanticAttributes;
import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

public class AwsLambdaTest extends AbstractAwsLambdaTest {
@@ -35,6 +40,22 @@ public class AwsLambdaTest extends AbstractAwsLambdaTest {
assertThat(testing.forceFlushCalled()).isTrue();
}

+ @Test
+ void awsLambdaInternalHandlerIgnoredAndUserHandlerTraced() {
+ RequestHandler<String, String> handler = new AwsLambdaInternalRequestHandler(handler());
+ String result = handler.handleRequest("hello", context());
+ assertThat(result).isEqualTo("world");
+ testing()
+ .waitAndAssertTraces(
+ trace ->
+ trace.hasSpansSatisfyingExactly(
+ span ->
+ span.hasName("my_function")
+ .hasKind(SpanKind.SERVER)
+ .hasAttributesSatisfyingExactly(
+ equalTo(SemanticAttributes.FAAS_INVOCATION_ID, "1-22-333"))));
+ }
+
private static final class TestRequestHandler implements RequestHandler<String, String> {

@Override
diff --git a/instrumentation/aws-lambda/aws-lambda-core-1.0/testing/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/AbstractAwsLambdaTest.java b/instrumentation/aws-lambda/aws-lambda-core-1.0/testing/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/AbstractAwsLambdaTest.java
index 94a85244e2..25a32896aa 100644
--- a/instrumentation/aws-lambda/aws-lambda-core-1.0/testing/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/AbstractAwsLambdaTest.java
+++ b/instrumentation/aws-lambda/aws-lambda-core-1.0/testing/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/AbstractAwsLambdaTest.java
@@ -53,6 +53,10 @@ public abstract class AbstractAwsLambdaTest {
assertThat(testing().forceFlushCalled()).isTrue();
}

+ protected Context context() {
+ return context;
+ }
+
@Test
void handlerTraced() {
String result = handler().handleRequest("hello", context);

0 comments on commit c8bce79

Please sign in to comment.