Allow customizing logged span events #40
Merged
Conversation
This file contains 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
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
This was referenced Aug 22, 2023
Closed
evanrelf
previously approved these changes
Aug 22, 2023
Comment on lines
+20
to
+30
| fn value_variants<'a>() -> &'a [Self] { | ||
| &[ | ||
| Self(FmtSpan::NEW), | ||
| Self(FmtSpan::ENTER), | ||
| Self(FmtSpan::EXIT), | ||
| Self(FmtSpan::CLOSE), | ||
| Self(FmtSpan::NONE), | ||
| Self(FmtSpan::ACTIVE), | ||
| Self(FmtSpan::FULL), | ||
| ] | ||
| } |
There was a problem hiding this comment.
-
I'm surprised this lifetime can't be elided?
-
Could this list be generated with the Rust equivalent of
[minBound .. maxBound]in Haskell? The compiler won't tell you if a new variant is added, making this list incomplete.
There was a problem hiding this comment.
- Me too! I guess the trait definition has an explicit lifetime so you're not allowed to elide it here?
Compilation error when I elide the lifetime
error[E0106]: missing lifetime specifier
--> src/clap/fmt_span.rs:20:28
|
20 | fn value_variants() -> &[Self] {
| ^ expected named lifetime parameter
|
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
help: consider using the `'static` lifetime
|
20 | fn value_variants() -> &'static [Self] {
| +++++++
error[E0195]: lifetime parameters or bounds on method `value_variants` do not match the trait declaration
--> src/clap/fmt_span.rs:20:22
|
20 | fn value_variants() -> &[Self] {
| ^ lifetimes do not match method in trait
Some errors have detailed explanations: E0106, E0195.
For more information about an error, try `rustc --explain E0106`.
- I wish! This has been an open issue since before Rust 1.0 (peep the Ruby-like syntax in the original issue here). There's a few crates that implement this functionality for
enumtypes with#[derive]macros, but they're pretty heavy so we don't use them here.
Comment on lines
+32
to
+46
| fn to_possible_value(&self) -> Option<PossibleValue> { | ||
| Some(match self.0 { | ||
| FmtSpan::NEW => PossibleValue::new("new").help("Log when spans are created"), | ||
| FmtSpan::ENTER => PossibleValue::new("enter").help("Log when spans are entered"), | ||
| FmtSpan::EXIT => PossibleValue::new("exit").help("Log when spans are exited"), | ||
| FmtSpan::CLOSE => PossibleValue::new("close").help("Log when spans are dropped"), | ||
| FmtSpan::NONE => PossibleValue::new("none").help("Do not log span events"), | ||
| FmtSpan::ACTIVE => { | ||
| PossibleValue::new("active").help("Log when spans are entered/exited") | ||
| } | ||
| FmtSpan::FULL => PossibleValue::new("full").help("Log all span events"), | ||
| _ => { | ||
| return None; | ||
| } | ||
| }) |
There was a problem hiding this comment.
Is this one of those permanently non-exhaustive enum things, or are you ignoring some of the options? If the latter, why?
There was a problem hiding this comment.
The former. The FmtSpan type is actually defined as a wrapper around a u8, not an enum:
pub struct FmtSpan(u8);
impl FmtSpan {
pub const NEW: FmtSpan = FmtSpan(1 << 0);
// ...
}
https://docs.rs/tracing-subscriber/latest/tracing_subscriber/fmt/format/struct.FmtSpan.html
So these look like enum variants, but they're actually constants in the type's namespace. (You can tell something funky is up because they're written in ALL_CAPS rather than TitleCase.)
In short: this is necessary to make the compiler not complain.
Base automatically changed from
rebeccat/dux-1318-move-ghci-out-of-arcmutex_
to
main
August 22, 2023 23:48
9999years
force-pushed
the
rebeccat/dux-1319-customizable-span-events
branch
from
488220b to
d7c407c
Compare
evanrelf
approved these changes
Aug 22, 2023
9999years
added a commit
that referenced
this pull request
Aug 23, 2023
Currently, new/close span events are logged (that is, when a span is created for the first time, usually when an `#[instrument]`ed function is called, and when spans are closed, usually when those functions return). It can be useful to disable those for terser logs, or to enable more events for greater detail. This change adds a `--trace-spans` argument to customize which span events are logged. Before shipping 1.0, we should clean up the option names. Maybe `--tracing-filter` could be `--log` or something.
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.
Currently, new/close span events are logged (that is, when a span is created for the first time, usually when an
#[instrument]ed function is called, and when spans are closed, usually when those functions return). It can be useful to disable those for terser logs, or to enable more events for greater detail.This change adds a
--trace-spansargument to customize which span events are logged.Before shipping 1.0, we should clean up the option names. Maybe
--tracing-filtercould be--logor something.