Skip to content
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

Support to set loadBalancerKey from request header #451

Open
wants to merge 1 commit into
base: 2.2.x
Choose a base branch
from

Conversation

alan-tang-tt
Copy link

When I write myself MyRule implements IRule, the key is always null, like below:

public class MyRule extends AbstractLoadBalancerRule {

    @Override
    public Server choose(Object key) {
          // here the key is always null
    }
}

By watching the source code, I find there's no where to set this loadBalancerKey. So I push this PR to resolve this problem by overriding FeignLoadBalancer#customizeLoadBalancerCommandBuilder method.

         // override FeignLoadBalancer#customizeLoadBalancerCommandBuilder
	@Override
	protected void customizeLoadBalancerCommandBuilder(RibbonRequest request, IClientConfig config, LoadBalancerCommand.Builder<RibbonResponse> builder) {
		builder.withServerLocator(request.request.headers().get("loadBalancerKey"));
	}

By this way, I can put my loadBalancerKey from feign.RequestInterceptor implementation like this:

public class MyRequestInterceptor implements RequestInterceptor {
    @Override
    public void apply(RequestTemplate template) {
        template.header("loadBalancerKey", "gray");
    }
}

Then in MyRule I can get the key:

public class MyRule extends AbstractLoadBalancerRule {

    @Override
    public Server choose(Object key) {
          // no the key equals to gray
    }
}

This is all.

@codecov
Copy link

codecov bot commented Dec 27, 2020

Codecov Report

Merging #451 (fdbc0c7) into 2.2.x (e32b10b) will increase coverage by 0.18%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##              2.2.x     #451      +/-   ##
============================================
+ Coverage     77.80%   77.98%   +0.18%     
- Complexity      476      477       +1     
============================================
  Files            64       64              
  Lines          1883     1885       +2     
  Branches        265      265              
============================================
+ Hits           1465     1470       +5     
+ Misses          298      295       -3     
  Partials        120      120              
Impacted Files Coverage Δ Complexity Δ
...work/cloud/openfeign/ribbon/FeignLoadBalancer.java 70.14% <100.00%> (+0.91%) 7.00 <1.00> (+1.00)
...ncer/RetryableFeignBlockingLoadBalancerClient.java 70.76% <0.00%> (+4.61%) 11.00% <0.00%> (ø%)

Copy link
Collaborator

@OlgaMaciaszek OlgaMaciaszek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alan-tang-tt Thanks for the PR. The idea is good, however some more work has to be done on the PR.
Please verify if the headers contain the header you want to use.
Please change the default header name to X-LodBalancer-Key.
Please allow changing header name via properties.
Please add tests.
Please add documentation.

@@ -93,6 +94,11 @@ public RibbonResponse execute(RibbonRequest request, IClientConfig configOverrid
return new RibbonResponse(request.getUri(), response);
}

@Override
protected void customizeLoadBalancerCommandBuilder(RibbonRequest request, IClientConfig config, LoadBalancerCommand.Builder<RibbonResponse> builder) {
builder.withServerLocator(request.request.headers().get("loadBalancerKey"));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And consider other parameter like url parameter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants