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.

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

Move the B3 propagator out of the API #1012

Closed
MrAlias opened this issue Jul 31, 2020 · 4 comments · Fixed by open-telemetry/opentelemetry-go-contrib#344
Closed

Move the B3 propagator out of the API #1012

MrAlias opened this issue Jul 31, 2020 · 4 comments · Fixed by open-telemetry/opentelemetry-go-contrib#344
Labels
pkg:API Related to an API package
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Jul 31, 2020

Related to open-telemetry/opentelemetry-specification#735 and recent feedback:

B3 propagator (an implementation) should not be living in this package. We are shifting away from B3 and it’s not a core tracing API.

Possible destinations:

  1. A new go.opentelemetry.io/otel/propagators package that is dedicated to opensource trace context standards.
    • This will keep dependencies on the B3 propagator to a single repository.
    • This will pollute the main otel package with an extension.
  2. A new go.opentelemetry.io/contrib/propagators package.
    • If other propagators are added (especially non-opensource ones like AWS X-Ray that would need to be in the contrib repo) it would be nice (similar to what is wanted for instrumentation) to have all extensions in a single repository.

My recommendation would be (2).

@MrAlias MrAlias added pkg:API Related to an API package release:required-for-ga labels Jul 31, 2020
@Aneurysm9
Copy link
Member

I'd second option (2).

@Aneurysm9
Copy link
Member

I'm slightly confused by the update to the spec from open-telemetry/opentelemetry-specification#735 that says:

Propagators implementing officially supported protocols such as B3 Propagation Specification MUST be maintained by the OpenTelemetry organization and MUST be distributed as OpenTelemetry extension packages.

My confusion comes from the fact that I don't see where the "officially supported protocols" are defined. The closest I can find is (also added in that PR):

The API layer MAY include the following Propagators:

  • A HTTPTextPropagator implementing the W3C TraceContext Specification.

Does the fact that B3 is not listed in the "MAY include" statement mean that the API MAY NOT include it? I think the contrib repo would qualify as "maintained by the OpenTelemetry organization", so moving the B3 propagator there wouldn't run afoul of the first requirement. Does the "MUST be distributed as OpenTelemetry extension packages" statement mean that it MUST be in contrib, or would simply having a separate Go module in this repo suffice?

@MrAlias
Copy link
Contributor Author

MrAlias commented Aug 13, 2020

My confusion comes from the fact that I don't see where the "officially supported protocols" are defined.

I share this confusion 😞

Does the fact that B3 is not listed in the "MAY include" statement mean that the API MAY NOT include it? I think the contrib repo would qualify as "maintained by the OpenTelemetry organization", so moving the B3 propagator there wouldn't run afoul of the first requirement.

Agreed.

Does the "MUST be distributed as OpenTelemetry extension packages" statement mean that it MUST be in contrib, or would simply having a separate Go module in this repo suffice?

I'm not sure. It seems to me that was written in a Java paradigm and I'm not exactly sure how to translate that as you seem to be as well.

I think the approach we decided on in the SIG meeting:

  1. A new go.opentelemetry.io/contrib/propagators package.
    • If other propagators are added (especially non-opensource ones like AWS X-Ray that would need to be in the contrib repo) it would be nice (similar to what is wanted for instrumentation) to have all extensions in a single repository.

Will conform to the spec based on our similar understanding and allow extension in the future.

@evantorrie
Copy link
Contributor

evantorrie commented Aug 15, 2020

Does the "MUST be distributed as OpenTelemetry extension packages" statement mean that it MUST be in contrib, or would simply having a separate Go module in this repo suffice?

I'm not sure. It seems to me that was written in a Java paradigm and I'm not exactly sure how to translate that as you seem to be as well.

In the Java codebase, the source code is in the main opentelemetry-java repo, but yes, it is in a different module artifact called io.opentelemetry.extension.trace.propagation.

I think having it in the contrib repo makes sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:API Related to an API package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants