fix: only record a trace if needed by an audit

[patrickhulce]

fixes #2088

needs coordination with #2062 and #2104

[paulirish]

love it. can you add a quick test?

[patrickhulce]

done

[brendankenny]

is it going to be weird that this only runs when creating a config with onlyCategories and/or onlyAudits? So a completely custom config won't get it (unless you're both writing your own and whitelisting).

Also should we provide an escape hatch if you want to override this and record a trace (e.g. if you're doing --save-artifacts/assets to get a trace saved?)

[patrickhulce]

is it going to be weird that this only runs when creating a config with onlyCategories and/or onlyAudits? So a completely custom config won't get it (unless you're both writing your own and whitelisting).

Seems fine to me, if you're explicitly including a trace without a reason, we shouldn't stand in your way.

Also should we provide an escape hatch if you want to override this and record a trace (e.g. if you're doing --save-artifacts/assets to get a trace saved?)

Yeah I was slightly worried about this and other scenarios that you might sneakily consume a trace through unofficial channels or something. I'd prefer to leave that an outstanding issue until we add unification of CLI and config flags in settings which I hope to start/finish/allnighter sometime before I/O :D

[paulirish]

aye. If you really want a trace, then including user-timings audit doesn't seem all that bad.

[brendankenny]

Seems fine to me, if you're explicitly including a trace without a reason, we shouldn't stand in your way.

I guess I meant weird the other way, that you could get a trace even if an audit didn't need one in your own config, but if you extend that config and whitelist everything suddenly it's gone

[brendankenny]

I'd be ok with waiting for the great settings/flags rationalization, but maybe add a log.warn here for now? Slightly easier to figure out when wondering where your trace is.

[patrickhulce]

I'd be ok with waiting for the great settings/flags rationalization, but maybe add a log.warn here for now? Slightly easier to figure out when wondering where your trace is.

yeah good call on ⚠️

[brendankenny]

LGTM

[brendankenny]

the only issue with this is that validatePasses hasn't run yet, so the pass might not have a passName (and we allow no name if there's only one unnamed pass, at which point it should be DEFAULT_PASS...ugh)

Maybe 'Trace not requested by an audit, dropping trace in pass '${pass.passName}'' so at least they get a pass 'undefined'? :)

(we should figure out ordering on that at some point, too. Makes sense to validate at the end, so the actual config that's going to be run is sure to be valid, but input is also supposed to be valid so...validate twice?)

[brendankenny]

remove listener when finished?

[patrickhulce]

done