Skip to content
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.

Sign up for GitHub

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

Jump to bottom

distinguish two resource detector metrics #3029

Closed
wants to merge 1 commit into from
Closed

distinguish two resource detector metrics #3029

wants to merge 1 commit into from

Conversation

snowplayfire
Copy link
Contributor

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

@karmada-bot karmada-bot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jan 9, 2023
@karmada-bot karmada-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 9, 2023
@snowplayfire snowplayfire force-pushed the update-resource-detector branch from f868a79 to 8065aeb Compare January 9, 2023 12:08
@snowplayfire snowplayfire force-pushed the update-resource-detector branch from 8065aeb to 19855ba Compare January 10, 2023 05:53
XiShanYongYe-Chang
XiShanYongYe-Chang reviewed Jan 10, 2023
View reviewed changes
Copy link
Member

@XiShanYongYe-Chang XiShanYongYe-Chang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to set its own concurrent-resource-template-syncs parameter for DependenciesDistributor?

@XiShanYongYe-Chang
Copy link
Member

Metric for workqueue:

before：
# TYPE workqueue_depth gauge
workqueue_depth{name="binding-eviction"} 0
workqueue_depth{name="cluster"} 0
workqueue_depth{name="cluster-binding-eviction"} 0
workqueue_depth{name="clusterPropagationPolicy reconciler"} 0
workqueue_depth{name="clusterresourcebinding"} 0
workqueue_depth{name="federatedresourcequota"} 0
workqueue_depth{name="namespace"} 0
workqueue_depth{name="propagationPolicy reconciler"} 0
workqueue_depth{name="resource detector"} 0
workqueue_depth{name="resourceBinding reconciler"} 0
workqueue_depth{name="resourcebinding"} 0
workqueue_depth{name="service-export"} 0
workqueue_depth{name="serviceimport"} 0
workqueue_depth{name="work"} 0
workqueue_depth{name="work-status"} 0
after：
# TYPE workqueue_depth gauge
workqueue_depth{name="binding-eviction"} 0
workqueue_depth{name="cluster"} 0
workqueue_depth{name="cluster-binding-eviction"} 0
workqueue_depth{name="clusterPropagationPolicy reconciler"} 0
workqueue_depth{name="clusterresourcebinding"} 0
workqueue_depth{name="dependencies distributor resource detector"} 0
workqueue_depth{name="dependencies distributor resourceBinding reconciler"} 0
workqueue_depth{name="federatedresourcequota"} 0
workqueue_depth{name="namespace"} 0
workqueue_depth{name="propagationPolicy reconciler"} 0
workqueue_depth{name="resource detector"} 0
workqueue_depth{name="resourcebinding"} 0
workqueue_depth{name="service-export"} 0
workqueue_depth{name="serviceimport"} 0
workqueue_depth{name="work"} 0
workqueue_depth{name="work-status"} 0

@RainbowMango
Copy link
Member 

workqueue_depth{name="resource detector"} 0

vs

workqueue_depth{name="dependencies distributor resource detector"} 0
workqueue_depth{name="dependencies distributor resourceBinding reconciler"} 0

/assign

@snowplayfire snowplayfire force-pushed the update-resource-detector branch from 19855ba to 35e4d06 Compare April 25, 2023 12:24
@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from rainbowmango after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@karmada-bot karmada-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 25, 2023
@snowplayfire snowplayfire force-pushed the update-resource-detector branch from 35e4d06 to 43af7e1 Compare April 25, 2023 12:27
@snowplayfire snowplayfire changed the title distinguish two resource detector distinguish two resource detector metrics May 10, 2023
@snowplayfire
Copy link
Contributor Author

@RainbowMango @XiShanYongYe-Chang updated, please have a look.

RainbowMango
RainbowMango reviewed Jan 7, 2025
View reviewed changes
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two metrics have been renamed in release 1.7 and 1.2 respectively, I guess we don't need it anymore.
Sorry for letting this sit.
/close

pkg/dependenciesdistributor/dependencies_distributor.go
@@ -84,7 +84,7 @@ func (d *DependenciesDistributor) Start(ctx context.Context) error {
d.stopCh = ctx.Done()

bindingWorkerOptions := util.Options{
Name: "resourceBinding reconciler",
Name: "dependencies distributor resourceBinding reconciler",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name has been changed to dependencies-distributor by #5799, and released in v1.12.0.
https://github.com/karmada-io/karmada/blame/82fc37ea04aca442f378a013e8846104595037ea/pkg/dependenciesdistributor/dependencies_distributor.go#L66

pkg/dependenciesdistributor/dependencies_distributor.go
@@ -103,7 +103,7 @@ func (d *DependenciesDistributor) Start(ctx context.Context) error {
d.InformerManager.ForResource(resourceBindingGVR, bindingHandler)
d.resourceBindingLister = d.InformerManager.Lister(resourceBindingGVR)
resourceWorkerOptions := util.Options{
Name: "resource detector",
Name: "dependencies distributor resource detector",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name has been changed to dependencies resource detector by #3811, and released in v1.7.0.

@karmada-bot karmada-bot closed this Jan 7, 2025
@karmada-bot
Copy link
Collaborator

@RainbowMango: Closed this PR.

In response to this:

The two metrics have been renamed in release 1.7 and 1.2 respectively, I guess we don't need it anymore.
Sorry for letting this sit.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RainbowMango RainbowMango RainbowMango left review comments

@jwcesign jwcesign jwcesign left review comments

@XiShanYongYe-Chang XiShanYongYe-Chang XiShanYongYe-Chang left review comments

@lonelyCZ lonelyCZ Awaiting requested review from lonelyCZ

Assignees

@RainbowMango RainbowMango

Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@snowplayfire @XiShanYongYe-Chang @RainbowMango @karmada-bot @jwcesign