cloud_controller_ng
cloud_controller_ng copied to clipboard
Add instance_id field in Process Stats response
A short explanation of the proposed change:
- Display the Diego instance identifier as part of the Process Stats response. This id is unique for each actual LRP instance and can be used as a substitute for the integer index id.
- Eirini does not support this in their API at this time, so the OPI variant of the instances stats reporter returns an empty string for this field. In the future, if Eirini is enhanced to provide this we can update that client.
- Issue: https://github.com/cloudfoundry/cloud_controller_ng/issues/2819
An explanation of the use cases your change solves:
The issue (https://github.com/cloudfoundry/cloud_controller_ng/issues/2819) describes this use case in more detail. The summary though is this can help developers debug their apps (this id is logged by app containers) and provide a pathway for clients to provide other container ids than just the integer index.
-
[x] I have reviewed the contributing guide
-
[x] I have viewed, signed, and submitted the Contributor License Agreement
-
[x] I have made this pull request to the
main
branch -
[x] I have run all the unit tests using
bundle exec rake
-
[ ] I have run CF Acceptance Tests
@Gerg some additional context for this change:
On Korifi we're wanting to support workloads other than StatefulSets
(e.g. Deployments
, knative Services
, etc.) so we're working toward reducing the reliance of system components on the numerical indexing of app instances. I'll be putting out a proposal / some sort of design doc to start discussing this in more detail later this week, but I see this change as a first step toward that.
I'm putting this into draft state. I was looking at the AppRecipeBuilder and noticed that instance_id
is that metric tag we're adding and it corresponds to instance index. We are calling this the process_instance_id
on metrics/logs so maybe that name should be used. Worth discussing I think.
https://github.com/cloudfoundry/cloud_controller_ng/blob/7698bc407195cb30de7455ab877d9526b6d4bafe/lib/cloud_controller/diego/app_recipe_builder.rb#L108
It's a little odd that we call index instance_id
in the metrics tags. Looks like it came from this story.
index
/guid
is the most intuitive to me, but I could see other options. Process.instance.process_instance_id
sounds a bit redundant.
@Gerg
Yeah, I started calling it instance_guid
here in the proposal around this stuff here:
https://docs.google.com/document/d/118yYN4WGj1F2si8gk94xFZsYdewtOKoqO2Dv-1I-PgE/edit
What about just guid
?