nagios plugin doesn't really work with --enable-debug

[ghost]

I'm packaging collectd for ALT Linux; a user reported an issue with nagios plugin in 5.1.0, the brief translation follows:

# collectd-nagios -s /var/run/collectd-unixsock -H host.node -n load/load -d
shortterm -w 2 -c 10 
send:    --> GETVAL "host.node/load/load"
receive: <-- 3 Values found
receive: <-- shortterm=4.800000e-01
receive: <-- midterm=1.600000e+00
receive: <-- longterm=1.300000e+00
OKAY: 0 critical, 0 warning, 1 okay | shortterm=0.480000;;;;

Nagios interface (particularly the service status cell) shows:
send: --> GGETVAL "host.node/load/load"
instead of
0 critical, 0 warning, 1 okay

# collectdctl listval
[...]
send:    --> LISTVAL
receive: <-- 196 Values found
receive: <-- 1342505724.426 host.node/conntrack/conntrack
receive: <-- 1342505726.892 host.node/cpu-0/cpu-idle
[...]
instead of
host.node/conntrack/conntrack
host.node/cpu-0/cpu-idle
host.node/cpu-0/cpu-interrupt
host.node/cpu-0/cpu-nice
host.node/cpu-0/cpu-softirq
host.node/cpu-0/cpu-steal
[...]

The fix was to configure --disable-debug but it's not a really great fix I guess.

The original bugreport in Russian is here: https://bugzilla.altlinux.org/27548

[faxm0dem]

IMHO when debug mode is enabled, all corresponding data should go to STDERR instead of STDIN.
I apply the following trivial patch to solve this behaviour in my .spec:

diff -ru collectd-5.1.0.ori/src/libcollectdclient/client.c collectd-5.1.0/src/libcollectdclient/client.c
--- collectd-5.1.0.ori/src/libcollectdclient/client.c   2012-04-17 13:50:04.000000000 +0200
+++ collectd-5.1.0/src/libcollectdclient/client.c       2012-04-17 13:55:42.000000000 +0200
@@ -87,7 +87,7 @@
 } while (0)

 #if COLLECT_DEBUG
-# define LCC_DEBUG(...) printf (__VA_ARGS__)
+# define LCC_DEBUG(...) fprintf (stderr, __VA_ARGS__)
 #else
 # define LCC_DEBUG(...) /**/
 #endif

[ghost]

On Thu, Jul 19, 2012 at 04:20:37AM -0700, Fabien Wernli wrote:

IMHO when debug mode is enabled, all corresponding data should
go to STDERR instead of STDIN. I apply the following trivial
patch to solve this behaviour in my .spec:

Thanks; the point is that it should be the upstream behaviour.

---- WBR, Michael Shigorin mike@altlinux.ru
------ Linux.Kiev http://www.linux.kiev.ua/

[faxm0dem]

Point taken, let's see what ff and sh comment about this. As they are quite busy, I usually add a pull request, and patches to my packages until it's accepted upstream.

[octo]

Hi Michael,

good point. I don't really like having debugging output on STDERR, but then libraries shouldn't print to STDOUT either. What do you think about this compromise: Write debug output to STDOUT only when there is a controlling terminal.

@tokkee Do you have another clever idea how to handle this?

Best regards,
—octo

[ghost]

On Mon, Sep 10, 2012 at 12:50:19PM -0700, Florian Forster wrote:

good point. I don't really like having debugging output on
STDERR, but then libraries shouldn't print to STDOUT either.
What do you think about this compromise: Write debug output to
STDOUT only when there is a controlling terminal.

Meaning "write debug to stdout at tty, otherwise don't write
debug to stdout/stderr at all"? Looks good so far :-)

---- WBR, Michael Shigorin mike@altlinux.ru
------ Linux.Kiev http://www.linux.kiev.ua/
---- Sep 29, Kiev, Ukraine:
-- http://conference.osdn.org.ua

[faxm0dem]

Hi,

On Mon, Sep 10, 2012 at 12:50:20PM -0700, Florian Forster wrote:

good point. I don't really like having debugging output on STDERR, but then libraries shouldn't print to STDOUT either. What do you think about this compromise: Write debug output to STDOUT only when there is a controlling terminal.

Any other reason apart from "I don't like"?
Having debug output and real output on a different channel makes sense (in
a tty) when you want to filter the debug output out, without having to grep
it out.

my 2 cents here

[ghost]

On Tue, Sep 11, 2012 at 07:35:01AM -0700, Fabien Wernli wrote:

Having debug output and real output on a different channel
makes sense (in a tty) when you want to filter the debug
output out, without having to grep it out.

A prefix like [debug] or module name might help with that either.

---- WBR, Michael Shigorin mike@altlinux.ru
------ Linux.Kiev http://www.linux.kiev.ua/
---- Sep 29, Kiev, Ukraine:
-- http://conference.osdn.org.ua

[ChrisLundquist]

You could do something like open a new debug file handle, which may or may not be the same as /dev/null, stderr, etc.

Looks like x264 just prefixes it.
https://github.com/DarkShikari/x264-devel/blob/37be55213a39db40cf159ada319bd482a1b00680/common/common.c#L1047-1087

[faxm0dem]

using an environment variable is quite elegant too, e.g. COLLECTD_TRACE=1 or COLLECTD_TRACE=/tmp/collectd-debug.log

[octo]

Hi,

I think I like the idea of using an environment variable better than my previous idea of testing for a controlling terminal. The terminal check has the problem, that users might want to grep for some debugging message they see and end up with empty results, which is a hard situation to debug if you're not aware of the behavior. The environment variable, on the other hand, is easy to look up if you want to know how to enable debugging …

Best,
—octo

[ghost]

On Fri, Nov 09, 2012 at 11:47:20PM -0800, Florian Forster wrote:

I think I like the idea of using an environment variable better
than my previous idea of testing for a controlling terminal.

The same here :)

---- WBR, Michael Shigorin mike@altlinux.ru
------ Linux.Kiev http://www.linux.kiev.ua/

[faxm0dem]

COLLECTD_TRACE it shall be then