Support trace-level sampling

[lilymara-onesignal]

The libhoney-rust library has a sample_rate configuration option,
which provides random sampling of events sent to honeycomb.
Unfortunately, this sampling is done at the event level, rather than the
trace level. This means that spans or events belonging to a single trace
will have separate sampling decisions made for them. This in turn means
that when libhoney's sampling is enabled, traces will never have
complete span information within them. This severely limits the utility
of this sampling.

This proposed change introduces sampling based on the trace ID, meaning
that spans and events which are a part of the same trace will always
have the same sampling decision made for them. This will ensure that a
complete picture of the trace is sent up to honeycomb.

There are no breaking changes in behavior or API in this changeset, you
can still use the sampling logic built into libhoney if desired, but the
new function new_honeycomb_telemetry_layer_with_trace_sampling has
been introduced, which creates a telemetry layer that uses this new
sampling logic.

Alternatives:

• Use the sample_rate from the libhoney config struct as the
  trace-level sample rate, and reset the field to 1 before giving it to
  libhoney. This is undesirable as it makes the current behavior
  impossible to use, however it also makes it impossible to misuse this
  new function by providing a sample_rate to this function, and libhoney
  at the same time.
• Perform this trace-level sampling within libhoney. This is
  undesirable as it would require either parsing the traceID from the
  string provided to libhoney, or using a hash of this string. It is
  simpler to do the calculation in this library, where we have the raw
  trace ID. This would also be a breaking change for libhoney, whereas
  here it can be opt-in.

Example

I ran the following code with libhoney's sample rate set to 5, and the new trace-level sample rate set to 5.

fn main() {
    register_global_subscriber();

    for i in 0..10 {
        let s = info_span!("running process loop", i);
        let _g = s.enter();

        register_dist_tracing_root(TraceId::generate(), None).unwrap();

        foo();
    }

    std::thread::sleep(std::time::Duration::from_secs(5));
}

#[instrument]
fn foo() {
    sleep(Duration::from_millis(500));
    bar();
}

#[instrument]
fn bar() {
    sleep(Duration::from_millis(500));
    baz();
}


#[instrument]
fn baz() {
    sleep(Duration::from_millis(500));
}

This screenshot illustrates the change in behavior between the current libhoney sampling, and the new trace-level sampling.

[Fishrock123]

I'll need this soon too. @inanna-malick are you able to take a look sometime soon?

[Fishrock123]

I also think the constructor api could be reworked, but that can wait for 0.3 imo.

[Fishrock123]

fwiw 0.2 isn't even out yet, see #5

[inanna-malick]

0.2 is out, I just never published it (sorry, 2020 has been.. very 2020 recently) https://github.com/inanna-malick/tracing-honeycomb/blob/master/tracing-honeycomb/Cargo.toml#L3

[Fishrock123]

This is clever but I would have expected the sample rate to be a 0 to 1 kind of range - is this sort of sample rate common in other tracing libraries?

[inanna-malick]

I would also expect a float of some sort here - maybe an f32, with a 0 to 1 range

[inanna-malick]

Also, it looks like you're using sample_rate <= 1 to signify 'sample everything' - how do you feel about making this an Option<SampleRate> instead, where type SampleRate = u32 (or f32 or w/e)

[inanna-malick]

This looks great, really the only thing I'd ask you to change is making sampling rate an Option. I'm fine with using a usize for the sampling rate, after talking to some friends it seems like that is a pretty standard way of handling sampling

[lilymara-onesignal]

Thanks for the review @inanna-malick! I pushed up a fixup commit which changes the implementation to use an Option instead of a sentinel value. I did also change the rate type from a usize to a u128 though, in order to avoid casting the TraceID to a usize every time - because I believe this operation may panic if the u128 doesn't fit into a usize.

[inanna-malick]

This is great, thanks!