load balancing - get(Request request) should be called on delegates

[thewmo]

I have a load balancing configuration that combines zone awareness with a method that need access to the request context/headers (sticky session):

@Bean
public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier(ConfigurableApplicationContext context) {
    return ServiceInstanceListSupplier.builder()
            // get the service instances from the discovery client (Eureka in our case)
            .withDiscoveryClient()
            // if the request specifies an instance, filter for it
            .withRequestBasedStickySession()
            // of the survivors, pick one that's in our availability zone, if any
            .withZonePreference()
            .build(context);
}

The comments make clear the desired logic - I want same-zone routing, unless the sc-lb-instance-id cookie specifies the ID of an instance in a different zone. Consequently withZonePreference() is specified after request based sticky session. Otherwise it will only ever be possible to "stick" to an instance in the same zone.

However, in debugging this, it appears the ZonePreferenceServiceInstanceListSupplier is only implementing the get() method that does not take a request, and consequently the request is not propagated to the downstream delegate (in this case, the sticky session mechanism). It seems the ZonePreferenceServiceInstanceListSupplier should also implement Flux<List<ServiceInstance>> get(Request request) and propagate the request to the delegate.

spring-cloud-commons/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplier.java

Line 53 in 51ee785

public Flux<List<ServiceInstance>> get() {

While the documentation does not show any examples of combining ServiceInstanceListSuppliers like this, nothing I found in the docs suggest the list suppliers were not intended to be composable in this way.

[thewmo]

Potential/proposed fix in 16ecfa1

[Linzyoo]

I need this too. When and which branch will it be merged into?

[OlgaMaciaszek]

Thanks for reporting the issue @thewmo. Will discuss this with the team and get back to you.

[OlgaMaciaszek]

We agree that this makes sense. I will be marking at as an enhancement. We should update all the suppliers accordingly. The plan would be to make it default on main and opt-in in other branches. Will get it done in July.

[OlgaMaciaszek]

Done, thanks for yuor input @thewmo. @Linzyoo, it's on all the branches, but it's by default inmain and in 3.1.x and 4.0.x, you'll need to swap a property. Have added info to docs.