-
Notifications
You must be signed in to change notification settings - Fork 89
Description
Issue Scope
Currently we do use #[tracing::instrument(..)] but we always use the same 4 common patterns:
#[instrument(parent = None, target: COMPONENT, name = "foo.bar.baz", skip_all, err)]
#[instrument(target: COMPONENT, name = "foo.bar.baz", skip_all, err)]
#[instrument(parent = None, target: COMPONENT, name = "foo.bar.baz", skip_all, ret(level = "debug"), err)]
#[instrument(parent = None, target: COMPONENT, name = "foo.bar.baz", skip_all, fields(path=%some_path_ident.display())), err]
Let's go through the meaning of each of the used attributes one by one:
name- a human readable name you'll find in honeycomblevel=debug- only the given level will this span appear in the outputerr- add a log message if this path is taken, as if it was done witherror!. Can be modified witherr(debug)skip_all- skip all parameters to be recorded, bad for debugging, good for speed and avoiding accidental secrets being leaked in logsret(level = "debug")- log the return value at this level or aboveparent- defines the parent span, orNonefor a new root level span
This is the maximum scope we use. There is little correlation (consistency?) in when we do which (bar parent usage).
Proposal
Simplify the API
#[instrument("foo.bar.baz", ret(level="debug"))]
// desugars to
// #[tracing::instrument(name="foo.bar.baz", target=COMPONENT, err(level="warn"), skip_all]
fn bar() -> u32 {..}#[instrument(root, "foo.bar.foo", %a=a.display(),b=b.printer())]
// desugars to
// #[tracing::instrument(parent = None, target=COMPONENT, name="foo.bar.foo", err(level="warn"), skip_all, fields(%a=a.display(),b=b.printer()))), err]
fn foo() -> Result<_> {..}Simplification serves two purposes:
- less boiler plate
- consistency using sane defaults
- make
COMPONENTexplicitly provided or implicitly assumed - allow customization for
Result<_,_>types, to utilize our ownReporttrait
Further considerations:
Since we are hardwiring some special cases, we might go one step further, i.e. allow to print fields of which we know their relevance, i.e. BlockNumber is always safe to print, adding a "safe-list" and always add them as fields might be a good usability trade-off, while providing a off-switch with i.e. -. This ofc smells a lot like skip_all again, just more restricted, however the scope we operate in, this might be a good balance.