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

Backport everything up to November 2024 #3144

Merged
merged 21 commits into from
Nov 22, 2024
Merged

Backport everything up to November 2024 #3144

merged 21 commits into from
Nov 22, 2024

Conversation

hds
Copy link
Contributor

@hds hds commented Nov 19, 2024

Motivation

It's probably time to get a release of the core tracing crates out. Also,
since tracing-mock is now ready to be released, it would make sense
to do that all together.

Solution

Backport all the missing changes since the last large backport, roughly
12 months ago.

Checklist

The list is in git log order and should be completed from bottom to top. Commits
marked with (X) and no checkbox were already backported. Commits marked
with (S) have been skipped on purpose.

Additional, older PRs, that missed being backported earlier:

@hds hds requested review from hawkw and a team as code owners November 19, 2024 22:30
@hds hds changed the base branch from v0.1.x to hds/backport-tracing-mock November 19, 2024 22:31
@hds hds marked this pull request as draft November 19, 2024 22:31
@hds hds force-pushed the hds/backport-2024-11 branch 3 times, most recently from 07d3404 to ec1aed2 Compare November 20, 2024 07:43
Copy link
Member

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

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

thank you for doing this one!

Base automatically changed from hds/backport-tracing-mock to v0.1.x November 20, 2024 14:57
SpeedReach and others added 8 commits November 21, 2024 12:27
fixes https://github.com/tokio-rs/tracing/actions/runs/6785393202/job/18443641813

cargo test runs tests in the same file in parallel by default, causing race condition,
this can be proven by running
`cargo test --test reload -- --test-threads=1` => successes
`cargo test --test reload -- --test-threads=2` => flaky
multiple times

This fix runs only the two tests in serial.
We could seperate the tests in different files, but they share the same testing dependencies, so I left them in the same file.
This allows to manually map tracing levels to journald levels.

It seems that @little-dude, who started the original PR, doesn't have
time to finish this, so I picked it up.  Reapplied the changes to the
newest master branch and fixed the latest comments/issues.

This will also fix/close:

Closes #2649
Closes #2661
Closes #2347 (the original pr)
Fix typo "synchonous" => "synchronous" in the crate level documentation.

This commit is empty, it's being backported so that we don't try and
baackport it later.
This results in a substantial performance improvement,
and is compatible with our MSRV.

Signed-off-by: Alex Saveau <[email protected]>
Co-authored-by: Eliza Weisman <[email protected]>
Turbo87 and others added 8 commits November 21, 2024 12:33
## Motivation

In the simple macro cases with e.g. only "name" or only "target" these
prefixes exist, but for the more complicated cases like "name + target"
the prefixes were not piped through and silently ignored. This led to
the compiler complaining about `Value` not being implemented despite the
usage of the `?` or `%` prefixes:

<img width="1056" alt="Bildschirmfoto 2024-02-14 um 12 38 00"
src="https://github.com/tokio-rs/tracing/assets/141300/f8b0b1af-fe66-41d8-9a08-2ecba2d67f9e">


## Solution

This change adds the missing prefixes to the `$($k)` tokens, like they
already exist for the simple cases mentioned above.
If something like `warn!(?foo)` and `warn!(name: "foo", ?foo)` works, then `warn!(name: "foo", target: "foo_events", ?foo)` should arguably also work. Before this change the more complicated variants of the macros however required a message argument, due to the usage of the `+` specifier in the macro definitions. This commit changes the `+` (1 or more) to `*` (0 or more), which matches how the `field` tokens are used in the simpler variants of the macro.
…tion (#2883)


## Motivation

Const argumetns in `level!` macros do not work when in the first
position.

This also seems to have fixed #2748 where literals for fields names like
`info!("foo" = 2)` could not be used outside the `event!` macro.


Fixes #2837
Fixes #2738

## Solution

Previsously, `level!($(args:tt))` was forwarded to `event!(target: ...,
level: ..., { $(args:tt) })` but the added curly braces seem to have
prevented the `event` macro from correctly understanding the arguments
and it tried to pass them to `format!`.

With this change there may have some performance impact when expanding
the macros in most cases where the braces could have been added as it
will take one more step.

These are the two relevant `event!` blocks I believe, the new tests used
to expand to the first one (with empty fields), now they expand to the
latter:
```
    (target: $target:expr, $lvl:expr, { $($fields:tt)* }, $($arg:tt)+ ) => (
        $crate::event!(
            target: $target,
            $lvl,
            { message = $crate::__macro_support::format_args!($($arg)+), $($fields)* }
        )
    );
    (target: $target:expr, $lvl:expr, $($arg:tt)+ ) => (
        $crate::event!(target: $target, $lvl, { $($arg)+ })
    );
```
Users may want to pass data to `Subscribe`s which is not valid UTF-8. Currently, it would have to be encoded into `&str` to be passed as a field value.

This branch adds a `record_bytes` method to `Visit`. It has an implementation falling back to `record_debug` so it is not be a breaking change.

`JsonVisitor` got an overridden implementation as it should not use `Debug` output and encode it as a string, but rather store the bytes as an array.

`PrettyVisitor` go an overridden implementation to make the output more pretty.

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

Co-authored-by: Matthijs Brobbel <[email protected]>
Co-authored-by: Hayden Stainsby <[email protected]>
Co-authored-by: Eliza Weisman <[email protected]>
In the documentation of the layer context span_scope method, the note
contained a reference to a `scope()` method, which was removed some time
ago. Also fixed a phrasing error above.

Fixes: #2890
Currently, a keyword like `type` fails compilation as (a path segment of) a field name, for no clear reason. Trying to use `r#type` instead leads to the `r#` being part of the field name, which is unhelpful¹.

Don't require the field path to match a `macro_rules!` `expr`, use repeated `tt` instead. I can't tell why this was ever required: The internal stringify macro was introduced in 55091c9#diff-315c02cd05738da173861537577d159833f70f79cfda8cd7cf1a0d7a28ace31b with an `expr` matcher without any explanation, and no tests are failing from making it match upstream's `stringify!` input format.

Special thanks to whoever implemented the unstable `macro-backtrace` feature in rustc, otherwise this would have been nigh impossible to track down!

¹ this can likely be fixed too by some sort of "unraw" macro that turns `r#foo` into `foo`, but that's a separate change not made in this PR
…os (#3024)

fix: prefix macro calls with `__macro_support ` to avoid clashes with local macros

Fixes: <#3023>

This ensures that the tracing lib correctly builds when a crate is named
`core`. See #2761 and
#2762 for more info.
Clippy in 1.80.0 alerted us to the fact that `SerializeField` was never
constructed (and due to its non-`pub` member, it can't be constructed
outside the `tracing-serde` crate where it's from).

This change fixes the definition to hold a reference to a `Field`, which
is what the other `Serialize*` types do. It also implements `AsSerde`
for this type and uses it inside the `SerializeFieldSet` type.

As a bonus, Clippy is now also happy that the type is constructed.

The example collector in the `tracing-serde` crate was also renamed from
`JsonSubscriber` to `JsonCollector`.

Some additional doc formatting issues in `tracing-subscriber` were fixed
so that list items that run to multiple lines are correctly indented.
mladedav and others added 3 commits November 22, 2024 14:50
## Motivation

I've created a library for better customization of JSON log lines and would like to make it more discoverable.

This subscriber could help with a lot of issues such as #1531 

## Solution

Add `json-subscriber` to the ecosystem.
## Motivation

Expose the index of the field in order to support cases such as sending field information
by index instead of by the string name in Tokio Console. Details:
tokio-rs/console#462 (comment)

## Solution

Adds a new API which exposes the index of a field, which is how it is stored internally
(together with a reference to the `FieldSet` containing the ordered field names.

Signed-off-by: hi-rustin <[email protected]>
Co-authored-by: Hayden Stainsby <[email protected]>
This modifies the `tracing_subscriber::reload` layer to also set the
`log` crate's max level with the current max `tracing` level filter
after reloading. If reloading the subscriber caused the max `tracing`
level to change, this ensures that the change is propagated to the `log`
crate as well. In the case where the max level was made more verbose,
this will ensure that `log` records which were previously disabled are
enabled correctly; in the case where it was made less verbose, this
improve performance by not having to perfrom more costly filtering for
those `log` records.

The `log` max level is set only after rebuilding the callsite interest
cache, which is what sets the max `tracing` level filter. This ensures
that we pass the latest state to the `log` crate.

Signed-off-by: Eliza Weisman <[email protected]>
@hds hds marked this pull request as ready for review November 22, 2024 14:41
@hds hds requested a review from yaahc as a code owner November 22, 2024 14:41
@hds hds merged commit 6f08af0 into v0.1.x Nov 22, 2024
52 checks passed
@hds hds deleted the hds/backport-2024-11 branch November 22, 2024 14:42
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.