Feature Request: Create a instrumentation for Feign

[rafabene]

"Feign makes writing java http clients easier".

It also supports adding Request Interceptors https://github.com/Netflix/feign#request-interceptors that could be used to instrument Feign

[ewhauser]

As an alternative, you can use OkHttp with Feign and use https://github.com/openzipkin/brave/tree/master/brave-okhttp

[codefromthecrypt]

ps feign isn't the easiest thing to manage right now as interceptor only affects inbound. Ex you have to play games to catch the other side (by using an invocation handler or hacking a decoder or the like).

rejigging this part would probably be good for https://github.com/OpenFeign/feign and emulate how okhttp does it.

The advantage of doing this in feign level is that you can use the template name or class name of the feign interface as the span name, which makes it look prettier than "GET"

[rafabene]

The greatest advantage for me at this moment is that Feign already provides an integration with Hystrix: https://github.com/OpenFeign/feign/tree/master/hystrix

That's why I wanted to see Feign being supported so I could easily have: Feign + Hystrix for circuit-breaker + Zipking for distributed tracing.

The implementation that I was able to write to have this integration working is too "complex" to be easily consumed: https://github.com/redhat-helloworld-msa/hola/blob/master/src/main/java/com/redhat/developers/msa/hola/HolaResource.java#L95-L114

[codefromthecrypt]

we can probably do this, but it is a little tricky, especially when layering with things like hystrix

in the mean time, you can look at sleuth which already supports feign+hystrix https://github.com/spring-cloud/spring-cloud-sleuth

[codefromthecrypt]

In thinking this through, I think that Feign would be best as a "local span".

For example, feign's span would correlate to the method being invoked. Considering that the feign.Client implementation is traced in the case of OkHttp, we'd end up with something like this.

For example, feign's local span would have a child RPC span for each http request attempt:

{name: github.contributors // start when the method is invoked
  {name: get ["cs", "error"] }, // new span logged by okhttp
  {name: get ["cs", "cr"] },  // new span logged by okhttp
} // finish when the method returns

We could add to it later, like add annotations, etc, but the easiest way to get started is to create a span and assume the client is instrumented.

[johnament]

So just wondering, is this still an issue considering the changes that have been pushed in by @adriancole ? I was also kind of curious about this feature set, good to see my old buddy @rafabene had the same idea.

[codefromthecrypt]

The tricky part with feign is not doing double-dip tracing. For example, if OkHttp is below, it should really own the client span, similarly ApacheHC. A "cheap" way out would be to wrap feign.Client, but it is scarcely better than, and could interfere with transport-level tracing. When thinking about how to implement this, see how it goes with and without a traced client underneath.

[codefromthecrypt]

The other tricky part is the entrypoint. a wrapped Feign builder is neat, though this interferes with the Hystrix one! You could make hystrix an optional dep and do 2 entrypoints: TracingFeignBuilder and TracingHystrixFeignBuilder :P

[codefromthecrypt]

probably good time to try this as Brave 4.3 is out

[ImFlog]

Sleuth defines a wrapper for feign clients (https://github.com/spring-cloud/spring-cloud-sleuth/tree/master/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign) Should we port the code from sleuth to brave instrumentation now that Sleuth is using Brave ? Would that make sense as it's how it is done for Kafka for example ? WDYT @adriancole @marcingrzejszczak ?

[marcingrzejszczak]

The way it's done in Sleuth is shameful :P It would be better for someone to actually try to write it from scratch.

[svenhaag]

nothing new here? :(

[codefromthecrypt]

we could do something like this, though I wouldn't integrate spring cloud loadbalancer, etc, as that's where it gets gnarly. This means this code might not be literally used by sleuth.

https://github.com/spring-cloud/spring-cloud-sleuth/blob/master/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java

Any volunteers?