search

ceph / ceph-csi-operator

Kubernetes operator for managing the CephCSI plugins
Apache License 2.0
16 stars 16 forks source link

Configurable sidecar log level #139

Open Madhu-1 opened 2 months ago

Madhu-1 commented 2 months ago

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

nb-ohad commented 2 months ago

@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 commented 2 months ago

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

Madhu-1 commented 2 months ago

@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 commented 1 month ago

@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