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.

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

clustermesh-apiserver: add missing metrics and documentation #26070

Merged

Conversation

giorio94
Copy link
Member

@giorio94 giorio94 commented Jun 9, 2023

This PR extends the documentation with information about clustermesh-apiserver metrics that have been overlooked in previous commits. It also corrects the selection of the rate limiter metrics exposed by the clustermesh-apiserver and adds a couple of missing metrics.

@giorio94 giorio94 added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/clustermesh Relates to multi-cluster routing functionality in Cilium. release-note/misc This PR makes changes that have no direct user impact. labels Jun 9, 2023
@giorio94 giorio94 requested review from a team as code owners June 9, 2023 09:57
@giorio94
Copy link
Member Author

giorio94 commented Jun 9, 2023

Integration tests hit unrelated flake: #24904. Rerunning manually.

@giorio94
Copy link
Member Author

giorio94 commented Jun 9, 2023

/test

Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Looks good to me

@giorio94
Copy link
Member Author

The last commit enables the goroutine sched latency metric in the clustermesh-apiserver, for consistency with the ones exposed by the agents: #26083 (comment)

@giorio94
Copy link
Member Author

/test

@giorio94
Copy link
Member Author

giorio94 commented Jun 12, 2023

/ci-awscni

Failed while checking out the code due to GH rate limiting: #26111

@giorio94
Copy link
Member Author

giorio94 commented Jun 12, 2023

/ci-ginkgo

Rerun to pick the fix for the matrix generation

@giorio94 giorio94 changed the title clustermesh-apiserver: add missing metrics documentation clustermesh-apiserver: add missing metrics and documentation Jun 13, 2023
@giorio94
Copy link
Member Author

The last commit exposes the cilium_clustermesh_apiserver_version, as suggested in #26083 (comment).

@giorio94
Copy link
Member Author

/test

@giorio94
Copy link
Member Author

Rebased onto main to pick the fixes for conformance-ginkgo

@giorio94
Copy link
Member Author

/test

The "processed requests" metric had been overlooked when selecting the
list of rate limiter related metrics to expose by the clustermesh-apiserver.
Hence, let's enable it now. Additionally, let's drop the one about the
adjustment factor since auto adjustment is disabled.

Fixes: 7908f20 ("etcd: add max inflight requests to rate limiter")
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
This commit extends the documentation with information about
clustermesh-apiserver metrics that have been overlooked in previous
commits. In particular, it mentions the new metrics in the upgrade
guide, and lists the metrics concerning rate limiting in the reference
table.

Fixes: 7e65ca1 ("docs: add clustermesh-apiserver metrics")
Fixes: 7908f20 ("etcd: add max inflight requests to rate limiter")
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Let's enable these optional metrics for consistency with the ones
enabled in the cilium agent. They allow to troubleshoot strange
behaviors in severely CPU constrained environments.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Let's expose the cilium_clustermesh_apiserver_version metric to convey
information about the version of the clustermesh-apiserver.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94 giorio94 force-pushed the mio/clustermesh-apiserver-missing-metric branch from 1104157 to 6395a4c Compare June 15, 2023 07:11
@giorio94
Copy link
Member Author

Rebased onto main to fix conflicts

@giorio94
Copy link
Member Author

/test

@giorio94
Copy link
Member Author

giorio94 commented Jun 15, 2023

/ci-aks

Hit #26075

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 15, 2023
@joestringer joestringer merged commit 9b18e1c into cilium:main Jun 15, 2023
59 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clustermesh Relates to multi-cluster routing functionality in Cilium. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants