watchdog.execute logs should be behind debug flag?

[g-hodgson-tup]

Hi, I was looking for a way to reduce the amount of log messages from watchdog.execute, and noticed that one of the log statements is not behind a debug flag

Currently the logic is:

    if debug && !result.Success {
		log.Printf("[watchdog.execute] Monitored group=%s; endpoint=%s; success=%v; errors=%d; duration=%s; body=%s", ep.Group, ep.Name, result.Success, len(result.Errors), result.Duration.Round(time.Millisecond), result.Body)
	} else {
		log.Printf("[watchdog.execute] Monitored group=%s; endpoint=%s; success=%v; errors=%d; duration=%s", ep.Group, ep.Name, result.Success, len(result.Errors), result.Duration.Round(time.Millisecond))
	}

and I wonder if it should instead be:

    if debug {
        if !result.Success {
            log.Printf("[watchdog.execute] Monitored group=%s; endpoint=%s; success=%v; errors=%d; duration=%s; body=%s", ep.Group, ep.Name, result.Success, len(result.Errors), result.Duration.Round(time.Millisecond), result.Body)
        } else {
            log.Printf("[watchdog.execute] Monitored group=%s; endpoint=%s; success=%v; errors=%d; duration=%s", ep.Group, ep.Name, result.Success, len(result.Errors), result.Duration.Round(time.Millisecond))
        }
    }

?

[TwiN]

This is the expected behavior.

The only difference between debug and non-debug, as far as that piece of code is concerned, is that if we're debugging, the response body should be printed

[garyhodgson]

Ok. thanks for the quick response!

Would you consider a PR which puts the second log statement behind a "verbose" flag (default true), in order to reduce the number of log messages?

[TwiN]

I've been putting this off for a while honestly, give me a couple hours, I'll sort this out.

[TwiN]

Alright, so because I like writing my own version of everything, I decided to write my own logging library to deal with this issue.

Having both debug and verbose as a parameter wouldn't quite sit right with me, so instead, I would rather we add support for logging levels - DEBUG, INFO, WARN and ERROR.

wrt our specific situation, we'd go from:

if debug && !result.Success {
    log.Printf("[watchdog.execute] Monitored group=%s; endpoint=%s; success=%v; errors=%d; duration=%s; body=%s", ep.Group, ep.Name, result.Success, len(result.Errors), result.Duration.Round(time.Millisecond), result.Body)
} else {
    log.Printf("[watchdog.execute] Monitored group=%s; endpoint=%s; success=%v; errors=%d; duration=%s", ep.Group, ep.Name, result.Success, len(result.Errors), result.Duration.Round(time.Millisecond))
}

to:

if loggingLevel == logr.LevelDebug {
    logr.Debugf("[watchdog.execute] Monitored group=%s; endpoint=%s; success=%v; errors=%d; duration=%s; body=%s", ep.Group, ep.Name, result.Success, len(result.Errors), result.Duration.Round(time.Millisecond), result.Body)
} else {
    logr.Infof("[watchdog.execute] Monitored group=%s; endpoint=%s; success=%v; errors=%d; duration=%s", ep.Group, ep.Name, result.Success, len(result.Errors), result.Duration.Round(time.Millisecond))
}

and to achieve what you want to achieve (having less logs), you'd be expected to set the threshold to warn like so:

logr.SetThreshold(logr.LevelWarn)

(of course, you'd have to make the logr.LevelWarn in the example above configurable as opposed to the hardcoded example above)

I just threw together https://github.com/TwiN/logr because I miss programming, I had some free time & I've been wanting to make a logging library for this for a long time, but what do you think?

For what it's worth, I wouldn't expect you to find/set the appropriate level for each existing log.Print* calls - I can take care of that once you've taken care of the aforementioned code.

[garyhodgson]

Hi.

Some form of log levels sounds sensible. I had a go at an implementation here: #872

[TwiN]

FYI this has now been implemented by #872 and #895