Skip to content

Support Dynamic Targets #1162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
johnny2002 opened this issue Jan 20, 2020 · 7 comments
Open

Support Dynamic Targets #1162

johnny2002 opened this issue Jan 20, 2020 · 7 comments
Labels
feign-12 Issues that are related to the next major release proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration

Comments

@johnny2002
Copy link

I am writting a RequestInterceptor, need to know current "Target" so that I can revise the url of the RequestTemplate. While, I found the RequestTemplate.target is still null when it come into RequestInterceptor.apply method. Suggest to put the "target" value before invoke the RequestInterceptors, and provide a getter of RequestTemplate.target.

@johnny2002
Copy link
Author

johnny2002 commented Jan 20, 2020

So my suggested code illustrate:

In RequestTemplate, add:

public String target() {
    	return target;
    }

Then, the most important thing is that the target instance must not be null in RequestInterceptor.apply() method, so in SynchronousMethodHandler, add a line:

  public Object invoke(Object[] argv) throws Throwable {
    RequestTemplate template = buildTemplateFromArgs.create(argv);
    **template.target(this.target.name());**

Then in my interceptor:

    @Bean
    public RequestInterceptor cloudContextInterceptor() {
    	return new RequestInterceptor() {
    		@Override
			public void apply(RequestTemplate template) {
				String target = template.target();
				if (target.contains("$CLUSTER_ID")) {
					target = target.replace("$CLUSTER_ID", getClusterId(template));
					template.target(target);
				}

@kdavisk6
Copy link
Member

This is not really something that RequestInterceptor is meant for. The Target is immutable once created. Can you please explain what your intention is?

If you want to do dynamic host resolution, I suggest that you look into something like Ribbon or an alternative that can do that for you. Another option is to use our Request URI override support here:

https://github.com/OpenFeign/feign#interface-annotations

@kdavisk6 kdavisk6 added the feedback provided Feedback has been provided to the author label Jan 21, 2020
@johnny2002
Copy link
Author

This is not really something that RequestInterceptor is meant for. The Target is immutable once created. Can you please explain what your intention is?

If you want to do dynamic host resolution, I suggest that you look into something like Ribbon or an alternative that can do that for you. Another option is to use our Request URI override support here:

https://github.com/OpenFeign/feign#interface-annotations

Our project is a very large application, need to divide a service to several cluster. e.g. CustomerService needs to be separated to CustomerService001, CustomerService002...
Then we need will dispatch requests to different cluster according the customer number scope in RequestInterceptor. e.g. customer number from 0 to 1,000,000 will dispatch to CustomerService001, 1,000,001 to 2,000,000 dispatch to CustomerService002 ...
We are not expected to change the target object, while we need the target name in RequestInterceptor.apply() method to determine whether we should dispatch the service to a cluster.

@qrqhuang
Copy link

qrqhuang commented Oct 10, 2020

I think it must be very normal appeals to access the target.
In my case, all micro service should sign the uri before request.
The sign rule is common method and need know the app-name wihch define in @FiegnClient(name='xxx-service')

So can feign.SynchronousMethodHandler#invoke modify like this.

for (RequestInterceptor interceptor : requestInterceptors) {
  interceptor.apply(target,  template);
}

@johnny2002
Copy link
Author

I think it must be very normal appeals to access the target.
In my case, all micro service should sign the uri before request.
The sign rule is common method and need know the app-name wihch define in @FiegnClient(name='xxx-service')

So can feign.SynchronousMethodHandler#invoke modify like this.

for (RequestInterceptor interceptor : requestInterceptors) {
  interceptor.apply(target,  template);
}

https://blog.csdn.net/weixin_45357522/article/details/106745468

@kdavisk6
Copy link
Member

Based on these comments, what you are looking for is a DynamicTarget implementation or component available that allows users to provide a Consumer or Function that can resolve the endpoint at runtime. I can see the utility of this and I think that's a good idea.

I'll rename this issue to reflect the need. If there is enough interest, we'll convert this into an enhancement and move forward.

@kdavisk6 kdavisk6 changed the title Cannot get the target in RequestInterceptor Support Dynamic Targets Dec 29, 2020
@kdavisk6 kdavisk6 added proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration feign-12 Issues that are related to the next major release and removed feedback provided Feedback has been provided to the author labels Dec 29, 2020
@johnny2002
Copy link
Author

Based on these comments, what you are looking for is a DynamicTarget implementation or component available that allows users to provide a Consumer or Function that can resolve the endpoint at runtime. I can see the utility of this and I think that's a good idea.

I'll rename this issue to reflect the need. If there is enough interest, we'll convert this into an enhancement and move forward.

With 3.x, we can do it now. No need to change for this any more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feign-12 Issues that are related to the next major release proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration
Projects
None yet
Development

No branches or pull requests

3 participants