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.

Sign up for GitHub

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

Jump to bottom

#[instrument(fields(...))] does not support field::Empty #1211

Closed
nikclayton-dfinity opened this issue Jan 26, 2021 · 2 comments
Closed

#[instrument(fields(...))] does not support field::Empty #1211

nikclayton-dfinity opened this issue Jan 26, 2021 · 2 comments
Labels
kind/bug Something isn't working

Comments

@nikclayton-dfinity
Copy link

Bug Report

Version

│   │   ├── tracing v0.1.15
│   │   │   ├── tracing-attributes v0.1.8
│   │   │   └── tracing-core v0.1.10
│   │   │       │   │   │   │   │   └── tracing v0.1.15 (*)
│   │   └── tracing v0.1.15 (*)
├── tracing v0.1.15 (*)
├── tracing-futures v0.2.4
│   └── tracing v0.1.15 (*)
├── tracing-opentelemetry v0.10.0
│   ├── tracing v0.1.15 (*)
│   ├── tracing-core v0.1.10 (*)
│   ├── tracing-log v0.1.1
│   │   └── tracing-core v0.1.10 (*)
│   └── tracing-subscriber v0.2.9
│       ├── tracing-core v0.1.10 (*)
│       ├── tracing-log v0.1.1 (*)
│       └── tracing-serde v0.1.2
│           └── tracing-core v0.1.1

Platform

Darwin Kernel Version 19.6.0: Mon Aug 31 22:12:52 PDT 2020; root:xnu-6153.141.2~1/RELEASE_X86_64 x86_64 i386 MacBookPro15,1 Darwin

Description

Since you can't record a new field on a span after it's been created (per the docs) I wanted to pre-create some fields on the #[instrument(fields(...))].

The documentation doesn't suggest this is possible, but gives the example:

let span = trace_span!("my_span", greeting = "hello world", parting = field::Empty);

Given that, I figured that

#[instrument(fields(foo = tracing::field::Empty))]

would make sense.

Sadly, that results in

error: expected literal
   --> redacted/mod.rs:168:36
    |
168 | #[instrument(fields(foo = tracing::field::Empty), err, skip(registry_client, metrics))]
    |                           ^^^^^^^
@nikclayton-dfinity
Copy link
Author

I checked out the code to prepare a PR to fix this, and saw that the code does actually support this now. Looks like d24a43a#diff-9cbd60b122df9aadd01f63b729cf92e850d39bbb6ba8bb58c1d1d5c8be0e4e3c was the change that landed this.

With a Cargo.toml file that contains:

tracing = "0.1"

cargo fetched tracing-attributes 0.1.8, which is the version immediately before this support landed. I had to explicitly add tracing-attributes like so:

tracing = "0.1"
tracing-attributes = "0.1"

before cargo would update to the latest version (0.1.11 at time of writing).

With that change the syntax to specify an empty field is just to list the field name without any value. For example:

#[instrument(fields(otel.kind = "server", http.url))]
fn handle(request: Request<Body>) {
    let span = Span::current();
    span.record("http.url", &request.uri().to_string().as_str());

    // handle the request
}

Perhaps this example, or something like it, could be added to the documentation?

@hawkw hawkw added the kind/bug Something isn't working label Jan 28, 2021
@hawkw hawkw mentioned this issue Jan 28, 2021
Merged
hawkw added a commit that referenced this issue Feb 2, 2021
@hawkw
attributes: update & improve docs (#1215)
a018071 
The `tracing-attributes` documentation for `instrument` is out of date
and could use some improvement. In particular, it doesn't mention that
empty fields can be created in `instrument`'s `fields` argument, and
states that dotted fields are forbidden and that field values must be
literals, both of which are no longer the case. See also #1210 and
#1211.

This branch updates the docs to correct the false statements, and also
fleshes out the usage examples a bit.

I wasn't sure what to do with the old examples --- some of them are now
redundant, but it's also useful to have a big list of examples for every
allowed form. Any thoughts?

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw
Copy link
Member

hawkw commented Feb 4, 2021

tracing v0.1.23 updates the minimum dependency on tracing-attributes to v0.1.12, and that release of tracing-attributes updates the docs to add examples with empty fields. Thanks for the report!

@hawkw hawkw closed this as completed Feb 4, 2021
kaffarell pushed a commit to kaffarell/tracing that referenced this issue May 22, 2024
@hawkw @kaffarell
attributes: update & improve docs (tokio-rs#1215)
36b4dcb 
The `tracing-attributes` documentation for `instrument` is out of date
and could use some improvement. In particular, it doesn't mention that
empty fields can be created in `instrument`'s `fields` argument, and
states that dotted fields are forbidden and that field values must be
literals, both of which are no longer the case. See also tokio-rs#1210 and
tokio-rs#1211.

This branch updates the docs to correct the false statements, and also
fleshes out the usage examples a bit.

I wasn't sure what to do with the old examples --- some of them are now
redundant, but it's also useful to have a big list of examples for every
allowed form. Any thoughts?

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
Assignees
No one assigned
Labels
kind/bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@hawkw @nikclayton-dfinity