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

Please Make Event Logging Verbosity Configurable #1285

Closed
shin-go opened this issue Jul 27, 2023 · 3 comments · Fixed by #1438
Closed

Please Make Event Logging Verbosity Configurable #1285

shin-go opened this issue Jul 27, 2023 · 3 comments · Fixed by #1438
Labels
help wanted Feature requests approved by maintainers that are not included in the project roadmap

Comments

@shin-go
Copy link

shin-go commented Jul 27, 2023

Which component:
controller

Is your feature request related to a problem? Please describe.
Unless it's tucked away someplace not obvious in documentation, it doesn't look like there is an arg that can be set on the Deployment template spec nor in the helm chart. This would be greatly appreciated as the controller can be extremely chatty when it iterates on decrypting SealedSecret resources, particularly in large Kubernetes clusters.

Such normal events would be considered INFO, which are usually not very interesting for day-to-day knowledge when you may instead be interested specifically in WARN or more severe.

Here's an example event log from the controller:

2023/07/27 22:11:21 Updating staging/account-secrets
2023/07/27 22:11:21 Event(v1.ObjectReference{Kind:"SealedSecret", Namespace:"staging", Name:"account-secrets", UID:"45ef60cc-c7bf-4508-a861-a0514dce9c1c", APIVersion:"bitnami.com/v1alpha1", ResourceVersion:"768125925", FieldPath:""}): type: 'Normal' reason: 'Unsealed' SealedSecret unsealed successfully 

And worse, some events that aren't timestamped. #1274 is a good example of this.

Describe the solution you'd like
A flag that can be set in args on the Deployment template spec and helm chart. Also if implemented please document clearly what the valid values are. Some tools abide by the native Kubernetes logging verbosities which are integers (0 = shut up, 1 = info, etc.) while others support logging verbosity but only note the flag expects a string type.

Describe alternatives you've considered
Are there any?

Additional context
This might take more effort to implement because none of the events produced by the controller are classified classically - e.g.: INFO, DEBUG, WARN, etc..

@shin-go shin-go added the triage Issues/PRs that need to be reviewed label Jul 27, 2023
@alvneiayu alvneiayu added help wanted Feature requests approved by maintainers that are not included in the project roadmap and removed triage Issues/PRs that need to be reviewed labels Aug 16, 2023
@alvneiayu
Copy link
Collaborator

hi @shin-go,

As you said, our Log library has limitations. We can not set up right now the debug level of our messages. We are using the standard Golang log library and can not set up this level. One option here is to move to another log library like Zap to make it possible or to improve our actual log library.

This will be an excellent contribution for the project. I invite you to collaborate to the project, helping us to improve this log process.

Thanks a lot

Álvaro

@mohamed-essam
Copy link
Contributor

@alvneiayu I would like to work on this issue, I agree on using Zap for better logging, but before I start on that I just wanted to make sure that Zap is the option we will go with

@alvneiayu
Copy link
Collaborator

alvneiayu commented Jan 9, 2024

hi @mohamed-essam

thanks a lot for your proposal. It would be nice to work on this issue, so feel free to contribute with.

About to use Zap or not, recently a native structure logging library has been released:

https://pkg.go.dev/log/slog

Maybe it is a good idea to investigate it and use it to reduce the dependencies needed.

Thanks a lot again

Álvaro

alemorcuq pushed a commit that referenced this issue Jan 26, 2024
**Description of the change**

Change controller logging to use `log/slog`.

**Benefits**

1. Allow structured logging.
2. Allow changing logging format between text and JSON.
3. Allow changing logging level.

**Possible drawbacks**

N/A

**Applicable issues**

- fixes #1285 

**Additional information**

* I took the liberty of removing log at line 91 in
pkg/controller/main.go as it only served as a separator
* To implement the functionality of `--log-info-to-stdout`, I had to
implement a custom handler that would hand logs down to one of two
slog.Loggers in place of the package in `pkg/log` that handled this
previously

---------

Signed-off-by: M Essam Hamed <github@messam.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Feature requests approved by maintainers that are not included in the project roadmap
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants