fix: make debug param log output work regardless of log level#1438
Merged
Conversation
Fixes issue reported here: prometheus#1381 (comment) In aligning behavior for how the `--log.prober` flag is currently expected to work, I didn't test it's effect on logs when used with the `debug` URL parameter, as is often done for testing. This changes the handler to always write to the internal buffer logger (which is what is used to store the logs for displaying with the `debug` url param), and it only writes through to the real logger if it's enabled for the level the prober is set to. This ensures that logs are always available with the `debug` param, while still correctly filtering levels for console output at stderr. As a consequence of thinking about how to make this work, I also realized it wasn't necessary to manually iterate record attrs or maintain our own attr list when hijacking the log call -- the level can be overridden directly from the record, we just need to grab a copy via Clone() first. So should be more efficient, as well. Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>
Contributor
Author
Contributor
Author
|
@electron0zero should we get this fix in now while we gather more input for #1377 (comment) ? |
Member
|
my bad for not getting to it, thanks for the PR 🙇🏼 |
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes issue reported here:
#1381 (comment)
In aligning behavior for how the
--log.proberflag is currentlyexpected to work, I didn't test it's effect on logs when used with the
debugURL parameter, as is often done for testing.This changes the handler to always write to the internal buffer logger
(which is what is used to store the logs for displaying with the
debugurl param), and it only writes through to the real logger if it's
enabled for the level the prober is set to. This ensures that logs are
always available with the
debugparam, while still correctly filteringlevels for console output at stderr.
As a consequence of thinking about how to make this work, I also
realized it wasn't necessary to manually iterate record attrs or
maintain our own attr list when hijacking the log call -- the level can
be overridden directly from the record, we just need to grab a copy via
Clone() first. So should be more efficient, as well.
Signed-off-by: TJ Hoplock t.hoplock@gmail.com