On 11/29/2021 1:40 PM, Jeff King wrote: > On Mon, Nov 29, 2021 at 01:47:44PM +0000, Derrick Stolee via GitGitGadget wrote: > >> As reported by Ævar [1] and diagnosed by Peff in a reply, the default >> GIT_TRACE2_EVENT_NESTING is set so low that tests that look for trace2 >> events can start failing due to an added trace2 region. This can even be >> subtle, including the fact that the progress API adds trace2 regions when in >> use (which can depend on the verbose output of a test script). > > I think this is a good change for fixing the immediate problem of the > test suite failing. > > But I have to wonder if this is masking a problem that real users will > see. Aren't we silently discarding trace2 output that could be useful to > somebody debugging or trying to get performance metrics? > > I.e., should we be bumping the internal nesting max of 2 to something > higher? If that would that cause problems with existing metrics, should > we be designing new metrics to avoid nesting? One thing this makes me think about is that we might want to have items such as trace2_data_intmax() and trace2_data_string() not be subject to the nesting limit. Only have the nesting apply to the regions (which nest). CC'ing Jeff Hostetler for his thoughts on that idea. We will have a discussion offline to see if the progress regions have started to cause inconsistencies in our existing telemetry stream of internal users. Thanks, -Stolee
