kubeslice / worker-operator

Kubeslice Worker Operator Opensource Repository: The KubeSlice Worker Operator is a Kubernetes operator that manages the lifecycle of KubeSlice worker clusters.
Apache License 2.0
60 stars 19 forks source link

Introduce standard log fields and replace zap with logrus #134

Closed kmjayadeep closed 1 year ago

kmjayadeep commented 1 year ago

To introduce standard logging fields and conventions, we need to fix the following limitations with the current codebase

To fix this, this PR introduces the following changes (more PRs to follow)

zap

log.Info("something", zap.string("slice", "green"))

* Change log field names
  * level -> severity
  * msg -> message
  * ts -> timestamp
* Added `logrusr` package to convert logrus to logr implementation (will replace with our own implementation in next PRs)
* go fmt and go.mod changes

Next steps

* Replace logrusr with our own implementation to support the log levels (https://github.com/bombsimon/logrusr/blob/main/logrusr.go)
* Export log levels as constants and replace code blocks like below

log.V(1).Info("App pod is not part of the slice", "pod", pod.Name, "slice", slice.Name) log.V(logger.DEBUG).Info("App pod is not part of the slice", "pod", pod.Name, "slice", slice.Name)

log.V(2).Info("App pod is not part of the slice", "pod", pod.Name, "slice", slice.Name) log.V(logger.WARN).Info("App pod is not part of the slice", "pod", pod.Name, "slice", slice.Name)


* Improve log messages and fields
NishantSingh10 commented 1 year ago

report link 'https://kubeslice.github.io/e2e-allure-reports/Kind-worker-operator-2022-11-13T08:51:04-master-235/index.html'

gourishkb commented 1 year ago

HI @kmjayadeep , this is a great description. Thank you. I am not clear about the following,

"Replace logrusr with our own implementation to support the log levels (https://github.com/bombsimon/logrusr/blob/main/logrusr.go)" and "Export log levels as constants and replace code blocks like below"

seem like the same steps to me or did I misunderstand you ?

The changes look good to me :+1:

kmjayadeep commented 1 year ago

thanks @gourishkb. I'm closing this PR. I found that it is easy to implement this with zap itself, without needing to maintain our own implementation of logr.Logger. I will raise a separate PR

kmjayadeep commented 1 year ago

@pnavali this PR was closed. Here is the new PR: https://github.com/kubeslice/worker-operator/pull/135 The new fields are added there