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

Simplify common case of immediately entering the span #1252

Merged
merged 8 commits into from
Feb 19, 2021

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Feb 19, 2021

This PR allows the following API:

let _guard = tracing::span!("foo").entered();

See https://github.com/tokio-rs/tracing/issues/zc for an extended
discussion.

This probably needs better docs, so that the people know that this API exists, but I would prefer to avoid further polishing the docs myself :)

Closes #1246
Closes #192
Closes #79

@matklad matklad requested review from davidbarsky, hawkw and a team as code owners February 19, 2021 08:47
This PR allows the following API:

```
let _guard = tracing::span!("foo").entered();
```

See tokio-rs#1246 for an extended
discussion.
This fns are monomorphic, so an `#[inline]` is necessary to alow
cross-crate inlining at all.
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

This looks great! I went ahead and added a suggestion with some more complete documentation, based on the existing ones for enter.

Once this PR merges, I'll go ahead and backport it to v0.1.x as well. We may also want to make some of the existing examples more concise by using entered instead of enter, but that's probably better off as a follow-up.

Thanks for working on this!

tracing/src/span.rs Show resolved Hide resolved
pub fn exit(mut self) -> Span {
// One does not simply move out of a struct with `Drop`.
let span = mem::replace(&mut self.span, Span::none());
span.do_exit();
Copy link
Member

Choose a reason for hiding this comment

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

That this will call do_exit again on Span::none() when the EnteredSpan drops at the end of the method, but that should just nop, since it's now None. Might be worth a comment noting that, but it's not really a blocker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought about doing

let me = ManuallyDrop::new(me);
mem::replace(&mut me, Span::none()).do_exit();

but figured that a simple implementation withe the generic "tricky bit" comment would be overall better. The drop behavior is checked by the test, so it is unlikely to regress.

Comment on lines +1300 to +1307
impl Deref for EnteredSpan {
type Target = Span;

#[inline]
fn deref(&self) -> &Span {
&self.span
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Good call!

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

hawkw commented Feb 19, 2021

This also closes #192 and #79. I updated the PR description to add those issues as well.

@hawkw hawkw self-assigned this Feb 19, 2021
@matklad
Copy link
Contributor Author

matklad commented Feb 19, 2021

Once this PR merges, I'll go ahead and backport it to v0.1.x as well.

That'd be great, thanks!

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

whoops, i made a couple mistakes in the doctests...this is what i get for trying to write examples directly in the github web UI rather than checking out the branch :/

tracing/src/span.rs Outdated Show resolved Hide resolved
tracing/src/span.rs Outdated Show resolved Hide resolved
tracing/src/span.rs Show resolved Hide resolved
tracing/src/span.rs Outdated Show resolved Hide resolved
@hawkw hawkw merged commit 02f3074 into tokio-rs:master Feb 19, 2021
@matklad matklad deleted the entered_span branch February 19, 2021 22:08
hawkw added a commit that referenced this pull request Feb 20, 2021
* Simplify common case of immediately entering the span

This PR allows the following API:

```
let _guard = tracing::span!("foo").entered();
```

See #1246 for an extended
discussion.

* Add missing inlines

This fns are monomorphic, so an `#[inline]` is necessary to alow
cross-crate inlining at all.

* improve method documentation

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Feb 22, 2021
* Simplify common case of immediately entering the span

This PR allows the following API:

```
let _guard = tracing::span!("foo").entered();
```

See #1246 for an extended
discussion.

* Add missing inlines

This fns are monomorphic, so an `#[inline]` is necessary to alow
cross-crate inlining at all.

* improve method documentation

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Feb 22, 2021
* Simplify common case of immediately entering the span

This PR allows the following API:

```
let _guard = tracing::span!("foo").entered();
```

See #1246 for an extended
discussion.

* Add missing inlines

This fns are monomorphic, so an `#[inline]` is necessary to alow
cross-crate inlining at all.

* improve method documentation

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Feb 23, 2021
* Simplify common case of immediately entering the span

This PR allows the following API:

```
let _guard = tracing::span!("foo").entered();
```

See #1246 for an extended
discussion.

* Add missing inlines

This fns are monomorphic, so an `#[inline]` is necessary to alow
cross-crate inlining at all.

* improve method documentation

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Feb 23, 2021
* Simplify common case of immediately entering the span

This PR allows the following API:

```
let _guard = tracing::span!("foo").entered();
```

See #1246 for an extended
discussion.

* Add missing inlines

This fns are monomorphic, so an `#[inline]` is necessary to alow
cross-crate inlining at all.

* improve method documentation

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Feb 23, 2021
Added

- `Span::entered` method for entering a span and moving it into a guard
  by value rather than borrowing it (#1252)

Thanks to @matklad for contributing to this release!

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Feb 23, 2021
Added

- `Span::entered` method for entering a span and moving it into a guard
  by value rather than borrowing it (#1252)

Thanks to @matklad for contributing to this release!

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Feb 23, 2021
Added

- `Span::entered` method for entering a span and moving it into a guard
  by value rather than borrowing it (#1252)

Thanks to @matklad for contributing to this release!

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
bors bot added a commit to comit-network/comit-rs that referenced this pull request Mar 17, 2021
3542: Bump tracing from 0.1.22 to 0.1.25 r=mergify[bot] a=dependabot[bot]

Bumps [tracing](https://github.com/tokio-rs/tracing) from 0.1.22 to 0.1.25.
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/tokio-rs/tracing/commit/4ad1e62a2dd9f3e97a06ead14285993a9df99ea5"><code>4ad1e62</code></a> tracing: prepare to release v0.1.25</li>
<li><a href="https://github.com/tokio-rs/tracing/commit/c22b62e28121b66522beeff516f8a1528ec8ff36"><code>c22b62e</code></a> tracing: highlight <code>Span::entered</code> in more docs</li>
<li><a href="https://github.com/tokio-rs/tracing/commit/a358728b1ef321751acead4580643b1150863c62"><code>a358728</code></a> tracing: simplify common case of immediately entering the span (<a href="https://github.com/tokio-rs/tracing/issues/1252">#1252</a>)</li>
<li><a href="https://github.com/tokio-rs/tracing/commit/4538d74d228e4a6856c0176ade5f01fc26129b8e"><code>4538d74</code></a> subscriber: prepare to release v0.2.16 (<a href="https://github.com/tokio-rs/tracing/issues/1256">#1256</a>)</li>
<li><a href="https://github.com/tokio-rs/tracing/commit/a0201ba798b86739ba633d15d297ec233b905e4e"><code>a0201ba</code></a> log: prepare to release v0.1.2</li>
<li><a href="https://github.com/tokio-rs/tracing/commit/0cdd5e88ac3478ae91223e9c3b53a1c3598a7893"><code>0cdd5e8</code></a> log: forward <code>LogTracer::enabled</code> to the subscriber  (<a href="https://github.com/tokio-rs/tracing/issues/1254">#1254</a>)</li>
<li><a href="https://github.com/tokio-rs/tracing/commit/8d83326a5fddfdd68dc8865680f8663dad76a2b4"><code>8d83326</code></a> subscriber: fix FmtCollector not forwarding max level  (<a href="https://github.com/tokio-rs/tracing/issues/1251">#1251</a>)</li>
<li><a href="https://github.com/tokio-rs/tracing/commit/31aa6afecc1fb97b1f2d4eb15cefdd456c120142"><code>31aa6af</code></a> subscriber: set the max <code>log</code> <code>LevelFilter</code> in <code>init</code> (<a href="https://github.com/tokio-rs/tracing/issues/1248">#1248</a>)</li>
<li><a href="https://github.com/tokio-rs/tracing/commit/2a9d17f73f8499b7e73cf6e8bee225190aac6990"><code>2a9d17f</code></a> log: compare <code>log</code> record <code>Level</code>s against the max level (<a href="https://github.com/tokio-rs/tracing/issues/1247">#1247</a>)</li>
<li><a href="https://github.com/tokio-rs/tracing/commit/d173c2de9af913e2b4ebb8555e46d65b416ca730"><code>d173c2d</code></a> tracing: prepare to release v0.1.24 (<a href="https://github.com/tokio-rs/tracing/issues/1244">#1244</a>)</li>
<li>Additional commits viewable in <a href="https://github.com/tokio-rs/tracing/compare/tracing-0.1.22...tracing-0.1.25">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=tracing&package-manager=cargo&previous-version=0.1.22&new-version=0.1.25)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually


</details>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
…-rs#1252)

* Simplify common case of immediately entering the span

This PR allows the following API:

```
let _guard = tracing::span!("foo").entered();
```

See tokio-rs#1246 for an extended
discussion.

* Add missing inlines

This fns are monomorphic, so an `#[inline]` is necessary to alow
cross-crate inlining at all.

* improve method documentation

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
Added

- `Span::entered` method for entering a span and moving it into a guard
  by value rather than borrowing it (tokio-rs#1252)

Thanks to @matklad for contributing to this release!

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants