-
Notifications
You must be signed in to change notification settings - Fork 662
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
log: add ignored target prefixes to LogTracer
#215
Conversation
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
/// [`init_with_filter`]: ../fn.init_with_filter.html | ||
#[derive(Debug)] | ||
pub struct LogTracer { | ||
ignore_crates: Box<[String]>, |
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.
Why dis over Vec<String>
?
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.
Once we actually construct a LogTracer
, the vector would never grow again, so I thought changing it to a Box<[String]>
indicated that more clearly. Theoretically, it is also slightly smaller since it lacks a capacity field, but since this is essentially a singleton that isn't really important.
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.
Interesting, hadn't considered that. 👍
/// initializing it. | ||
/// | ||
/// [`builder`]: #method.builder | ||
pub fn init_with_filter(level: log::LevelFilter) -> Result<(), log::SetLoggerError> { |
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.
Do you think it makes sense to keep this method on LogTracer
now it can be done with the builder? (and maybe for the init
method too).
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 left it because I thought it was better not to break any code currently using this method.
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.
Ok 👍
Motivation
People writing libraries using tracing may wish to expose regular
log
support via the
tracing/log
feature. At the same time, to get tracingsupport for crates using
log
, users may turn totracing_log::LogTracer
. Enabling both means logging events twice.Solution
This branch adds a
Builder
to thetracing_log::LogTracer
type, whichallows configuring the
LogTracer
to ignore log records with targetsbeginning with a certain string. This can be used to ignore any crates
which are emitting log records via the
tracing/log
feature.Ideally, we would provide a way to do this automatically, without
requiring configuration, but that'll take some thinking. The ability to
ignore certain sets of log records may be valuable even if we have an
automatic method for crates to signal that they are using the
tracing/log
feature in the future.I've also moved the
LogTracer
andTraceLogger
types intracing-log
out of
lib.rs
into their own modules, since the main lib file wasgetting a bit lengthy.
Refs: #204
Signed-off-by: Eliza Weisman eliza@buoyant.io