Support custom log levels in slog.Handler Implementation
lvlcn-t opened this issue ยท 2 comments
Issue
When implementing an issue for my logging library, I encountered an issue with your slog.Handler
implementation. Since v0.4.0 introduced custom levels in combination with using your Level
type, I noticed that the slog handler implementation does not support custom log levels. I'd like to see this feature extended to the slog handler implementation as well.
Problem Description
Currently your implementation of the slog.Handler
interface, the conversion from slog.Level
to log.Level
is handled through a static map, fromSlogLevel
. This map explicitly associates slog's default log levels to your own levels. However, this direct mapping presents a limitation when attempting to extend logging levels beyond the defaults provided by you and slog, as any new custom levels are not included in this map, causing the map lookup to fail. This issue does not result in a panic, but it prevents the correct handling of records with custom log levels.
This limitation hinders the development of logging libraries that are built on top of slog and also use charmbracelet/log, as they are forced to implement workarounds like this, to ensure that extended log levels are recognized and handled appropriately:
type Level = slog.Level
const LevelFatal Level = slog.Level(16)
type logger struct { *slog.Logger }
// Fatal logs at [LevelFatal] and then calls os.Exit(1).
func (l *logger) Fatal(msg string, args ...any) {
// This is necessary since the log.Logger slog implementation only recognizes the default slog log levels.
// So this means we need to use their own Log method so that the log level is correctly set.
if h, ok := l.Handler().(*log.Logger); ok {
h.Log(log.Level(LevelFatal), msg, args...)
} else {
l.logAttrs(context.Background(), LevelFatal, msg, args...)
}
os.Exit(1)
}
This workaround is not ideal, as it adds complexity to codebases and requires to manually handle the conversion of custom log levels. It would be more beneficial if you could directly support custom log levels without the need for such workarounds.
Solution Proposal
To address this issue and enhance the flexibility of log level extension, I propose modifying the way of handling slog levels. Instead of relying on a static map for level conversion, you could dynamically cast the provided slog.Level
(of base type int
) to your own Level
type (base type int32
). This approach would allow for a more generic handling of log levels, accommodating both the default levels and any extended levels introduced by developers. Here is a conceptual outline of how the level conversion may be adapted:
// Example adjustment in the Handle method or similar
level := Level(int32(record.Level)) // Directly cast from slog.Level (int) to log.Level (int32)
This solution would simplify the process of integrating extended log levels, eliminating the need for workarounds and fostering greater flexibility and creativity in logging practices.
Additional Context
https://github.com/charmbracelet/log/blob/main/level_121.go#L9
https://github.com/charmbracelet/log/blob/main/logger_121.go#L36
Hy @lvlcn-t, thanks for reporting this issue. I think the proposed proposal is totally reasonable and should be part of Log. With that being said, PRs are always welcome at Charm, feel free to send a patch if you feel like it ๐
@aymanbagabas Sure, I'd be happy to contribute, so feel free to assign me this issue ๐