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

[Misc] Standardizing klog Level Guidelines in AIBrix #69

Closed
kr11 opened this issue Aug 1, 2024 · 8 comments · Fixed by #202
Closed

[Misc] Standardizing klog Level Guidelines in AIBrix #69

kr11 opened this issue Aug 1, 2024 · 8 comments · Fixed by #202
Assignees
Labels
area/observation kind/documentation Improvements or additions to documentation kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@kr11
Copy link
Collaborator

kr11 commented Aug 1, 2024

Summary

This issue aims to standardize klog level usage across Kubernetes controllers by establishing clear logging guidelines.

For the immediate phase, we will standardize on klog.V(4).

Motivation

With varying klog levels observed in files like modeladapter_controller.go and podautoscaler_controller.go, it's challenging to maintain and analyze logs effectively. Standardizing log levels and creating detailed guidelines will enable more consistent and meaningful logging, aiding in development and troubleshooting.

Proposed Change

No response

Alternatives Considered

No response

@kr11 kr11 added the kind/documentation Improvements or additions to documentation label Aug 1, 2024
@kr11 kr11 self-assigned this Aug 1, 2024
@Jeffwan Jeffwan added kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. area/observation labels Aug 29, 2024
@Jeffwan Jeffwan added this to the v0.1.0-rc.2 milestone Aug 29, 2024
@Jeffwan Jeffwan changed the title [RFC]: Standardizing klog Level Guidelines in AIBrix Standardizing klog Level Guidelines in AIBrix Sep 9, 2024
@Jeffwan
Copy link
Collaborator

Jeffwan commented Sep 9, 2024

info, warning and error should be revisited for all the codes using this opportunity. Varun notice some references are incorrect #135 (comment)

@Jeffwan
Copy link
Collaborator

Jeffwan commented Sep 17, 2024

image

Let's use V(2) and V(4) in logs, there's no need to use V(3) which complex the management

@Jeffwan Jeffwan changed the title Standardizing klog Level Guidelines in AIBrix [Misc] Standardizing klog Level Guidelines in AIBrix Sep 18, 2024
@Jeffwan
Copy link
Collaborator

Jeffwan commented Sep 20, 2024

@kr11 Can you help find all v(3) and v(5) instance and change to V(2) or V(4) based on the log information? BTW, v(2) is the default level and klog.v(4).InfoS equivalent to klog.InfoS. Let's probably also remove v(2) for all logs that explicitly using v(2)

@Jeffwan
Copy link
Collaborator

Jeffwan commented Sep 20, 2024

if you can add some guidance in the development docs. that would be great! l

@Jeffwan
Copy link
Collaborator

Jeffwan commented Sep 22, 2024

Besides the log level, since we use klog to simplify the logger import. now it have different log formats which needs to be refactored

image

we probably can use this adaptor https://github.com/kubernetes/klog/blob/main/klogr/klogr.go or just change to structure logging

@kr11
Copy link
Collaborator Author

kr11 commented Sep 23, 2024

@kr11 Can you help find all v(3) and v(5) instance and change to V(2) or V(4) based on the log information? BTW, v(2) is the default level and klog.v(4).InfoS equivalent to klog.InfoS. Let's probably also remove v(2) for all logs that explicitly using v(2)

@Jeffwan I just add a PR #202 about this issue, please take a look.

And, is klog.v(4).InfoS equivalent to klog.InfoS a typo? Should it be klog.V(2).InfoS = klog.InfoS instead? 😊

@Jeffwan
Copy link
Collaborator

Jeffwan commented Sep 23, 2024

@kr11 good catch. that's a typo.

@Jeffwan
Copy link
Collaborator

Jeffwan commented Oct 5, 2024

#264 addressed this issue.

@Jeffwan Jeffwan closed this as completed Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/observation kind/documentation Improvements or additions to documentation kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants