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

Refine etcdctl snapshot restore error logs #9290

Closed
hongchaodeng opened this issue Feb 6, 2018 · 3 comments
Closed

Refine etcdctl snapshot restore error logs #9290

hongchaodeng opened this issue Feb 6, 2018 · 3 comments

Comments

@hongchaodeng
Copy link
Contributor

Continue discussion: #9179

The following lines are always printed to stderr

2018-01-19 23:42:54.398661 I | pkg/netutil: resolving test-etcd-backup-restore-0-0000.test-etcd-backup-restore-0.e2e-etcd-operator-flaketest-580.svc:2380 to 10.28.5.36:2380
2018-01-19 23:42:54.402454 I | pkg/netutil: resolving test-etcd-backup-restore-0-0000.test-etcd-backup-restore-0.e2e-etcd-operator-flaketest-580.svc:2380 to 10.28.4.183:2380

It has two issues:

  • It causes confusion. These seems normal message. As a user I wouldn't expect to see it on stderr for successful run.
  • For DNS resolving issues, the DNS and IP were already included in the error message: *: add detailed error message on URLs equal check #9210. So the messages are duplicate.
@xiang90
Copy link
Contributor

xiang90 commented Feb 6, 2018

It causes confusion. These seems normal message. As a user I wouldn't expect to see it on stderr for successful run.

No. This is the whole point of why stderr exists. Users should only redirect stdout if they only care about the final result rather than any informational or diagnostic messages.

@hongchaodeng
Copy link
Contributor Author

hongchaodeng commented Feb 7, 2018

@xiang90 I understand your argument. But let's be honest that this mixing of diagnostic information and user-facing error message is confusing.

If etcdctl runs successfully, it logs what it affected users like writing files. If it fails, it logs what failures should user expect -- in this case, Error: --initial-cluster ....
Isn't it better to separate diagnostic and user messages?

As a side note, stdout/stderr doesn't really matter.

@gyuho
Copy link
Contributor

gyuho commented May 2, 2018

I've improved error messaging to be more clear. And now you can specify log output (default is stderr) with --log-outputs stdout,a.log flag.

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

No branches or pull requests

3 participants