Skip to content
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

chore: port PRs pointed at v0.1.x to master #2288

Merged
merged 2 commits into from Aug 24, 2022

Conversation

davidbarsky
Copy link
Member

I noticed that #1871 and #2102 targeted the v0.1.x branch and they were never ported to master. This branch fixes that.

(On an aside, I discovered the -Xfind-renames flag on git cherry-pick, which makes handling the layer -> subscriber rename much easier.)

@davidbarsky davidbarsky requested review from hawkw and a team as code owners August 24, 2022 20:11
@davidbarsky davidbarsky force-pushed the david/port-formatting-event-to-master branch from bdee4e0 to 2ff8467 Compare August 24, 2022 20:23
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this --- and I wish I'd known about the -Xfind-renames flag ages ago...

looks like CI is failing: https://github.com/tokio-rs/tracing/runs/8003371895?check_suite_focus=true, can we fix that up? this should be good to merge once CI passes!

@davidbarsky
Copy link
Member Author

davidbarsky commented Aug 24, 2022

Thanks for catching this --- and I wish I'd known about the -Xfind-renames flag ages ago...

Yeah, same, honestly—it makes porting changes between branches so much less painful.

looks like CI is failing: https://github.com/tokio-rs/tracing/runs/8003371895?check_suite_focus=true, can we fix that up? this should be good to merge once CI passes!

Whoops, yeah—it should be fixed now.

`SubscriberBuilder`s and `Layer`s configured with custom event/field
formatters do not provide any means of accessing or mutating those
formatters. Any configuration that needs to be done must be done before
setting them on the builder/layer. This is frustrating as it makes it
difficult to provide a pre-configured API akin to
`tracing_subscriber::fmt()` along with accessors like `.compact()` that
modify the formatter.

Add accessors `.map_event_format()` and `.map_fmt_fields()` to
`SubscriberBuilder` and `Layer` that map the existing formatter through
a closure. This allows the closure to modify it or to derive a new
formatter from it with a different type.

Also add a `.map_writer()` method that does the same thing for the
`MakeWriter`, to round out the accessors for the various type
parameters.

The filter type is currently restricted to just `LevelFilter` or
`EnvFilter` and so this does not add a corresponding `.map_filter()`.
That can be added later if we add the ability to attach arbitrary
filters.

Also fix some minor docs issues that were spotted as part of
implementing this.

Fixes #1756
@davidbarsky davidbarsky force-pushed the david/port-formatting-event-to-master branch from 2ff8467 to 561903e Compare August 24, 2022 20:39
…ter (#2102)

Motivation:
When `Format_event::format_event(...)` returns an error, we are
currently silently dropping that
Event. tokio-rs/valuable#88 explains one
such case in which this was encountered (due to a bug in
valuable-serde). We want to be made aware whenever an Event is dropped.

Solution:
Write to the Writer with an error message to let the user know that
we were unable to format a specific event. If writing to the Writer fails,
we fall back to writing to stderr. We are not emitting an actual tracing
Event, to avoid the risk of a cycle (the new Event could trigger the
same formatting error again).

Resolves #1965.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: David Barsky <me@davidbarsky.com>
@davidbarsky davidbarsky force-pushed the david/port-formatting-event-to-master branch from 561903e to ca3c05b Compare August 24, 2022 20:54
@davidbarsky davidbarsky requested a review from hawkw August 24, 2022 21:05
@hawkw hawkw enabled auto-merge (rebase) August 24, 2022 21:07
@hawkw hawkw merged commit 370a7c1 into master Aug 24, 2022
@hawkw hawkw deleted the david/port-formatting-event-to-master branch August 24, 2022 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants