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

tracing: add Span::or_current to help with efficient propagation #1538

Merged
merged 7 commits into from
Sep 4, 2021

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Sep 4, 2021

This adds a new Span::or_current method that returns the span it's
called on, if that span is enabled, or the current span if that span
is not enabled.

This should provide a more efficient alternative to writing code like

tokio::spawn(
    future
        .instrument(some_span)
        .in_current_span()
);

since it doesn't wrap the future in two different spans which are
(potentially) both entered on polls. It's also a little more concise
to write, which (hopefully) will encourage people to use it instead
of the less efficient alternative.

Span::or_current may be useful in other cases as well.

This should provide a more efficient alternative to writing
```rust
tokio::spawn(
    future
        .instrument(some_span)
        .in_current_span()
);
```
since it doesn't wrap the future in two different spans which are
(potentially) both entered on polls.

As a follow-up, we may want to also consider adding an `Instrument`-like
function that automatically calls `or_current`.
@hawkw hawkw added the crate/tracing Related to the `tracing` crate label Sep 4, 2021
@hawkw hawkw requested a review from a team September 4, 2021 03:43
@hawkw hawkw self-assigned this Sep 4, 2021
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.

Two notes:

  1. I feel like this the necessity of this combinator illustrates why don't really like levels in spans.
  2. When backporting, be sure to swap out "collector" for "subscriber"!

tracing/src/span.rs Outdated Show resolved Hide resolved
/// the [current span], if this span [is disabled].
///
/// This method can be useful when propagating spans to spawned threads or
/// [async tasks]. For example, consider the following:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// [async tasks]. For example, consider the following:
/// [async tasks]. Consider the following:

tracing/src/span.rs Outdated Show resolved Hide resolved
tracing/src/span.rs Outdated Show resolved Hide resolved
tracing/src/span.rs Outdated Show resolved Hide resolved
tracing/src/span.rs Outdated Show resolved Hide resolved
Co-authored-by: David Barsky <me@davidbarsky.com>
@hawkw
Copy link
Member Author

hawkw commented Sep 4, 2021

@davidbarsky

  1. I feel like this the necessity of this combinator illustrates why don't really like levels in spans.

Sure...but it's also not exclusively due to span levels. Even if spans don't have levels, you could still have a similar scenario if you're enabling spans based on their targets. For example, a parent span might be propagated by a child in a module you disable by target.

It's a consequence of any kind of filtering of spans, not just by level. Although, I will admit that it's easier to end up in this situation with level-based filtering, I think that's more a consequence of most people doing level-based filtering.

tracing/src/span.rs Outdated Show resolved Hide resolved
@hawkw
Copy link
Member Author

hawkw commented Sep 4, 2021

@davidbarsky couple quick questions before I merge this:

  1. As a follow-up, what do you think about adding a method to Instrument that automatically calls or_current...maybe Instrument::propagate? Alternatively, we could change instrument to always call .or_current(), and add a separate combinator to Instrument that's "this span, or nothing"...but I'm a little more leery of changing the current behavior, especially in 0.1.
  2. Do you think the docs ordering makes sense? i put the example with threads before the example with async tasks, but I think more users will probably be spawning tasks than threads. However, I thought the thread based example was a little more straightforward since it uses enter rather than Instrument, and I thought it made more sense to introduce the basic behavior of or_current without also having to talk about the Instrument combinator...

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw merged commit 8817e40 into master Sep 4, 2021
@hawkw hawkw deleted the eliza/or-current-span branch September 4, 2021 18:15
hawkw added a commit that referenced this pull request Sep 4, 2021
…1538)

This adds a new `Span::or_current` method that returns the span it's
called on, if that span is enabled, or the current span if that span
is not enabled.

This should provide a more efficient alternative to writing code like
```rust
tokio::spawn(
    future
        .instrument(some_span)
        .in_current_span()
);
```
since it doesn't wrap the future in two different spans which are
(potentially) both entered on polls. It's also a little more concise
to write, which (hopefully) will encourage people to use it instead
of the less efficient alternative.

`Span::or_current` may be useful in other cases as well.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Sep 4, 2021
…1538)

This adds a new `Span::or_current` method that returns the span it's
called on, if that span is enabled, or the current span if that span
is not enabled.

This should provide a more efficient alternative to writing code like
```rust
tokio::spawn(
    future
        .instrument(some_span)
        .in_current_span()
);
```
since it doesn't wrap the future in two different spans which are
(potentially) both entered on polls. It's also a little more concise
to write, which (hopefully) will encourage people to use it instead
of the less efficient alternative.

`Span::or_current` may be useful in other cases as well.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Sep 13, 2021
# 0.1.27 (September 13, 2021)

This release adds a new [`Span::or_current`] method to aid in
efficiently propagating span contexts to spawned threads or tasks.
Additionally, it updates the [`tracing-core`] version to [0.1.20] and
the [`tracing-attributes`] version to [0.1.16], ensuring that a number
of new features in those crates are present.

### Fixed

- **instrument**: Added missing `WithSubscriber` implementations for
  futures and other types ([#1424])

### Added

- `Span::or_current` method, to help with efficient span context
  propagation ([#1538])
- **attributes**: add `skip_all` option to `#[instrument]` ([#1548])
- **attributes**: record primitive types as primitive values rather than
  as `fmt::Debug` ([#1378])
- **core**: `NoSubscriber`, a no-op `Subscriber` implementation
  ([#1549])
- **core**: Added `Visit::record_f64` and support for recording
  floating-point values ([#1507], [#1522])
- A large number of documentation improvements and fixes ([#1369],
  [#1398], [#1435], [#1442], [#1524], [#1556])

Thanks to new contributors @dzvon and @mbergkvist, as well as @teozkr,
@maxburke, @LukeMathWalker, and @jsgf, for contributing to this release!

[`Span::or_current`]: https://docs.rs/tracing/0.1.27/tracing/struct.Span.html#method.or_current
[`tracing-core`]: https://crates.io/crates/tracing-core
[`tracing-attributes`]: https://crates.io/crates/tracing-attributes
[`tracing-core`]: https://crates.io/crates/tracing-core
[0.1.20]: https://github.com/tokio-rs/tracing/releases/tag/tracing-core-0.1.20
[0.1.16]: https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.16
[#1424]: #1424
[#1538]: #1538
[#1548]: #1548
[#1378]: #1378
[#1507]: #1507
[#1522]: #1522
[#1369]: #1369
[#1398]: #1398
[#1435]: #1435
[#1442]: #1442
hawkw added a commit that referenced this pull request Sep 13, 2021
# 0.1.27 (September 13, 2021)

This release adds a new [`Span::or_current`] method to aid in
efficiently propagating span contexts to spawned threads or tasks.
Additionally, it updates the [`tracing-core`] version to [0.1.20] and
the [`tracing-attributes`] version to [0.1.16], ensuring that a number
of new features in those crates are present.

### Fixed

- **instrument**: Added missing `WithSubscriber` implementations for
  futures and other types (#1424)

### Added

- `Span::or_current` method, to help with efficient span context
  propagation (#1538)
- **attributes**: add `skip_all` option to `#[instrument]` (#1548)
- **attributes**: record primitive types as primitive values rather than
  as `fmt::Debug` (#1378)
- **core**: `NoSubscriber`, a no-op `Subscriber` implementation
  (#1549)
- **core**: Added `Visit::record_f64` and support for recording
  floating-point values (#1507, #1522)
- A large number of documentation improvements and fixes (#1369,
  #1398, #1435, #1442, #1524, #1556)

Thanks to new contributors @dzvon and @mbergkvist, as well as @teozkr,
@maxburke, @LukeMathWalker, and @jsgf, for contributing to this release!

[`Span::or_current`]: https://docs.rs/tracing/0.1.27/tracing/struct.Span.html#method.or_current
[`tracing-core`]: https://crates.io/crates/tracing-core
[`tracing-attributes`]: https://crates.io/crates/tracing-attributes
[`tracing-core`]: https://crates.io/crates/tracing-core
[0.1.20]: https://github.com/tokio-rs/tracing/releases/tag/tracing-core-0.1.20
[0.1.16]: https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.16
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
# 0.1.27 (September 13, 2021)

This release adds a new [`Span::or_current`] method to aid in
efficiently propagating span contexts to spawned threads or tasks.
Additionally, it updates the [`tracing-core`] version to [0.1.20] and
the [`tracing-attributes`] version to [0.1.16], ensuring that a number
of new features in those crates are present.

### Fixed

- **instrument**: Added missing `WithSubscriber` implementations for
  futures and other types (tokio-rs#1424)

### Added

- `Span::or_current` method, to help with efficient span context
  propagation (tokio-rs#1538)
- **attributes**: add `skip_all` option to `#[instrument]` (tokio-rs#1548)
- **attributes**: record primitive types as primitive values rather than
  as `fmt::Debug` (tokio-rs#1378)
- **core**: `NoSubscriber`, a no-op `Subscriber` implementation
  (tokio-rs#1549)
- **core**: Added `Visit::record_f64` and support for recording
  floating-point values (tokio-rs#1507, tokio-rs#1522)
- A large number of documentation improvements and fixes (tokio-rs#1369,
  tokio-rs#1398, tokio-rs#1435, tokio-rs#1442, tokio-rs#1524, tokio-rs#1556)

Thanks to new contributors @dzvon and @mbergkvist, as well as @teozkr,
@maxburke, @LukeMathWalker, and @jsgf, for contributing to this release!

[`Span::or_current`]: https://docs.rs/tracing/0.1.27/tracing/struct.Span.html#method.or_current
[`tracing-core`]: https://crates.io/crates/tracing-core
[`tracing-attributes`]: https://crates.io/crates/tracing-attributes
[`tracing-core`]: https://crates.io/crates/tracing-core
[0.1.20]: https://github.com/tokio-rs/tracing/releases/tag/tracing-core-0.1.20
[0.1.16]: https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/tracing Related to the `tracing` crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants