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

importing etcdserver messes up log object #4115

Closed
purpleidea opened this issue Jan 3, 2016 · 11 comments · Fixed by coreos/pkg#53
Closed

importing etcdserver messes up log object #4115

purpleidea opened this issue Jan 3, 2016 · 11 comments · Fixed by coreos/pkg#53

Comments

@purpleidea
Copy link
Contributor

I've started work on trying to embed etcdserver into existing projects. This will be useful for a bunch of reasons and great for the etcd project. I am aware that the api's are not necessarily stable, and that this might not be supported at this time, but nevertheless, I am going to try and report/fix some issues as I go. If anyone is able to help out in this area, I'd appreciate it.

First issue: importing either of these modules:
_ "github.com/coreos/etcd/etcdserver" _ "github.com/coreos/etcd/etcdserver/etcdhttp"

Somehow affects the log output. My logs get a prefix of:

2016-01-02 19:15:55.887368 I | <existing log output>

IOW, this looks like:

2016-01-02 19:15:55.887414 I | 19:15:55 main.go:212: blah blah blah

The existing log output includes timestamps and whatever I have specified with log.SetFlags.
I haven't tracked down where the prefix is getting injected, but it's making a mess of anything that uses these modules. I'm also not sure why there is an I (capital i) character. Maybe it's some sort of custom status char used by etcd.

Thanks in advance,
James

@xiang90
Copy link
Contributor

xiang90 commented Jan 3, 2016

etcd uses https://github.com/coreos/pkg/tree/master/capnslog for logging. Check that repo to see if you can get what you want.

We want to provide great experience for embedding etcd into other projects. And we design etcd in that way. But we do not have enough bandwidth to support a stable API and good experience right now.

@gyuho
Copy link
Contributor

gyuho commented Jan 3, 2016

I agree this would be great to have, but it is hard to support configurable logger to external packages without changing lots of code, since capnslog is used almost in every etcd component. I hit the similar issue when I use standard http package. Some of its internal functions use standard log package that cannot be changed by outside packages.

@purpleidea
Copy link
Contributor Author

But we do not have enough bandwidth to support a stable API and good experience right now.

Completely understandable, and thank you for getting etcd as far as it is now.

I'll look into capnslog, I figured it was responsible while grepping through the source. I'll look into trying to come up with a patch.

Thanks for your time.

@xiang90
Copy link
Contributor

xiang90 commented Jan 12, 2016

@purpleidea Any luck with capnslog?

@purpleidea
Copy link
Contributor Author

On Tue, Jan 12, 2016 at 1:30 PM, Xiang Li [email protected] wrote:

@purpleidea https://github.com/purpleidea Any luck with capnslog?

I have not looked at it extensively yet sorry. There's another feature that
got pushed up higher on my todo list, but I plan to return to the issue.
Thank you for your patience and for your pointers.

@xiang90 xiang90 added this to the unplanned milestone Jan 14, 2016
@xiang90
Copy link
Contributor

xiang90 commented Mar 29, 2016

@purpleidea I am closing this one due to low activity. It seems that there is nothing we can do at etcd side. Let us know if you need any further information.

@xiang90 xiang90 closed this as completed Mar 29, 2016
@purpleidea
Copy link
Contributor Author

@xiang90 Fair enough! The other priority turned into two and are now (as of last night) in git master https://github.com/purpleidea/mgmt/ -- woo! -- I'm now trying to focus on doing the etcd parts before coresos fest in Berlin. Will you be coming?

Cheers
James

@purpleidea
Copy link
Contributor Author

Did some looking into capnslog today, found out where it all goes wrong ;)

Here: https://github.com/coreos/pkg/blob/master/capnslog/log_hijack.go#L26

log.SetOutput(w)

The main logger is hijacked, and as a result pipes everything through capnslog instead... In my code, I added to init() (or main()):

log.SetOutput(os.Stderr) // XXX: un-hijack from capnslog

which fixed the output for my logging, but still shows the capns style logs for etcd software...

I think if I were to embed etcd, I'd probably have to vendor override the capnslog package with one that doesn't have the hijack in there. That file has a comment which seems to suggest there's an easier way, but I haven't figured that out for now. At least my logs aren't disrupted anymore. Hope this was a helpful comment for anyone going down the same path as me, and advice is welcome.

@purpleidea
Copy link
Contributor Author

@xiang90 Here is one patch I've made to work around this: coreos/pkg#53 It doesn't affect any of the capnslog functionality anywhere else as it's only an additional formatter which nobody has to use. I think it could be useful for embedding etcd. Cheers and sorry for the delay.

@xiang90
Copy link
Contributor

xiang90 commented Apr 4, 2016

@purpleidea Thanks for the update!

@purpleidea
Copy link
Contributor Author

This is merged, and so is officially not an issue for me anymore: coreos/pkg#53 woo.

purpleidea added a commit to purpleidea/mgmt that referenced this issue May 18, 2016
LuausDalmolin pushed a commit to LuausDalmolin/mgmt that referenced this issue Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants