Make module data dir and config reading logs debug level #4566
Conversation
viambot
added
the
safe to test
| @@ -73,7 +73,7 @@ func newCloudWatcher(ctx context.Context, config *Config, logger logging.Logger) | |||
| } | |||
| newConfig, err := readFromCloud(cancelCtx, config, prevCfg, false, checkForNewCert, logger) | |||
| if err != nil { | |||
| logger.Errorw("error reading cloud config", "error", err) | |||
| logger.Debugw("error reading cloud config; will try again", "error", err) | |||
There was a problem hiding this comment.
I (and apparently SEs) have found this log to be output quite frequently on machines with a poor network connection. Particularly when we're in the firstRead case, and the timeout for reading is lowered, this path is not really an "error." I would be in favor of lowering this to the debug level.
There was a problem hiding this comment.
I'm bought into not needing to log this every 10 seconds. I do like having something loosely track whether we're online or offline, but happy to find a way to get that into FTDC.
For robot startup, do we at least log whether we've got a cloud config or are using the cached config? I would like to keep a log line for that. But happy to have it manifest as a simple "we're using a cloud config" or "we're using the cached config". And not inferring which config is getting used based on seeing "error reading cloud config".
There was a problem hiding this comment.
do we at least log whether we've got a cloud config or are using the cached config?
Good question; we only log when we are relying on the cache. These logs still appear with my changes:
2024-11-19T18:44:06.644Z WARN rdk config/reader.go:621 failed to read config from cloud, checking cache {"error":"context deadline exceeded"}
2024-11-19T18:44:06.646Z WARN rdk config/reader.go:637 unable to get cloud config; using cached version {"config last updated":"2024-11-19T13:43:54.075-0500","error":"context deadline exceeded"
We do not log when we are relying on a cloud config. I could add that, but I also feel that since that's the default expected behavior, maybe we don't want a log for it?
There was a problem hiding this comment.
We do not log when we are relying on a cloud config. I could add that, but I also feel that since that's the default expected behavior, maybe we don't want a log for it?
I think we can live with that. Being explicit wouldn't hurt either.
The config/reader.go:621 log line -- is there any value in that one existing? My guess is that the "error, context deadline exceeded' that's logged as part of 637 is just a copy of the error reported by 621?
There was a problem hiding this comment.
The config/reader.go:621 log line -- is there any value in that one existing?
Yeah I don't think so; removed because your guess below is exactly correct and nothing too critical happens between the two log lines.
My guess is that the "error, context deadline exceeded' that's logged as part of 637 is just a copy of the error reported by 621?
| @@ -73,7 +73,7 @@ func newCloudWatcher(ctx context.Context, config *Config, logger logging.Logger) | |||
| } | |||
| newConfig, err := readFromCloud(cancelCtx, config, prevCfg, false, checkForNewCert, logger) | |||
| if err != nil { | |||
| logger.Errorw("error reading cloud config", "error", err) | |||
| logger.Debugw("error reading cloud config; will try again", "error", err) | |||
There was a problem hiding this comment.
I'm bought into not needing to log this every 10 seconds. I do like having something loosely track whether we're online or offline, but happy to find a way to get that into FTDC.
For robot startup, do we at least log whether we've got a cloud config or are using the cached config? I would like to keep a log line for that. But happy to have it manifest as a simple "we're using a cloud config" or "we're using the cached config". And not inferring which config is getting used based on seeing "error reading cloud config".
|
Converting to draft until we find fix for data races that new goutils version brought up. |
viambot
added
safe to test
benjirewis
force-pushed
the
quieter-logs
branch
from
6306dde
to
0a250a2
Compare
viambot
added
safe to test
viambot
added
safe to test
No description provided.