tracing, tracing-futures: instrument Future inside Drop
#2562
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hawkw
reviewed
Apr 14, 2023
There was a problem hiding this comment.
overall, this change looks good to me! i had some minor suggestions, but no hard blockers with the approach used here.
also, it would be nice to update the documentation for Instrumented (and the instrument methods) to state that the span is also entered when the Instrumented future is dropped, so that users are aware of this feature.
as a follow up, we might want to also consider making WithDispatch do something similar, so that the dispatcher context is entered when dropping the future.
…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 <[email protected]>
|
I've addressed all the review suggestions and plan to follow up with similar changes to |
hawkw
added a commit
that referenced
this pull request
Apr 21, 2023
## 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 <[email protected]> Co-authored-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this pull request
Apr 21, 2023
## 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 <[email protected]> Co-authored-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this pull request
Apr 21, 2023
## 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 <[email protected]> Co-authored-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this pull request
Apr 25, 2023
# 0.1.38 (April 25th, 2023) This `tracing` release changes the `Drop` implementation for `Instrumented` `Future`s so that the attached `Span` is entered when dropping the `Future`. This means that events emitted by the `Future`'s `Drop` implementation will now be recorded within its `Span`. It also adds `#[inline]` hints to methods called in the `event!` macro's expansion, for an improvement in both binary size and performance. Additionally, this release updates the `tracing-attributes` dependency to [v0.1.24][attrs-0.1.24], which updates the [`syn`] dependency to v2.x.x. `tracing-attributes` v0.1.24 also includes improvements to the `#[instrument]` macro; see [the `tracing-attributes` 0.1.24 release notes][attrs-0.1.24] for details. ### Added - `Instrumented` futures will now enter the attached `Span` in their `Drop` implementation, allowing events emitted when dropping the future to occur within the span (#2562) - `#[inline]` attributes for methods called by the `event!` macros, making generated code smaller (#2555) - **attributes**: `level` argument to `#[instrument(err)]` and `#[instrument(ret)]` to override the level of the generated return value event (#2335) - **attributes**: Improved compiler error message when `#[instrument]` is added to a `const fn` (#2418) ### Changed - `tracing-attributes`: updated to [0.1.24][attrs-0.1.24] - Removed unneeded `cfg-if` dependency (#2553) - **attributes**: Updated [`syn`] dependency to 2.0 (#2516) ### Fixed - **attributes**: Fix `clippy::unreachable` warnings in `#[instrument]`-generated code (#2356) - **attributes**: Removed unused "visit" feature flag from `syn` dependency (#2530) ### Documented - **attributes**: Documented default level for `#[instrument(err)]` (#2433) - **attributes**: Improved documentation for levels in `#[instrument]` (#2350) Thanks to @nitnelave, @jsgf, @Abhicodes-crypto, @LukeMathWalker, @andrewpollack, @quad, @klensy, @davidpdrsn, @dbidwell94, @ldm0, @NobodyXu, @ilsv, and @daxpedda for contributing to this release! [`syn`]: https://crates.io/crates/syn [attrs-0.1.24]: https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.24
hawkw
added a commit
that referenced
this pull request
Apr 25, 2023
# 0.1.38 (April 25th, 2023) This `tracing` release changes the `Drop` implementation for `Instrumented` `Future`s so that the attached `Span` is entered when dropping the `Future`. This means that events emitted by the `Future`'s `Drop` implementation will now be recorded within its `Span`. It also adds `#[inline]` hints to methods called in the `event!` macro's expansion, for an improvement in both binary size and performance. Additionally, this release updates the `tracing-attributes` dependency to [v0.1.24][attrs-0.1.24], which updates the [`syn`] dependency to v2.x.x. `tracing-attributes` v0.1.24 also includes improvements to the `#[instrument]` macro; see [the `tracing-attributes` 0.1.24 release notes][attrs-0.1.24] for details. ### Added - `Instrumented` futures will now enter the attached `Span` in their `Drop` implementation, allowing events emitted when dropping the future to occur within the span (#2562) - `#[inline]` attributes for methods called by the `event!` macros, making generated code smaller (#2555) - **attributes**: `level` argument to `#[instrument(err)]` and `#[instrument(ret)]` to override the level of the generated return value event (#2335) - **attributes**: Improved compiler error message when `#[instrument]` is added to a `const fn` (#2418) ### Changed - `tracing-attributes`: updated to [0.1.24][attrs-0.1.24] - Removed unneeded `cfg-if` dependency (#2553) - **attributes**: Updated [`syn`] dependency to 2.0 (#2516) ### Fixed - **attributes**: Fix `clippy::unreachable` warnings in `#[instrument]`-generated code (#2356) - **attributes**: Removed unused "visit" feature flag from `syn` dependency (#2530) ### Documented - **attributes**: Documented default level for `#[instrument(err)]` (#2433) - **attributes**: Improved documentation for levels in `#[instrument]` (#2350) Thanks to @nitnelave, @jsgf, @Abhicodes-crypto, @LukeMathWalker, @andrewpollack, @quad, @klensy, @davidpdrsn, @dbidwell94, @ldm0, @NobodyXu, @ilsv, and @daxpedda for contributing to this release! [`syn`]: https://crates.io/crates/syn [attrs-0.1.24]: https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.24
kaffarell
pushed a commit
to kaffarell/tracing
that referenced
this pull request
May 22, 2024
…#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 <[email protected]> Co-authored-by: Eliza Weisman <[email protected]>
kaffarell
pushed a commit
to kaffarell/tracing
that referenced
this pull request
May 22, 2024
# 0.1.38 (April 25th, 2023) This `tracing` release changes the `Drop` implementation for `Instrumented` `Future`s so that the attached `Span` is entered when dropping the `Future`. This means that events emitted by the `Future`'s `Drop` implementation will now be recorded within its `Span`. It also adds `#[inline]` hints to methods called in the `event!` macro's expansion, for an improvement in both binary size and performance. Additionally, this release updates the `tracing-attributes` dependency to [v0.1.24][attrs-0.1.24], which updates the [`syn`] dependency to v2.x.x. `tracing-attributes` v0.1.24 also includes improvements to the `#[instrument]` macro; see [the `tracing-attributes` 0.1.24 release notes][attrs-0.1.24] for details. ### Added - `Instrumented` futures will now enter the attached `Span` in their `Drop` implementation, allowing events emitted when dropping the future to occur within the span (tokio-rs#2562) - `#[inline]` attributes for methods called by the `event!` macros, making generated code smaller (tokio-rs#2555) - **attributes**: `level` argument to `#[instrument(err)]` and `#[instrument(ret)]` to override the level of the generated return value event (tokio-rs#2335) - **attributes**: Improved compiler error message when `#[instrument]` is added to a `const fn` (tokio-rs#2418) ### Changed - `tracing-attributes`: updated to [0.1.24][attrs-0.1.24] - Removed unneeded `cfg-if` dependency (tokio-rs#2553) - **attributes**: Updated [`syn`] dependency to 2.0 (tokio-rs#2516) ### Fixed - **attributes**: Fix `clippy::unreachable` warnings in `#[instrument]`-generated code (tokio-rs#2356) - **attributes**: Removed unused "visit" feature flag from `syn` dependency (tokio-rs#2530) ### Documented - **attributes**: Documented default level for `#[instrument(err)]` (tokio-rs#2433) - **attributes**: Improved documentation for levels in `#[instrument]` (tokio-rs#2350) Thanks to @nitnelave, @jsgf, @Abhicodes-crypto, @LukeMathWalker, @andrewpollack, @quad, @klensy, @davidpdrsn, @dbidwell94, @ldm0, @NobodyXu, @ilsv, and @daxpedda for contributing to this release! [`syn`]: https://crates.io/crates/syn [attrs-0.1.24]: https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.24
hds
added a commit
to tokio-rs/console
that referenced
this pull request
Oct 22, 2024
For users who build tokio-console by installing it with `cargo`, it is common to use the `--locked` flag to indicate that the dependency versions from the lock file should be used. So it's worth having those dependencies up to date. This change checks in the result of running `cargo update` without changing the manifest (`Cargo.toml`) files. The patch (pre-1.0 minor) version change to `tonic-build` included a deprecation (rename) and some clippy lint allow directives in the generated Rust files. The update to `tracing` in the lockfile from 0.1.37 to 0.1.40 includes the changes from tokio-rs/tracing#2562. This change causes an instrumented future to be entered one final time when the future is dropped, so that any code run in the drop implementation will be in the scope of the span. This causes the number of polls recorded for a task to be incremented by 1 right before it is dropped. Due to this change, some of the `console-subscriber` tests needed to be updated. This behaviour was already present for many application using the `console-subscriber`, but not yet in the tests because we hadn't updated the lockfile for a while.
hds
added a commit
to tokio-rs/console
that referenced
this pull request
Oct 22, 2024
For users who build tokio-console by installing it with `cargo`, it is common to use the `--locked` flag to indicate that the dependency versions from the lock file should be used. So it's worth having those dependencies up to date. This change checks in the result of running `cargo update` without changing the manifest (`Cargo.toml`) files. The patch (pre-1.0 minor) version change to `tonic-build` included a deprecation (rename) and some clippy lint allow directives in the generated Rust files. The update to `tracing` in the lockfile from 0.1.37 to 0.1.40 includes the changes from tokio-rs/tracing#2562. This change causes an instrumented future to be entered one final time when the future is dropped, so that any code run in the drop implementation will be in the scope of the span. This causes the number of polls recorded for a task to be incremented by 1 right before it is dropped. Due to this change, some of the `console-subscriber` tests needed to be updated. This behaviour was already present for many application using the `console-subscriber`, but not yet in the tests because we hadn't updated the lockfile for a while.
hds
added a commit
to tokio-rs/console
that referenced
this pull request
Oct 22, 2024
For users who build tokio-console by installing it with `cargo`, it is common to use the `--locked` flag to indicate that the dependency versions from the lock file should be used. So it's worth having those dependencies up to date. This change checks in the result of running `cargo update` without changing the manifest (`Cargo.toml`) files. The patch (pre-1.0 minor) version change to `tonic-build` included a deprecation (rename) and some clippy lint allow directives in the generated Rust files. The update to `tracing` in the lockfile from 0.1.37 to 0.1.40 includes the changes from tokio-rs/tracing#2562. This change causes an instrumented future to be entered one final time when the future is dropped, so that any code run in the drop implementation will be in the scope of the span. This causes the number of polls recorded for a task to be incremented by 1 right before it is dropped. Due to this change, some of the `console-subscriber` tests needed to be updated. This behaviour was already present for many application using the `console-subscriber`, but not yet in the tests because we hadn't updated the lockfile for a while.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
#2541
Solution
Wrap inner
FutureofInstrumentedinManuallyDropto execute innerDropwith enteredSpan.