OTLP MetricExporter deadlock issue

[cijothomas]

https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-otlp/src/exporter/tonic/metrics.rs#L58

Tonic MetricExporter uses std::Mutex to protect inner client, and the lock is held across await points. This can cause deadlock. The fix would be to use tokio::Mutex, but then shutdown/cleanup cannot be easily done, like already faced by LogExporter. But that is better than having code that deadlocks.

Opening an issue to track fixing this.

[cijothomas]

Looks like same issue in HTTP also https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-otlp/src/exporter/http/mod.rs#L271

[fraillt]

I might be missing something, but current code doesn't hold the lock across the await point...
Given that inner is accessed in two places (export and shutdown), I pretty sure that code is correct and it's not possible to deadlock...
What do I miss here?

[cijothomas]

.and_then(|g| match &*g {
Some(client) => Ok(Arc::clone(client)),
_ => Err(OTelSdkError::AlreadyShutdown),
})?;

if the guard is dropped here itself, then yes, this looks correct. (I was incorrectly assuming the lock guard is not dropped until export() method ends..

[jakubdyszkiewicz]

Hey @cijothomas 👋

I need to dynamically reconfigure the exporter during binary execution, but the SDK seems to be stuck on shutdown. Is this the same issue?

Here is a minimal repro

#[cfg(test)]
mod tests {
    use super::*;

    #[tokio::test]
    async fn test_otel_recorder_shutdown() -> anyhow::Result<()> {
        let telemetry_url = Url::parse("http://localhost:8090/otlp/v1/metrics").expect("should parse");
        let exporter = opentelemetry_otlp::MetricExporter::builder()
            .with_http()
            .with_endpoint(telemetry_url)
            .with_protocol(Protocol::HttpBinary)
            .with_timeout(Duration::from_secs(1))
            .with_http_client(reqwest::Client::builder().build().expect("should build"))
            .build()
            .expect("should build");

        let reader = PeriodicReader::builder(exporter, runtime::Tokio)
            .with_interval(Duration::from_secs(10))
            .build();

        let sdk = SdkMeterProvider::builder()
            .with_reader(reader)
            .build();

        let _ = sdk.shutdown(); // called on drop(sdk);

        Ok(())
    }
}

The test never finishes, it's stuck on the shutdown().