search

wetopi / docker-volume-rbd

Docker Engine managed plugin to manage RBD volumes.
MIT License
69 stars 18 forks source link

Capture STDERR output for better error messages #13

Closed diurnalist closed 2 years ago

diurnalist commented 4 years ago

This output is not parsed or otherwise used for business logic, so a combined output stream makes sense, especially given it is returned as part of the error message.

diurnalist commented 2 years ago

@sitamet is there any appetite for this change or shall I close it out? Thanks 🙏🏻

sitamet commented 2 years ago

@diurnalist, If I understand your request, you mean, capturing the output when using shell calls. The block of code related to this is here:

https://github.com/wetopi/docker-volume-rbd/blob/c89b9e6bdd9419f6cc0afeb62001de88be2b2239/lib/sh.go#L15-L27

and this post shows how to capture stderr so we can log it on debug mode: https://blog.kowalczyk.info/article/wOYk/advanced-command-execution-in-go-with-osexec.html#ca6597d7-e802-4d4e-9522-0e6197de19c9

diurnalist commented 2 years ago

@sitamet yes, it is possible to capture them separately. I recall that there was a benefit of the stdout and stderr being part of one string. However, I am looking at the code again and can't see where the output is actually displayed anywhere. In one part of the code it's parsed to check if RBD got a particular error. My memory is that this change helped me debug some stuff when using with an incompatible version of Ceph, because otherwise I couldn't get enough information about why the rbd invocations were failing. So, it was helpful to get stderr from the command, which often contains the error message. I can't see that the output of the command is included in any error messages though, so I'm wondering if there's another patch I did that needs to go with this? Or, otherwise, it's no longer relevant. I'll close for now, thanks for looking :)

sitamet commented 2 years ago

ok