x/tools/gopls/internal/telemetry/cmd/stacks: allow dups to be reported by simply appending a comment

[adonovan]

The stacks command de-dups stack counter reports based on their hash. A GitHub issue search looks for this hash in the first comment attached to an issue ("in:body"). So, when triaging reports one must edit the first comment and append the new stack.

It would be more convenient (and easier to see the timeline) if we could simply copy the stack into a new comment, but this won't show up in the issues search. We could remove the in:body restriction, but extra work is required to fetch all the comment bodies for an issue: in principle there may be hundreds, so only the first is returned "for free".

Alternatively we could just stick with having to modify the first comment.

[adonovan]

Looking at some of the issues with more numerous field reports, such as:

#64235: x/tools/gopls: bug in typeCheckBatch.importPackage reported by telemetry [open] (n=80)
#64236: x/tools/gopls: bug in analysis importer reported by telemetry [open] (n=54)
#60890: x/tools/gopls: getImportPackage crash (PackagePath == "unsafe", id != "unsafe") (Bazel?) [closed] (n=39)

it's clear that de-duping by stack hash is not effective because of the wide variation in the call stack. Manual effort is currently required to match up each stack with a previous issue. However it would be easy for the triageur to summarize the crucial part of a stack as a predicate, for example "must contain a frame matching golang.Hover:+4", similar to the way watchflakes works. All existing predicates would then be applied to each incoming new stack and if a single predicate matches, the stack would be associated with that issue; only if there is no unique match would the triageur have to do any work.

It's not clear how sophisticated the predicate needs to be, but I think we can get away without a means of specifying relationships (e.g. this frame must be a parent or ancestor of that one). At a bare minimum, it needs to support conjunctions (e.g. stack contains "bug.Report" AND "importMap.func1:+6") and disjunctions (e.g. stack contains "importMap.func1:+6" OR "importMap.func1:+7", to deal with line number perturbation). So this grammar may suffice:

expr = "string literal" 
         | ( expr )
         | expr '&&' expr
         | expr '||' expr
         .

where a string literal (defined as for Go) implies a substring match. I don't think even regexps are needed.

Example

# stacks
"bug.Report" && ("importMap.func1:+6" || "importMap.func1:+7")

[gopherbot]

Change https://go.dev/cl/613215 mentions this issue: gopls/internal/telemetry/cmd/stacks: predicate de-duplication