Opt in to traced fields [WIP - Some open questions]

[LukeMathWalker]

Motivation

Closes #651

Solution

• Start emitting a compiler warning if skip is used within #[tracing::instrument];
• Remove existing skip logic, converting the annotation from opt-out to opt-in;
• Adapt existing tests to the new behaviour

Open questions

The PR is not ready for prime-time yet (mostly because I need to update the documentation), but I wanted to clarify a few questions before moving forward:

• Is emitting a warning when skip is used the desired behaviour? Or do we want to emit a compiler error?
• Currently

#[tracing::instrument(fields(a))]
fn a_function(a: usize) { ... }

behaves quite surprisingly: instead of adding a to the span, using its Debug representation, it adds a new field with its value set to Empty. To get the expected behaviour you need to use

#[tracing::instrument(fields(?a))]
fn a_function(a: usize) { ... }

This was perhaps rarely encountered in the wild when the macro worked according to an opt-out philosophy but I am sure it's going to become a very common mistake now that it's opt-in by default and developers need to spell out the fields they want to capture.

Looking at the source code, there is a comment by @hawkw on the issue

            // XXX(eliza): I don't like that fields without values produce
            // empty fields rather than local variable shorthand...but,
            // we've released a version where field names without values in
            // `instrument` produce empty field values, so changing it now
            // is a breaking change. agh.

Is this something you want to resolve as part of the set of breaking changes for the next release? Should it be part of this PR?

[Stargateur]

I don't feel like this is a good solution, then people will ask for opt-out feature. I think there should be two mode. and default to opt-in

[LukeMathWalker]

Is there an interest to progress this one (a.k.a. me bringing it up to speed with trunk and updating the docs) or should I close it? @hawkw?

[bryangarza]

Thanks for working on this, unfortunately, seems we were not able to get your open questions answered (due to lack of bandwidth probably).

@hawkw thoughts on this? Something we want to merge, or should we close?

[LukeMathWalker]

Happy to bring it back to a mergeable state @bryangarza if desired.

[bryangarza]

Thanks, I'll follow up on the Discord about this and see if we can get some direction on if we want to move forwards or not

[cdmistman]

hey! any updates on design considerations?

[bryangarza]

Hi -- this one must have fallen through the cracks. I'm no longer contributing to tracing; if anyone is interested in following up on this, you should probably reach out to the maintainers on the Tokio discord.