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

Kaifastromai public writer new #1

Merged
merged 29 commits into from
Aug 10, 2023
Merged

Conversation

kaifastromai
Copy link
Owner

Motivation

As seen here tokio-rs#2512 and tokio-rs#2223. Previously pr'ed here tokio-rs#2525, but no progress has been made there for quite some. I have applied the suggested changes. Not sure the formatting of the doc is sufficient or otherwise correct

Solution

Make the format::Writer::new() function public. I don't see any obvious trade-offs, but I am not familiar with the larger direction of tracing/subscriber, so I may be wrong.

davidbarsky and others added 29 commits April 4, 2023 16:18
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.)
As part of upgrading syn to 2.0 (e.g.,
tokio-rs#2516), we need to bump the MSRV
to 1.56. As part of this PR, I've:
- Updated the text descriptions of what would be an in-policy MSRV bump
  to use more recent versions of rustc. The _niceness_ of said version
  numbers are purely coincidental.
- I've removed some of the exceptions made in CI.yml in order to support
  some crates with a higher MSRV.
…-rs#2555)


## 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
## Motivation

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.

## Solution

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>
Bump up the version of the `time` crate so that we don't need to
build with `--cfg unsound_local_offset` for using `fmt::time::LocalTime`.
## 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>
…#2550)"

This reverts commit 9744ec0. This
change breaks MSRV compatibility, and was accidentally auto-merged due
to what appears to be an issue with the CI configuration, which
(apparently) doesn't require the MSRV minimal-versions check runs to
complete before allowing a branch to merge.

We'll have to solve the original issue here through documentation for
now. See [this comment] for details.

[this comment]: tokio-rs#2550 (comment)
…rs#2532)

## 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>
…#2562)

## 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>
…okio-rs#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 (tokio-rs#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 tokio-rs#2565

Authored-by: Devin Bidwell <dbidwell@biddydev.com>
We should make it explicit that tracing uses the security policy of Tokio.
…tokio-rs#2590)

## Motivation

This was inconsistent with other features, which mention their
requirements, and a reader might assume they don't need to inspect the
feature flags page or manifest.
…okio-rs#2609)

Co-authored-by: Hayden Stainsby <hds@caffeineconcepts.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
The purpose of this test is to assert two clones of the same span are
equal to each other, so the clone is kind of the whole point of the
test. This commit adds an allow attribute to make clippy shut up about
it.
## Motivation

Module `tracing::field` documentation is missing a word.

## Solution

Fixing the typo by adding `event`.
## Motivation

tokio-rs#2609 added an allow to generated code to allow a lint that was added in
Clippy 1.70.0. This was released with a patch bump so anyone who uses an
older version and latest tracing gets a compilation warning about an
unkonwn lint.

## Solution

Allowing unkonwn lints should fix this now and prevent similar issues in
the future. If the lints are unknown it will most likely be because the
lints are introduced only in newer compiler. There is just a higher risk
that a future contributor tries to add another allow and if they make a
typo, the issue will not be caught.
## Motivation

This PR adds two new accessor functions that are useful for creating a
structured serde implementation for tracing.

This is a sort of "distilled" version of
tokio-rs#2113, based on the `v0.1.x`
branch.

As it is unlikely that "structured serde" will be 1:1 compatible with
the existing JSON-based `tracing-serde` (or at least - I'm not sure how
to do it in a reasonable amount of effort), these functions will allow
me to make a separate crate to hold me over until breaking formatting
changes are possible in `tracing-serde`.

CC @hawkw, as we've discussed this pretty extensively
There has been interest around publishing `tracing-mock` to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.

This change adds documentation to the collector module itself and to all the
public APIs in the module. This includes doctests on all the methods
that serve as examples.

Additionally the implementation for the `Expect` struct has been moved
into the module with the definition, this was missed in tokio-rs#2369.

Refs: tokio-rs#539
## Motivation

A deadlock exists when a collector's `register_callsite` method calls
into code that also contains tracing instrumentation and triggers a
second `register_callsite` call for the same callsite. This is because
the current implementation of the `MacroCallsite` type holds a
`core::sync::Once` which it uses to ensure that it is only added to the
callsite registry a single time. This deadlock was fixed in v0.1.x in PR
tokio-rs#2083, but the issue still exists on v0.2.x.

## Solution

This branch forward-ports the solution from tokio-rs#2083. Rather than using a
`core::sync::Once`, we now track the callsite's registration state
directly in `MacroCallsite`. If a callsite has started registering, but
has not yet completed, subsequent `register` calls will just immediately
receive an `Interest::sometimes` until the registration has completed,
rather than waiting to attempt their own registration.

I've also forward-ported the tests for this that were added in tokio-rs#2083.
…std prelude (tokio-rs#2621)

## Motivation

Currently, in many places, macros do not use fully qualified names for
items exported from the prelude. This means that naming collisions
(`struct Some`) or the removal of the std library prelude will cause
compilation errors.

## Solution

- Identify and use fully qualified names in macros were we previously
  assumed the Rust std prelude. We use `::core` rather than `::std`.
- Add
  [`no_implicit_prelude`](https://doc.rust-lang.org/reference/names/preludes.html#the-no_implicit_prelude-attribute)
  to `tracing/tests/macros.rs`. I'm unsure if this is giving us good
  coverage - can we improve on this approach? I'm not confident I've
  caught everything.
A number of people (myself included) have expressed significant interest in having Writer::new() public for various reasons, with some difficulty getting traction.
@kaifastromai kaifastromai merged commit 76a4fbd into master Aug 10, 2023
@kaifastromai kaifastromai deleted the kaifastromai-public-writer-new branch August 10, 2023 17:29
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.