offer function-level control over tracing

[joeshaw]

This adds to the tracing boolean from the from_witx! macro a disabled_for list of identifiers that allow for suppression of tracing on a per-function basis.

Example:

wiggle::from_witx!({
    tracing: true disable_for {
        module1::func,
        module2::another_func,
    },
    witx: ["..."],
});

Fixes #5193

[github-actions]

Subscribe to Label Action

cc @kubkon

Details

This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

• kubkon: wasi

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[pchickey]

Thanks. I'd rather not break the current use of tracing: true | false in the macro if we can avoid it. For suppressing it on certain functions, could you add an optional suffix like, perhaps, tracing: true except { module::func, module2::func2 }?

[joeshaw]

Thanks for the feedback, I'll look into it. My worry with except is that you might expect something different if you did tracing: false except { module::func } but there's probably another word we can use.

[pchickey]

It should probably give an error if used in the false case? exclude is another word we could use.

[alexcrichton]

Perhaps:

tracing: true, //  all functions

// ... or ...

tracing: false, // no functions

// ... or ...

tracing: ["..."], // only these functions

Would that work for your use case? I agree we'd ideally avoid various directions of lists here if we could. One possibility would be to support globs as well if families of functions to trace all have similar names. I suppose this could even go so far as to have:

tracing: ["trace-me", "!this-is-not-trace"],

where a ! in front disables tracing for that function (or any other matching globs)

[joeshaw]

@alexcrichton For my use case, I am looking to suppress tracing from a function that carries sensitive information, so negation is the only thing that'd work.

Taking a step back: is there a strong reason to have the tracing config be highly customizable? I think I have a compelling reason for disabling tracing on a particular function but is it important to be able to turn it on for just a few functions?

I'm also hesitant to diverge too much from syntax that already exists. async allows either a catchall (async: *) or a list of identifiers in braces (async: { module::func1, module::func2 }).

[alexcrichton]

Keeping consistent with async seems reasonable, but I think that it's fine to change the syntaxes for both at the same time. I would agree that there's probably not much of a use case for "just this function". Perhaps:

tracing: true,
tracing: false,
tracing_skip: [ ... ]

and that may work?

[joeshaw]

I pushed a new set of commits that implements what @pchickey suggested (but with disable_for instead of except because I think it flows better even if you change true to false -- but happy to change this back).

Example usage:

wiggle::from_witx!({
    tracing: true disable_for {
        module1::func,
        module2::another_func,
    },
    witx: ["..."],
});

I'm also happy to switch this to be a separate argument altogether (@alexcrichton's tracing_skip suggestion) if that's preferred.

[alexcrichton]

Seems reasonable to me 👍 but I defer to @pchickey for the final approval

[pchickey]

Thanks @joeshaw, this is good work.