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

Tracing GA tracking #330

Closed
15 tasks done
jtescher opened this issue Nov 2, 2020 · 12 comments
Closed
15 tasks done

Tracing GA tracking #330

jtescher opened this issue Nov 2, 2020 · 12 comments

Comments

@jtescher
Copy link
Member

jtescher commented Nov 2, 2020

Spec compliance:

API ergonomics:

  • Allow &str and String as parameters where possible

Backwards compatibility:

  • Ensure all pub items should be exposed
  • Ensure dependencies in public APIs are 1.0
  • consider dropping serde feature Serde support #576
@TommyCpp
Copy link
Contributor

TommyCpp commented Dec 11, 2020

Ensure dependencies are 1.0

Feels like would be a difficult criteria to meet now as most of the crates in Rust are pre 1.0

@jtescher
Copy link
Member Author

@TommyCpp good point, clarified it to only include ones in public interfaces

@TommyCpp
Copy link
Contributor

TommyCpp commented Feb 25, 2021

The spec around tracing has stabilized and I see they are checking with language SIG on their progress to GA on tracing in the maintainer meeting logs. We might want to review the spec and see what we are missing. I can tell we still need to add http/json and http/protobuf support for opentelemetry-otlp.

Also, we might need to add logs in our API and SDK. Now that the opentelemetry log spec has decided that they will respect log facade from languages. I think we can just use Rust's log facade to instrument the logs for now and figure out if we want to use env_logger or our otlp log SDK later.

Rust also provides a guideline on library https://rust-lang.github.io/api-guidelines/checklist.html, we could walk through this and see if there are improvements we should make.

@djc
Copy link
Contributor

djc commented Feb 25, 2021

I think we should probably use tracing instead of log?

@TommyCpp
Copy link
Contributor

I think we should probably use tracing instead of log?

@djc I am a little worried that we would have cycle dependency if we want to use opentelemetry-log for log SDK and tracing for logging API because tracing-opentelemetry depends on opentelemetry

@LukeMathWalker
Copy link
Contributor

Related to @TommyCpp - have you considered splitting out the sdk module in its own crate?
It would be ideal to be able to isolate the instrumentation side (e.g. most of the traits) in a module moving faster towards 1.0 while keep iterating on the sdk side.

@TommyCpp
Copy link
Contributor

TommyCpp commented Apr 8, 2021

have you considered splitting out the sdk module in its own crate?

That could work, but we need to first make sure the api doesn't use anything from SDK part otherwise there will be a circular dependency. Another problem is I believe there is a crate called opentelemetry-sdk already 😞

On the other hand, we can still declare API stable while working on the SDK if I understand the VERSIONING.md correctly.

@LukeMathWalker
Copy link
Contributor

LukeMathWalker commented Apr 8, 2021

If they are all part of the same crate and you break the public API of the sdk sub-module you are forced to publish a new major (if you are past 1.0) of the whole crate - api included.
If you have two incompatible versions of opentelemetry around (e.g. 1.0 and 2.0) stuff like

static ref GLOBAL_TRACER_PROVIDER: RwLock<GlobalTracerProvider> = RwLock::new(GlobalTracerProvider::new(trace::NoopTracerProvider::new()));
stops working and it becomes very cumbersome for users of the crate to get traces propagation working correctly (i.e. everything that uses OpenTelemetry MUST be on the same version of opentelemetry).

I have just gone through this exciting exercise for 20+ applications and 10+ libraries (REST APIs, gRPC APIs, AMQP consumers, REST clients, etc.) - it takes a long time.
That's why I am suggesting, if it reduces the chances of breaking changes in the past, to separate the api module from the sdk one - it would make it much easier for libraries in the Rust ecosystem to start bundling OpenTelemetry instrumentation (relying mostly on api) without having to do regular new releases.

@TommyCpp
Copy link
Contributor

TommyCpp commented Apr 9, 2021

Yeah, you have a good point. I think we should review our status on backward compatibility and how far we are towards tracing GA. If we decided we still need a while to stable the SDK public API after we stabilized tracing API, maybe it's a good idea to separate the SDK from API. @jtescher / @djc / @frigus02 any thoughts?

@jtescher
Copy link
Member Author

jtescher commented Apr 9, 2021

A lot of these are now done, could make sense to do another pass to see what changes are needed before GA, if the majority of the changes are in the SDK only then we may make life easier for some folks by separating them.

@TommyCpp
Copy link
Contributor

Docs:

@hdost hdost mentioned this issue Feb 21, 2024
41 tasks
@hdost
Copy link
Contributor

hdost commented Feb 21, 2024

Didn't realize this existed when I created #977, but I have been keeping it updated there. So going to close this here. Please follow in #977

@hdost hdost closed this as not planned Won't fix, can't repro, duplicate, stale Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants