Better support for request-specific headers

[chadjaros]

This is more of a question than an issue, but there is no mailing list for Feign that I could find.

From what I can tell, feign does not support request-specific headers unless they are declared as a method parameter. This is problematic from the standpoint of incorporating Feign into a set of stateless web services as a singleton bean, and wanting to optionally forward a set of headers from service to service without having the client explicitly have to deal with them.

It seems that creating feign clients per-request could be a performance concern since reflection is used to generate the proxy. I couldn't find anywhere in the Feign builder infrastructure which caches the reflected object state to improve the performance of repeated builds of the same interface with minor adjustments to the builder object.

Has any thought been put into either adding some mechanism for allowing modifications to a specific request before sending it, or reducing the cost of creating Feign instances? Either of these approaches would help me achieve my goal. The first suggestion could potentially dovetail with the ticket for asynchronous requests that I saw on the board.

Thanks,
Chad

[codefromthecrypt]

Hi, Chad. Good point on no mailing list :) I suppose if we get enough issues etc we'll start looking at SO or maybe a mailing list. So, it is true that one should cache instances created by feign. It isn't impossible to make a feign compiler, and that sort of thing would be welcome. Since feign has only one method, it shouldn't be too hard to make a forwarding caching function. Also, I might look at RequestInterceptor, as this is what I use in denominator for example to decorate requests. (Also look at denominator, as a lot of patterns are there). Does any of this help? If not, can you give some concrete examples you'd like to enable?

[chadjaros]

Hi Adrian,

I was looking at using RequestInterceptor, but it seems like it would be less than straightforward to leverage that functionality to achieve per-request header forwarding if one were to share a single Feign instance across multiple requests.

My plan right now is to test out the possibility of using ThreadLocal to write an interceptor capable of being hooked into Spring MVC. Since SpringMVC uses the thread-per-request model, this should theoretically allow injection of request-specific headers in my use case. This is obviously a very tailored solution for my platform, and wouldn't work for a platform based on Netty's non-blocking IO (like the Play framework). Assuming that works, I might not have an immediate need for improved performance when creating Feign instances.

If it doesn't work, I'll likely look into adding reflection caching for the feign builder in the near future and submitting a pull request. The improved build performance would likely allow me to use per-request scoped Feign clients, built with a simple interceptor which adds the request's headers. I'd like to add this functionality either way because it seems like it is a more resilient mode of operation.

If you have any concrete examples of dealing with per-request-variation types of problems, I'd be glad to see them. I searched through the denominator project, but didn't find any examples of interceptor use.

Thanks

[codefromthecrypt]

oh. you are right.. actually denominator pretty much always customizes request headers using the Target object. Ex. this is how it adds authentication data. Chad, could you help me by being more concrete about the use case? (vs the implementation) Ex. can you sketch a sample client that and what you would be doing to it? Would the caller interact with a hook before invocation or would they set state? Would a system just set state for them? What is the state (ex. is this a more generalizable security or tracing thing?) A couple use cases will help me understand better. I think there's room for defining a request scope (or even making clients cheaper), just want to make sure we can write a test case for it.

[codefromthecrypt]

Ps more on places that can help workaround Target is the last step before the request is invoked by the client. https://github.com/Netflix/feign/blob/master/core/src/main/java/feign/Target.java You could also play with a forwarding Client. https://github.com/Netflix/feign/blob/master/core/src/main/java/feign/Client.java Or something more fancy like what spring cloud does by overriding invocation https://github.com/Netflix/feign/blob/master/core/src/main/java/feign/InvocationHandlerFactory.java https://github.com/spring-cloud/spring-cloud-netflix/tree/master/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign Cc @spencergibb Hope this helps!

[codefromthecrypt]

Apologies for the idea storm. I bet JMH would show most time is spent parsing the annotations (Contract). This might be a quicker and more stable place to get some caching quick wins.

[chadjaros]

Concrete Example:

So I'm trying to write a server + client library as succinctly as possible. The primary use case for the client library is to enable simple service-to-service communication. I would like to have the Authorization header automatically pass through when doing such a service-to-service communiqué. This is primarily to reduce the boilerplate associated with declaring the Authorization header as a method parameter on every method, in every server.

So, lets say I have two service-oriented servers: One to get Cookies and one to get Ingredients. I want to define the interfaces exactly once, and have those be usable by both Feign and SpringMVC. So I'm using the spring-cloud-netflix SpringContract to enable the SpringMVC annotation support in Feign. Thus I have:

// Defined in Cookie API
public interface CookiesTemplate {
  @RequestMapping(value="/cookies/{id}" method=GET, mediaType=JSON)
  public Cookie get(@PathParam("id") String id);
}

---

// Defined in Ingredient API
public interface IngredientsTemplate {
  @RequestMapping(value="/ingredients/{id}" method=GET, mediaType=JSON)
  public Ingredient get(@PathParam("id") String id);
}

I can implement these interfaces in my servers (one each) to write my SpringMVC based web services. I can also build a client around these interfaces using Feign. This is awesome, because it just so happens that the Cookie server depends on the Ingredient server. I need the get() method of the Cookie server to call the get() method of the Ingredient server.

Ideally I want to set up the Feign clients as Spring beans, so that they can be accessed freely in my services.

@Configuration // Inside of Cookie Service
public class ConfigForCookie {
  @Bean
  public IngredientsTemplate ingredientClient() {
    return ...; // return Feign client built around the IngredientTemplate class
  }  
}

Here the Feign client is defined as a bean and provided to any place within the Cookie server that needs to use it.

Every endpoint is secured, meaning that it requires an Authorization header. Since both of these endpoints are secured using the Authorization header, I'm trying to figure out how to pass that around properly. The Authorization header contains a user's access token, and it can be different for every request. We have a security library that uses Spring Interceptors to validate the Authorization header on every request, and don't have to deal with it directly. I'd like to set up something similar such that the user's token is passed along the service-to-service call chain without having to do it manually.

---

I was considering adding caching within the Contract, and I just now see that you can create a class-based cache at that level. My assumption is that a class only needs to be interpreted once by any specific Contract implementation, since it generally will not change at runtime. If the MethodMetadata instances could be cached at the completion of processing an interface, you could bypass all future calls to that Contract for that interface. Making the Feign building performant would allow me to add a couple of Spring annotations and call it a day with the very simple interceptors.

Example:

@Autowired
private HttpHeaders headers;
...
@Bean(scope= DefaultScopes.REQUEST)
@ScopedProxy
public IngredientsTemplate ingredientsClient() {
  return ...;// Feign client with basic interceptor using simple headers from the HttpHeaders object
  // The returned object is good for the current request's scope only, and a new one will be built 
  // for the next request
}

I basically want to make service-to-service calls as simple as one would expect from Feign.

!!! - new potential avenue
I might be able to build a Feign interceptor around the HttpHeaders object directly. That object is supposed to be request scoped already. I'm going to pursue that avenue first.

But - I still think it would be good to have a better way to achieve this sort of functionality independent of anything Spring supplies.

[codefromthecrypt]

Good idea wrt http headers via interceptor (or target which is basically the same thing).

I will process this further later tonight or tomorrow morning. Thanks a million for taking the time to elaborate this, Chad.

[spencergibb]

@chadjaros I'll also take a further look.

[codefromthecrypt]

Apologies for the delay. I'm using all OSS energy to get a denominator release cut by april 1. I will circle back at the end of the week.

[chadjaros]

Not a problem

[chadjaros]

I've come up with a pattern that I think would allow both for request-specific parameters, and for asynchronous operations as requested in this ticket. Take a look and let me know what you think:

public class FeignClient<T> {

    // Things that you want to modify on a per-request basis. This includes
    // interceptors and load balancing rules.
    private List<RequestInterceptor> interceptors;
    private List<IRule> rules;

    public T go() {
        return proxy(interceptors, rules);
    }

    public RequestBuilder request() {
        return new RequestBuilder(interceptors, rules);
    }

    public AsyncRequestBuilder asyncRequest() {
        return new AsyncRequestBuilder(interceptors, rules);
    }

    protected T proxy(List<RequestInterceptor> interceptors, List<IRule> rules) {
        // TODO apply additional interceptors and rules
        // TODO Return proxy with injected settings
        return null;
    }

    @Data
    @Accessors(chain = true, fluent = true)
    class RequestBuilder {
        private List<RequestInterceptor> builderInterceptors;
        private List<IRule> builderRules;

        public RequestBuilder(Collection<RequestInterceptor> interceptors, List<IRule> rules) {
            builderInterceptors = Lists.newArrayList(interceptors);
            builderRules = Lists.newArrayList(rules);
        }

        private RequestBuilder addInterceptor(RequestInterceptor interceptor) {
            builderInterceptors.add(interceptor);
            return this;
        }

        private RequestBuilder addRule(IRule rule) {
            builderRules.add(rule);
            return this;
        }

        public T go() {
            return proxy(builderInterceptors, builderRules);
        }
    }

    @Data
    @Accessors(chain = true, fluent = true)
    class AsyncRequestBuilder {

        private List<RequestInterceptor> builderInterceptors;
        private List<IRule> builderRules;

        public AsyncRequestBuilder(Collection<RequestInterceptor> interceptors, List<IRule> rules) {
            builderInterceptors = Lists.newArrayList(interceptors);
            builderRules = Lists.newArrayList(rules);
        }

        private AsyncRequestBuilder addInterceptor(RequestInterceptor interceptor) {
            builderInterceptors.add(interceptor);
            return this;
        }

        private AsyncRequestBuilder addRule(IRule rule) {
            builderRules.add(rule);
            return this;
        }

        public <U> ListenableFuture<U> go(Function<T, U> function) {

            SettableFuture<U> future = SettableFuture.create();
            final T instance = proxy(builderInterceptors, builderRules);
            {   //TODO execute asynchronously
                future.set(function.apply(instance));
            }

            return future;
        }
    }
}

After an initial look, the code that would need to change to support this is somewhat substantial. The binding to a target would be delayed until the call to FeignClient#proxy, so code that is executed up to that point would need some refactoring to make it more efficient than it currently is.

I'm going to take a run at making something like this work, and I'll let you know how it goes.

The reason I like this pattern is that you can set up an asynchronous method call's parameters on your current thread prior to dispatching it. If you're using something that wraps a ThreadLocal object, like Spring and Jersey do for accessing request-specific headers, you need to extract those prior to throwing them on the queue to be executed asynchronously. If I tried to execute a method call asynchronously with the code that I have deployed today, I would lose my request specific headers.

[codefromthecrypt]

Thanks, Chad. I plan to have a deep look at this tomorrow.

[chadjaros]

Here is a quick and dirty proof of concept. Completely untested, but I think it illustrates the emphasis on late binding.

https://github.com/chadjaros/feign/compare/master...chadjaros:late-binding-client?expand=1

[codefromthecrypt]

Cool. Today got away from me. Will try again tomorrow!

[codefromthecrypt]

@chadjaros I've looked at this, and I'm wondering if this can't be solved with composed Targets?

Ex. a Target can have an instance of a feign client. It completely controls the request sent, so can do anything. If there's something of asynchronous nature, we'd probably best decouple that from collaborative clients as it makes the topic easier to grok.

Ex. take a look at the existing thing here:

https://github.com/Netflix/feign/blob/master/core/src/test/java/feign/TargetTest.java

I targets to deal with credential supplying in denominator, too. For example, there's one where a discovery client needs to be invoked to get the endpoint and header to pass to other things. Target was literally made for this usecase.

Thoughts?

[chadjaros]

For example, there's one where a discovery client needs to be invoked to get the endpoint and header to pass to other things.

I definitely see how Target can be used to provide endpoints and headers for certain use cases. I have plans to create some custom targets for use cases around discovery and load balancing.

It completely controls the request sent, so can do anything.

I think that your assertion makes sense, and could be suitable for my use case if it were in a single threaded context. In this respect, I could do something such as:

CustomTarget target = new CustomTarget(MyInterface.class, "https://server");
MyInterface proxy = Feign.builder()
    ...
    .target(target);

target.setLoadBalancingKey("Only servers on port 4444");
target.setHeader("headerKey", "headerVal1", "headerVal2");

Cake chocolateCake = proxy.getCake("chocolate");

Is this along the lines of what you were thinking?

In a multi-threaded context I don't think this continues to work. If I tried to execute a second request concurrently using the same target and proxy, the header and load balancer key values become unpredictable, and neither request can be guaranteed to execute properly.

As I understand it, the interface proxy to Target relationship is 1 to 1. Any parameters that I pass into the target will affect the proxy generated from that target. To make this paradigm safe in a concurrent application, you must pass all of the modifiable parameters in a single scope. The way that the proxy object and Target are defined means that you likely have to create a new target and proxy for arguments that fall outside of the interface's method arguments, and as we discussed before that doesn't seem to be very efficient. The code that I provided is an attempt to make creating the new target and proxy as efficient as possible, and thread-safe as well.

As for asynchronous execution, I'm not sure how it would be possible to handle this within the target. Since the call to your proxy (and subsequent return value) is synchronous, any asynchronous work you do within the target must be re-synchronized to return a valid resultant conforming to your interface's method declaration. I'd like using an asynchronous interface to be as straightforward as using the synchronous one, and I think that the pattern that I proposed above does just that:

FeignClient<MyInterface> client = ....target(...);
ListenableFuture<Cake> futureCake = client.asyncRequest().go( request -> {
    return request.getCake("chocolate");
});

Am I way off base here? If so, can you help illustrate more clearly how these use cases could be solved using the existing model?

[codefromthecrypt]

No.. I don't think you are way off base. It is hard to relate nuances like this in text, but you've done a surprisingly good job, Chad. I will make another pass at thinking through this. Personal deadline is Weds but maybe sooner. Thanks for championing this feature!

[chadjaros]

Thanks Adrian, it's my pleasure.

[codefromthecrypt]

Ok so another round, and looking for least efforts and new concepts to support request-scoped overrides, such as headers, applied via thread-locals.

base case: cache feign and do .newInstance for each request

Feign (the object) has a .newInstance(Target) method, which for at least the case of synchronous commands could achieve the goal of late binding context including any headers. Without any tuning, "laptop speeds" are 12K contract parses per second, and could be improved.

alternative: no change to feign, but deeper integration in a framework.

One way to avoid the performance burden could be to fix the number of calls to feign.newInstance (instances) to the threads that affect state.

i.e. the source of request-scoped data is a thread-local, coordinate with it. Ex. have feign also be a thread-local, use an invocation handler, custom target or otherwise to ensure that when the framework applies state, it goes to whatever hook is present at the same time.

alternative: make it cheaper to customize feign (aka chad's idea)

Looking closer at chad's idea, deferring the application of interceptors or target. I think this can work.

If you look carefully at Feign.newInstance, just about everything deals only with Target.type, the interface type T. Everything else is not very important until a request is made.

I like Chad's idea, but would feel comfortable with a little less initial scope. For example, by limiting scope to target, there are less new concepts in feign, and should still be possible to construct new abstractions.

Feigned<Foo> foo = Feign.prepare(Foo.class); // forgive the name, this does everything expensive
Foo requestScoped = foo.target(new FooTarget()); // call this as often as per request

Something like this we'd call beta until async is done. In any case I'd really like to ensure whatever we make has test cases that would break if we screwed up the impl (ex. actually set a thread-local in a test.)

Thoughts?

cc @spencergibb
ps apologies but I can only participate on this thread more tomorrow, as I'll be offline till early june.

[chadjaros]

I like the second idea. I think that it solves both per-request and asynchronous use cases, and is generic enough to work with any framework.

I agree that separating the target from the interface type is something that could happen. In my quick and dirty example I considered it, but then decided not to since it was quick and dirty.

If we did this, you could even supply a default target in the builder. Overriding the target could be optional as a late binding effort to support advanced scenarios.

[codefromthecrypt]

cool, so if you are cool from moving from quick and dirty land to implementation, give it a go! I can review on June 2. If you choose to, make sure no deps are added. Otherwise, I can have a go when I get back. Thanks regardless for thinking this through including mockups, Chad.

[chadjaros]

I have to wait for GrubHub to put together our github organization and get our legal ducks in a row before I can submit such an implementation. I'd like to do that, but it might take a couple more weeks to get everything in order.

I'll let you know.

[codefromthecrypt]

Okie. Keep us posted!

[zampettim]

@chadjaros I too am very much interested in this solution. I have the same basic requirements, where I'm building a set of micro-services using Spring Cloud and need to propagate the request header information so that I can pass along the request-specific authorization information from one service to the next.

[zampettim]

@chadjaros Any updates on your possible solution?

[chadjaros]

Sorry for the delay guys. I never was able to get my old company to set up an OSS policy, and was hamstrung by that. I've since changed jobs and now should have the freedom to work on this on the side.

[PedroAlvarado]

@chadjaros I ran into this thread as we are running into difficulties trying to set per-request headers as well. Is this feature something we can look forward to? Is there other ways to tackle this problem today?

[faizanahemad]

We were trying out Feign to replace our clients written in Jersey. Not being able to set request headers per-request is preventing us from a full blown adoption. So why can't we use the Target class? Thread safety?
Any plans to include this in a future release or any simple workarounds?

[codefromthecrypt]

folks, to make it more clear what you're looking for, can you vote for any of the proposals above? or should we assume you want "alternative: make it cheaper to customize feign (aka chad's idea)"

[chadjaros]

Again, sorry for the delay.

@faizanahemad You can't use the target class for this exactly because of thread safety. The target is logically disconnected from the act of making a request. I wasn't able to figure a way to tie something in the target to a specific request without making the target synchronized for the duration of each request. This, of course, seriously impedes performance in a multi-threaded web server that uses a singleton instance of a feign client.

[breznik]

I have the exact same use case, except environment is jersey/guice. +1 for alternative: chad's idea

[GarimaJainATOS]

@adriancole
adriancole
Hi adriancole,
I am trying to call below method through feignClient

ResponseEntity<PagedResources> getTransactionList(
@Valid TransactionSearchRequest categorizedTransactionSearchRequest,
@apiparam("Pageable information") @pageabledefault(size = 05, page = 0) Pageable p);

But i am getting below exception

Caused by: java.lang.IllegalStateException: Method has too many Body parameters: public abstract org.springframework.http.ResponseEntity com.worldline.fpl.banking.budget.controller.IBudgetController.getTransactionList(
com.worldline.fpl.banking.budget.json.TransactionSearchRequest,org.springframework.data.domain.Pageable)
at feign.Util.checkState(Util.java:128)
at feign.Contract$BaseContract.parseAndValidateMetadata(Contract.java:114)
at org.springframework.cloud.netflix.feign.support.SpringMvcContract.parseAndValidateMetadata(SpringMvcContract.java:133)
at feign.Contract$BaseContract.parseAndValidatateMetadata(Contract.java:64)
at feign.hystrix.HystrixDelegatingContract.parseAndValidatateMetadata(HystrixDelegatingContract.java:34)
at feign.ReflectiveFeign$ParseHandlersByName.apply(ReflectiveFeign.java:146)
at feign.ReflectiveFeign.newInstance(ReflectiveFeign.java:53)
at feign.Feign$Builder.target(Feign.java:209)
at org.springframework.cloud.netflix.feign.HystrixTargeter.target(HystrixTargeter.java:48)
at org.springframework.cloud.netflix.feign.FeignClientFactoryBean.loadBalance(FeignClientFactoryBean.java:146)
at org.springframework.cloud.netflix.feign.FeignClientFactoryBean.getObject(FeignClientFactoryBean.java:167)
at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.doGetObjectFromFactoryBean(FactoryBeanRegistrySupport.java:168)
... 44 common frames omitted

When i debugged it, i got know that these

interface org.springframework.data.web.PageableDefault
interface javax.validation.Valid

are not the feign supported annotations and that's why it is failing
But i am not getting the solid fix for it .
Can you help me on this as soon as possible ?

[codefromthecrypt]

@GarimaJainATOS your message is on an unrelated issue about spring-cloud-netflix. please raise a new issue there!

[TehBakker]

Same here, stuck on having to set request specific header.
Issue has been open for 2 years and seems to be a common use case, is there any plan to make it this available?
I'm using maven codegen so I have noway to modify the method declaration interface.

[nickbr23]

I've just ran into this issue - has there been any update?

[JokerSun]

@adriancole I have several question abort the interceptor of open feign that is it thread-safe when i use it to set header like 'token' ?

[suphiya]

Any updates on this, ran into the same issue to make request specific header with short life jwt

[timpamungkas]

Any update on this?

[kdavisk6]

There are a number of ways to accomplish this today. I've included the relevant documentation below:

Headers: https://github.com/OpenFeign/feign#headers
Dynamic Per Target: https://github.com/OpenFeign/feign#setting-headers-per-target
Request Interceptor: https://github.com/OpenFeign/feign#request-interceptors

JHipster also has one approach for building authorized Feign Clients: https://www.jhipster.tech/using-uaa/

For this specific request, no work has started. Chad's idea is one we could consider, which involves creating a new Feign target per request, but I want to see if there are any other alternative proposals. We will then consider this feature for the next major release of Feign (11)

[TehBakker]

This is such a blocking issue for me, we are gonna move away from feign solely because of this