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

operator: fix errors/warnings metric. #31214

Merged
merged 1 commit into from Mar 15, 2024
Merged

operator: fix errors/warnings metric. #31214

merged 1 commit into from Mar 15, 2024

Conversation

tommyp1ckles
Copy link
Contributor

@tommyp1ckles tommyp1ckles commented Mar 7, 2024 •
edited

operator: fix errors/warnings metric.

This was broken during transition of pkg/metrics to integrate with Hive where relevant operator metrics where never initialized.
This adds a init func specific for operator and cleans up the "flush" logic used as a work around for errors/warnings emitted prior to agent starting (in the case of the operator).

@tommyp1ckles tommyp1ckles requested review from a team as code owners March 7, 2024 00:04
@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 Mar 7, 2024
@tommyp1ckles tommyp1ckles added release-note/bug This PR fixes an issue in a previous release of Cilium. area/metrics Impacts statistics / metrics gathering, eg via Prometheus. labels Mar 7, 2024
@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 Mar 7, 2024
@tommyp1ckles tommyp1ckles marked this pull request as draft March 7, 2024 00:16
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-operator-err-warn-metrics branch 3 times, most recently from c7d5b03 to 745b50f Compare March 12, 2024 05:45
@tommyp1ckles tommyp1ckles marked this pull request as ready for review March 12, 2024 05:45
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-operator-err-warn-metrics branch from 745b50f to b8061b4 Compare March 12, 2024 05:46
@tommyp1ckles
Copy link
Contributor Author

/test

@tommyp1ckles tommyp1ckles added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Mar 12, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.8 Mar 12, 2024
pippolo84
pippolo84 requested changes Mar 12, 2024
View reviewed changes
Copy link
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
I've left a suggestion inline to do this in a (hopefully) simpler way and avoid the panic.

operator/cmd/root.go Outdated Show resolved Hide resolved
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-operator-err-warn-metrics branch 3 times, most recently from 58a7df0 to 941c700 Compare March 12, 2024 14:12
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-operator-err-warn-metrics branch from 941c700 to 5c4b5ec Compare March 13, 2024 03:50
@tommyp1ckles
Copy link
Contributor Author

/test

pippolo84
pippolo84 approved these changes Mar 13, 2024
View reviewed changes
Copy link
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@tommyp1ckles
Copy link
Contributor Author

/test

@thorn3r thorn3r added this to Needs backport from main in 1.14.9 Mar 13, 2024
@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 Mar 14, 2024
@julianwiedmann
Copy link
Member

julianwiedmann commented Mar 15, 2024 •
edited

Just to double-check - we want a backport to 1.14, but 1.15 is not affected?

@julianwiedmann julianwiedmann added this pull request to the merge queue Mar 15, 2024
Merged via the queue into cilium:main with commit f61651f Mar 15, 2024
62 checks passed
@tommyp1ckles tommyp1ckles added the needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch label Mar 15, 2024
@tommyp1ckles
Copy link
Contributor Author

@julianwiedmann Mistake on my part, both are affected

@gandro gandro mentioned this pull request Mar 19, 2024
Merged
21 tasks
@gandro gandro added backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Mar 19, 2024
@gandro
Copy link
Member

gandro commented Mar 19, 2024

operator/metrics/metrics.go looks very different on 1.14. I lack context to backport this myself. Marking as "backport/author".

@gandro gandro added the backport/author The backport will be carried out by the author of the PR. label Mar 19, 2024
@gandro gandro mentioned this pull request Mar 19, 2024
Merged
10 tasks
@github-actions github-actions bot added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. labels Mar 21, 2024
@jrajahalme jrajahalme mentioned this pull request Mar 26, 2024
Merged
@jrajahalme jrajahalme added this to Needs backport from main in 1.14.10 Mar 26, 2024
@jrajahalme jrajahalme removed this from Needs backport from main in 1.14.9 Mar 26, 2024
This was referenced Apr 11, 2024
Merged
Merged
@asauber asauber added this to Needs backport from main in 1.14.11 Apr 11, 2024
@asauber asauber removed this from Needs backport from main in 1.14.10 Apr 11, 2024
@github-actions github-actions bot added the backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. label Apr 24, 2024
This was referenced Apr 29, 2024
Closed
Closed
Closed
Merged
@julianwiedmann julianwiedmann removed the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label May 3, 2024
@nebril nebril moved this from Needs backport from main to Backport done to v1.14 in 1.14.11 May 10, 2024
@nebril nebril mentioned this pull request May 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chancez chancez chancez approved these changes

@pippolo84 pippolo84 pippolo84 approved these changes

Assignees
No one assigned
Labels
area/metrics Impacts statistics / metrics gathering, eg via Prometheus. backport/author The backport will be carried out by the author of the PR. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.14.11
Backport done to v1.14
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tommyp1ckles @julianwiedmann @gandro @chancez @pippolo84