Propagate Tracing Headers in ext_proc

[scrocquesel]

Title: B3 headers should be propagated to the gRPC context call

Description:

Unlike ext_authz, ext_proc do not send tracing headers. Or I don't find how to do it.

[optional Relevant Links:]

We should be able to fetch the header like in ext_authz with injectContext : #6520

[scrocquesel]

I mean like https://github.com/envoyproxy/envoy/blob/main/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc

[scrocquesel]

From what I understand, tracing span for streaming grpc call was not implemented during #1885.

[wbpcode]

cc @snowp

[wbpcode]

ext_proc will sends all request headers include B3 headers to the server as part of grpc requests.

So do you mean that you want to add tracing support of grpc rqeuests self?

[scrocquesel]

Yes, span should begin with the start of the stream and should end at the end of it. And a child span can be created in the filter to align with how ext_authz is doing it. Use the header from the request as parent span seems not right as envoy may have create child span itself.

Maybe a new span be created for each message in the stream and pass as metadata also but it seems that gRPC tracing interceptor are not working this way. At least, https://github.com/opentracing-contrib/java-grpc/blob/master/src/main/java/io/opentracing/contrib/grpc/TracingServerInterceptor.java, is just creating a span for the whole stream and adding logs.

[scrocquesel]

I tried to set the initial_metadata with %REQ(x-b3-traceid)% but if the downstream request do not have those headers, envoy generate a new trace and I don't get those values at all (stream request nor message request)

[wbpcode]

if the downstream request do not have those headers, envoy generate a new trace and I don't get those values at all (stream request nor message request)

This is because Envoy inject the tracing headers at the last of request header processing. So you get nothing in the ext_proc.

[wbpcode]

Looks like this is a general issue for the external grpc requests. But sorry I have no enough bandwidth for this recently. 😞 I will mark this help wanted.

[xvzf]

@wbpcode would love to get my hands dirty on this, but don't really know where to start - wanna give me a hand?

[nahk-ivanov]

@xvzf It seems like the reason lies in the fact that ext_proc filter is using stream API, while ext_authz is using simple request/response. For request/response (aka send), there is a dedicated parent_span parameter in the gRPC client, which will eventually add tracing information to the outgoing request. For the gRPC stream API, there is no mentioning of trace headers anywhere near.

I ain't no gRPC expert, but it appears to me that with streaming, you only send the headers once and not per message, so perhaps what needs to happen is AsyncClientImpl::startRaw should be updated to take in parent_span (similar to basic send), then we spawn new child span like this in the AsyncStreamImpl constructor, and finally populate the trace headers in AsyncStreamImpl::initialize before sending them like this. I don't know if gRPC streaming API headers (Http::RequestMessagePtr) are handled anyhow different compared to basic request/response (Http::RequestHeaderMap), so there might be some additional work we need to do to adopt/shim one to another.

In the ext_proc code you can then get the span information from decoder_callbacks_ field, like this and pass it to the start (that you will modify to accept span information) here.

We were thinking to work-around this by adding a Lua filter at the beginning of the filter chain, that will inject tracing header(s) with our manually generated IDs, so that we can reference it through initial_metadata with %REQ(..) and therefore share between main request processing pipeline and calls to ext_proc, but it seems like that is not going to work either, because connection manager will generate new Span structure at the beginning before calling filters (which I assume makes sense, as filters can log/trace), and that structure is used throughout the code, instead of reading headers all the time. It doesn't seem to care about any headers' changes afterwards. So you will still get different IDs in the main processing pipeline and everything that tries to read the trace id from the headers, unless I missed something.

[wbpcode]

cc @xvzf sorry for the delayed respnose. I almost forget this PR. But I think @nahk-ivanov has given a great explanation about this work.
Sorry again for the delay.

[cainelli]

It's been some time since the last comment. Is there an update on this issue? We're transitioning from Lua filters to exproc, but it appears that traces are still not being propagated to the grpc server.

[cainelli]

I'm really interested on this and decided to take a stab #33665. Thank you for the very detailed pointers @nahk-ivanov, it was sufficient for one with no prior C++ knowledge to follow. I would appreciate any review.

[nahk-ivanov]

@cainelli LEGEND!!!!

[ekkinox]

coming after the war but @cainelli thanks a lot for this work !!!