More fully describe OTEL_PROPAGATOR and OTEL_EXPORTERS, remove confus… #1316
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ing language about SDK-specific conventions for third party propagators.
anuraaga
commented
Jan 5, 2021
| @@ -2,6 +2,8 @@ | |||
|
|
|||
| The goal of this specification is to unify the environment variable names between different OpenTelemetry SDK implementations. SDKs MAY choose to allow configuration via the environment variables in this specification, but are not required to. If they do, they SHOULD use the names listed in this document. | |||
|
|
|||
| For variables which accept a known value out of a set (i.e., an enum value), SDK implementations MAY support additional values not listed here. | |||
There was a problem hiding this comment.
@carlosalberto @Oberon00 I believe this line clarifies the concern in #1311 (comment)
anuraaga
commented
Jan 5, 2021
| - `"b3"`: [B3 Single](https:/openzipkin/b3-propagation#single-header) | ||
| - `"b3multi"`: [B3 Multi](https:/openzipkin/b3-propagation#multiple-headers) | ||
| - `"jaeger"`: [Jaeger](https://www.jaegertracing.io/docs/1.21/client-libraries/#propagation-format) | ||
| - `"xray"`: [AWS X-Ray](https://docs.aws.amazon.com/xray/latest/devguide/xray-concepts.html#xray-concepts-tracingheader) (_third party_) |
There was a problem hiding this comment.
After adding the above line at the top, I understand better what the intent of the "third party Propagators" caveat was before - the example wasn't great because our SDKs actually sometimes support xray and ottracer, but I think the point was about completely separate propagators like Dynatrace.
I'm wondering if given the line I added at the beginning, I can remove these noisy (_third party_) notes since they don't seem that helpful. Let me know.
Oberon00
approved these changes
Jan 5, 2021
carlosalberto
approved these changes
Jan 6, 2021
|
@Oberon00 Please verify that you are still fine with the latest iteration ;) If you are ok, let's merge this. |
tigrannajaryan
approved these changes
Jan 8, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ing language about SDK-specific conventions for third party propagators.
Changes
Adds more detail about the actual formats for propagators and exporters, and removes wording about third party propagator names being in SDK documentation to ensure different languages use the same configuration values even for third party propagators. Instead, it documents how SDKs should deal with enum-type variables.
Follow up to #1311