feat(aws-lambda): disableAwsContextPropagation config option

[nirsky]

Which problem is this PR solving?

Lambda instrumentation is trying by default to use the x-ray context header and extract the span context from it.
It appears AWS is attaching this header to lambda even if x-ray is disabled, this results in a non-sampled context, which creates a NonRecordingSpan when using parent-based sampler.
If, for example, I have a lambda triggered by a cron job and have x-ray disabled, I will always end up with a NonRecordingSpan.

Short description of the changes

Added disableAwsContextPropagation config option, to disable this aws x-ray propagation altogether.
The change was discussed with instrumentation author @anuraaga

[codecov]

Codecov Report

Merging #546 (c01c29f) into main (c1b6eec) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #546      +/-   ##
==========================================
+ Coverage   94.91%   94.93%   +0.01%     
==========================================
  Files         166      166              
  Lines       10313    10337      +24     
  Branches     1026     1027       +1     
==========================================
+ Hits         9789     9813      +24     
  Misses        524      524              

Impacted Files	Coverage Δ
...-instrumentation-aws-lambda/src/instrumentation.ts	91.79% <0.00%> (+0.06%)	⬆️
...ws-lambda/test/integrations/lambda-handler.test.ts	97.87% <0.00%> (+0.15%)	⬆️

[dyladan]

Why is the instrumentation doing its own extraction and not simply calling the propagation API?

[Flarna]

Why is the instrumentation doing its own extraction and not simply calling the propagation API?

see #431

propagation across AWS services seems to work only via X-Ray but not via W3C headers.

[anuraaga]

Yeah unfortunately there seems to be a bug in API gateway or Lambda which causes the trace context in the environment variable to be set to something even when tracing is disabled. The environment variable is meant for propagation through AWS services but shouldn't be set if there's nothing to propagate nor tracing is enabled, but something seems to be up necessitating this sort of option.

The parent-based hint is interesting though - @nirsky do you think an approach works that doesn't involve a separate option, it just checks whether the currently configured sampler is parent-based or not?

[anuraaga]

Sorry to have this idea this late but does it work to add a check that that span context is valid and no need for the option? I just realized that I think in the problematic case the context has sampled = false, but actually doesn't have a valid span ID either and that could be enough of a signal

[nirsky]

What do you mean by checking that the span context is valid? Doesn't the if (spanContext ... check covers it?
Anyway, for our usage, we'd be interested to have this ability even if x-ray is turned on, I'll try to describe the scenario, hope it'll be clear:

Consider the following:

• Mircrosevice sends span with id x to the collector
• API Gateway sends span with id y to x-ray
• Lambda sends span with id z and parent span id of y.

We end up only having spans x and z in the collector, resulting in a broken trace.

[anuraaga]

I think if spanContext is only in the case that there is no span in the context, in practice I don't know if it can actually happen with this code.

There is an isSpanContextValid function

https://www.github.com/open-telemetry/opentelemetry-js-api/tree/main/src%2Fapi%2Ftrace.ts

For that use case, is the idea for the customer to have the trace both in x-ray and another backend via the collector and show the same trace in them?

[nirsky]

In this use case, the customer is interested only in having x and z where the parentSpanId of z is x.

[anuraaga]

In that case they can disable x-ray right? Is there a reason to enable it and be charged even without needing y?

[nirsky]

Some of our customers may have an existing x-ray set-up and may want to use our product in addition to other usages, without integrating x-ray traces into our product.
Our product provides some other value on top of observability.

Anyway, IMO it's up for the user to decide, each one has different considerations and it's reasonable to let the user decide how to configure this

[anuraaga]

Giving the option seems fine to me. Thanks