Add span builder

[jtescher]

This patch adds a SpanBuilder struct that allows Tracers to provide
a method for easily configuring Span attributes before the spans are
started.

Example:

let tracer = global::trace_provider().get_tracer("example-tracer");

// Use span builder to assign span properties, note that a tracer reference
// must be passed to `start`.
let _span = tracer
    .span_builder("example-span-name")
    .with_kind(api::SpanKind::Server)
    .start(&tracer);

Resolves #13, resolves #23

[MikeGoldsmith]

Looks great, just one query regarding passing the tracer in earlier.

[MikeGoldsmith]

Could we take a tracer reference as a parameter here? It means we wouldn't need to pass the tracer into the Start fn below.

[jtescher]

Yeah it definitely is not ideal to have to pass a reference to start. Seems like there are a few options:

The expected API would be something like:

let _span = tracer
    .span_builder("example-span-name")
    .with_kind(api::SpanKind::Server)
    .start();

The simple way to accomplish this would be for SpanBuilder to have a type parameter:

impl SpanBuilder<T: api::Tracer> {
    pub fn from_name(name: String, tracer: &T) -> Self {}
    pub fn start(self) -> T::Span {}
   ...
}

However that would mean the Tracer impl would look like:

pub trait Tracer {
    fn span_builder(&self) -> SpanBuilder<Self> {}
}

And we need Tracers to be trait objects when used in the global module so they cannot have Self in parameters or return types.

Another option could be that instead people use SpanBuilder directly:

    SpanBuilder::from_name("example-span-name", &tracer).start()

But that isn't very ergonomic either.

The current solution seems like a middle ground, Tracers can still be trait objects as their methods do not reference Self, but it requires an explicit reference on span builder's start.

Definitely open to alternatives if I've missed a better option that allows for the ideal API above. Thoughts?

[jtescher]

Alternatively SpanBuilder could accept an &dyn api::Tracer and return a Box<dyn api::Span>? Tradeoff would be having an explicitly boxed span as the return type, but that might be preferable to the tracer reference?

[MikeGoldsmith]

Hey - sorry for slow reply.

I think using Box<..> types for the tracer and span would be okay. Would it be possible to support both your initial and boxed tracer options?

[jtescher]

Yeah can keep trying. Might be another option that doesn't have these downsides...

[jtescher]

@MikeGoldsmith I think this is the best we can get currently with two options:

 // The builder can be used to create a span directly with the tracer
 let _span = tracer.build(SpanBuilder {
     name: "example-span-name".to_string(),
     span_kind: Some(SpanKind::Server),
     ..Default::default()
 });

 // Or used with builder pattern
 let _span = tracer
     .span_builder("example-span-name")
     .with_kind(SpanKind::Server)
     .start(&tracer);

The builder has a hard time taking a reference because the types can't be known ahead of time, e.g. tracer: &dyn Tracer<Span=?> can't encode that without needing the builder to be generic, which doesn't currently work in the global module.

A middle ground is having the builder impl Default so it can be used as a form of default args as I show above. The upside of the start method taking a reference to the tracer is builders can be passed around without worrying about the tracer lifetime or having to clone tracers for every builder.

Any other ideas on how this API could be nicer? or should we go with this for now

[MikeGoldsmith]

I think the second example, where we use the builder pattern is the most readable. Let's go in that direction for now, and we can re-visit in the future if needed - this PR has been hanging around for awhile so lets get it merged 😄

[jtescher]

Ok great, the PR currently supports both patterns I pasted there so we should be good to go if you want to review 🎉