Add requirement to use X-Ray propagation for aws-sdk client calls

[tylerbenson]

X-Ray is necessary because it is the only format currently supported by AWS to allow context propagation through AWS managed services, for example SNS -> SQS -> Lambda, or S3 -> Lambda.

[carlosalberto]

Changelog entry please ;)

[Oberon00]

X-Ray is necessary because it is the only format currently supported by AWS to allow context propagation through AWS managed services, for example SNS -> SQS -> Lambda

At Dynatrace, we have actually been using W3C-tracecontext-based propagation successfully with this scenario, as SNS message attributes are mapped to SQS message attributes or transported as part of the message body (JSON payload). Our backend relies on additional information in the tracestate field that cannot be transported over the X-Ray header.
So we rely on the fact that many existing implementations of this (e.g. Node.Js, Ruby, Python) use (non-system) message attributes and the configured global propagator for this.

If these were changed to only use X-Ray, it would break these scenarios for Dynatrace and our customers (the scenarios are documented e.g. here: https://www.dynatrace.com/support/help/shortlink/opentel-lambda#python-sqs-sns). For example the Python PR open-telemetry/opentelemetry-python-contrib#1673 that probably results from this Spec PR would be problematic for us (we'll also comment there).

or S3 -> Lambda.

This is indeed only possible with using X-Ray headers (but doesn't invalidate the previous scenario)

I propose that we clarify that additional propagation mechanisms MAY be used where it makes sense (e.g. messaging operations).

Also, I think there is no need to use the strong MUST which basically bans instrumentations from offering an option to disable X-Ray propagation.

[tsloughter]

I think for HTTP headers in AWS requests the Xray propagator should always be used (until a time AWS changes this).

For per-service instrumentation like MessageAttributes in SQS there is no need because it is invisible to AWS anyway.

What I'm not sure on is whether SQS instrumentation should be additionally inserting context into MessageAttributes or not.

@Oberon00 I notice dynatrace python guide has the user defining the extraction themselves because python's lambda instrumentation doesn't do it -- something I'd like to change but it would be a Link and not the parent, so not a replacement for what you have here. Would requiring similar for injection not be acceptable?

[Oberon00]

Would requiring similar for injection not be acceptable?

For extraction this is conceptually required for batch processing. For injection, it would be a shame if the user would have to do it manually. That would also mean that they would have to create the span manually (since you can only do manual injection before calling the function in which the span is created by the instrumentation). Basically we would not be able to use the instrumentation at all anymore and instruct users to do everything manually, or create and distribute a fork of the instrumentation code.

[carlosalberto]

So trying to summarize the current state after the latest comments, just to be clear (please do correct me if I'm wrong):

• X-Ray seems to be the only propagated format currently, but we would like to keep the door open for future AWS approved/supported formats?
• Propagation support through message attributes: although the plan has been to allow instrumentation to fallback to this, is this something that can be totally unnecessary by allowing custom propagation formats? Can tracestate really be propagated this way? Based on the previous comments, I'd say no?

On a slightly related note, there's a (merged) OTEP around Messaging Context Propagation which mentions that per-Message context should be propagated, rather than per-request context (cc @pyohannes in case you have two cents to add here, etc).

[Oberon00]

@carlosalberto

X-Ray seems to be the only propagated format currently, but we would like to keep the door open for future AWS approved/supported formats?

This is true in the general case. But specific APIs will often provide other ways where you can transport extract/inject key-value-pairs, e.g. message attributes for SNS or SQS.

[trask]

/easycla

[nickmango]

/easycla

[trask]

/easycla

[trask]

/easycla

[jarias-lfx]

/easycla

[carlosalberto]

@jsuereth @Aneurysm9 @Oberon00 please review the updated PR - the content itself remains the same, albeit located now in the non-formal supplementary-guides/ directory.

[Oberon00]

I realize I added a lot of suggestions despite approving -- but none of them are blocking, so I would actually be OK-ish with merging this as-is (but please do consider my suggestions nevertheless 🙂)

[reyang]

This PR has been existing for long time and already received multiple approvals, I will merge it once the CI is clear. The change would be released in the new https://github.com/open-telemetry/semantic-conventions (to be created) repository.

Related to #3474 (review).