Unify the server logs and the gokit logging.#870
Unify the server logs and the gokit logging.#870tomwilkie merged 4 commits intocortexproject:masterfrom
Conversation
tomwilkie
force-pushed
the
logging
branch
2 times, most recently
from
f96a91a to
db27c66
Compare
tomwilkie
changed the title
|
Testing & this works nicely - we get decent logs from the servers again. |
bboreham
left a comment
bboreham
left a comment
There was a problem hiding this comment.
Looks ok; one nit.
Also I'm not clear what replaced AllowedLevel, but that's mostly curiosity.
Please rebase on the updated weaveworks/common
| // This code copy-pasted from prometheus/common/promlog.New() | ||
| l := log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr)) | ||
| l = al.Filter(l) | ||
| logger := log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr)) |
There was a problem hiding this comment.
Is the comment still correct? ("This code copy-pasted from prometheus/common/promlog.New()")
There was a problem hiding this comment.
Not in any meaningful way, no. Will remove.
Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
Thats basically replaced with https://github.com/weaveworks/common/blob/master/logging/level.go |
Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
|
Can I interpret "Looks ok" as LGTM? :-) |
bboreham
left a comment
bboreham
left a comment
There was a problem hiding this comment.
LGTM.
I notice kuberesolver was updated; don't know what difference that makes.
Good catch. Let me look into that, as there hasn't been semver on that repo before, and a bunch of breaking changes have gone in... |
|
Looks safe to me - its pre v2 (when the API changed) and pretty much only adds a change by you, Bryan! |
|
I missed that the metric I filed weaveworks/promrus#11 |
2 participants
Fixes #835
Also tidies up the
prometheusLogger(which keep metrics on number of log lines per level). Probably want to move this into common.Depends on weaveworks/common#96, so start there first