EurekaDiscoveryClient.getIntsances(String)
doesn't use the serviceId
or application name but the VIP Address to lookup instances:
```!java
public List
here's the doc for `DiscoverClient.getInstances(String serviceId)`
```!java
/**
* Get all ServiceInstances associated with a particular serviceId
* @param serviceId the serviceId to query
* @return a List of ServiceInstance
*/
List<ServiceInstance> getInstances(String serviceId);
So far I haven't noticed this as EurekaInstanceConfigBean
uses spring.application.name
as virtualHostName
which is used by InstanceInfoFactory
as VIPAddress
here's the doc for InstanceInfo
:
```!java /* * Gets the Virtual Internet Protocol address for this instance. Defaults to * hostname if not specified. * * @return - The Virtual Internet Protocol address / @JsonProperty("vipAddress") public String getVIPAddress() { return vipAddress; }
/**
* Sets the Virtual Internet Protocol address for this instance. The
* address should follow the format <code><hostname:port></code> This
* address needs to be resolved into a real address for communicating
* with this instance.
*
* @param vipAddress - The Virtual Internet Protocol address of this instance.
* @return the instance builder.
*/
public Builder setVIPAddress(String vipAddress) {
result.vipAddressUnresolved = StringCache.intern(vipAddress);
result.vipAddress = StringCache.intern(resolveDeploymentContextBasedVipAddresses(vipAddress));
return this;
}
so it looks like this implementation treats application name and VIP address as synonyms - and don't rely use VIP addresses anywhere - so that's why this bug went undetected.
So the proper implementation would be this:
```!java
@Override
public List<ServiceInstance> getInstances(String serviceId) {
Application application = this.eurekaClient.getApplication(serviceId);
List<ServiceInstance> instances = new ArrayList<>();
if (application != null) {
for (InstanceInfo info : application.getInstances()) {
instances.add(new EurekaServiceInstance(info));
}
}
return instances;
}
Comment From: spencergibb
I think, at most, there should be some clarifying documentation.
Comment From: spencergibb
Thinking of changing getInstances()
to query using this.eurekaClient.getInstancesByVipAddressAndAppName(null, serviceId, false);
. Thoughts, @sfussenegger? I know you've moved on from eureka.
Comment From: spencergibb
I think I used vip address for parity with the eureka ribbon integration.
Comment From: spencergibb
I think the best thing would be to document uses, see https://github.com/spring-cloud/spring-cloud-netflix/pull/1789#issuecomment-287807084
Comment From: spencergibb
More reasons for https://github.com/spring-cloud/spring-cloud-commons/pull/227
Comment From: sfussenegger
@spencergibb it's been a while but I agree that really writing down the meaning of the terms application name, virtual host name, host name, instance id, and service id would go a long way towards solving the issue.
What's the difference though between eurekaClient.getInstancesByVipAddressAndAppName(null, serviceId, false)
and eurekaClient.getApplication(serviceId)
other than the secure
flag? Both use serviceId
to query for an appName
so it looks like the approach I recommended at the end of my original comment.
I'm really somewhat removed from the whole Eureka and Ribbon thing. Though I do trust my 5-month-younger self enough to say that this seems to be a good idea ;)
Comment From: codefromthecrypt
ps I lost a lot of time here, because there's no parity for reasons mentioned above. If you know the vip address, likely you have little to use eureka for. It seems odd that the service list returns applications (which makes sense), but you have to guess a vip address to well. discover metadata around that vip address. basically you already have to know a lot of info to get started.