kubernetes/kubernetes

Use a forked version of glog in kubernetes

dims opened this issue · 26 comments

dims commented

Based on the discussion in SIG-Arch on Oct 25 and previous experiment in #69333, we should start by forking glog, cleanup the code to suit our use cases and switch over to it. Also see issue #61006

Tasks:

  • Request a new repository for the forked code in https://github.com/dims/klog in the kubernetes org(?)
  • Update k8s.io/klog README.md to explain it is a permanent fork.
  • update github.com/kubernetes/repo-infra/kazel/kazel.go to use klog
  • update k8s.io/gengo/ to use klog
  • update k8s.io/kube-openapi/ to use klog
  • update github.com/google/cadvisor to use klog
  • vendor all the repos above into k/k and switch k/k from glog to klog
  • add verify scripts to prevent adding any new vendor dependencies that use glog

cc @thockin @tallclair @erictune @justinsb @lavalamp

/kind feature

dims commented

/sig architecture

dims commented

/assign @thockin
/assign @tallclair

erictune commented

Thank you for doing this, @dims!

Also:

  • Update k8s.io/klog README.md to explain it is a permanent fork.
liggitt commented

I think we would also need to add verify scripts to prevent adding any new vendor dependencies that use glog. I'm actually very surprised we don't have more already:

$ grep -R 'glog"' vendor | egrep -v 'k8s.io|cadvisor'
vendor/github.com/golang/glog/BUILD:    importpath = "github.com/golang/glog",
vendor/github.com/kubernetes/repo-infra/kazel/kazel.go:	"github.com/golang/glog"
dims commented

+1 @erictune @liggitt updating task list above

deads2k commented

I think we would also need to add verify scripts to prevent adding any new vendor dependencies that use glog. I'm actually very surprised we don't have more already:

I think that's because this project uses godep, not something like glide which pulls in entire trees. github.com/coreos/etcd vendors github.com/grpc-ecosystem/grpc-gateway which vendors glog.

liggitt commented

we would also need to add verify scripts to prevent adding any new vendor dependencies that use glog

given the prevalence of glog usage in the go ecosystem, we also have to consider the impact of k8s.io/* components using a logging library that cannot be built into a binary that also includes glog. If a downstream consumer imports k8s.io/apimachinery or k8s.io/client-go (and therefore "klog"), and also has another dependency that uses glog, this change will break them without much recourse.

deads2k commented

Based on the discussion in SIG-Arch on Oct 25 and previous experiment in #69333, we should start by forking glog, cleanup the code to suit our use cases and switch over to it. Also see issue #61006

@dims I've had a look at your fork. Is there a list of specific objectives beyond flags and output? I think I'd be looking for something more pluggable overall if we commit to this pain.

dims commented

@liggitt to allow klog and glog to co-exist

  • We have to drop the flags registration from "init" and require an explicit Init for klog
  • end users can use klog's setOutput and redirect the logs from klog to glog
dims commented

@deads2k, there are a lot of discussions in many of the previous issues about things that can be done. I am deliberately trying to start simple here and unblock folks who want to do more things.

For example, i would like to:

-- Dims

  • 👍1
liggitt commented

@liggitt to allow klog and glog to co-exist

  • We have to drop the flags registration from "init" and require an explicit Init for klog
  • end users can use klog's setOutput and redirect the logs from klog to glog

I think allowing co-existence is a requirement if klog is referenced by our published repos.

dims commented

@liggitt agree. we should document what someone has to do if they are using glog today and want to import our stuff.

deads2k commented

@liggitt agree. we should document what someone has to do if they are using glog today and want to import our stuff.

It's not just about code directly owned by the vendorer. I vendor kube libraries and I vendor libraries from other authors that I do not own. Anything that requires me to change code to import and run with kube libraries can make that impossible.

@deads2k, there are a lot of discussions in many of the previous issues about things that can be done. I am deliberately trying to start simple here and unblock folks who want to do more things.

@dims do you have a specific roadmap in mind? This feels like a switch from A to B, where B is more or less equivalent and any future direction change will face the same roadblocks as before.

liggitt commented

Anything that requires me to change code to import and run with kube libraries can make that impossible.

Right. klog should allow co-existence by default with no action taken by importers. The types of things required to do this are good practices anyway (don't register global flags in init blocks, etc).

dims commented

@deads2k regarding same roadblocks this time, we have a place where we can make changes :) klog repo! we did not have the luxury of modifying glog before :)

tallclair commented

copying here for visibility (from kubernetes/org#195 (comment)):

Are we still intending for [klog] to be a transitional step towards our logging end goal? If so, can we call that out in the description & readme, and discourage others from using this? My concern is that as soon as we put out a "kubernetes-log" repo, other projects will start cargo-culting it...
Along similar lines, I wonder if we should stick with "glog" for the name? It further emphasizes that this is a fork of glog, and not a new kubernetes-endorced log library. It also means fewer lines need to be touched in the migration.

  • 👍1
BenTheElder commented

I think we would also need to add verify scripts to prevent adding any new vendor dependencies that use glog. I'm actually very surprised we don't have more already:

Noting that one of the examples:

vendor/github.com/kubernetes/repo-infra/kazel/kazel.go: "github.com/golang/glog"

Is actually a dev-time dependency rather than being built into any k8s components. I don't think we need to worry too much about that (though we might as well migrate it too) cc @ixdy

/cc

dims commented

@tallclair ack.

One more data point for folks here. thanks @aledbf for showing me how Istio does this. based on his pointers here's a repo - https://github.com/dims/glog-minimal that redirects glog calls to klog

This can work with some usecases like https://github.com/istio/istio/blob/master/Gopkg.toml#L61 (override the source using dep)

  • 👍2
ixdy commented

yes, migrating kazel is fine with me.

  • 👍1
dims commented

We have 2 ways of co-existing with glog. the glog-minimal repo above or see the example in klog. So i believe we can move on to the steps listed above.

@thockin @tallclair i am looking to you both to approve the org request as well :)

kubernetes/org#195

Yes, please decide on the name as well. (klog vs glog)

tallclair commented

One more data point for folks here. thanks @aledbf for showing me how Istio does this. based on his pointers here's a repo - https://github.com/dims/glog-minimal that redirects glog calls to klog

That's awesome that istio has already done this - that's exactly the route I was picturing us going.

thockin commented

I'm not sure I see how this fixes conflicts with imports on "real" glog?

If I have an app that uses github.com/golang/glog pervasively, and I decide to use client-go, I will have glog and klog loaded, and the logs will not be merged.

What am I missing?

dims commented

@thockin if you are using github.com/golang/glog pervasively and want to use client-go, then you have 2 choices.

Choice #1 - Keep github.com/golang/glog in charge of actual file operations using example/pattern in:

Choice #2 - I don't care much about github.com/golang/glog` in charge of writing to disk, but i don't want to change all my existing code, what is my option?

thockin commented
dims commented

/sig instrumentation

dims commented

@kubernetes/sig-instrumentation-feature-requests @kubernetes/sig-architecture-feature-requests