atlassian / escalator

Escalator is a batch or job optimized horizontal autoscaler for Kubernetes
Apache License 2.0
663 stars 59 forks source link

`cloud_provider_*` metrics include configured node name as label #125

Closed wendorf closed 5 years ago

wendorf commented 6 years ago

The cloud_provider_* metrics include an id label that represents the name of the ASG in the cloud provider, and not the name of the node group configured in nodegroup.yml. Other metrics include the node_group label that does represent the configured name. This difference makes it difficult to correlate metrics since there's no shared key:value pair.

I started making a PR, but it got pretty ugly adding a Name() function to cloudprovider.NodeGroup and felt wrong to have the provider abstraction include a field just for tying back to the non-provider nodegroup config. I decided to abort the PR and file this issue instead to start the discussion.

Does this seem like a reasonable change to the metrics? What's the best way to implement it? Should I just extend cloudprovider.NodeGroup and take the aesthetic hit?

awprice commented 6 years ago

Great suggestion! I'm completely happy to have the Name() field added to the NodeGroup interface as it's something that should of originally been added to the metrics. I suggest updating CloudProvider. RegisterNodeGroups to take a slice of some sort of struct containing both the ID + name instead of a slice of strings so we can nicely pass in the node group name.

Let me know what you think.