Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FaaS: Change requirements regarding handling of AWS Lambda-provided SpanContext #272

Closed
wants to merge 3 commits into from
Closed

FaaS: Change requirements regarding handling of AWS Lambda-provided SpanContext #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
07435bc
FaaS: Clarify requirements regarding handling of AWS Lambda-provided …
Aneurysm9 Aug 18, 2023
4304636
Merge branch 'main' of github.com:open-telemetry/semantic-conventions…
Aneurysm9 Aug 21, 2023
fc59e27
update changelog to reflect requested change to change description
Aneurysm9 Aug 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ release.
([#60](https://github.com/open-telemetry/semantic-conventions/pull/60))
- BREAKING: Remove pluralization from JVM metric namespaces.
([#252](https://github.com/open-telemetry/semantic-conventions/pull/252))
- FaaS: Change requirements regarding handling of AWS Lambda-provided `SpanContext`
([#272](https://github.com/open-telemetry/semantic-conventions/pull/272))

## v1.21.0 (2023-07-13)

Expand Down
30 changes: 18 additions & 12 deletions docs/faas/aws-lambda.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use cases.
<!-- toc -->

- [All triggers](#all-triggers)
* [AWS X-Ray Environment Span Link](#aws-x-ray-environment-span-link)
* [AWS X-Ray Environment `SpanContext`](#aws-x-ray-environment-spancontext)
- [API Gateway](#api-gateway)
- [SQS](#sqs)
* [SQS Event](#sqs-event)
Expand Down Expand Up @@ -54,19 +54,25 @@ and the [cloud resource conventions][cloud]. The following AWS Lambda-specific a
[faasres]: /docs/resource/faas.md (FaaS resource conventions)
[cloud]: /docs/resource/cloud.md (Cloud resource conventions)

### AWS X-Ray Environment Span Link
### AWS X-Ray Environment `SpanContext`

If the `_X_AMZN_TRACE_ID` environment variable is set, instrumentation SHOULD try to parse an
OpenTelemetry `Context` out of it using the [AWS X-Ray Propagator](https://github.com/open-telemetry/opentelemetry-specification/tree/v1.22.0/specification/context/api-propagators.md). If the
resulting `Context` is [valid](https://github.com/open-telemetry/opentelemetry-specification/tree/v1.22.0/specification/trace/api.md#isvalid) then a [Span Link][] SHOULD be added to the new Span's
[start options](https://github.com/open-telemetry/opentelemetry-specification/tree/v1.22.0/specification/trace/api.md#specifying-links) with an associated attribute of `source=x-ray-env` to
indicate the source of the linked span.
Instrumentation MUST check if the context is valid before using it because the `_X_AMZN_TRACE_ID` environment variable can
contain an incomplete trace context which indicates X-Ray isn’t enabled. The environment variable will be set and the
`Context` will be valid and sampled only if AWS X-Ray has been enabled for the Lambda function. A user can
disable AWS X-Ray for the function if the X-Ray Span Link is not desired.
If the `_X_AMZN_TRACE_ID` environment variable contains a non-empty value then instrumentation MUST attempt to extract an
OpenTelemetry `Context` from it using the [AWS X-Ray Propagator](https://github.com/open-telemetry/opentelemetry-specification/tree/v1.22.0/specification/context/api-propagators.md). If the
resulting `Context` contains a [valid `SpanContext`](https://github.com/open-telemetry/opentelemetry-specification/tree/v1.22.0/specification/trace/api.md#isvalid) then the
instrumentation MUST utilize it in one of the following ways when creating a `Span` representing the invocation (the `Invocation Span`):

**Note**: When instrumenting a Java AWS Lambda, instrumentation SHOULD first try to parse an OpenTelemetry `Context` out of the system property `com.amazonaws.xray.traceHeader` using the [AWS X-Ray Propagator](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/api-propagators.md) before checking and attempting to parse the environment variable above.
* as the parent `Context`
* as a `SpanContext` associated with a [Span Link][] added to the `Invocation Span`'s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this SpanContext is used as a span link instead of the parent, it won't work with ParentBased samplers like the OTel default ParentBased(root=AlwaysOn).

Does AWS provide head sampling through this SpanContext?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Head sampling can be provided through this SpanContext. It is my understanding that that is irrelevant to the users who wish to have it associated as a link because they are not using X-Ray as their tracing backend and do not desire to have a trace including the Lambda-vended segments. I believe the expectation is that those users will obtain a SpanContext from another source, such as HTTP headers included in the invocation event, and would want to use that SpanContext for any sampling decisions.

[start options](https://github.com/open-telemetry/opentelemetry-specification/tree/v1.22.0/specification/trace/api.md#specifying-links) with an associated `String`-typed
attribute with a key of `source` and a value of `x-ray-env`, in order to indicate the source of the linked span.

Instrumentation MUST provide a mechanism for the user to select the behavior that they require it to utilize. In the absence of an expressed user preference
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence flips the previous default setting. Is that intended / agreed upon by the FAAS SiG?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the current state of most Lambda instrumentation. It is preserving the functioning behavior for users who had functioning instrumentation while clarifying that it is permissible to have an alternate behavior. I feel that this is a clarification as the spec as written permits this behavior and that was not apparent to all in the discussion during the last FaaS SIG meeting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Aneurysm9 Java and the semconv docs are not inline with this, and we would be indeed flipping the current default in this repo.

As said before: having this as default as direct parent will work great with X-Ray but will break the other vendors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As said before: having this as default as direct parent will work great with X-Ray but will break the other vendors.

I do not believe that this is a correct interpretation of the situation. Having the default behavior to be to use the value as a parent SpanContext will preserve the existing behavior for for current users in all languages but Java. It will not break any users. Users who require a different behavior will be able to select that behavior.

Java and the semconv docs are not inline with this, and we would be indeed flipping the current default in this repo.

Java implemented a spec change that was incomplete and acknowledged to require changes. This is the semconv repo and this change is fixing the "semconv docs". In any event, this is "experimental" specification and subject to change. Is there a special process for such changes I'm unaware of that I should be following?

instrumentation SHOULD utilize the extracted `Context` as the parent context for the `Invocation Span`. Instrumentation MAY require that the user explicitly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to allow explicit configuration as a requirement? I'd prefer if we could all agree on a default behavior here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, explicit configuration is a requirement. The Lambda-provided context does different things for different people and they need a mechanism for communicating their intent to the instrumentation.

choose a behavior.

**Note**: When instrumenting a Java AWS Lambda function, instrumentation SHOULD first try to extract an OpenTelemetry `Context` from the system property `com.amazonaws.xray.traceHeader`
using the [AWS X-Ray Propagator](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/api-propagators.md) before attempting to
extract a `Context` from the environment variable above.

[Span Link]: https://opentelemetry.io/docs/concepts/signals/traces/#span-links

Expand Down