Trace module exports #255
Trace module exports #255
Conversation
Flatten trace exports under the trace sub module.
| @@ -32,22 +32,3 @@ pub use context::propagation::{ | |||
| text_propagator::TextMapFormat, Extractor, Injector, | |||
| }; | |||
| pub use context::Context; | |||
|
|
|||
There was a problem hiding this comment.
So baggage and context are feature flagged to trace but not under the trace module. Is there a strong opinion about whether these should be reorganized under the trace mod?
There was a problem hiding this comment.
Probably makes sense to keep the directory structure in line with https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/library-layout.md#api-package, but if features aren't used outside of trace then feature gating with trace probably makes sense. I think context is used by metrics though, so it should probably not be.
There was a problem hiding this comment.
Ok - I'll leave baggage and context alone.
Clean up api/trace exports to be flattened under api/trace instead of just api.
awiede
force-pushed
the
trace-import-nesting
branch
from
e6021e9
to
fe836b9
Compare
benches/trace.rs
Outdated
| @@ -64,13 +67,13 @@ fn criterion_benchmark(c: &mut Criterion) { | |||
| }); | |||
| } | |||
|
|
|||
| fn trace_benchmark_group<F: Fn(&sdk::Tracer)>(c: &mut Criterion, name: &str, f: F) { | |||
| fn trace_benchmark_group<F: Fn(&sdk::trace::Tracer)>(c: &mut Criterion, name: &str, f: F) { | |||
There was a problem hiding this comment.
These are kinda unergonomic now with the double module prefix, but not sure how to avoid that as they share names with the traits. Any ideas here? or is this the best we can do currently.
There was a problem hiding this comment.
Looks like go uses the equivalent of:
use opentelemetry::sdk::trace as sdktrace;
// ...
let tracer: sdktrace::Tracer;
// or
use opentelemetry::sdk::trace::Tracer as SdkTracer;
// ...
let tracer: SdkTracer;Alternatively we could have a convention like our examples always import api::Tracer as _ and Tracer is always in reference to sdk, or vice versa.
There was a problem hiding this comment.
I mean, we could pub use it in src/sdk/mod.rs, but then we go back to the inconsistency. It's weird, but if we flatten this into sdk, then do we do the same for any type that is prefixed with Trace*? And then we're back where we started. 🤷
There was a problem hiding this comment.
Yeah, I think example aliases are probably good for now.
There was a problem hiding this comment.
@jtescher updated examples, doc tests and tracing bench to use the alias.
There was a problem hiding this comment.
Ok cool, the aliases are not breaking changes so looks good to me even if we want to experiment here
awiede
force-pushed
the
trace-import-nesting
branch
from
d1c56d7
to
b1b5f75
Compare
Alias sdk::trace and api::trace in examples and doc tests.
awiede
force-pushed
the
trace-import-nesting
branch
from
b1b5f75
to
e395235
Compare
Resolves #254 Resolves #249
apiandsdktoapi::traceandsdk::tracemetricsfrom the default features