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

Unknown arguments to #[instrument] are ignored #772

Closed
mcarton opened this issue Jun 29, 2020 · 2 comments · Fixed by #786
Closed

Unknown arguments to #[instrument] are ignored #772

mcarton opened this issue Jun 29, 2020 · 2 comments · Fixed by #786

Comments

@mcarton
Copy link

mcarton commented Jun 29, 2020

Bug Report

Version

└── tracing v0.1.15
    ├── tracing-attributes v0.1.8
    └── tracing-core v0.1.10

Crates

  • tracing-attributes

Description

I made the mistake to use

#[instrument(foo="bar", id=1, show=true)]
fn foo() {}

instead of

#[instrument(fields(foo="bar", id=1, show=true))]
//           ^^^^^^^                          ^
fn foo() {}

This is silently accepted by the attribute, but does nothing. I'd expect an error that foo, id and show don't exist, with a possible suggestion to use fields.

@hawkw
Copy link
Member

hawkw commented Jun 29, 2020

I believe this was fixed in PR #672 --- but it hasn't been released yet. We should get on that!

@hawkw hawkw added this to the tracing-attributes 0.2 milestone Jul 7, 2020
@hawkw
Copy link
Member

hawkw commented Jul 7, 2020

I'm closing this as it's a duplicate of #727. We'll try to publish the fix for it soon.

@hawkw hawkw closed this as completed Jul 7, 2020
hawkw added a commit that referenced this issue Jul 8, 2020
## Motivation

In PR #672, the `#[instrument]` attribute was rewritten to use a custom
`syn` parser rather than using `syn::AttributeArgs`. This was primarily
to allow the use of arbitrary expressions as fields, but it had the side
benefit of also allowing more sophisticated error reporting. In
particular, we were able to replace the previous behavior of silently
ignoring unknown input with a compiler error, which has been requested a
few times (see #772 and #727). However, emitting a compiler error for
inputs that we previously allowed (even if they did nothing) is
technically a breaking change --- see
#773 (comment) and
following comments. This means that the other new features added in #672
cannot be released until we release `tracing-subscriber` 0.2.

## Solution 

This branch avoids the breaking change by downgrading these errors to
warnings. However, it isn't currently possible to emit a warning from a
proc-macro on stable --- the `proc_macro::Diagnostic` API which would
allow this requires a nightly feature. This branch hacks around this by
generating a fake deprecated item with for each unrecognized token in
the input, emitting the skipped item's parse error as the deprecation's
`note`, and "using" the deprecated item.

The deprecated warning is used here rather than other code that will
emit compiler warnings, because it gives us a way to customize the
warning output (via the `note` field). I've tried to make it fairly
obvious that this is not a "real" deprecation. Sample output looks like
this:
![Screenshot_20200707_153151](https://user-images.githubusercontent.com/2796466/86850948-fce0a780-c066-11ea-9b1c-5085c0fd43e0.png)

Fixes #727 
Fixes #772 

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

Successfully merging a pull request may close this issue.

2 participants