Setting extensions on a outgoing client is not visible inside interceptors / tower service wrappers

[mtoader]

Bug Report

Version

➜ cargo tree | grep tonic
│ └── tonic v0.5.2
│ └── tonic-build v0.5.1
│ └── tonic v0.5.2 ()
├── tonic v0.5.2 ()

Platform

Darwin xxxxx.local 20.6.0 Darwin Kernel Version 20.6.0: Wed Jun 23 00:26:31 PDT 2021; root:xnu-7195.141.2~5/RELEASE_X86_64 x86_64

Crates

this is either tonic itself or tonic-build depending on where we want to put the fix.

Description

I have a generated gRPC client from a set of protobufs, and I am trying to do a generic metric / logger wrappers. I ended up using a tower wrapped target service to build the client. However in order to properly define the per request metrics I have to link a metricsclient like abstraction (preconfigured properly) to each request. What I did was to do req.extensions_mut().insert(ctx.metrics.clone()); on a tonic::Request<T> that I get from using a let mut req = req.into_request(); after I build my generated req type.

The expectation was that I would be able to access that on the hyper request that is being passed on the service abstraction itself. However, this is not happening.

Looking into the code I see that in the generated GrpcClient this happens in the execute method for a service endpoint:

            let path = http::uri::PathAndQuery::from_static("/proto.service.Name/Method");
            self.inner
                .server_streaming(request.into_request(), path, codec)
                .await

the request.into_request() here has the extensions set properly, however the server_streaming method does this:

        let request = request.map(|m| stream::once(future::ready(m)));
        self.streaming(request, path, codec).await

and the request.map() method does this:

  let message = f(self.message);

        Request {
            metadata: self.metadata,
            message,
            extensions: Extensions::new(),
        }

basically dropping the extensions on the floor.

My questions:

• are the req extensions the right place to pass context like values into a request processing pipeline (I would expect that to be true) and if so why is the Request.map() dropping them (looks like a bug)?
• if they are not, what is the right way to do this (apart from threading a context like object along the pipeline)?

[davidpdrsn]

Seems like a bug to me. Request::map should probably be

#[doc(hidden)]
pub fn map<F, U>(self, f: F) -> Request<U>
where
    F: FnOnce(T) -> U,
{
    let message = f(self.message);

    Request {
        metadata: self.metadata,
        message,
        extensions: self.extensions,
    }
}

[mtoader]

That's what it seems to be me. Happy to open a PR with mtoader@1427c5c if people agree that this is the right solution.

[davidpdrsn]

That would be great!

[mtoader]

Opened #771.

[mtoader]

@davidpdrsn are you able to take a look at #771 ?

[davidpdrsn]

Yes I'll take a look some time this week.

[mtoader]

Great, Thank you!