Skip to content

backport tracing subscriber changes #2038

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

Merged
merged 4 commits into from
Apr 1, 2022
Merged

backport tracing subscriber changes #2038

merged 4 commits into from
Apr 1, 2022

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Apr 1, 2022

this backports some stuff from master

@hawkw hawkw requested a review from a team as a code owner April 1, 2022 01:08
@hawkw hawkw enabled auto-merge (rebase) April 1, 2022 01:08
@hawkw hawkw force-pushed the eliza/backport-2035 branch 2 times, most recently from 2d2ed28 to ac62e13 Compare April 1, 2022 01:40
hawkw and others added 4 commits April 1, 2022 09:08
Depends on #2028

## Motivation

In many cases, it is desirable to have a variable number of `Layer`s
at runtime (such as when reading a logging configuration from a config
file). The current approach, where types implementing `Layer` are
composed at the type level into `Layered` layers, doesn't work well
when the number of layers varies at runtime.

## Solution

To solve this, this branch adds a `Layer` implementation for
`Vec<L> where L: Layer`. This allows variable-length lists of
layers to be added to a subscriber. Although the impl for `Vec<L>`
requires all the layers to be the same type, it can also be used in
 onjunction with `Box<dyn Layer<S> + ...>` trait objects to
implement a variable-length list of layers of multiple types.

I also wrote a bunch of docs examples.

## Notes

Alternatively, we could have a separate type defined in
`tracing-subscriber` for a variable-length list of type-erased
`Layer`s. This would have one primary usability benefit, which is
that we could have a `push` operation that takes an `impl Layer`
and boxes it automatically. However, I thought the approach used
here is nicer, since it's based on composing together existing
primitives such as `Vec` and `Box`, rather than adding a whole new API.
Additionally, it allows avoiding the additional `Box`ing in the case
where the list consists of layers that are all the same type.

Closes #1708

Signed-off-by: Eliza Weisman <[email protected]>
This adds more modification methods for use with reloading. Replaces
#2032.

## Motivation

I have a `Filtered` layer that I'd like to modify with a reload handle. If
I use `reload` then the filter doesn't work. If I use `modify` with
`filter_mut` I can't update the writer.

## Solution

Adds some additional accessor methods to allow the writer to be modified
in `modify`:

* `Filtered::inner`
* `Filtered::inner_mut`
* `fmt::Layer::writer`
* `fmt::Layer::writer_mut`
* `fmt::Layer::set_ansi`
## Motivation

Closes #1892

## Solution

This branch adds new `span_enabled!` and `event_enabled!` macros.

I went withe sharing as much of the macro as possible. The new macros
were implemented by adding a `kind:` parameter to `enabled!`, and invoking
`enabled!` with the appropriate `kind:` argument. This means in the
docs, the macro only shows the `($($rest:tt)*)=> (...)` variant, but I
think linking to the `enabled!` docs should be fine. Also, I find the
macro pattern part of rustdoc to be very hard to understand, and I think
many people dont look at it.

Co-authored-by: Eliza Weisman <[email protected]>
## Motivation

Currently, `EnvFilter` will always interpret all field value filter
directives that are not numeric,or boolean literals as regular
expressions that are matched against a field's `fmt::Debug` output. In
many cases, it may not be desirable to use regular expressions in this
case, so users may prefer to perform literal matching of `fmt::Debug`
output against a string value instead. Currently, this is not possible.

## Solution

This branch introduces the ability to control whether an `EnvFilter`
interprets field value `fmt::Debug` match filters as regular expressions
or as literal strings. When matching literal `fmt::Debug` patterns, the
string is matched without requiring a temporary allocation for the
formatted representation by implementing `fmt::Write` for a matcher type
and "writing" the field's `Debug` output to it. This is similar to the
technique already used for matching regular expression patterns.

Since there is not currently a nice way to specify configurations prior
to parsing an `EnvFilter` from a string or environment variable, I've
also added a builder API. This allows setting things like whether field
value filters should use strict matching or regular expressions.

## Notes

Ideally, I think we would want the filter language to allow specifying
whether a field value filter should be interpreted as a regular
expression or as a literal string match. Instead of having a global
toggle between regular expressions and literal strings, we would
introduce new syntax for indicating that a value match pattern is a
regular expression. This way, a single filter can have both regular
expression and literal string value matchers. The `with_regex(false)`
configuration would just return an error any time the regex syntax was
used when parsing the filter string.

However, this would be a breaking change in behavior. Currently, field
value filters are interpreted as regex by default, so changing the
parser to only interpret a value filter as a regex if there's additional
syntax indicating it's a regex would break existing filter
configurations that rely on regex matching.

In `tracing-subscriber` 0.4, we should definitely consider introducing
new syntax to indicate a match pattern is a regex, and change the
`with_regex` method's behavior to disallow the use of that syntax. For
now, however, making it a global toggle at least allows users to control
whether or not we use regex matching, so this is a significant
improvement for v0.3.x.

Signed-off-by: Eliza Weisman <[email protected]>
@hawkw hawkw force-pushed the eliza/backport-2035 branch from ac62e13 to 261e372 Compare April 1, 2022 16:08
@hawkw hawkw merged commit e81ba60 into v0.1.x Apr 1, 2022
@hawkw hawkw deleted the eliza/backport-2035 branch April 1, 2022 16:37
hawkw pushed a commit that referenced this pull request Apr 11, 2022
## Motivation

There's a broken link and broken formatting in the [mod-level docs] for
`tracing_subscriber::layer` referring to the `Layer` impl for `Vec`s of
`Layer`s. Looks like it got mangled by search+replace in #2038 so it's
not an issue in master.

[mod-level docs]: https://docs.rs/tracing-subscriber/0.3.11/tracing_subscriber/layer/index.html#runtime-configuration-with-layers

## Solution

I have stolen the correct markdown and link from master.
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
## Motivation

There's a broken link and broken formatting in the [mod-level docs] for
`tracing_subscriber::layer` referring to the `Layer` impl for `Vec`s of
`Layer`s. Looks like it got mangled by search+replace in tokio-rs#2038 so it's
not an issue in master.

[mod-level docs]: https://docs.rs/tracing-subscriber/0.3.11/tracing_subscriber/layer/index.html#runtime-configuration-with-layers

## Solution

I have stolen the correct markdown and link from master.
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.

4 participants