hudl / fargo

Golang client for Netflix Eureka
MIT License
133 stars 53 forks source link

Missing InstanceId into fargo.Instance struct #39

Open jerome-laforge opened 8 years ago

jerome-laforge commented 8 years ago

Hello, Is it possible to add into fargo.Instance this field:

InstanceId       string   `xml:"instanceId" json:"instanceId"`

because if I use UniqueID func(i Instance) string xml:"-" json:"-"

I have this error into eureka console:

Not Found (Renew): xxxx - 10.193.204.167:xxxx:9001:2016-07-08 18:30:19.779

Thx in adv

seh commented 8 years ago

I see that this behavior changed in Eureka since my company took our internal fork from the upstream source.

In our fork, Eureka extracts the instance ID from the "data center info" structure if its corresponding Java representation implements com.netflix.appinfo.UniqueIdentifier, which for the data center named "Amazon" means the "instance-id" field. Failing that, it uses the "hostName" field value.

In our fork one can't supply an instance ID like you're proposing above without changing the Eureka server code—defining a new data center info type that implements UniqueIdentifierto use at registration time. However, now I see that the current Eureka adds an "instanceId" field to the com.netflix.appinfo.InstanceInfo class. That makes registration much easier, assuming that Eureka will accept such a proposed ID in the registration payload. Have you found that it works?

jerome-laforge commented 8 years ago

With Eureka 1.3.1, I try to use this InstanceId without success.

For my point of view, The only correct way (that I found) is to add below line into Instance struct.

InstanceID string                  `xml:"instanceId,omitempty" json:"instanceId,omitempty"` 

Is it possible to add this? I can submit a PR.

seh commented 8 years ago

That's not where the Eureka server is going to look for an instance ID, though. The server will consult AmazonMetadataType.InstanceID if your Instance.DataCenterInfo.Name field's value is "Amazon". Otherwise, without customizing the server—as my company has done internally—it won't.

My reading of the current Eureka code shows the client acquiring its instance ID using the field you mention, but not the server honoring it. I'd need to hear more about your usage scenario to figure out why you're finding success using it.

jerome-laforge commented 8 years ago

I have to plug my Go application with Spring Cloud Netflix Eureka server that already exists. Maybe Spring Cloud crew has customized this Eureka server.

With Instance.DataCenterInfo.Name set "Amazon" and AmazonMetadataType.InstanceID that works. Thx for your response.