Move RuntimeChannel type arg T to batch_message_channel and associated types

[frigus02]

Changes

RuntimeChannel::batch_message_channel needs to be generic over the message type. The type used to be declared on the RuntimeChannel<T> trait. This means a RuntimeChannel can only be used with one particular message type, which feels unfortunate.

fn install<R: RuntimeChannel<??::BatchMessage>>(runtime: R) {
    // Can't use the same runtime here. :-(
    TracerProvider::builder().with_batch_exporter(e, runtime);
    LoggerProvider::builder().with_batch_exporter(e, runtime);
}

This change moves the type argument to the batch_message_channel<T> function and the associated types Receiver<T> and Sender<T>. Channels are still specific to a message type, but a RuntimeChannel can be used with any number of message types.

fn install<R: RuntimeChannel>(runtime: R) {
    // It works. :-)
    TracerProvider::builder().with_batch_exporter(e, runtime);
    LoggerProvider::builder().with_batch_exporter(e, runtime);
}

This also means the BatchMessage types no longer need to be public.

Merge requirement checklist

• CONTRIBUTING guidelines followed
• Unit tests added/updated (if applicable)
  • not applicable, I think
• Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
• Changes in public API reviewed (if applicable)
  • Removes logs::BatchMessage and trace::BatchMessage
  • RuntimeChannel is public, so all changes to it here are breaking

[djc]

What are you trying to achieve with this change? What's your use case?

[jtescher]

Looks like CI isn't passing, but this does appear to simplify the external interface which I'm supportive of 👍

[frigus02]

Sorry, I should have really described my use case first:

I recently added live metrics support to opentelemetry-application-insights. It's a separate span processor, which polls Application Insights and sends a handful of automatically collected metrics (e.g. requests per second) in a much more frequent interval to the server as long as a user is watching the metrics.

I couldn't quite use the batch span processor because the polling interval varies. It's shorter when someone is watching. Otherwise it has the same structure: a separate task/thread that calls the server comminuates with the main task via a channel. I think the nicest API here uses RuntimeChannel. Developers already have to provide that when configuring OpenTelemetry and it has everything I need. But I want to use my own message enum and I don't want developers to have to provide 2 different RuntimeChannels.

I want to provide a function like:

fn install_batch<R: RuntimeChannel>(runtime: R) {
    let live_metrics = LiveMetrics::new(runtime.clone());
    TracerProvider::builder().with_batch_exporter(e, runtime).with_span_processor(live_metrics)
}

[frigus02]

Ha, but I see this isn't compatible with the MSRV:

error[E0658]: generic associated types are unstable

I think they were stabilised in https://blog.rust-lang.org/2022/11/03/Rust-1.65.0.html#generic-associated-types-gats

I might try if that's possible without generic associated types somehow.

[codecov]

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Files	Coverage Δ
...elemetry-contrib/src/trace/exporter/jaeger_json.rs	0.0% <ø> (ø)
opentelemetry-jaeger/src/exporter/runtime.rs	0.0% <ø> (ø)
opentelemetry-otlp/src/span.rs	59.4% <100.0%> (ø)
opentelemetry-sdk/src/runtime.rs	92.9% <100.0%> (+0.8%)	⬆️
opentelemetry-sdk/src/trace/mod.rs	100.0% <ø> (ø)
opentelemetry-sdk/src/trace/span_processor.rs	82.8% <ø> (ø)
opentelemetry-datadog/src/exporter/mod.rs	50.7% <0.0%> (+0.4%)	⬆️
opentelemetry-sdk/src/logs/log_emitter.rs	0.0% <0.0%> (ø)
opentelemetry-sdk/src/logs/log_processor.rs	0.0% <0.0%> (ø)
opentelemetry-sdk/src/trace/provider.rs	89.8% <0.0%> (-0.6%)	⬇️
... and 5 more

📢 Thoughts on this report? Let us know!.

[djc]

I think we can maybe bump the MSRV to 1.65?

[hdost]

I can push a patch on bumping msrv

[frigus02]

Yeah, I don't know how to make this work without generic associated types. Bumping the MSRV sounds good to me.

[frigus02]

Thanks for bumping the MSRV, hdost@. I rebased and also updated the changelog.

[hdost]

Thank you @frigus02 !

[TommyCpp]

Thanks!