proposal: testing: add goroutine labels to tests

[dfinkel]

Proposal Details

Debugging tests timeouts can be frustrating when multiple tests fail to complete.

This was improved recently with the code that prints the set of tests that were still running, but that doesn't tell you which goroutines were associate with which test.

I propose adding one or two goroutine labels to all test invocations to make it easy to see which goroutine is for which test and make it easy to see which tests spawn a ton of goroutines.

I posted a CL to #23458 which would update runtime tracebacks (e.g. printed for recovered panics) to include goroutine labels in the status-line. (we can leave any discussion about where to put the labels on that bug)

My initial (straw-man) proposal for the labels:

• a label with the key test.name and value of the full test-name (the same as from t.Name() (or f.Name() for fuzz tests, and b.Name() for benchmarks)
• a label with the key test.iter which is set for the second and later iterations of a test that's run with -count.
• a label with the key bench.name for benchmarks (including sub-benchmarks)
• a label with the key bench.run.iter for benchmarks with later calls (from -count as well)
• a label with the key fuzz.name as the only label for direct fuzz tests (-fuzz FuzzFoo)
• a label with the key fuzz.corpus.name for fuzz tests run as normal tests based on corpus (both seed and testdata)
• a label with the key fuzz.corpus.iter for later corpus fuzz test iterations (-count)

I don't feel strongly about either label-name, but including the test iteration makes it easy to see how far a test got before timing out. However, omitting the iteration on the iteration number on the first run eliminates visual noise in the common-case.

https://go.dev/cl/740301 and https://go.dev/cl/740320 would implement this proposal.

[gopherbot]

Change https://go.dev/cl/696595 mentions this issue: testing: goroutine labels with name & iteration

[gopherbot]

Change https://go.dev/cl/696117 mentions this issue: runtime/pprof: Move goroutine labeling to internal

[glycerine]

Many is the time I have wished that my stack dumps could show me some set of the current values of some local stack frame variables in the goroutine stack dump. Maybe a mechanism that generalizes to both goroutine labeling and printing the values of the arguments to each call would be beneficial.

[dfinkel]

Hmm, I would love to make the existing function-call arguments a bit easier to read. However:

• teaching the traceback printing code to handle non-integer values while walking the stack after a panic would be tricky.
  • In particular, dereferencing pointers after a segfault may trigger another segfault if not properly guarded.
  • that would apply for any struct passed by reference on the stack, slice, or string.
• that's out of scope for this proposal, since adding goroutine labels to the tracebacks is part of runtime/pprof: labels are not added to profiles #23458. (I want to be careful about scope-creep)

[dfinkel]

It just occurred to me that even without https://go.dev/cl/694119, having goroutine labels associated with individual tests can be useful with -test.cpuprofile/-cpuprofile, since the pprof protobuf-formatted CPU-profiles already include goroutine labels. (I rarely use that facility, which is why I didn't think about it before)

[dfinkel]

A quick update: https://go.dev/cl/694119 landed at the end of November and is in go 1.26 behind GODEBUG=tracebacklabels=1. #76349 is a bug to decide on a stable format for the goroutine labels to use going forward.

@adonovan and @prattmic this was my motivating use-case for adding goroutine labels to tracebacks. (I don't see it in the proposal review queue. Is there a step I missed?)

[gopherbot]

Change https://go.dev/cl/740301 mentions this issue: runtime/pprof: Move goroutine labeling to internal

[gopherbot]

Change https://go.dev/cl/740320 mentions this issue: testing: goroutine labels with name & iteration

[dfinkel]

https://go.dev/cl/751580 was merged last night, and I've rebased the two CLs attached to this issue along with doing a bunch of cleanup. I think they're ready for review now. (I've now removed the WIP bit from both CLs)

While doing that cleanup, I made a few changes to labels, so I've updated the initial comment on this issue to reflect that. (most notably, the benchmark and fuzz test handling have some additional cases that I added distinct labels for)