-
Notifications
You must be signed in to change notification settings - Fork 380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
observe metrics in recommendation controller #693
Conversation
🎉 Successfully Build Images. Docker RegistryOverview: https://hub.docker.com/u/gocrane
Quick Deploy - Helm helm repo add crane https://finops-helm.pkg.coding.net/gocrane/gocrane
helm install crane -n crane-system --create-namespace \
--set craned.image.repository=gocrane/craned \
--set craned.image.tag=pr-693-30b4d6b \
--set metricAdapter.image.repository=gocrane/metric-adapter \
--set metricAdapter.image.tag=pr-693-30b4d6b \
--set craneAgent.image.repository=gocrane/crane-agent \
--set craneAgent.image.tag=pr-693-30b4d6b \
--set cranedDashboard.image.repository=gocrane/dashboard \
--set cranedDashboard.image.tag=pr-693-30b4d6b crane/crane Coding RegistryOverview: https://finops.coding.net/public-artifacts/gocrane/crane/packages
Quick Deploy - Helm helm repo add crane https://finops-helm.pkg.coding.net/gocrane/gocrane
helm install crane -n crane-system --create-namespace \
--set craned.image.repository=finops-docker.pkg.coding.net/gocrane/crane/craned \
--set craned.image.tag=pr-693-30b4d6b \
--set metricAdapter.image.repository=finops-docker.pkg.coding.net/gocrane/crane/metric-adapter \
--set metricAdapter.image.tag=pr-693-30b4d6b \
--set craneAgent.image.repository=finops-docker.pkg.coding.net/gocrane/crane/crane-agent \
--set craneAgent.image.tag=pr-693-30b4d6b \
--set cranedDashboard.image.repository=finops-docker.pkg.coding.net/gocrane/crane/dashboard \
--set cranedDashboard.image.tag=pr-693-30b4d6b crane/crane Ghcr RegistryOverview: https://github.com/orgs/gocrane/packages?repo_name=crane
Quick Deploy - Helm helm repo add crane https://finops-helm.pkg.coding.net/gocrane/gocrane
helm install crane -n crane-system --create-namespace \
--set craned.image.repository=ghcr.io/gocrane/crane/craned \
--set craned.image.tag=pr-693-30b4d6b \
--set metricAdapter.image.repository=ghcr.io/gocrane/crane/metric-adapter \
--set metricAdapter.image.tag=pr-693-30b4d6b \
--set craneAgent.image.repository=ghcr.io/gocrane/crane/crane-agent \
--set craneAgent.image.tag=pr-693-30b4d6b \
--set cranedDashboard.image.repository=ghcr.io/gocrane/crane/dashboard \
--set cranedDashboard.image.tag=pr-693-30b4d6b crane/crane |
pkg/metrics/analysis.go
Outdated
@@ -13,7 +13,7 @@ var ( | |||
Name: "resource_recommendation", | |||
Help: "The containers' CPU/Memory recommended value", | |||
}, | |||
[]string{"apiversion", "owner_kind", "namespace", "owner_name", "container", "resource", "owner_replicas"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
owner_replicas
Why should we discard this label?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I thought owner_replicas is not a value that resource recommend needs to care about, there is also no related value in recommendation CRD.
Another reason is that I refactor thses code with a common method (the input parameter of the observe function was changed from RecommendationContext to Recommendation). If this field needs to be displayed, it needs to be processed separately.
Please let me know if there is a need to keep this field or any other information I am missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replicas * resource = total workload resource now
so i think it is useful here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I will adjust it later
093db6c
to
b1311ee
Compare
@qmhu Already updated, PTAL |
Please fix the lint |
b1311ee
to
30b4d6b
Compare
Sorry, miss that. It is fixed now. |
What type of PR is this?
optimize
What this PR does / why we need it:
Currutly recommendation metrics will lost after restart craned because the metrics is created in the progress of calculation of recommend.
If we observe recommendation status in recommedation controller, that can avoid this issue.
Which issue(s) this PR fixes:
Fixes #672
Special notes for your reviewer: