Do not force xray propagator, default adds W3C+B3

[NathanielRN]

Description:

Follow up to #140 which added the new xray name instead of aws_xray, we update this last lingering reference to also use xray we drop forcing the xray propagator entirely, and make the default composite propagator include:

• tracecontext
• b3
• xray

Link to tracking Issue: N/A

Testing: Will leave the testing to open-telemetry/opentelemetry-lambda#124

Documentation: N/A

[anuraaga]

Hmm - in other languages, we don't have this behavior of forcing in the xray propagator if the user set a propagator.

https://github.com/aws-observability/aws-otel-lambda/blob/main/java/scripts/otel-handler#L5

https://github.com/aws-observability/aws-otel-lambda/blob/main/nodejs/wrapper-adot/src/adot-extension.ts#L12

I feel this is better since in the end users always need to be allowed a choice, but currently it's not possible to disable the xray propagator no matter what.

Also good to have the default include w3c and b3 as the others

[NathanielRN]

Yeah I think it's a bit surprising that it forces its inclusion. I can see why though, it's just automatically trying to parse the _X_AMZN_TRACE_ID if it's available, and ensure the correct parenting is done without the users haven't to fret about it.

I think more customers would want this automatic behavior and worry that more customers would be confused as to why them setting OTEL_PROPAGATORS=tracecontext suddenly breaks their parenting when they call downstream services.

But either way that's a digression, the PR is just to fix this reference which didn't get updated in the previous PR. We can open a new PR if we don't like the current behavior.

Also good to have the default include w3c and b3 as the others

I can see why customers might come across _X_AMZN_TRACE_ID haphazardly tinkering with AWS services and settings, but maybe W3C and B3 contexts would require going a bit more out of their way to integrate with services that integrate these? I don't think there is any harm in adding them. We can do that in that next PR I mentioned above.

[anuraaga]

We explicitly specify the x-ray propagator when parsing from the environment variable so this setting doesn't affect that behavior

https://github.com/open-telemetry/opentelemetry-lambda/blob/main/python/src/otel/otel_sdk/opentelemetry/instrumentation/aws_lambda/__init__.py#L168

This should only be for extracting from HTTP or injecting into downstream HTTP - if the user explicitly set a format then it's the one to use, let's not insert the xray propagator. I think we can repurpose this PR given it would mean just deleting the currently modified line.

but maybe W3C and B3 contexts would require going a bit more out of their way to integrate with services that integrate these?

The idea is to allow the default when nothing is set to be as compatible as possible - tracecontext,b3,xray just means any format will be extracted and all formats will be injected. It ensures low chance of accidentally misparenting at the expense of some more overhead on injecting of formats that may not be used downstream - where it's a concern the customer would then specify the env variable to restrict to what they use.

This could be in a different PR - but seems fine to just do it together to keep the fixes to the defaults together.

[NathanielRN]

We explicitly specify the x-ray propagator when parsing from the environment variable so this setting doesn't affect that behavior

That is only for the span generated by the Lambda instrumentation on the code you linked to. I was talking more in general every time the OTel Python SDK calls propagator.inject which could be in several places in the instrumentation. Just a quick example

if the user explicitly set a format then it's the one to use, let's not insert the xray propagator. I think we can repurpose this PR given it would mean just deleting the currently modified line.

Okay I will repurpose this PR to do this.

This could be in a different PR - but seems fine to just do it together to keep the fixes to the defaults together.

Yeah it's a small change, I'll include it.

[anuraaga]

I was talking more in general every time the OTel Python SDK calls propagator.inject which could be in several places in the instrumentation.

Yeah - since we can't know if the downstream will be using the X-Ray SDK or not (possibly indirectly by being another Lambda function), we trust the user if they explicitly opt in, they have more information than us. Since the default when nothing is set is highly compatible, this is still generally seamless.

[anuraaga]

We explicitly specify the x-ray propagator when parsing from the environment variable so this setting doesn't affect that behavior

https://github.com/open-telemetry/opentelemetry-lambda/blob/main/python/src/otel/otel_sdk/opentelemetry/instrumentation/aws_lambda/__init__.py#L168

This should only be for extracting from HTTP or injecting into downstream HTTP - if the user explicitly set a format then it's the one to use, let's not insert the xray propagator. I think we can repurpose this PR given it would mean just deleting the currently modified line.

but maybe W3C and B3 contexts would require going a bit more out of their way to integrate with services that integrate these?

The idea is to allow the default when nothing is set to be as compatible as possible - tracecontext,b3,xray just means any format will be extracted and all formats will be injected. It ensures low chance of accidentally misparenting at the expense of some more overhead on injecting of formats that may not be used downstream - where it's a concern the customer would then specify the env variable to restrict to what they use.

This could be in a different PR - but seems fine to just do it together to keep the fixes to the defaults together.

[anuraaga]

I was talking more in general every time the OTel Python SDK calls propagator.inject which could be in several places in the instrumentation.

Yeah - since we can't know if the downstream will be using the X-Ray SDK or not (possibly indirectly by being another Lambda function), we trust the user if they explicitly opt in, they have more information than us. Since the default when nothing is set is highly compatible, this is still generally seamless.