Skip to content

Conversation

aanm
Copy link
Member

@aanm aanm commented Apr 14, 2020

klog currently redirects all of its output to stderr which might
prevent users from detecting potentially error messages generated by
klog since the logging format is different from logrus. Redirecting klog
output to logrus will make the logging messages consistent.

Master:

level=info msg="Waiting until all pre-existing resources related to policy have been received" subsys=k8s-watcher
W0410 09:55:21.017837   31587 reflector.go:402] githu....

With commit:

level=info msg="Waiting until all pre-existing resources related to policy have been received" subsys=k8s-watcher
level=warning msg="github.com/cilium/cilium/pkg/k8s/watchers/network_p...
level=info msg="github.com/cilium/cilium/pkg/k8s/watchers/network_p...

Unfortunately, klog as a bug / feature that prints the each log level
message on each writer. This will be fixed upstream.

Candidate to backport this to 1.7?

@aanm aanm added pending-review release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Apr 14, 2020
@aanm aanm requested a review from a team as a code owner April 14, 2020 10:42
@aanm
Copy link
Member Author

aanm commented Apr 14, 2020

test-me-please

@aanm aanm force-pushed the pr/set-klog-to-logrus branch from ed0633a to 3a9a5e1 Compare April 14, 2020 18:29
@aanm aanm requested a review from a team April 14, 2020 18:29
@aanm aanm force-pushed the pr/set-klog-to-logrus branch from 3a9a5e1 to 71aa686 Compare April 14, 2020 19:02
@aanm
Copy link
Member Author

aanm commented Apr 14, 2020

test-me-please

@joestringer
Copy link
Member

I think 1.7 backport makes sense, to pick up on issues in user environments. v1.7 will be supported for some time yet.

klog currently redirects all of its output to stderr which might
prevent users from detecting potentially error messages generated by
klog since the logging format is different from logrus. Redirecting klog
output to logrus will make the logging messages consistent.

Master:
```
level=info msg="Waiting until all pre-existing resources related to policy have been received" subsys=k8s-watcher
W0410 09:55:21.017837   31587 reflector.go:402] githu....
```
With commit:
```
level=info msg="Waiting until all pre-existing resources related to policy have been received" subsys=k8s-watcher
level=warning msg="github.com/cilium/cilium/pkg/k8s/watchers/network_p...
level=info msg="github.com/cilium/cilium/pkg/k8s/watchers/network_p...
```

Unfortunately, klog as a bug / feature that prints the each log level
message on each writer. This will be fixed upstream.

Signed-off-by: André Martins <andre@cilium.io>
@aanm aanm force-pushed the pr/set-klog-to-logrus branch from 71aa686 to ce1e848 Compare April 15, 2020 05:19
@aanm
Copy link
Member Author

aanm commented Apr 15, 2020

test-me-please

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 46.661% when pulling ce1e848 on pr/set-klog-to-logrus into 7abdfa3 on master.

@aanm
Copy link
Member Author

aanm commented Apr 15, 2020

hit #10118

@aanm aanm changed the title pkg/logging: redirect klog output to logurs pkg/logging: redirect klog output to logrus Apr 15, 2020
@aanm aanm merged commit 4b138e6 into master Apr 15, 2020
@aanm aanm deleted the pr/set-klog-to-logrus branch April 15, 2020 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants