Repository navigation
Conversation
irpc currently depends on `opentelemetry` and `tracing-opentelemetry` behind the `tracing-opentelemetry` feature, and exposes their types in its public API. Their versions change often, so irpc 1.0 could not update them without a major release (#111). This PR removes both dependencies from irpc. irpc keeps the wire format and gets a small `Propagator` trait: one method writes the context of a span into the text headers of a request, one sets the parent of a span from them. An application installs one propagator per process with `irpc::span_propagation::set_propagator`. The propagator does not change the wire format: a protocol with `span_propagation` always sends the `Option<SpanContextCarrier>`, and without a propagator its value is `None`. The new crate `irpc-opentelemetry` (0.x) implements the trait with the global text map propagator of `opentelemetry`, and can follow new `opentelemetry` versions with breaking releases. Other tracing backends can implement the trait too. The span propagation tests and example move to the new crate. A new test in irpc checks the hook with a fake propagator. For cargo-release, the new crate has its own version and does not share the version of irpc. * removed: the feature `tracing-opentelemetry`. Add `irpc-opentelemetry` and call `irpc_opentelemetry::install()` once at startup. * removed: `SpanContextCarrier::from_current` and `SpanContextCarrier::to_context`, and the `Injector` and `Extractor` impls of `SpanContextCarrier`. * changed: `rpc` enables the `rt` feature of `tokio`, for the task-local of the span context.
The span propagator of irpc is currently a process-wide static, set once with `set_propagator`. A process can only have one, a second call fails, and tests that need different propagators must live in separate test binaries. The setup is also apart from the rest of the tracing setup. This PR removes the static. irpc gets a `PropagatorLayer`, a tracing layer that holds a propagator and records nothing. irpc finds it with `Dispatch::downcast_ref`, the same way `tracing-opentelemetry` finds its own layer. The client looks in the subscriber of the current thread, and the server in the subscriber of the request span. Without the layer, requests send `None` as their span context, as before. The wire format does not change. `rpc` now enables `tracing-subscriber` without default features, for the `Layer` trait. `irpc-opentelemetry` replaces `install()` with `layer()`, which the application adds to its subscriber next to the `tracing-opentelemetry` layer. ## Breaking changes * removed: `span_propagation::set_propagator` and `PropagatorAlreadySet`. Add a `PropagatorLayer` to the subscriber. * removed: `irpc_opentelemetry::install`. Use `irpc_opentelemetry::layer()` in the subscriber.
With the previous commit, `rpc` enables `tracing-subscriber`, and `PropagatorLayer` puts its `Layer` trait into the public API of irpc. `tracing-subscriber` is 0.x, and most users of irpc do not propagate span context. This PR moves `Propagator`, `PropagatorLayer`, and the task-local of the server behind the new feature `span-propagation`, which is not a default feature. `irpc-opentelemetry` enables it. The wire format stays under `rpc`: a protocol with `span_propagation` always sends `Option<SpanContextCarrier>`. Without the feature, the client sends `None` and the server ignores the carrier, the same as without a layer. `set_span_parent_from_remote` stays without the feature, because the code of the macro calls it, and does nothing. `tokio/rt`, which only the task-local needs, moves from `rpc` to the new feature. ## Breaking changes * changed: `span_propagation::Propagator` and `span_propagation::PropagatorLayer` need the feature `span-propagation`.
dd0d8ed to
4d5d564
Compare
|
I had a previous version that used a per-process static to store the propagator. However, I like the current version more: We can use the installed tracing subscriber and its layers to store the propagator. This makes it possible to have different propagators within one process as long as tracing-subscriber is configured accordingly, which nicely matches how tracing-opentelemetry already works. The downside is that this adds |
…span context Without context activation in tracing-opentelemetry, an empty carrier made the request span a new root. Only set the parent if the extracted context has a valid span.
The crate has no opentelemetry types in its API, so the re-exports are not needed. The README and crate docs state the version bound on opentelemetry and tracing-opentelemetry.
|
|
||
| /// Returns a tracing layer that makes irpc propagate span context with [`OtelPropagator`]. | ||
| pub fn layer() -> PropagatorLayer { | ||
| PropagatorLayer::new(OtelPropagator) |
There was a problem hiding this comment.
My AI tells me that if you do it this way there will be a performance overhead for every tracing call when you use this layer. It suggests to set .with_filter(LevelFilter::OFF).
pub fn layer<S>() -> impl Layer<S>
where
S: Subscriber + for<'a> LookupSpan<'a>,
{
PropagatorLayer::new(OtelPropagator).with_filter(LevelFilter::OFF)
}There was a problem hiding this comment.
Good finding. Fixed in the latest commit.
|
|
||
| /// Run `fut` with `carrier`'s context installed as the per-task scope read by | ||
| /// [`set_span_parent_from_remote`]. | ||
| #[cfg(feature = "span-propagation")] |
There was a problem hiding this comment.
This is the only place where we have a dependency to tracing-subscriber 0.3 in the public API.
I was wondering if we should gate this with a feature flag that is named after the dependency version, e.g.
#[cfg(feature = "tracing-subscriber-03")]
impl<S: tracing::Subscriber> tracing_subscriber_03::Layer<S> for PropagatorLayer {}
There was a problem hiding this comment.
It is currently gated on span-propagation. I think we could add a tracing-subscriber-04 feature already now, that would then add the corresponding impl for tracing-subscriber 0.4. but yeah then we'd have to depend on both versions. so maybe adding tracing-subscriber-03 now makes sense. but span-propagation without this feature wouldn't do anything I think. Let me think this through a bit more
An unfiltered layer enables everything. With per-layer filters on the other layers, adding it raised the max level to TRACE, so every `debug!` and `trace!` reached the other layers. `span_propagation::layer(propagator)` now returns the layer with a `LevelFilter::OFF` per-layer filter. `PropagatorLayer` is private. ## Breaking changes * removed: `span_propagation::PropagatorLayer`. Use `span_propagation::layer`. * changed: `irpc_opentelemetry::layer` returns `impl Layer<S>`, and needs a subscriber that implements `LookupSpan`.
2 participants
Fixes #111
irpc currently depends on
opentelemetryandtracing-opentelemetrybehind thetracing-opentelemetryfeature and exposes their types in the public API. Both crates are at 0.* versions with rather frequent releases, so we shouldn't expose them in our 1.0 API.This PR removes both dependencies from irpc. Instead, irpc offers a generic way to inject span context through a small
Propagatortrait, with one method to write span context into request headers (on the client) and one method to set the parent of a span from the request headers (on the server). The propagator impl to use is found through the tracing subscriber: add the layer fromirpc::span_propagation::layer(propagator)to it. The layer has aLevelFilter::OFFper-layer filter, because an unfiltered layer would raise the max level to TRACE when other layers use per-layer filters. So the subscriber must implementLookupSpan, astracing-opentelemetryalso requires. irpc finds the layer withDispatch::downcast_ref, the same waytracing-opentelemetryfinds its own layer, so there is no process-wide global, and tests can use different propagators withtracing::subscriber::set_default(which is thread-local, so the server task must run on the same thread, as in the new test). The propagator and the layer are behind the new featurespan-propagation(not default), sotracing-subscriber(0.x) is not in the public API of a default build. The wire format does not depend on the feature: without it, a client sends no span context.This PR then adds a new in-repo crate
irpc-opentelemetry, which we would not release as 1.0. It implements the trait with the global text map propagator ofopentelemetry, and can follow newopentelemetryversions with breaking releases. It has its own version (shared-version = falsefor cargo-release), and the OpenTelemetry tests and example move into it.Other tracing backends can implement the trait too, see the new test in irpc that checks the hook with a fake propagator.
Breaking changes
tracing-opentelemetry. Addirpc-opentelemetryand addirpc_opentelemetry::layer()to the tracing subscriber.irpc-opentelemetryenables thespan-propagationfeature of irpc.SpanContextCarrier::from_currentandSpanContextCarrier::to_context, and theInjectorandExtractorimpls ofSpanContextCarrier.Additions
span-propagation(not default), withspan_propagation::Propagatorandspan_propagation::layer.SpanContextCarrier::get,SpanContextCarrier::set, andSpanContextCarrier::keys.