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

Replace klog with zerolog in Lighthouse Agent #728

Merged
merged 1 commit into from
Oct 20, 2022
Merged

Replace klog with zerolog in Lighthouse Agent #728

merged 1 commit into from
Oct 20, 2022

Conversation

tpantelis
Copy link
Contributor

@tpantelis tpantelis commented Apr 21, 2022 •
edited
Loading

Using the kzerolog package from admiral.

Note: some internal k8s packages uses klog directly, so few logs may still be printed in the klog format.

Note: this was originally submitted with #692 however the original author left the project.

Signed-off-by: David Ohana [email protected]

@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr728/tpantelis/zerolog
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@tpantelis tpantelis mentioned this pull request Apr 21, 2022
Closed
@tpantelis
Copy link
Contributor Author

tpantelis commented Apr 21, 2022 •
edited
Loading

This is a little more complicated. Now that K8s was bumped to v1.21, kzerolog in admiral needs to updated to use logr 1.2.0.

@vthapar
Copy link
Contributor

vthapar commented Apr 25, 2022

@tpantelis Do we need a fresh PR for this after 1.21 changes?

@tpantelis
Copy link
Contributor Author

@tpantelis Do we need a fresh PR for this after 1.21 changes?

This is a new PR. We need submariner-io/admiral#342.

@github-actions github-actions bot removed the dependent label May 2, 2022
@stale
Copy link

stale bot commented May 31, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label May 31, 2022
@dfarrell07 dfarrell07 removed the wontfix This will not be worked on label May 31, 2022
@stale
Copy link

stale bot commented Jun 18, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jun 18, 2022
@tpantelis tpantelis removed the wontfix This will not be worked on label Jun 19, 2022
@dfarrell07
Copy link
Member

I think this was blocked on a K8s version bump? We now have 1.24, can we revisit this?

@skitt
Copy link
Member

skitt commented Jun 28, 2022

I think this was blocked on a K8s version bump? We now have 1.24, can we revisit this?

We don’t have 1.24 yet, see submariner-io/shipyard#825

@dfarrell07
Copy link
Member

I think this was blocked on a K8s version bump? We now have 1.24, can we revisit this?

We don’t have 1.24 yet, see submariner-io/shipyard#825

Ah thanks, getting ahead of myself.

@stale
Copy link

stale bot commented Jul 13, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jul 13, 2022
@dfarrell07 dfarrell07 removed the wontfix This will not be worked on label Jul 19, 2022
@stale
Copy link

stale bot commented Aug 12, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Aug 12, 2022
@dfarrell07 dfarrell07 removed the wontfix This will not be worked on label Aug 23, 2022
@tpantelis tpantelis changed the title Replace klog with zerolog for human-friendly logging in Lighthouse Agent Replace klog with zerolog in Lighthouse Agent Sep 1, 2022
@tpantelis tpantelis self-assigned this Sep 1, 2022
@tpantelis tpantelis requested a review from dfarrell07 September 1, 2022 17:39
@stale
Copy link

stale bot commented Sep 28, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Sep 28, 2022
@nyechiel nyechiel removed the wontfix This will not be worked on label Sep 28, 2022
@stale
Copy link

stale bot commented Oct 14, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Oct 14, 2022
@tpantelis tpantelis removed the wontfix This will not be worked on label Oct 14, 2022
@submariner-bot submariner-bot added the ready-to-test When a PR is ready for full E2E testing label Oct 19, 2022
@skitt
Copy link
Member

skitt commented Oct 19, 2022

@tpantelis this needs a manual rebase.

@davidohana @tpantelis
Replace klog with zerolog in Lighthouse Agent
215cbb0 
...using the kzerolog package from Admiral.

Note: some internal k8s packages uses klog directly, so few logs may
still be printed in the klog format.

Signed-off-by: David Ohana <[email protected]>
Signed-off-by: Tom Pantelis <[email protected]>
@tpantelis tpantelis enabled auto-merge (rebase) October 20, 2022 12:06
@tpantelis tpantelis merged commit d35bdf8 into submariner-io:devel Oct 20, 2022
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr728/tpantelis/zerolog]

@tpantelis tpantelis deleted the zerolog branch January 17, 2023 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dfarrell07 dfarrell07 dfarrell07 approved these changes

@skitt skitt skitt approved these changes

@aswinsuryan aswinsuryan Awaiting requested review from aswinsuryan aswinsuryan is a code owner

@Oats87 Oats87 Awaiting requested review from Oats87 Oats87 is a code owner

@sridhargaddam sridhargaddam Awaiting requested review from sridhargaddam sridhargaddam is a code owner

@vthapar vthapar Awaiting requested review from vthapar vthapar is a code owner

@mkolesnik mkolesnik Awaiting requested review from mkolesnik

@maayanf24 maayanf24 Awaiting requested review from maayanf24 maayanf24 is a code owner

Assignees

@tpantelis tpantelis

Labels
ready-to-test When a PR is ready for full E2E testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tpantelis @submariner-bot @vthapar @dfarrell07 @skitt @nyechiel @davidohana