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

capnslog: deprecation notice #57

Open
tmrts opened this issue May 2, 2016 · 9 comments
Open

capnslog: deprecation notice #57

tmrts opened this issue May 2, 2016 · 9 comments

Comments

@tmrts
Copy link

tmrts commented May 2, 2016

The point of this issue is to have a discussion about moving on from capnslog to a better alternative Logrus, which is a well developed structured & pluggable logging library.

cc @philips

@philips
Copy link

philips commented May 2, 2016

We would need to fixup clair, etcd, and go-oidc.

https://godoc.org/github.com/coreos/pkg/capnslog?importers

cc @xiang90 @Quentin-M @ericchiang for their opinions.

@ericchiang
Copy link
Contributor

Should be easy for go-oidc. Not particularly picky about which logging package we use.

@tmrts
Copy link
Author

tmrts commented May 2, 2016

Also we should create a wrapper interface around Logrus and use that for our projects,
which would make it easy to replace Logrus and add more features to it.

As long as we don't use reflection in the wrapper, there should be no resource overhead.

Maybe with the pkg name "clog" for CoreOS Log

@Quentin-M
Copy link

@jsqware and I already discussed about replacing capnslog by logrus very recently. The only con was that capnslog is the one that every CoreOS project use.

@chancez
Copy link
Contributor

chancez commented May 2, 2016

@tmrts Would one of these interfaces work?

https://github.com/Sirupsen/logrus/blob/master/logrus.go#L97-L143

@xiang90
Copy link

xiang90 commented May 2, 2016

For etcd, people might want to control the pkg level logging when they embed etcd server into their own projects. Is this something easy to do in logrus?

@tmrts
Copy link
Author

tmrts commented May 6, 2016

@chancez That is an idiomatic interface. It would be fine.

But, I'd suggest going one step forward, consider the following example:

m, err := p.FetchMetadata()
if err != nil {
    log.Fatal("Failed to fetch meta-data from pod",
        log.Fields{
            Event: "k8s.Pod.FetchMetadata",
            Error: err,
        },
        log.Details{
            "pod": p,
        },
    )
}

Advantage would be consistent log messages and they would provide actionable & meaningful information. Developers wouldn't be thinking how to format the messages and it wouldn't resemble a stack dump after a crash to bug hunters.

The underlying logging library would be Logrus in this case, but it can be the stdlib logging library just as well. And the log.Details would be discarded if log.Level wasn't Debug.

Also we should name it something short and not advise people to rename this package to log it causes problems to goimports, etc. Something like clog, coslog, corelog would be good, or a better suggestion

@purpleidea
Copy link
Contributor

For etcd, people might want to control the pkg level logging when they embed etcd server into their own projects. Is this something easy to do in logrus?

Above @xiang90 makes an excellent point. This is what we have to do in mgmt to avoid log pains due to capnslog:

https://github.com/purpleidea/mgmt/blob/master/lib/main.go#L186

I like the suggestion of using an internal (to etcd) interface to control logs, and whether that uses logrus or logfoo or whatever is hidden to the user embedding etcd.

@jonboulle
Copy link
Contributor

jonboulle commented Mar 15, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants