-
Notifications
You must be signed in to change notification settings - Fork 15
[log] [metric] Log Setting API, More TRACE Logs, and Gatherer Support #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…patibility Signed-off-by: hfuss <hayden.fuss@kaleido.io>
…for logging Signed-off-by: hfuss <hayden.fuss@kaleido.io>
…lementation Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
…amically via monitoring server; all dbsql logs to trace Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
EnriqueL8
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @onelapahead - I agree with moving those logs to Trace, I've felt the same level of pain debugging logs and being polluted by that.
I have a few questions on the way the log level is set and some confusions there
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
… name w/ comment Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Co-authored-by: Peter Broadhurst <peter.broadhurst@kaleido.io> Signed-off-by: hayden <haydenfuss@gmail.com>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
| // Wrap the request itself in a log wrapper, that gives minimal request/response and timing info | ||
| l := log.L(ctx) | ||
| l.Infof("--> %s %s", req.Method, req.URL.Path) | ||
| l.Logf(hs.apiEntryLoggingLevel, "--> %s %s", req.Method, req.URL.Path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was thinking you'd grab the local/remote fields back out the context to log here.
peterbroadhurst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working through this. Important stuff 👍
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
EnriqueL8
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great - thanks @onelapahead for working through all the comments 🙇🏼
This is a slightly philosophical PR - but working on codebases using ff-common, I've found I'm often failing to find the log message I need, and often fighting to filter out all the noise of the logs I rarely need in production (but do find helpful in development). Adapted from #200 originally.
In short - I think we log too much at the
debuglevel, especially in these libraries which should mostly logtrace, and not enough at theinfolevel. I worry we depend too much ondebugto "see everything", and we can't guarantee users will actually havedebugon when they hit a new problem. We need to make sureinfohas "enough" (which is very hard to known) to debug know issues / bugs / edge cases, and compensate additionally with metrics and tracing. For unknown issues / networking / database-related issues - thats wheretracecan be enabled temporarily to help triage.Between human SREs, log aggregation systems, and agentic AI - there is a cost to every byte of waste in terms of storage, processing, and context. A "less is more" mindset is necessary. Especially as we consider adopting more modern, performant logging frameworks like https://github.com/uber-go/zap which encourage structured logging and sampling.
All that said - this PR proposes two significant changes:
ffapi,ffresty,fftls,andtodbsqltraceto avoid logging 10-100s of lines per API request (especially when TLS and databases are in play)PUTAPI on the monitoring server:tracelogs referred to above, or go frominfotodebugin a less expensive way, w/o continually over logging.Additionally, exposes the
prometheus.Gatherwithin the metrics managers' Prometheus registry to allow for custom metrics exporting and filtering.