Generic http client handler does not work well with okhttp

[lmolkova]

I'm trying to use HttpClientHandler with OkHttp and use it inside network interceptor.

When HttpClientHandler.handleStart is invoked, it supposed to inject header.
Unfortunately, OkHttp uses builder pattern to construct requests and the only way to add a header is through builder (see https://stackoverflow.com/questions/32196424/how-to-add-headers-to-okhttp-request-interceptor)

It is possible to supply HttpClientHandler with request builder, but it seems to be inefficient (re-building and extra allocations).

The only efficient way to instrument okhttp seems to be:

0. create request builder
1. start span
2. inject trace context
3. build request
4. fill attributes (yes, you can fill them from request, but not the builder)

But this approach also does not work with the network interceptor (and it nice to be able track redirects and retries without thinking about the position in the handler list)

So I think the right long-term plan is to fix it on the okhttp side.

@rghetia am I missing something in the handler design? what http libs it intends to work with?

What version of OpenCensus are you using?

0.19.0

[rghetia]

@lmolkova I have used it with jetty. I am not familiar with okhttp but let me look into it.

[rghetia]

I looked at okhttp and agree with your assessment. Another problem with creating span during request builder is that the actual request may be executed later and there by creating longer span than what it is.

[lmolkova]

@rghetia thanks for looking into this.

I've created feature request for okttp: square/okhttp#4459

[lmolkova]

As per conversation in the square/okhttp#4459 there is no support from the okhttp team to save a couple of allocations, at least until we have more info on how bad it is for performance

The request object is immutable by design. I don't see any reason to change
that and lose all the benefits therein without proof these allocations are
actually prohibitive to performance. And that finding would be extremely
surprising given the other allocations and work that must be done
per-request.

It also makes it hard to use HttpClientHandler with okhttp as it requires special treatment - rebuilding. Probably the easiest approach is to have a special handler implementation.