configs which contain jaeger config will hang the router when updated

[garypen]

If a configuration is updated which contains jaeger configuration, then
at some point a synchronous call to:
opentelemetry::global::set_tracer_provider()
will arise. Even though this call is made from a synchronous function,
the root context is asynchronous and it seems to cause the runtime to
mis-behave: i.e.: hang...

If we make the call from a newly spawned thread (no performance concerns
here because re-configuration is not hot code) then the problem goes
away.
fixes: #331

[Geal]

joining on a thread from inside an async context might give the same result as waiting on a RwLock, since joining is blocking. But I don't see another way to do it (except maybe through channels with that thread)

[garypen]

It's not ideal, but, for whatever reason, it's certainly better behaved with this fix in than otherwise.

[cecton]

would tokio::spawn_blocking work better?

[garypen]

I thought the thread spawn looked cleaner. I need a way to wait on the spawn_blocking and the code looks a little bit messier (to my eyes).
e.g.:

            let jh = tokio::task::spawn_blocking(|| {
                opentelemetry::global::set_tracer_provider(provider);
            });
            futures::executor::block_on(jh).expect("block on set tracer");

I don't have strong feelings. I don't think either is more "correct" than the other.

[garypen]

Ok. I used spawn_blocking(), since that's what it's designed for.
resolved in: bef9051

[cecton]

oh geez I should have seen that xD

[garypen]

Yeah, it's a tricky one. It's hard to know what the correct thing to do is in this scenario.

[cecton]

🧶 The comment should actually mention the ticket ID, a link and summarize what the issue is exactly.

[cecton]

Less of "prevents the router from working correctly"
More like "makes the async runtime hang for some reason"

[garypen]

resolved in: bef9051

[cecton]

1. I'm not entirely sure this issue is a apollo-router issue. It would be nice if you could ask the question to the people of opentelemetry and seek for advice. My reason is that their documentation explicitly says that this function is intended to be called multiple time. It's not like tracing's global subscriber which can be called only once otherwise it panics.
2. Would it be cleaner to spawn a thread in main.rs that will loop on a synchronous channel for new providers? cc @Geal

   fn main() {
       let (tx, rx) = some_channel();
       let otlp_global_tracer_thread = std::thread::spawn(move || {
           while let Some(provider) = rx.next() {
               opentelemetry::global::set_tracer_provider(provider);
           }
       });
       
       // ...
       
       tx.close();
       otlp_global_tracer_thread.join();
       Ok(())
   }

[BrynCooke]

Approved, I guess if it comes up again we dig in further.

[cecton]

Also approved but more as a temporary fix.