spring-cloud / spring-cloud-openfeign

Support for using OpenFeign in Spring Cloud apps
Apache License 2.0
1.2k stars 780 forks source link

Support lb:// url in FeingClient annotation #489

Open antechrestos opened 3 years ago

antechrestos commented 3 years ago

Current behaviour

In a project where is used spring cloud gateway, feign client (with target put in configuration), we ended up having feign clients declared as follows

@FeignClient(name="${service.name:any-name}", url="${service.direct-url:}")

With configuration as follows

service:
   name: service-id
   # url: http://direct-url (only used when service discovery is not available)

This way we are able, by configuration, to either run in service discovery/load balancing mode and direct mode.

Proposal

I kind of like the way spring cloud gateway handle that by allowing lb:// protocol. Allowing this protocol in url attribute would allow the following implementation

@FeignClient(name="technical-name", url="${service.uri}")

with following configuration

service:
   uri: "lb://service-id" # or "http//direct-url"

this way it will let common behaviour, allowing to have a common behaviour in url specification.

I spotted the code responsible for feign client building and think that it would be easily doable by

I am eager to implement it if you allow me to do so. I am also pleased to discuss about it and about any alternative I missed.

Thank you

antechrestos commented 3 years ago

@OlgaMaciaszek what the point on this issue? Can I make a PR?

antechrestos commented 3 years ago

PR submitted

OlgaMaciaszek commented 3 years ago

@antechrestos Actually, I am not sure this is a change we want to make. We are going to see if there's interest in the community and, if yes, then, we will discuss it in the team.

antechrestos commented 3 years ago

@OlgaMaciaszek Agreed. Just for clarification, is there any documentation describing "the waiting for votes" adhesion and what is the comunity? Is there a number of :+1: expected on the issue? Or is it based on an adhesion of an inner community (spring developers for instance)? Thank you

OlgaMaciaszek commented 3 years ago

We check how many users request a feature. If we see considerable interest, then we discuss the possible issues and implications within the Spring Cloud team, taking into account the discussion in the issue and other considerations related to maintenance, planned future project development and possible issues.

antechrestos commented 3 years ago

@OlgaMaciaszek ok that's more clear

antechrestos commented 3 years ago

To my mind, I think the real question is if there is value to apply the concept of lb:// url used in spring cloud gateway in other spring projects or if it needs to be restricted to it.

As an alternative, it may be useful to let the user decide whether (s)he wants to skip load balancing/discovery behaviour. Because currently, the url use hides the fact that we force the use of delegated client... What do you think of it?

antechrestos commented 3 years ago

@OlgaMaciaszek do you think I should open a new issue proposing to handle load-balancing/discovery in a dedicated properties rather than hidden behind url attribute?