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

hive: Fix start hook log output #30712

Merged
merged 1 commit into from Feb 12, 2024
Merged

hive: Fix start hook log output #30712

merged 1 commit into from Feb 12, 2024

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented Feb 12, 2024 •
edited

Commit 07595da added module ID to the start/stop hook output by carrying the module ID in the hook. Unfortunately I forgot to fix the getHooKFuncName to actually handle this wrapped handle which caused the log output to always show "augmentedHook.Start" as the function name. Fix this by looking through the wrapper.

Fixes: 07595da ("hive: Add module ID to hive start/stop hook output")

Before:

pkg/hive/example % go run .    
...                                                                                                                                                             
level=info msg="Start hook executed" duration="28.273µs" function=cell.augmentedHook.Start subsys=hive

After:

pkg/hive/example % go run . 
...
level=info msg="Start hook executed" duration="18.316µs" function="*main.exampleEventSource.Start" subsys=hive

@joamaki
hive: Fix start hook log output
47d3ee7 
Commit 07595da added module ID to the start/stop hook output by carrying
the module ID in the hook. Unfortunately I forgot to fix the getHooKFuncName to
actually handle this wrapped handle which caused the log output to always show
"augmentedHook.Start" as the function name. Fix this by looking through the
wrapper.

Fixes: 07595da ("hive: Add module ID to hive start/stop hook output")
Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki joamaki added release-blocker/1.14 This issue will prevent the release of the next version of Cilium. needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch release-blocker/1.15 This issue will prevent the release of the next version of Cilium. needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Feb 12, 2024
@joamaki joamaki requested a review from a team as a code owner February 12, 2024 14:29
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Feb 12, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.15.1 Feb 12, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.7 Feb 12, 2024
@joamaki joamaki added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Feb 12, 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 Feb 12, 2024
@joamaki
Copy link
Contributor Author

joamaki commented Feb 12, 2024

/test

@joamaki joamaki added this pull request to the merge queue Feb 12, 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 Feb 12, 2024
Merged via the queue into cilium:main with commit f8c68e4 Feb 12, 2024
62 of 63 checks passed
@joamaki joamaki deleted the pr/joamaki/fix-start-hook-log branch February 12, 2024 16:25
@sayboras sayboras mentioned this pull request Feb 12, 2024
Merged
2 tasks
@sayboras sayboras added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Feb 12, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.7 Feb 12, 2024
@sayboras sayboras mentioned this pull request Feb 13, 2024
Merged
2 tasks
@sayboras sayboras 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 Feb 13, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.15 in 1.15.1 Feb 13, 2024
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Feb 13, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.7 Feb 13, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Backport done to v1.14 in 1.14.7 Feb 13, 2024
@michi-covalent michi-covalent mentioned this pull request Feb 13, 2024
Merged
@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 Feb 13, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Backport pending to v1.15 in 1.15.1 Feb 13, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport done to v1.14 in 1.14.7 Feb 13, 2024
@michi-covalent michi-covalent mentioned this pull request Feb 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dylandreimerink dylandreimerink dylandreimerink approved these changes

@bimmlerd bimmlerd bimmlerd approved these changes

Assignees
No one assigned
Labels
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-blocker/1.14 This issue will prevent the release of the next version of Cilium. release-blocker/1.15 This issue will prevent the release of the next version of Cilium. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.14.7
Backport done to v1.14
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@joamaki @dylandreimerink @bimmlerd @sayboras