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

Jump to bottom

Separate SDK into its own package #721

Closed
hdost opened this issue Feb 7, 2022 · 17 comments · Fixed by #735
Closed

Separate SDK into its own package #721

hdost opened this issue Feb 7, 2022 · 17 comments · Fixed by #735

Comments

@hdost
Copy link
Contributor

hdost commented Feb 7, 2022 •
edited
Loading

Based on the Usage Guidelines in https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/library-guidelines.md does it make sense to split the SDK out into its own crate?
I feel like with the current implementation it kinda already achieves the noop and since unused code won't be in the final binary I suppose it doesn't matter all that much.

Related: #330
@jtescher
Copy link
Member

jtescher commented Feb 7, 2022

Yeah could be nice to split out the api and sdk into their own crates. @GregBowyer would you be open to giving this project access to https://crates.io/crates/opentelemetry-ap and https://crates.io/crates/opentelemetry-sdk?

@GregBowyer
Copy link

GregBowyer commented Feb 7, 2022 via email

@hdost
Copy link
Contributor Author

hdost commented Feb 7, 2022 •
edited
Loading

I would just assume keep opentelemetry for the API segment and opentelemetry-sdk be the SDK. Follows kinda the go implementation a bit https://github.com/open-telemetry/opentelemetry-go/blob/main/example/prometheus/main.go not sure if there's an advantage other than maybe delineating between the two.

Potentially opentelemetry could be an alias or a meta package of opentelemetry-api and opentelemetry-sdk ?

@GregBowyer
Copy link

GregBowyer commented Feb 8, 2022 via email

@jtescher
Copy link
Member

jtescher commented Feb 9, 2022

@GregBowyer Not sure what their transfer method is, but I think you can add a github group like @open-telemetry/rust-maintainers to the crate owners list, or if that is a hassle just add @djc, @TommyCpp and myself 🙏

@GregBowyer
Copy link

GregBowyer commented Feb 9, 2022 via email

@djc
Copy link
Contributor

djc commented Feb 9, 2022

@GregBowyer thanks!

@TommyCpp
Copy link
Contributor

TommyCpp commented Feb 9, 2022

@GregBowyer Thanks a lot!

@hdost
Copy link
Contributor Author

hdost commented Feb 11, 2022

@TommyCpp @jtescher did you have any opinions on my suggestion ? #721 (comment)
I personally like the idea of the opentelemetry crate being the API and opentelemetry_sdk and then maybe if people want re-export the API on opentelemetry_api.

@djc
Copy link
Contributor

djc commented Feb 11, 2022

I also like the idea of using the opentelemetry crate for the API. Don't know what they do in other languages, though?

@hdost
Copy link
Contributor Author

hdost commented Feb 13, 2022

I also like the idea of using the opentelemetry crate for the API. Don't know what they do in other languages, though?

Java

  • io.opentelemetry:opentelemetry-api
  • io.opentelemetry:opentelemetry-sdk

Python

  • opentelemetry-api
  • opentelemetry-sdk

Ruby

  • opentelemetry-api
  • opentelemetry-sdk

Erlang

  • opentelemetry-api
  • opentelemetry (for the sdk)

Go

  • go.opentelemetry.io/otel
  • go.opentelemetry.io/otel/sdk

Swift

  • OpenTelemetryApi
  • OpenTelemetrySdk

Js

  • opentelemetry/api
  • opentelemetry/sdk-node

I think the ergonomics of use opentelemetry is a bit nicer but there's nothing preventing use opentelemetry_api as opentelemetry or ... as otel etc.

With that being said i think I'd agree that it probably makes more sense to align and make the packages opentelemetry-api and opentelemetry-sdk.

Also, then maybe have opentelemetry just re-export or even just include both 🤷‍♂️ maybe leave that aside for now.

@jtescher
Copy link
Member

Took a quick look at splitting things, will help but there are some breaking changes necessary as some of the api trats depend on types currently in the sdk (InstrumentationLibrary, Resource, possibly SpanData, etc).

Also there is a choice to be made on where "sdk api" traits (e.g. SpanExporter, IdGenerator, TraceRuntime) would go. If we keep them in the sdk crate like most of the other otel languages do, then potential alternative sdk implementations will need to depend on the sdk directly to be able to be interoperable with the exporter ecosystem. Alternatively we could move them into the API crate somewhere, but that would deviate a bit from otel expectations.

@hdost
Copy link
Contributor Author

hdost commented Feb 13, 2022 •
edited
Loading

If we keep them in the sdk crate like most of the other otel languages do, then potential alternative sdk implementations will need to depend on the sdk directly to be able to be interoperable with the exporter ecosystem.

100% agree, not necessarily sure if that's an issue. what would the downsides be with that?

@hdost
Copy link
Contributor Author

hdost commented Feb 13, 2022

Took a quick look at splitting things, will help but there are some breaking changes necessary as some of the api trats depend on types currently in the sdk (InstrumentationLibrary, Resource, possibly SpanData, etc).

For the SpanData it looks like there's a Noop Span Exporter, which doesn't really need to exist and then also a testing span exporter, which if we follow others again might even be it's own package which would rely on SDK. However it doesn't need to exist in the API either.

@TommyCpp
Copy link
Contributor

Generally speaking. We can build opentelemetry-api and opentelemtry-sdk and use opentelemetry as all in one, battery included solution including both API and SDK. I think the general idea can be to provide a ready-to-use solution while giving people the choice to dig into the code and replace the component as needed.

@TommyCpp
Copy link
Contributor

TommyCpp commented Feb 13, 2022 •
edited
Loading

If we keep them in the sdk crate like most of the other otel languages do, then potential alternative sdk implementations will need to depend on the sdk directly to be able to be interoperable with the exporter ecosystem

I think if the users just want to custom exporter then it's OK for them to rely on SDK create. IMHO rebuilding SDK only applies to those who find the current span processing are not idea for their use cases and want to replace everything from SpanProcessor to SpanExporter

@jtescher jtescher mentioned this issue Feb 14, 2022
Merged
@hdost
Copy link
Contributor Author

hdost commented Feb 18, 2022

Thank you @jtescher :)

@hdost hdost mentioned this issue Aug 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Split out opentelemetry-api and opentelemetry-sdk jtescher/opentelemetry-rust-1
5 participants
@GregBowyer @djc @jtescher @hdost @TommyCpp