-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Allow a regex filter for RUST_LOG #16553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -89,6 +89,23 @@ hello,std::option // turns on hello, and std's option logging | |||
error,hello=warn // turn on global error logging and also warn for hello | |||
``` | |||
|
|||
## Filtering results | |||
|
|||
A RUST_LOG directive may include a regex filter. The syntax is to append `/` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you describe what is being filtered here? (i.e. that it is applied to the message that will be printed, rather than the module path.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and can you specify a regex for each module, or can there only be one for the whole RUST_LOG
, applying globally?
@huonw updated for your comments, thanks! |
Sadly, I can't reproduce these errors locally. |
Am I correct in assuming the regex cannot include a comma? It might be nice to state this explicitly. Ideally there would be some syntax of escaping a comma for inclusion, such as backslash-escaping it, but it's probably ok to ignore that for now. |
Actually, reading the code, the regex applies to all crates, not just to a single crate, and thus can contain commas. That's very surprising. I would have expected the regex to be per-crate. |
To be more specific, I would expect the actual syntax to look a bit more like the following:
With this syntax, the filter is per-crate, and actually occurs before the log level. Which suggests that the following should be possible:
i.e. set the base log level for the mod to Also, with this syntax, I would expect commas and equals signs within the regex to be treated as part o the regex. Similarly, I would expect to be able to backslash-escape the closing
|
@kballard this is by design. My use case is that I insert a load of debug statements in different modules, and use a tag word in each. Then I want to match the same regex for each module (just to pick out the tag). |
@huonw r? |
let mods = parts.next(); | ||
let filter = parts.next(); | ||
if parts.next().is_some() { | ||
println!("warning: invalid logging spec '{}', \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two println!
s should go to stderr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was following convention in the file. Do you prefer to change all uses to stderr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@huonw ping on this question
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, it probably should; but I'm happy with just leaving as is and filing a bug.
r=me with the minor comments. |
I'm still concerned that this design is tailored very closely to one particular use-case of @nick29581, does not match the semantics that one would expect upon learning that |
@kballard I'm not sure what your use case is for the more flexible approach, so I'm erring in the direction of the YAGNI principle here. I think it is fine to break this in the future if there is need for the more flexible approach - there is no backwards-compatibility guarantee with logging. I also don't think the more flexible approach is necessarily what "one would expect upon learning that RUST_LOG supports regexes", that seems purely subjective. (I should also state a broader use case which is logging multiple modules but filtering on some cross-cutting string, e.g., finding all references to a particular set of expressions across type checking and trans). |
That's a fair point. Given that, I am much less concerned about committing this in its current form.
My feeling is that while you might want to find some cross-cutting string across a bunch of modules, I might want to simply enable debug logs on several modules and filter each module to just the output I want from that module. The interesting output from one module might look different than the interesting output from a second. |
When specifying RUST_LOG, the programmer may append `/regex` to the end of the spec. All results will then be filtered using that regex.
@bors: retry |
When specifying RUST_LOG, the programmer may append `/regex` to the end of the spec. All results will then be filtered using that regex. r?
fix: Imrpove recover on `=` for record field initializer and pattern
When specifying RUST_LOG, the programmer may append
/regex
to the end of the spec. All results will then be filtered using that regex.r?