Simplified logging middleware; Fields are now "any" type; Moved logging providers to examples only. #552
Conversation
|
You might be interested in this @yashrsharma44 - I changed a lot the API, IMO for simpler. Let me know what you think! (: |
bwplotka
force-pushed
the
v2slog
branch
2 times, most recently
from
eadbe9e
to
0553e09
Compare
…ng providers to examples only. Fixes #517 * Simpler interface, only one method. This allows adapter to be ultra simple, so moved them to examples only (!). * Fields are string, values are any. * "More" slog compatibility. I did not go for using slog natively as it's still experimental (see bigger discussion: https://gophers.slack.com/archives/C029RQSEE/p1679860885943619) Signed-off-by: bwplotka <bwplotka@gmail.com>
| return logging.LoggerFunc(func(_ context.Context, lvl logging.Level, msg string, fields ...any) { | ||
| largs := append([]any{"msg", msg}, fields...) | ||
| switch lvl { | ||
| case logging.LevelDebug: |
There was a problem hiding this comment.
Should we use ERROR instead of introducing yet another enum for logging levels? Would be nice to have parity with logging conventions, for enums, instead of having yet another enum?
While it's understandable, I would still prefer using ERROR instead, unless there are some breaking changes in using the older convention :)
There was a problem hiding this comment.
What is ERROR? ERROR was replaced by LevelError and comon levels from slog there.
There was a problem hiding this comment.
Okay, I meant we were using logging.ERROR before, but if slog uses that - link, then let's use that convention.
| // Second, we wanted to make it easy to use levels to specify logger verbosity. | ||
| // Since a larger level means a more severe event, a logger that accepts events | ||
| // with smaller (or more negative) level means a more verbose logger. Logger | ||
| // verbosity is thus the negation of event severity, and the default verbosity |
There was a problem hiding this comment.
Should using "inverse" would be a better option? Negation implies that our definition is tied up in the implementation(we are negating in this case), however using inverse means that lower the log level(enum) included, more verbose our logging is, which is simple to understand.
There was a problem hiding this comment.
I just copied all of this from slog, so there is less maintenance on our side (: So if you don't like this you can talk to slog authors 🙈 (https://pkg.go.dev/golang.org/x/exp/slog#pkg-overview)... I feel we can use whatever, so I thought we could use what others most likely start doing more.
| // Logger requires Log method, similar to experimental slog, allowing logging interceptor to be interoperable. Official | ||
| // adapters for popular loggers are in `provider/` directory (separate modules). It's totally ok to copy simple function | ||
| // implementation over. | ||
| // TODO(bwplotka): Once slog is official, we could use slog method directly. Currently level is copied over, so we don't |
Fixes #517
I did not go for using slog natively as it's still experimental (see bigger discussion: https://gophers.slack.com/archives/C029RQSEE/p1679860885943619)