Customize load balanced requests according to the chosen ServiceInstance #164

Merged
merged 1 commit into from Jan 6, 2017

Projects

None yet

2 participants

@william-tran
Contributor
william-tran commented Jan 5, 2017 edited

Applications can define their own LoadBalancerRequestTransformer beans
which can modify the HttpRequest to be executed. These beans can be
@Ordered in case of multiple transformers.

Fixes #162

@william-tran
Contributor

There are public signature changes that I'm not sure we need to be concerned with:

  • Constructors for LoadBalancerInterceptor and RetryLoadBalancerInterceptor now require a LoadBalancerRequestFactory
  • ServiceRequestWrapper is now UriReconstructingRequestWrapper

If we need to be concerned with backwards compatibility for anyone whose directly using these classes outside of what gets autoconfigured, we could be additive and create a default LoadBalancerRequestFactory in the existing constructor.

@ryanjbaxter

I would say we should be additive and not make any signature changes since we want this to go into Camden.

+ @Override
+ public HttpRequest transformRequest(HttpRequest request, ServiceInstance instance,
+ LoadBalancerClient loadBalancer) {
+ return new UriReconstructingRequestWrapper(request, instance, loadBalancer);
@ryanjbaxter
ryanjbaxter Jan 5, 2017 Contributor

Why is this a transformer? I would think Spring Cloud would create the initial request and then pass that request to any transformers that might be present. Its not really transforming the request at all.

@william-tran
william-tran Jan 5, 2017 edited Contributor

I'm not sure if your issue is with nomenclature or structure but I'll try to address both.

I chose the term transformer to mean something that takes in an HttpRequest, and spits out a HttpRequest (that could be a different object). In this case, this is the default behaviour of returning a request that has its URI reconstructed.

This default behaviour is provided as a LoadBalancerRequestTransformer so if an application wanted to replace it entirely it could do so. See https://github.com/spring-cloud/spring-cloud-commons/pull/164/files#diff-be91ae702a070a885afe88254370b793R184

@william-tran
william-tran Jan 5, 2017 Contributor

This default behaviour is provided as a LoadBalancingRequestTransformer so if an application wanted to replace it entirely it could do so.

Another approach would be to hard code this behaviour into LoadBalancerRequestFactory, and if you wanted to replace that behaviour, provide your own implementation of LoadBalancerRequestFactory that overrides the behaviour

@william-tran william-tran Customize load balanced requests according to the chosen ServiceInstance
Applications can define their own LoadBalancerRequestTransformer beans
which can modify the HttpRequest to be executed. These beans can be
@Ordered in case of multiple transformers.

Fixes #162
c87b8b3
@ryanjbaxter ryanjbaxter merged commit d70cc62 into spring-cloud:master Jan 6, 2017

2 checks passed

ci/pivotal-cla Thank you for signing the Contributor License Agreement!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment