opentelemetry-java-instrumentation icon indicating copy to clipboard operation
opentelemetry-java-instrumentation copied to clipboard

Published 20 hours ago • verified publisher icon open-telemetry

[aws-lambda] POC - Request for Comments - Add support to carrier pre-processing in aws-lambda instrumentation

Open rapphil opened this issue 2 years ago • 4 comments

This PR is a POC for the proposal FaaS: Change requirements regarding handling of AWS Lambda-provided SpanContext

In this PR we:

  • Change the default behaviour of lambda to always try to include AWS X-Ray environment traces keys in the carrier that will be offered to the extract operation of the global propagators. ~* Adds a new option otel.instrumentation.aws-lambda.link-xray-traces (also as env var OTEL_INSTRUMENTATION_AWS_LAMBDA_LINK_XRAY_TRACES) to allow users to include X-Ray traces originated from the lambda environment as span links.~

p.s.: there is no way to configure the global context propagators to include the X-Ray propagator to fully validate this change.

rapphil avatar Aug 24 '23 21:08 rapphil

@tylerbenson could you help reviewing this

laurit avatar Aug 25 '23 07:08 laurit

I'm trying to get clarification/confirmation from the TC. Once I get that, I'll review.

tylerbenson avatar Aug 28 '23 16:08 tylerbenson

I'm trying to get clarification/confirmation from the TC. Once I get that, I'll review.

Can you provide more information on what clarification/confirmation you are seeking?

bryan-aguilar avatar Aug 29 '23 18:08 bryan-aguilar

So there's a few fundamental decisions that were made in OTEL:

  1. Message passing tracing will use links as default - see approved OTEP 220.
  2. The TC weighed in on AWS lambda discussions from FAAS WG here.

Given these two, there's one central problem I see with this PR:

We should be preparing users for the new default parenting of spans in message passing scenarios to be links, not spans. We need to be working on a transition period and configuraiton to help users perserve the behavior they have today, but also let them know the default will be changing in OTEL (OTEP 220 guarantees this).

Given we are talking about breaking changes for users, I'd rather not churn how the instrumentation works any more than is strictly necessary. This PR does not move in the direction of OTEP 220, and I'm concerned the new environment variable that's added has the wrong default for the future of OTEL.

jsuereth avatar Aug 29 '23 21:08 jsuereth

is this PR still relevant?

jaydeluca avatar Feb 04 '25 12:02 jaydeluca

I don't think so.

tylerbenson avatar Feb 10 '25 16:02 tylerbenson