-
Notifications
You must be signed in to change notification settings - Fork 7
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
Initial Project Layout #1
Comments
Looks good to me 👍
|
|
On traits/generics: say we have a trait like this one: pub trait Span<Context> {
fn span_context() -> Context;
// ...
} That means that If the tracer had an API looking like the below it is not possible to have runtime configuration of the tracer. pub trait Tracer<Context> {
pub span(...) -> Span<Context>;
// ...
} Does this make sense? |
@stefano-pogliani the way I understood it is that at runtime they are not the same trait implementation, so if you want to switch at runtime your API needs to take a |
@stefano-pogliani here is a simple version with generics: https://play.rust-lang.org/?gist=478ac54c30e3f9535eb5c988d740e47d&version=stable but honestly I think it works much better with associated types: https://play.rust-lang.org/?gist=7287f8e211c291087f7ddb722613b7f9&version=stable |
@daschl I like the associated types much better too but I still think they would cause issues with applications that want to configure the tracers in I have added to your examples to show you what I mean:
I think it stands from the fact that in both cases To support this in my crate I used the |
@stefano-pogliani when you update the gist you need to generate new links, I don't see your changes on those unfortunately. |
Also, lets discuss this further in #2 since API is different from project layout. Relating to this, I'll come up with an initial PR for the project structure and we can take it from there on the PR. |
This change applies the basic repository layout discussed in Fixes #1
* Add initial cargo crate layout. This change applies the basic repository layout discussed in Fixes #1 * Extend Cargo.toml with more info
Just realised: @daschl do we want a changelog somewhere in the repo? I don't think we need anything too advanced (version, maybe date, list of changes, highlight breaking changes is what I tend to use them for). |
Hi,
as a first step, I think we need to figure out a rough project structure. Not that we can't change it later, but we need to come up with something.
Since I think OT Java is currently the most active developed/used (correct me if I'm wrong) it makes sense to take this as a starting point.
With that in mind, I propose the following:
opentracing-rust
is a cargo workspace for the following subprojects.opentracing-api
provides only the traits which are required to implement a tracer but doesn't have any implementation by itself. It should be very lightweight and not make any assumptions on how the actual tracer implementation is done.opentracing-noop
a "noop" implementation of the API, can be used if no tracer needs to be used at runtime and also serves as a sanity check durning compilation.opentracing-mock
a simple mock implementation of the API, useful for testing and also showcases how to implement a simple tracer.We might also need utils (which would hold the global tracer) and of course examples, but we can add those as soon as we need them.
ping @hanscj1 @stefano-pogliani @sile
The text was updated successfully, but these errors were encountered: