Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: provide a SetLog() func #18

Closed
aanm opened this issue Oct 18, 2016 · 34 comments
Closed

Proposal: provide a SetLog() func #18

aanm opened this issue Oct 18, 2016 · 34 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@aanm
Copy link
Contributor

aanm commented Oct 18, 2016

Is it possible to provide a SetLog() in the packages? For example, I'm using 3 cache.NewInformer to watch for changes on 3 different resources, but if, for whatever reason, the connection between the client and the apiserver doesn't exist, I have this error messages in my log:

ERROR: logging before flag.Parse: E1018 14:16:07.808569    5156 reflector.go:214] k8s.io/client-go/1.5/tools/cache/reflector.go:109: Failed to list *v1.Endpoints: Get http://192.168.33.11:8080/api/v1/endpoints?resourceVersion=0: dial tcp 192.168.33.11:8080: getsockopt: connection refused
ERROR: logging before flag.Parse: E1018 14:16:08.809443    5156 reflector.go:214] k8s.io/client-go/1.5/tools/cache/reflector.go:109: Failed to list *v1beta1.NetworkPolicy: Get http://192.168.33.11:8080/apis/extensions/v1beta1/networkpolicies?resourceVersion=0: dial tcp 192.168.33.11:8080: getsockopt: connection refused
ERROR: logging before flag.Parse: E1018 14:16:08.809458    5156 reflector.go:214] k8s.io/client-go/1.5/tools/cache/reflector.go:109: Failed to list *v1.Service: Get http://192.168.33.11:8080/api/v1/services?resourceVersion=0: dial tcp 192.168.33.11:8080: getsockopt: connection refused
ERROR: logging before flag.Parse: E1018 14:16:08.809526    5156 reflector.go:214] k8s.io/client-go/1.5/tools/cache/reflector.go:109: Failed to list *v1.Endpoints: Get http://192.168.33.11:8080/api/v1/endpoints?resourceVersion=0: dial tcp 192.168.33.11:8080: getsockopt: connection refused
ERROR: logging before flag.Parse: E1018 14:16:09.811722    5156 reflector.go:214] k8s.io/client-go/1.5/tools/cache/reflector.go:109: Failed to list *v1beta1.NetworkPolicy: Get http://192.168.33.11:8080/apis/extensions/v1beta1/networkpolicies?resourceVersion=0: dial tcp 192.168.33.11:8080: getsockopt: connection refused
ERROR: logging before flag.Parse: E1018 14:16:09.811733    5156 reflector.go:214] k8s.io/client-go/1.5/tools/cache/reflector.go:109: Failed to list *v1.Endpoints: Get http://192.168.33.11:8080/api/v1/endpoints?resourceVersion=0: dial tcp 192.168.33.11:8080: getsockopt: connection refused
ERROR: logging before flag.Parse: E1018 14:16:09.811733    5156 reflector.go:214] k8s.io/client-go/1.5/tools/cache/reflector.go:109: Failed to list *v1.Service: Get http://192.168.33.11:8080/api/v1/services?resourceVersion=0: dial tcp 192.168.33.11:8080: getsockopt: connection refused

I would like to at least change the layout to be the same as my logs or/and silent the log messages until I get a new kubernetes connection.

@jimmidyson
Copy link
Member

Would love to have a proper log interface & be able to provide an impl that the client could use. glog is a bit invasive here.

@ae6rt
Copy link

ae6rt commented May 10, 2017

Thank you for this k8s client. It's very helpful.

Do you have any advice around suppressing glog's hoisting of its command line flags into the help output of tools that use the k8s client.

For example, those flags whose descriptions are not decorated with "MyApp" are coming from glog's init function: https://github.com/golang/glog/blob/master/glog.go#L398

Usage of build/dist/darwin/myapp:
  -alsologtostderr
    	log to standard error as well as files
  -config string
    	MyApp: Config file that points to the K8S master. (default "./config")
  -log_backtrace_at value
    	when logging hits line file:N, emit a stack trace
  -log_dir string
    	If non-empty, write log files in this directory
  -logtostderr
    	log to standard error instead of files
  -reload
    	MyApp: Send the initial system state read from K8S to the load balancer. (default true)
  -stderrthreshold value
    	logs at or above this threshold go to stderr
  -v value
    	log level for V logs
  -vmodule value
    	comma-separated list of pattern=N settings for file-filtered logging

@fejta-bot
Copy link

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.

Prevent issues from auto-closing with an /lifecycle frozen comment.

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 24, 2017
@DavidHuie
Copy link

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 2, 2018
@DavidHuie
Copy link

I would also appreciate having this feature. We're currently building tooling around client-go without using glog, so we would love to have the ability to override it.

@deads2k
Copy link
Contributor

deads2k commented Jan 15, 2018

If we undertake the effort to switch logging frameworks, (It's not just going to be client-go), we need to have

  1. flag compatibility - there are numerous "real" deployments using these. Make the optional to register.
  2. dynamic package log level changes
  3. logging formatter pluggability (one size does NOT fit all)
  4. logging handler pluggability (different sinks exist and have value)
  5. logging filter pluggability (dynamic and per project filtering needs exist)
  6. glog compatibility desired.

@lavalamp @eparis @bparees you've also dealt with our logging shortcomings, other things to add to the list?

I'm disinclined to undertake the effort of a replacement that didn't address those requirements.

@bparees
Copy link

bparees commented Jan 15, 2018

@deads2k that covers my wishlist, anyway. I suppose rate limiting or de-duping might be nice options as well, but not must haves.

@deads2k
Copy link
Contributor

deads2k commented Jan 15, 2018

@deads2k that covers my wishlist, anyway. I suppose rate limiting or de-duping might be nice options as well, but not must haves.

I agree those are nice to have. I think that given pluggable filters and handlers it is possible to build de-duping and rate limiting. Given pieces to build them, I won't block progress on not having them.

@liggitt
Copy link
Member

liggitt commented Jan 15, 2018

logging formatter pluggability (one size does NOT fit all)

If you're wanting structured logs, then you really have to replace all usage of glog (even from our dependencies) in order to be useful ("almost all JSON" log output doesn't really do much good)

logging handler pluggability (different sinks exist and have value)

same comment about logger usage by dependencies

@deads2k
Copy link
Contributor

deads2k commented Jan 15, 2018

logging formatter pluggability (one size does NOT fit all)
If you're wanting structured logs, then you really have to replace all usage of glog (even from our dependencies) in order to be useful ("almost all JSON" log output doesn't really do much good)

logging handler pluggability (different sinks exist and have value)
same comment about logger usage by dependencies

The same sort of argument would apply to fmt and to capnslog and whatever else other libraries do. Formatting of most messages does help significantly (ie: I get <package>.<file> in 90% of messages) . The same applies for handlers (I get a well present systemd log), getting 90% working really helps.

Mixing log4j and jsr47 doesn't work perfectly, but you don't have to have perfect coherence for big gains.

@luxalpa
Copy link

luxalpa commented Jan 22, 2018

same comment about logger usage by dependencies

In our projects, we have more that 80 services with countless of dependencies and so far we had no problem with structured logging (except from k8s/client-go). I found that usually dependencies just use the log.Logger interface for logging.

Could you make a list of dependencies that use glog?

@lavalamp
Copy link
Member

Sadly it does seem reasonable to want glog to not be mandatory to use the client. This would be a fairly large effort, though. We'd have to define a new logging interface & provide a glog implementation which gives zero difference from today. (glog's interface is not easy (or possible, even?) to reimplement due to the .V(). pattern--I consider it a go language defect :( )

If we had an interface and a 100% compatible glog implementation, we could slowly roll changes out. But it's a lot of effort.

@sttts
Copy link
Contributor

sttts commented Jan 23, 2018

Technically you can reimplement github.com/golang/glog using another library, place the wrapper in your vendor/ dir and make e.g. golang/dep to point to your wrapper:

[[constraint]]
  name = "github.com/golang/glog"
  source = "https://github.com/yourorg/glog-wrapper"

This way you won't hit the "go language defect" @lavalamp is referring to and client-go will use your logging library without any code change.

@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 23, 2018
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

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 rotten
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 23, 2018
@aanm
Copy link
Contributor Author

aanm commented May 23, 2018

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label May 23, 2018
@aanm
Copy link
Contributor Author

aanm commented May 23, 2018

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed enhancement labels Jun 5, 2018
@fiunchinho
Copy link

does anyone know a workaround for this? I'd like to set the format for glog logs, or disable it all together.

@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 26, 2018
@aanm
Copy link
Contributor Author

aanm commented Nov 26, 2018

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 26, 2018
@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 24, 2019
@aanm
Copy link
Contributor Author

aanm commented Feb 25, 2019

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 25, 2019
@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 26, 2019
@DingGGu
Copy link

DingGGu commented May 29, 2019

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 29, 2019
@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 27, 2019
@aanm
Copy link
Contributor Author

aanm commented Sep 2, 2019

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 2, 2019
@coryrc
Copy link
Contributor

coryrc commented Nov 20, 2019

Since client-go now uses klog, doesn't klog.SetLogger() fulfill this proposal?

https://github.com/kubernetes/klog/blob/8422fac62d1e961e89426ffb5ae3a07f2d0bcca2/klog.go#L776

@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 18, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

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 rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 19, 2020
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

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

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

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

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.

@kishoregv
Copy link

Since client-go now uses klog, doesn't klog.SetLogger() fulfill this proposal?

https://github.com/kubernetes/klog/blob/8422fac62d1e961e89426ffb5ae3a07f2d0bcca2/klog.go#L776

To be able to use klog.SetLogger() client-go needs to start using k8s.io/klog/v2

@kishoregv
Copy link

/remove-lifecycle rotten

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests