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

Configurable sidecar log level #139

Open
Madhu-1 opened this issue Sep 18, 2024 · 4 comments
Open

Configurable sidecar log level #139

Madhu-1 opened this issue Sep 18, 2024 · 4 comments

Comments

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Sep 18, 2024

Describe the feature you'd like to have

Currently, we have a single configuration to configure the log level for all the containers in the csi pods, we should have 2 configurations like what we have today in Rook and cephcsi to configure the log level for the cephcsi container and Kubernetes sidecar containers

Who is the end user and what is the use case where this feature will be valuable?

This is required for the feature compatibility with existing deployments and also to avoid too verbose logs on the sidecar when not required.

How will we know we have a good solution? (acceptance criteria)

Introduce a new argument in the Log struct for the sidecar log level configurations

@Madhu-1 Madhu-1 added the good first issue Good for newcomers label Sep 18, 2024
@nb-ohad
Copy link
Collaborator

nb-ohad commented Sep 19, 2024

@Madhu-1 Beside the fact that this was the way it was used in the past integration of CSI, I would like to understand why it is needed. From my perspective, this is an overkill and a middle ground at the same time. and if we are going that route I would prefer to have finer control in the API where we control every individual sidecar log settings

@nb-ohad
Copy link
Collaborator

nb-ohad commented Sep 19, 2024

Removing the good first issue label until we have the API design discussion settled

@nb-ohad nb-ohad removed the good first issue Good for newcomers label Sep 19, 2024
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Sep 20, 2024

@Madhu-1 Beside the fact that this was the way it was used in the past integration of CSI, I would like to understand why it is needed. From my perspective, this is an overkill and a middle ground at the same time. and if we are going that route I would prefer to have finer control in the API where we control every individual sidecar log settings

@nb-ohad having a sidecar log-level was the overkill, we cannot use the same log level for both the cephcsi container and the sidecar containers, Most of the time we end up having very huge logs, Ideally in most the clusters the issues happen on the cephcsi containers where the log level will be set to 5 to generate the logs and all the sidecar log level will be set to 1 to generate only error logs we don't need to have to debug logs for the sidecar containers. IMO advertizing the log level per container can be done again as I mentioned it an overkill, For example, the GRPC timeout its 1 argument common for all the sidecars. This is going to be the same.

@nb-ohad
Copy link
Collaborator

nb-ohad commented Sep 24, 2024

@Madhu-1 I am not sure it is an overkill. what will happen if future changes for one of the sidecars will result in considerably bigger logs for that sidecar vs the other ones vs the operator? I would like us to design the API change in a way that allows a central control and a granular control as an overwrite.

We can set up a call to discuss the needed changes and create a proposal for an API change in the docs section. When we have that, we can link it to this issue for someone to pick up the implementation part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants