You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
typeLevel= slog.LevelconstLevelFatalLevel=slog.Level(16)
typeloggerstruct { *slog.Logger }
// Fatal logs at [LevelFatal] and then calls os.Exit(1).func (l*logger) Fatal(msgstring, 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.ifh, 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 similarlevel:=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.
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 🙂
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 yourLevel
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 fromslog.Level
tolog.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:
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 typeint
) to your ownLevel
type (base typeint32
). 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: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
The text was updated successfully, but these errors were encountered: