Use of glog for logging is problematic

[obeattie]

FEATURE REQUEST:

/kind feature

What happened:

Kubernetes uses golang/glog extensively for logging. While this is reasonable enough for the Kubernetes binaries, sizeable parts of the codebase are re-used as client libraries (eg. k8s.io/client-go). This presents several problems for users of the libraries:

• glog registers a bunch of flags in an init function, and offers no programmatic way to configure its behaviour. It can be surprising to users of the k8s libraries that they have to call flag.Parse(), and this is easy to get wrong (eg. No way to configure glog coredns/coredns#1597).
• glog's default behaviour is to log to a file on disk. A user of a library wouldn't typically expect it to write files without explicitly configuring it to do so.
• Worse, if glog cannot create a file, it calls os.Exit. This can be very harmful, and since it's common to run containerised binaries with a read-only root filesystem, can be easy to trigger.
• glog doesn't perform any management of the files it writes, so without something like logrotate running (especially problematic in a container) the log files just accumulate.

I think we need to find a way to fix this: the use of glog makes working with the k8s client libraries very difficult. A recent Twitter discussion on this topic shows this is a widely-held concern.

[obeattie]

@kubernetes/sig-architecture-feature-requests

[k8s-ci-robot]

@obeattie: Reiterating the mentions to trigger a notification:
@kubernetes/sig-architecture-feature-requests

In response to this:

@kubernetes/sig-architecture-feature-requests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[obeattie]

Personal 2¢: while replacing glog completely with something else might be the best long-term solution, doing so seems like it would be a long road. Coming to a consensus on a replacement and rewriting all existing calls to glog to use that replacement doesn't seem like it will happen quickly.

The problems caused by this are quite harmful now, so I think it's pragmatic to mitigate those without replacing glog completely. It seems like that would involve something like:

1. Forking glog
2. Making it programmatically configurable
3. Making it log to stdout/stderr by default
4. Removing its flags, adding those as explicit k8s flags instead, and using them to configure the library

I'd be happy to take a go at this, but it's probably only worthwhile doing that if there's enough agreement that it would be an acceptable thing to do (a PR that changes the glog import path would benefit from merging pretty fast since it would get out of date very quickly.)

[errordeveloper]

@obeattie thanks for enumerating these issue in full, I agree with all of them and we should start doing something about it. I think most developers are partly aware of some of these issues, but I am not sure if anyone else has enumerated these issues as well as you did here.

[bboreham]

I agree glog should be fixed, forked or replaced.
Note the glog README says "Feature requests will be ignored.", which points towards the latter two.

Interested in your view @thockin

[alexellis]

I was part of the initial Twitter thread - my main concerns are: the init() does too much and cannot be deferred.

A forked/updated glog should ideally not run init() or have an environmental variable that can be set to avoid it running init() until it's desirable - for instance how can you unit test this (you can't)?

The override should ideally not mandate the use of args/flags to configure logging.

To the point @thockin raised by his Go logging interface project (mentioned in the tweet thread) - there should be an interface available so that the underlying test framework/implementation can be abstracted away from the consuming code.

https://github.com/thockin/logr

[alexellis]

Related issue: coredns/coredns#1597 (closed)

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[alexellis]

@obeattie wdyt?