Skip to content

operator: add identity GC metrics #14254

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
Merged
merged 1 commit into from
Dec 17, 2020
Merged

operator: add identity GC metrics #14254

merged 1 commit into from
Dec 17, 2020

Conversation

ArthurChiao
Copy link
Contributor

@ArthurChiao ArthurChiao commented Dec 3, 2020 •
edited
Loading

operator: add new metric cilium_operator_identity_gc_entries_total

This patch adds identity GC stats to cilium-operator's metric list,
which allows users to track the identity GC activities,
e.g. deleted & alive identities during each GC run.

Preview:

$ curl <cilium-operator-ip>:6942/metrics 2>/dev/null | grep identity
# HELP cilium_operator_identity_gc_entries_total The number of alive and deleted identity entries at the end of a garbage collector run
# TYPE cilium_operator_identity_gc_total gauge
cilium_operator_identity_gc_entries_total{status="alive"} 157
cilium_operator_identity_gc_entries_total{status="deleted"} 0

Add metrics for identity garbage collection in cilium-operator

@ArthurChiao ArthurChiao requested review from a team as code owners December 3, 2020 09:07
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 3, 2020
@ArthurChiao ArthurChiao requested a review from fristonio December 3, 2020 09:07
aanm
aanm reviewed Dec 3, 2020
View reviewed changes
@ArthurChiao ArthurChiao force-pushed the add_identity_gc_metrics_github branch from c4b6ca1 to 74fac62 Compare December 3, 2020 10:35
@ArthurChiao ArthurChiao changed the title WIP: operator: add identity GC metrics operator: add identity GC metrics Dec 3, 2020
@ArthurChiao ArthurChiao force-pushed the add_identity_gc_metrics_github branch from 74fac62 to 32c49a4 Compare December 3, 2020 14:07
@ArthurChiao ArthurChiao requested a review from a team as a code owner December 3, 2020 14:07
@ArthurChiao ArthurChiao force-pushed the add_identity_gc_metrics_github branch from 32c49a4 to 24424e2 Compare December 6, 2020 10:58
@ArthurChiao ArthurChiao changed the title operator: add identity GC metrics WIP: operator: add identity GC metrics Dec 6, 2020
@ArthurChiao ArthurChiao force-pushed the add_identity_gc_metrics_github branch 2 times, most recently from fe0f122 to fffc6a1 Compare December 7, 2020 12:35
@aanm
Copy link
Member

aanm commented Dec 7, 2020

@ArthurChiao it seems that you are still working on this PR. I'll mark it as draft, feel free to mark it ready for review once it's complete.

@aanm aanm marked this pull request as draft December 7, 2020 18:00
@ArthurChiao ArthurChiao force-pushed the add_identity_gc_metrics_github branch from fffc6a1 to a0143d0 Compare December 8, 2020 01:57
@maintainer-s-little-helper
Copy link

Commit 366e2b4a6c9567e31b54f228f854e91371fc501f does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 8, 2020
@ArthurChiao ArthurChiao marked this pull request as ready for review December 8, 2020 02:55
@ArthurChiao ArthurChiao force-pushed the add_identity_gc_metrics_github branch from 366e2b4 to abe1d88 Compare December 8, 2020 03:11
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 8, 2020
@ArthurChiao
Copy link
Contributor Author

@aanm Thanks!

Ready for reviewing now. Any comments welcomed.

@aanm aanm added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Dec 8, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 8, 2020
@aanm aanm changed the title WIP: operator: add identity GC metrics operator: add identity GC metrics Dec 8, 2020
@aanm
Copy link
Member

aanm commented Dec 8, 2020

@ArthurChiao the changes LGTM, can you add a good release note in the PR description? Thank you!

@aanm
Copy link
Member

aanm commented Dec 8, 2020

test-me-please

@aanm aanm added the area/metrics Impacts statistics / metrics gathering, eg via Prometheus. label Dec 8, 2020
@maintainer-s-little-helper
Copy link

Commits 72b4a65f1609ded36e2e39a2c60a3c6ff5f44119, bf46dee8ebbfa2bf5b8c96c5d04dde0bc74d2aba, 1d02be060733c8bc3894816ca221686a52f99976, 622065ef25a550c6ceec3a4b5b799e9339fdfaa1 do not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

1 similar comment
@maintainer-s-little-helper
Copy link

Commits 72b4a65f1609ded36e2e39a2c60a3c6ff5f44119, bf46dee8ebbfa2bf5b8c96c5d04dde0bc74d2aba, 1d02be060733c8bc3894816ca221686a52f99976, 622065ef25a550c6ceec3a4b5b799e9339fdfaa1 do not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@ArthurChiao ArthurChiao force-pushed the add_identity_gc_metrics_github branch from 622065e to 31171f9 Compare December 11, 2020 06:58
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 11, 2020
@ArthurChiao ArthurChiao force-pushed the add_identity_gc_metrics_github branch 2 times, most recently from 3e35281 to d8771aa Compare December 11, 2020 07:14
@ArthurChiao
Copy link
Contributor Author

test-me-please

@ArthurChiao
Copy link
Contributor Author

ArthurChiao commented Dec 11, 2020 •
edited
Loading

@christarazi thanks for your inline comments! All comments addressed and rebased code to the latest master commit.

My local make unit-tests passed, but I noticed the github CI failed at:

{"level":"warn","ts":"2020-12-11T07:31:16.654Z","caller":"clientv3/retry_interceptor.go:62","msg":"retrying of unary invoker failed","target":"endpoint://client-0756e0c5-c5e8-47d1-8589-95b15ddfa74b/127.0.0.1:4002","attempt":0,"error":"rpc error: code = Canceled desc = context canceled"}

FAIL: services_test.go:217: ClusterMeshServicesTestSuite.TestClusterMeshServicesUpdate
services_test.go:272:
s.expectEvent(c, k8s.DeleteService, svcID, nil)
services_test.go:129:
c.Assert(event.Action, Equals, action)
... obtained k8s.CacheAction = 0 ("service-updated")
... expected k8s.CacheAction = 1 ("service-deleted")
OOPS: 5 passed, 1 FAILED
--- FAIL: Test (90.49s)
FAIL
coverage: 4.9% of statements in ./...
FAIL github.com/cilium/cilium/pkg/clustermesh 90.589s
FAIL
make: *** [Makefile:223: unit-tests] Error 1
The command "./.travis/build.sh" exited with 2.

@ArthurChiao
operator: add metrics for identity garbage collection
d0d9440 
Signed-off-by: ArthurChiao <arthurchiao@hotmail.com>
@ArthurChiao ArthurChiao force-pushed the add_identity_gc_metrics_github branch from d8771aa to d0d9440 Compare December 14, 2020 08:38
@aanm
Copy link
Member

aanm commented Dec 15, 2020

test-me-please

@aanm aanm merged commit c60023b into cilium:master Dec 17, 2020
@ArthurChiao ArthurChiao deleted the add_identity_gc_metrics_github branch December 18, 2020 07:11
@fristonio fristonio mentioned this pull request Jan 7, 2021
Closed
@rscampos rscampos mentioned this pull request May 3, 2021
Merged
@christarazi
Copy link
Member

Adding for backport to v1.9 & v1.10 as this change is valuable to have on stable releases.

@christarazi christarazi mentioned this pull request Sep 14, 2021
Merged
This was referenced Sep 27, 2021
Closed
Closed
@jibi jibi mentioned this pull request Sep 29, 2021
Merged
@glibsm
Copy link
Member

glibsm commented Oct 4, 2021 •
edited
Loading

In the last round of backports, this PR was omitted due to non-trivial merge. Temporarily dropping the 1.10 backport so it doesn't resurface until we figure out a course of action

image

edit: it's already in 1.10 c60023b

@joestringer joestringer mentioned this pull request Nov 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aanm aanm aanm approved these changes

@christarazi christarazi christarazi approved these changes

@fristonio fristonio fristonio approved these changes

Assignees

@christarazi christarazi

@fristonio fristonio

Labels
area/metrics Impacts statistics / metrics gathering, eg via Prometheus. area/operator Impacts the cilium-operator component release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ArthurChiao @aanm @joestringer @fristonio @christarazi @glibsm