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

Events missing using tracing-core 0.1.24 but works using 0.1.23 #2050

Closed
davidpdrsn opened this issue Apr 6, 2022 · 4 comments
Closed

Events missing using tracing-core 0.1.24 but works using 0.1.23 #2050

davidpdrsn opened this issue Apr 6, 2022 · 4 comments
Assignees
Labels
crate/core Related to the `tracing-core` crate kind/bug Something isn't working

Comments

@davidpdrsn
Copy link
Member

davidpdrsn commented Apr 6, 2022

Bug Report

Version

tracing-core v0.1.24

Platform

Darwin Davids-MacBook-Air.local 20.4.0 Darwin Kernel Version 20.4.0: Thu Apr 22 21:46:41 PDT 2021;
root:xnu-7195.101.2~1/RELEASE_ARM64_T8101 x86_64

The bug is also there on Linux and Windows.

Crates

tracing-core, maybe tracing-subscriber?

Description

At Embark Studios we noticed that our volume of logs was significantly lower after updating to tracing-core 0.1.24. Turns out we were loosing events emitted after a certain point in out bootstrap process. No events were being sent to the subscriber. Downgrading to tracing-core 0.1.23 fixes it.

Reproducing it:

fn main() {
    // our final subscriber requires a bunch of dependencies and is created quite late in our app's
    // bootstrap process. So we create a very simple "early subscriber" to still get events from before
    // the final subscriber is created.
    //
    // This `early_log_guard` represents that.
    let early_log_guard = tracing::dispatcher::set_default(&tracing::dispatcher::Dispatch::new(
        tracing_subscriber::fmt::fmt().finish(),
    ));

    // log something while the "early subscriber" is active
    tracing::info!("from early subscriber");

    // create the final subscriber. This is much more involved in our actual code but this is
    // enough to reproduce the bug
    let subscriber = tracing_subscriber::fmt::fmt().finish();

    // set the final logger as the global subscriber
    tracing::dispatcher::set_global_default(tracing::dispatcher::Dispatch::new(subscriber))
        .expect("logger already set");

    // log something
    tracing::info!("set global subscriber");

    // remove the early log guard
    drop(early_log_guard);

    // log something else
    tracing::info!("dropped early subscriber");
}

Cargo.toml:

[package]
name = "foo"
version = "0.1.0"
edition = "2021"

[dependencies]
tracing-core = "=0.1.23"
tracing = "0.1"
tracing-subscriber = "0.3"

Running this works as expected and we see the three log events printed:

2022-04-06T09:28:24.721223Z  INFO foo: from early subscriber
2022-04-06T09:28:24.721793Z  INFO foo: set global subscriber
2022-04-06T09:28:24.721845Z  INFO foo: dropped early subscriber

However changing tracing-core = "=0.1.23" to tracing-core = "=0.1.24" causes the final "dropped early subscriber" event to not be printed. Instead we just see

2022-04-06T09:30:04.342320Z  INFO foo: from early subscriber
2022-04-06T09:30:04.342492Z  INFO foo: set global subscriber

I would expect both of these to work the same.

@shaun-cox
Copy link
Contributor

I'm seeing this behavior too and can confirm that locking to tracing-core = "=0.1.23" is a work around. Thanks @davidpdrsn for spotting it and giving a nice repro.

@hawkw hawkw added kind/bug Something isn't working crate/core Related to the `tracing-core` crate labels Apr 11, 2022
@hawkw
Copy link
Member

hawkw commented Apr 11, 2022

Yup, that's definitely a bug --- I think this was almost certainly introduced in #2001.

@hawkw hawkw self-assigned this Apr 11, 2022
hawkw added a commit that referenced this issue Apr 11, 2022
hawkw added a commit that referenced this issue Apr 11, 2022
## Motivation

PR #2001 introduced --- or rather, _uncovered_ --- a bug which occurs
when a global default subscriber is set *after* a scoped default has
been set.

When the scoped default guard is dropped, it resets the
thread-local default cell to whatever subscriber was the default when
the scoped default was set. This allows nesting scoped default contexts.
However, when there was *no* default subscriber when the `DefaultGuard`
was created, it sets the "previous" subscriber as `NoSubscriber`. This
means dropping a `DefaultGuard` that was created before any other
subscriber was set as default will reset that thread's default to
`NoSubscriber`. Because #2001 changed the dispatcher module to stop
using `NoSubscriber` as a placeholder for "use the global default if one
exists", this means that the global default is permanently clobbered on
the thread that set the scoped default prior to setting the global one.

## Solution

This PR changes the behavior when creating a `DefaultGuard` when no
default has been set. Instead of populating the "previous" dispatcher
with `NoSubscriber`, it instead leaves the `DefaultGuard` with a `None`.
When the `DefaultGuard` is dropped, if the subscriber is `None`, it will
just clear the thread-local cell, rather than setting it to
`NoSubscriber`. This way, the next time the cell is accessed, we will
check if a global default exists to populate the thread-local, and
everything works correctly. As a side benefit, this also makes the code
a bit simpler!

I've also added a test reproducing the bug.

This PR is against `v0.1.x` rather than `master`, because the issue does
not exist on `master` due to other implementation differences in v0.2.
We may want to forward-port the test to guard against future
regressions, though.

Fixes #2050
@hawkw
Copy link
Member

hawkw commented Apr 11, 2022

PR #2065 fixes this.

hawkw added a commit that referenced this issue Apr 12, 2022
## Motivation

PR #2001 introduced --- or rather, _uncovered_ --- a bug which occurs
when a global default subscriber is set *after* a scoped default has
been set.

When the scoped default guard is dropped, it resets the
thread-local default cell to whatever subscriber was the default when
the scoped default was set. This allows nesting scoped default contexts.
However, when there was *no* default subscriber when the `DefaultGuard`
was created, it sets the "previous" subscriber as `NoSubscriber`. This
means dropping a `DefaultGuard` that was created before any other
subscriber was set as default will reset that thread's default to
`NoSubscriber`. Because #2001 changed the dispatcher module to stop
using `NoSubscriber` as a placeholder for "use the global default if one
exists", this means that the global default is permanently clobbered on
the thread that set the scoped default prior to setting the global one.

## Solution

This PR changes the behavior when creating a `DefaultGuard` when no
default has been set. Instead of populating the "previous" dispatcher
with `NoSubscriber`, it instead leaves the `DefaultGuard` with a `None`.
When the `DefaultGuard` is dropped, if the subscriber is `None`, it will
just clear the thread-local cell, rather than setting it to
`NoSubscriber`. This way, the next time the cell is accessed, we will
check if a global default exists to populate the thread-local, and
everything works correctly. As a side benefit, this also makes the code
a bit simpler!

I've also added a test reproducing the bug.

This PR is against `v0.1.x` rather than `master`, because the issue does
not exist on `master` due to other implementation differences in v0.2.
We may want to forward-port the test to guard against future
regressions, though.

Fixes #2050
@hawkw
Copy link
Member

hawkw commented Apr 22, 2022

Fixed by #2065

@hawkw hawkw closed this as completed Apr 22, 2022
kaffarell pushed a commit to kaffarell/tracing that referenced this issue May 22, 2024
…2065)

## Motivation

PR tokio-rs#2001 introduced --- or rather, _uncovered_ --- a bug which occurs
when a global default subscriber is set *after* a scoped default has
been set.

When the scoped default guard is dropped, it resets the
thread-local default cell to whatever subscriber was the default when
the scoped default was set. This allows nesting scoped default contexts.
However, when there was *no* default subscriber when the `DefaultGuard`
was created, it sets the "previous" subscriber as `NoSubscriber`. This
means dropping a `DefaultGuard` that was created before any other
subscriber was set as default will reset that thread's default to
`NoSubscriber`. Because tokio-rs#2001 changed the dispatcher module to stop
using `NoSubscriber` as a placeholder for "use the global default if one
exists", this means that the global default is permanently clobbered on
the thread that set the scoped default prior to setting the global one.

## Solution

This PR changes the behavior when creating a `DefaultGuard` when no
default has been set. Instead of populating the "previous" dispatcher
with `NoSubscriber`, it instead leaves the `DefaultGuard` with a `None`.
When the `DefaultGuard` is dropped, if the subscriber is `None`, it will
just clear the thread-local cell, rather than setting it to
`NoSubscriber`. This way, the next time the cell is accessed, we will
check if a global default exists to populate the thread-local, and
everything works correctly. As a side benefit, this also makes the code
a bit simpler!

I've also added a test reproducing the bug.

This PR is against `v0.1.x` rather than `master`, because the issue does
not exist on `master` due to other implementation differences in v0.2.
We may want to forward-port the test to guard against future
regressions, though.

Fixes tokio-rs#2050
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/core Related to the `tracing-core` crate kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants