WithContext not populated when a reload layer is used

[arlyon]

Bug Report

Hi there. I am building a cli which may-or-may-not have opentelemetry enabled. We initialize tracing early on and conditionally enable opentelemetry based on some flags. We use reload layers starting with None to allow enabling layers later on. Everything works, except context propagation. If the layer is not present when the registry is init'ed, WithContext is not set up correctly meaning set_parent and friends silently fails due to not being able to downcast the subscriber (L142).

tracing-opentelemetry/src/span_ext.rs

Lines 139 to 150 in 5e3354e

	fn set_parent(&self, cx: Context) {
	let mut cx = Some(cx);
	self.with_subscriber(move |(id, subscriber)| {
	if let Some(get_context) = subscriber.downcast_ref::<WithContext>() {
	get_context.with_context(subscriber, id, move |data, _tracer| {
	if let Some(cx) = cx.take() {
	data.parent_cx = cx;
	}
	});
	}
	});
	}

I can confirm this does not work as expected even if the reload layer is initialized with Some(T) either. The layer does work if you use a Some(T) without the reload layer, however.

Version

│   │   │   │   └── tracing v0.1.37
│   │   │   │       ├── tracing-attributes v0.1.26 (proc-macro)
│   │   │   │       └── tracing-core v0.1.31
│   │   │   └── tracing v0.1.37 (*)
│   │   ├── tracing v0.1.37 (*)
│   │   └── tracing v0.1.37 (*)
│   │   │   └── tracing v0.1.37 (*)
│   │   ├── tracing v0.1.37 (*)
│   └── tracing v0.1.37 (*)
│   ├── tracing v0.1.37 (*)
│   ├── tracing v0.1.37 (*)
│   │       └── tracing v0.1.37 (*)
├── tracing v0.1.37 (*)
├── tracing-appender v0.2.2
│   └── tracing-subscriber v0.3.18
│       ├── tracing v0.1.37 (*)
│       ├── tracing-core v0.1.31 (*)
│       └── tracing-log v0.2.0
│           └── tracing-core v0.1.31 (*)
├── tracing-chrome v0.7.1
│   ├── tracing-core v0.1.31 (*)
│   └── tracing-subscriber v0.3.18 (*)
├── tracing-opentelemetry v0.23.0
│   ├── tracing v0.1.37 (*)
│   ├── tracing-core v0.1.31 (*)
│   ├── tracing-log v0.2.0 (*)
│   └── tracing-subscriber v0.3.18 (*)
├── tracing-subscriber v0.3.18 (*)
│   ├── tracing v0.1.37 (*)
│   │   ├── tracing v0.1.37 (*)
│   │   │   └── tracing v0.1.37 (*)
│   ├── tracing v0.1.37 (*)
│   │   ├── tracing v0.1.37 (*)
│   ├── tracing v0.1.37 (*)
│   ├── tracing v0.1.37 (*)
│   ├── tracing-test v0.2.4
│   │   ├── tracing-core v0.1.31 (*)
│   │   ├── tracing-subscriber v0.3.18 (*)
│   │   └── tracing-test-macro v0.2.4 (proc-macro)
│   │   ├── tracing v0.1.37 (*)
│   │   │   └── tracing v0.1.37 (*)
│   │   │   ├── tracing v0.1.37 (*)
│   │   │   ├── tracing v0.1.37 (*)
│   │   │   │   ├── tracing v0.1.37 (*)
├── tracing v0.1.37 (*)
├── tracing-test v0.2.4 (*)

Platform

macOS / Linux

[tomhoule]

We are running into the same problem with tracing_opentelemetry 0.23.0. Removing the reload layer makes the downcast_ref() in set_parent() return Some(_) again.

[djc]

I probably won't have time to look into this myself but happy to review a PR that tries to address this.

[SamPeng87]

that bug is always exists

[SamPeng87]

I am trying to investigate this issue

Discovered, the reload method. It will only downcast_ref NoneLayerMarker.

This is the code for tracing-subscriber:

    #[doc(hidden)]
    unsafe fn downcast_raw(&self, id: TypeId) -> Option<*const ()> {
        // Safety: it is generally unsafe to downcast through a reload, because
        // the pointer can be invalidated after the lock is dropped.
        // `NoneLayerMarker` is a special case because it
        // is never dereferenced.
        //
        // Additionally, even if the marker type *is* dereferenced (which it
        // never will be), the pointer should be valid even if the subscriber
        // is reloaded, because all `NoneLayerMarker` pointers that we return
        // actually point to the global static singleton `NoneLayerMarker`,
        // rather than to a field inside the lock.
        if id == TypeId::of::<layer::NoneLayerMarker>() {
            return try_lock!(self.inner.read(), else return None).downcast_raw(id);
        }

        None
    }

According to my investigation, this function always returns None. It will never downcast_raw load WithContext. How can I modify it?

[mladedav]

I think the only way to solve this issue is to either

• use something else than tracing::reload::Layer. That layer does not correctly downcast (it even says so in its own docs)
• get rid of WithContext in this crate, which will definitely not happen soon and may never happen.

I want to look into the first option, but that's out of scope of this repository.

I'll keep this open for visibility for now as this is possibly an issue more people will stumble upon.

[mladedav]

Two tests for future reference that fail because we depend on downcasting but the reload layer will not allow us to:

    #[test]
    fn downcast_with_reload() {
        let tracer = TestTracer::default();
        let layer = layer().with_tracer(tracer.clone());
        let (layer, _handle) = tracing_subscriber::reload::Layer::new(layer);
        let subscriber = tracing_subscriber::registry().with(layer);

        let dispatch = Dispatch::new(subscriber);
        dispatch.downcast_ref::<WithContext>().unwrap();
    }

    #[test]
    fn context_with_reload() {
        let tracer = TestTracer::default();
        let layer = layer().with_tracer(tracer.clone());
        let (layer, _handle) = tracing_subscriber::reload::Layer::new(layer);
        let subscriber = tracing_subscriber::registry().with(layer);

        tracing::subscriber::with_default(subscriber, || {
            let span = info_span!("root");
            let context = span.context();
            assert!(context.span().span_context().is_valid());
        });
    }