env var carrier must be part of OTel Core repo#5042
env var carrier must be part of OTel Core repo#5042pellared wants to merge 5 commits intoopen-telemetry:mainfrom
Conversation
|
CC @open-telemetry/semconv-cicd-approvers |
Co-authored-by: Christophe Kamphaus <christophe.kamphaus@gmail.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
| - applying any truncation or other format-specific behaviors | ||
|
|
||
| Environment variable carrier implementation SHOULD be maintained and distributed | ||
| as part of the Core OpenTelemetry repositories. |
There was a problem hiding this comment.
I agree with the sentiment. Do we use this language about "core" anyway else? I imagine core means different things for different language sigs based on how they've designed their artifact topology, which is fine and expected.
I would expect the carrier implementation to be in the same artifact that holds w3c trace, baggage propagators, and the core context implementation (i.e. APIs and implementation).
There was a problem hiding this comment.
I would expect the carrier implementation to be in the same artifact that holds w3c trace, baggage propagators, and the core context implementation (i.e. APIs and implementation).
Based on the discussion #5042 (comment) I think artifact is not the concern here. The repo is what we're trying to clarify.
|
@pellared I don't quite understand what is/are "Core OpenTelemetry repositories". I think the spec has a position - anything in the spec should be in the Here is my understanding:
1 and 4 are very clear. |
reyang
left a comment
reyang
left a comment
There was a problem hiding this comment.
Blocking for now since it is unclear whether we're talking about repositories or artifacts. Also, it is unclear what does "OTel Core" mean.
This is not what the specification says. From https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/overview.md#contrib-packages:
The specification explicitly defines that optional plugins are Contrib. We follow this in OTel Go. Also the specification clearly says that carrier is not API. This is a "plugin". While the "Core repository" is not defined it is used in other places as I mentioned in #5042 (comment).
What is your proposal? Do we need to define "OTel Core repository" and "OTel Contrib repository" in the specification beforehand to unblock this PR? |
pellared
changed the title
Would you elaborate what's your concern here? I think this is clear to me. For example, Prometheus Exporter is part of the metrics SDK spec, it is implemented as part of the SDK (not necessarily in a single monolithic binary artifact though), you as maintainer can decide whether you bundle it in the SDK (which doesn't make a lot of sense to me) or you provide it as a package (which I believe most SIGs do), but it will be in the opentelemetry-language repo rather than the opentelemetry-language-contrib repo. |
|
@reyang, my concern is that the specification does not say that environment variable propopagion carrier should be part of the opentelemetry-language repo.
What is the way in the specification to say that a component is required by the specification? I do not think that there is any rule saying that it something is not explicitly optional then it is required. The only rule I found is https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification#notation-conventions-and-compliance:
Currently, the SIG can interpret it as an optional plugin and have it under Contrib.
|
Got it, thanks! I think we should clarify that everything required by the specification needs to happen in the opentelemetry-language repository rather than the opentelemetry-language-contrib repository. Do folks agree? |
Please create a separate issue for it. Why? Personally, I am not sure if this is change is going to be welcome.
There may be many reasons why it may be preferred, but this is something that should be discussed separately. PS. I changed SHOULD to MUST as I think there is a strong preference to have it in the "Core" repo: be61465 |
pellared
changed the title
| - validating and parsing values | ||
| - applying any truncation or other format-specific behaviors | ||
|
|
||
| Environment variable carrier implementation MUST be maintained and distributed |
There was a problem hiding this comment.
If we don't implement it within this PR, it would be helpful in a subsequent PR to include a footnote linking to w/e the definition of a Core OpenTelemetry repository is.
6 participants
Towards #5040
Clarify that the environment variable carrier implementation must be maintained and distributed as part of the Core OpenTelemetry repositories. I have used wording that I found in other parts of the spec. See #5042 (comment)
This is important because instrumentations that want to propagate context through environment variables should be able to reuse a common carrier implementation instead of re-creating their own. Making it a core component gives users and instrumentation authors something stable and reliable to depend on, improves consistency across languages, and reduces the risk of fragmented or incompatible implementations.