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 macro: provide way to include additional fields #650

Closed
carllerche opened this issue Mar 30, 2020 · 1 comment · Fixed by #672
Closed

instrument macro: provide way to include additional fields #650

carllerche opened this issue Mar 30, 2020 · 1 comment · Fixed by #672
Labels
crate/attributes Related to the `tracing-attributes` crate kind/feature New feature or request

Comments

@carllerche
Copy link
Member

It would be nice to have an API for including additional fields with values set by calling methods on self.

For example (making up an API that probably isn't possible)

struct Foo {
    conn: TcpStream,
}

impl Foo {
    #[instrument(fields(local_addr = self.local_addr))]
    fn run(&self) {
    }

    fn local_addr(&self) -> SocketAddr {
        self.conn.local_addr()
    }
}
@carllerche carllerche mentioned this issue Mar 30, 2020
Open
@hawkw
Copy link
Member

hawkw commented Mar 30, 2020

I think the proposed API may actually work as an extension of the existing fields API...we could just parse an arbitrary expression that's then evaluated when the span is constructed. Depending on how this works, it might even be possible to get decent diagnostics out of the compiler when these are malformed.

@hawkw hawkw added kind/feature New feature or request crate/attributes Related to the `tracing-attributes` crate labels Mar 31, 2020
@carllerche carllerche mentioned this issue Apr 1, 2020
Merged
This was referenced Apr 3, 2020
Closed
Merged
hawkw added a commit that referenced this issue May 23, 2020
@hawkw
attributes: support arbitrary field expressions (#672)
d24a43a 
## Motivation

Fields on spans generated by `#[instrument]` consist of the parameters
to the `instrument`ed function, and a list of optional fields with
literal values passed in the `fields` argument of the attribute. The
current API for additional user-defined fields is pretty limited, since
it only accepts literals (and doesn't accept dotted field names). It
would be nice to have an API for including additional fields with values
set by calling methods on `self` or on parameters, or by accessing
values from parameters.

## Solution

This branch extends the currently supported `fields` argument to allow
providing arbitrary expressions which are invoked within the function's
scope. This is a superset of the previous support for literal values
only. In addition, field names passed to `fields` may now contain dotted
Rust identifiers.

Implementing this required rewriting how arguments to the
`#[instrument]` attribute are parsed. Previously, we used `syn`'s
`AttributeArgs` type, which parses a comma-separated of "nested meta"
items, consisting of `$ident($nested meta)`, `$ident = $nested_meta`,
paths, and literals. By replacing the use of `AttributeArgs` with our
own types implementing `syn::parse::Parse`, we can accept more
expressions in the attribute arguments position. This also lets us
reject more invalid inputs at parse-time, which improves syntax error
reporting a bit.

One thing that's worth noting is that the current implementation will
generate an _empty_ field when a field name is provided without an `=`
and a value. This makes sense when only simple fields with literal
values are permitted. However, if we accept arbitrary expressions in the
macro, this is not the ideal behavior --- we would prefer to use the
same local variable shorthand as the function-like `tracing` macros.
However, changing this now is a breaking change. Any code which uses a
name that doesn't exist in the current scope to declare an empty field
would fail to compile, because it attempts to reference a name that
doesn't exist. Instead, I left a comment noting that this is not the
ideal behavior and it should be changed next time we're ready to break
the proc macros.

Fixes: #650

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

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
crate/attributes Related to the `tracing-attributes` crate kind/feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

attributes: support arbitrary field expressions tokio-rs/tracing
2 participants
@carllerche @hawkw