feat(promslog): add Level() method to get slog.Level #795
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a `Level()` method to the promslog.Level type. We already expose the configured level with the `String()` method, but only in a specifically processed format. This means that downstream projects which need a true `slog.Level` value need to do shenanigans like we do in the blackbox_exporter here[1]. The slog.Level type is really just an int behind the scenes, but buys us access to the type's methods, obviously. If we want to make this a bit more flexible/"future proof", we could instead have the method return an slog.Leveler interface type. I don't think it's necessary though, as we'd probably only want to do that if we intended to expose access to promslog.Level's internal slog.LevelVar (since that also satisfies the slog.Leveler interface), and I don't think that's a good idea -- giving direct access would allow users to interact with the level var directly, providing an avenue to change level configurations outside of promslog's knowledge, resulting in unexpected behavior at best and incompatible behavior at worst. I think providing access to the value of the level as a proper slog.Level value as done here is useful enough on it's own and the safer option. 1. https://github.com/prometheus/blackbox_exporter/blob/f77c50ed7c0f39b734235931e773cf7b5af1fc8a/prober/handler.go#L236-L249 2. https://pkg.go.dev/log/slog#Level Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a
Level()method to the promslog.Level type. We already exposethe configured level with the
String()method, but only in aspecifically processed format. This means that downstream projects which
need a true
slog.Levelvalue need to do shenanigans like we do in theblackbox_exporter here[1]. The slog.Level type is really just an int
behind the scenes, but buys us access to the type's methods, obviously.
If we want to make this a bit more flexible/"future proof", we could
instead have the method return an slog.Leveler interface type. I don't
think it's necessary though, as we'd probably only want to do that if we
intended to expose access to promslog.Level's internal slog.LevelVar
(since that also satisfies the slog.Leveler interface), and I don't
think that's a good idea -- giving direct access would allow users to
interact with the level var directly, providing an avenue to change
level configurations outside of promslog's knowledge, resulting in
unexpected behavior at best and incompatible behavior at worst. I think
providing access to the value of the level as a proper slog.Level value
as done here is useful enough on it's own and the safer option.
Signed-off-by: TJ Hoplock t.hoplock@gmail.com