Updated Documentation For Change in Behaviour of Reading X-Ray Tracing Info From Lambda #3502
Conversation
| contain an incomplete trace context which indicates X-Ray isn’t enabled. The environment variable will be set and the | ||
| There are two common ways to read the tracing information if AWS X-Ray is enabled for a lambda function: | ||
|
|
||
| 1. Via the system property `com.amazonaws.xray.traceHeader`. |
There was a problem hiding this comment.
As this is language-independent spec, you need to add more context here on what a "system property" is. I believe this is something specific to the Java runtime?
There was a problem hiding this comment.
ok sure - I have updated this now - is this any better?
There was a problem hiding this comment.
Linking to the docs of system properties seems a bit overkill, probably any Java programmer should know them. I think just mentioning that it is Java specific is enough. If linking anything, maybe https://docs.aws.amazon.com/lambda/latest/dg/lambda-java.html
There was a problem hiding this comment.
ok sure I have removed this now
| If the system property `com.amazonaws.xray.traceHeader` is set, instrumentation SHOULD try to parse an OpenTelemetry `Context` out of it using the [AWS X-Ray Propagator](../../../context/api-propagators.md). | ||
| Alternatively, if the system property is not set then instrumentation SHOULD try to parse an OpenTelemetry `Context` out of the `_X_AMZN_TRACE_ID` environment variable using the [AWS X-Ray Propagator](../../../context/api-propagators.md). | ||
| If the resulting `Context` is [valid](../../api.md#isvalid) then a [Span Link][] SHOULD be added to the new Span's | ||
| [start options](../../api.md#specifying-links) with an associated attribute of `source=x-ray-env` to indicate the source of the linked span. |
There was a problem hiding this comment.
Such a link attribute should have a proper semantic convention YAML entry (type attribute_group since we don't have a specific type for links yet).
I expect this will also lead to larger discussions, e.g. whether an unprefixed attribute is OK, or which prefix to use, or, since this seems to be a generally useful attribute, whether we want something similar for the span parent.
There was a problem hiding this comment.
Sorry Christian - I'm not quite sure what you mean here - can you elaborate on this?
There was a problem hiding this comment.
When the spec recommends to set an attribute with a particular name, we always have a corresponding entry in the semantic conventions which are at just this moment moving from this repository to https://github.com/open-telemetry/semantic-conventions.
Adding them there will make many OpenTelemetry languages (semi-)automatically generate string constants with the keys and documentation comments.
There was a problem hiding this comment.
Ok understood - can this be tackled in another PR? This PR is mainly to cover the update to the docs to cover the change made here:
| If the resulting `Context` is [valid](../../api.md#isvalid) then a [Span Link][] SHOULD be added to the new Span's | ||
| [start options](../../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 or the system property `com.amazonaws.xray.traceHeader` can | ||
| contain an incomplete trace context which indicates X-Ray isn’t enabled. The environment variable or system property will be set and the |
There was a problem hiding this comment.
This seems redundant with the next sentence
There was a problem hiding this comment.
ok sure - I have now removed this
| 1. Via the system property `com.amazonaws.xray.traceHeader` in the event of a Java function. More information on Java system properties can be found [here](https://docs.oracle.com/javase/tutorial/essential/environment/sysprop.html) | ||
| 2. Via an environment variable that is set, titled `_X_AMZN_TRACE_ID`. |
There was a problem hiding this comment.
Did you ever find documentation for either of these on AWS's side? I think it would be highly beneficial to have a link to more authoritative documentation on what these properties are and how they're used.
There was a problem hiding this comment.
I'm working with AWS doc team to get this done
There was a problem hiding this comment.
This PR needs to be moved to the https://github.com/open-telemetry/semantic-conventions repository
Fixes #
Changes
Related issues #
open-telemetry/opentelemetry-java-instrumentation#8368
Related OTEP(s) #