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 a gigantic pile of stuff #2570

Merged
merged 28 commits into from
Apr 21, 2023
Merged

backport a gigantic pile of stuff #2570

merged 28 commits into from
Apr 21, 2023

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Apr 21, 2023

hawkw and others added 23 commits April 21, 2023 14:36
Currently, the Netlify docs are failing because of a warning that one of
the lints we explicitly enable is no longer a warning on nightly but
becoming a hard error. This is because the docs are built on nightly
with `-D warnings`, which denies all warnings...which seems not ideal.

For now, let's not remove that lint from the `deny` list, as it's still
a valid lint on stable. Instead, this PR explicitly allows that lint on
the docs build.
## Motivation

There's currently a `fmt::Display` impl for `EnvFilter` that emits an
equiovalent filter string that can be parsed back into an `EnvFilter`,
but the `Targets` filter does not have a `fmt::Display` impl. We ought
to have one, especially to make using `Targets` with `clap` v4.0 easier.
 
## Solution

This branch adds a `fmt::Display` impl for `filter::Targets`. The
implementation is pretty straightforward.

I also added tests that a `Targets`' `fmt::Display` output can be parsed
back into a filter that's equivalent to the original.
This branch adds the ability to override the level of the events
generated by the `ret` and `err` arguments to `#[instrument]`. An
overridden level can be specified with:

```rust

```
```rust

```
and so on.

This syntax is fully backwards compatible with existing uses of the
attribute.

In addition, some refactoring was done to how levels are parsed and how
the tokens for a specified level is generated.

Fixes #2330
; Conflicts:
;	tracing-attributes/src/lib.rs
…2350)

This branch adds documentation and tests noting that the `#[instrument]`
macro accepts `tracing::Level` directly. Using `tracing::Level` directly
allows for IDE autocomplete and earlier detection of typos.

The documentation for tracing-attributes was also rewritten to remove
the usage of the second-person perspective, making it more consistent
with the rest of tracing's documentation.

Co-authored-by: David Barsky <me@davidbarsky.com>
; Conflicts:
;	tracing-attributes/Cargo.toml
;	tracing-attributes/src/lib.rs
This `doc_cfg` attribute's `cfg` part has multiple predicates without an
`all`, which iis what's breaking the netlify build. I'm...kind of
surprised this ever succeeded, since the cfg is malformed...
## Motivation

The worker thread name in non blocking mode is always "tracing-appender".
It can be convenient to quickly identify the appender threads for
audit reasons or affinity pinning.

## Solution

This patch adds a new setter to the builder and propagates the info to
the thread initialization.

Closes #2364
## Motivation

PR #2270 added an unreachable branch with an explicit return value to
`#[instrument]` in `async fn`s in order to fix type inference issues.
That PR added the appropriate `#[allow]` attribute for the Rust
compiler's unreachable code linting, but not Clippy's, so a Clippy
warning is still emitted. See:
#2270 (comment)

## Solution

Adding the clippy lint warning as discussed here:
#2270 (comment)
The `MakeWriter` trait comes from the `tracing-subscriber` crate,
not `tracing-appender`.
## Motivation

There is a small wording mistake in the tracing-subscriber docs
that can be fixed quickly and easily.

## Solution

Two duplicate words were removed.
Fixes #2383.
; Conflicts:
;	tracing-appender/src/lib.rs
;	tracing-attributes/src/lib.rs
;	tracing-core/src/lib.rs
;	tracing-error/src/lib.rs
;	tracing-futures/src/lib.rs
;	tracing-log/src/lib.rs
;	tracing-opentelemetry/src/lib.rs
;	tracing-serde/src/lib.rs
;	tracing-subscriber/src/lib.rs
;	tracing/src/lib.rs
## Motivation

The `#[instrument]` macro cannot be used on `const fn`s, because the
generated code will perform runtime tracing behavior. However, when
adding the attribute to a `const fn`, the compiler errors generated
currently are somewhat unclear (see #2414). It would be better if we
generated a less verbose error that simply states that `#[instrument]`
is not supported on `const fn`s.

## Solution

This branch changes the `#[instrument]` macro to detect when the
annotated function is a `const fn`, and emit a simpler, more descritpive
error message. The new error simply states that the `#[instrument]`
attribute cannot be used on `const fn`s, and should be much less
confusing to the user.

Fixes #2414
There are new warnings as errors reported by clippy in Rust 1.67.0.

This are causing builds to fail, e.g.:
https://github.com/tokio-rs/tracing/actions/runs/4027112923/jobs/6922513360

In both cases they are reports of lifetimes that can be elided. This
change removes the unnecessary lifetime annotations to make clippy
happy.
## Motivation

The current description for the default level of the `err` return value
event _strongly implies_ it's the same as the span. However, the
implementation actually defaults to `ERROR`.

## Solution

This PR documents that, so future generations don't have to chase down
the truth, like I did. 😉
This PR removes tracing-opentelemetry to a dedicated repo located at
https://github.com/tokio-rs/tracing-opentelemetry. (Note that at time of
writing this PR, the new repo has not be made public). We're moving
tracing-opentelemetry to a dedicated repository for the following
reasons:
1. opentelemetry's MSRV is higher than that of `tracing`'s.
2. more importantly, the main `tracing` repo is getting a bit unweildy
   and it feels unreasonable to maintain backports for crates that
   integrate with the larger tracing ecosystem.

(https://github.com/tokio-rs/tracing-opentelemetry does not have the
examples present in this repo; this will occur in a PR that will be
linked from _this_ PR.)
## Motivation

Make `tracing::event!` codegen smaller

## Solution

Add `inline` to several functions called by `tracing::event!`.

Simple example: https://github.com/ldm0/tracing_test

After inlining, executable size drops from 746kb to 697kb
(`cargo build --release + strip`), saves 50 bytes per `event!`.

Test environment:
```
toolchain: nightly-aarch64-apple-darwin
rustc-version: rustc 1.70.0-nightly (88fb1b922 2023-04-10)
```

There are also performance improvements in the benchmarks:

```
event/scoped [-40.689% -40.475% -40.228%]
event/scoped_recording [-14.972% -14.685% -14.410%]
event/global [-48.412% -48.217% -48.010%]
span_fields/scoped [-25.317% -24.876% -24.494%]
span_fields/global [-39.695% -39.488% -39.242%]
span_repeated/global [-27.514% -26.633% -25.298%]
static/baseline_single_threaded [-32.275% -32.032% -31.808%]
static/single_threaded [-29.628% -29.376% -29.156%]
static/enabled_one [-29.777% -29.544% -29.305%]
static/enabled_many [-30.901% -30.504% -30.140%]
dynamic/baseline_single_threaded [-20.157% -19.880% -19.603%]
```

I retried benchmark several times and the improvements seem to be fairly
stable.

raw log: https://gist.github.com/ldm0/6573935f4979d2645fbcf5bde7361386
Same reason as rust-lang/log#536 :

`cfg_if` is only used in a single place and `tracing` is used by many
other crates, so even removing one dependency will be beneficial.

Remove dependency `cfg-if` and replace `cfg_if::cfg_if!` with a `const
fn get_max_level_inner() -> LevelFilter` and uses `if cfg!(...)` inside.

Using if in const function is stablised in
[1.46](https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1460-2020-08-27)
so this should work fine in msrv 1.56

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
; Conflicts:
;	tracing/Cargo.toml
;	tracing/src/level_filters.rs
Remove unused `syn`s feature `visit`
; Conflicts:
;	tracing-attributes/Cargo.toml
## Motivation

syn 2.0 is out!

## Solution

Update to syn 2.0 🚀

Co-authored-by: David Barsky <me@davidbarsky.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation

Currently it is not possible to disable ANSI in `fmt::Subscriber`
without enabling the "ansi" crate feature. This makes it difficult for
users to implement interoperable settings that are controllable with
crate features without having to pull in the dependencies "ansi" does.

I hit this while writing an application with multiple logging options
set during compile-time and I wanted to cut down on dependencies if
possible.

## Solution

This changes `fmt::Subscriber::with_ansi()` to not require the "ansi"
feature flag. This way, `with_ansi(false)` can be called even when the
"ansi" feature is disabled. Calling `with_ansi(true)` when the "ansi"
feature is not enabled will panic in debug mode, or print a warning if
debug assertions are disabled.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw requested review from a team, jtescher and yaahc as code owners April 21, 2023 23:26
hawkw and others added 4 commits April 21, 2023 16:46
The `tracing-subscriber` module `tests::support` included functionality
to mock a layer (via the `Layer` trait). This code depends on
some items from `tracing_mock::collector` which should otherwise not be
public.

This change moves the mocking functionality inside `tracing-mock` behind
a feature flag. Allowing the `Expect` enum and `MockHandle::new` from
`tracing_mock::collector` to be made `pub(crate)` instead of `pub`.
Since it's now used from two different modules, the `Expect` enum has
been moved to its own module.

This requires a lot of modifications to imports so that we're not doing
wildcard imports from another crate (i.e. in `tracing-subscriber`
importing wildcards from `tracing-mock`).

This PR is based on @hds' PR #2369, but modified to track renamings. I
also deleted all the doc comments temporarily because updating them was
a lot of work and I need to get a release of `tracing-subscriber` out
first.

Closes: #2359
## Motivation

Currently it is not possible to disable ANSI in `fmt::Subscriber`
without enabling the "ansi" crate feature. This makes it difficult for
users to implement interoperable settings that are controllable with
crate features without having to pull in the dependencies "ansi" does.

I hit this while writing an application with multiple logging options
set during compile-time and I wanted to cut down on dependencies if
possible.

## Solution

This changes `fmt::Subscriber::with_ansi()` to not require the "ansi"
feature flag. This way, `with_ansi(false)` can be called even when the
"ansi" feature is disabled. Calling `with_ansi(true)` when the "ansi"
feature is not enabled will panic in debug mode, or print a warning if
debug assertions are disabled.

Co-authored-by: daxpedda <daxpedda@gmail.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
…2568)

updated UI tests using TRYBUILD=overwrite with the latest stable version of Rust

## Motivation

UI tests are failing on the latest stable version of Rust

## Solution

Run `TRYBUILD=overwrite cargo test` to update the effected files.
… dependency (#2566)

 ## Motivation

Missing features for the `regex` crate were causing build time errors
due to the the use of unicode characters in the regex without using
the proper features within the regex crate

 ## Solution

Add the missing feature flags.

Fixes #2565

Authored-by: Devin Bidwell <dbidwell@biddydev.com>
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.