Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls/internal/telemetry/cmd/stacks: automate stack context #64654

Open
findleyr opened this issue Dec 11, 2023 · 3 comments
Open

x/tools/gopls/internal/telemetry/cmd/stacks: automate stack context #64654

findleyr opened this issue Dec 11, 2023 · 3 comments
Assignees
Labels
gopls Issues related to the Go language server, gopls. telemetry x/telemetry issues Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@findleyr
Copy link
Contributor

Reminder issue: we should automate the context added to issues created by the stacks command (https://github.com/golang/go/issues?q=is%3Aissue+is%3Aopen+label%3Agopls%2Ftelemetry-wins).

In order to do this, we need a func(symbol, tag string) (linenumber int) in order to build the source location. Our benchmarks have some helpers to shallow clone, which we could extract and reuse.

Since we may need to adjust the relevant source location, we should make this functionality available as a subcommand (in addition to guessing it in the initial issue template). E.g. stacks context golang.org/x/tools/gopls/internal/server.diagnose 23.

CC @adonovan

@findleyr findleyr added this to the gopls/v0.16.0 milestone Dec 11, 2023
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Dec 11, 2023
@findleyr findleyr modified the milestones: gopls/v0.16.0, gopls/v0.17.0 Mar 13, 2024
@adonovan
Copy link
Member

adonovan commented Mar 25, 2024

Shallow cloning is the easy part. Parsing the stack frame out of the symbol name is the tricky bit. Consider this line from #66490:

golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3:+5

If I understand correctly, this refers to a call 5 lines into an anonymous function defined within protocol.ServerHandler, which is inlined into (*streamServer).ServeStream, and because of the inlining becomes the third anonymous function within the combined function. The actual package name (protocol) isn't even mentioned! So not only do you have to parse the symbol name quite carefully, it then needs resolving relative to type information.

Inlining should be a compiler implementation detail. Instead of printing the runtime's inline-mangled symbol name, we should resolve it to a logical (file name, relative line, function name) triple upfront in x/telemetry's crashmonitor (#66517). This will break all our existing counter names, but will allow us to automate more of the triage burden.

@findleyr
Copy link
Contributor Author

findleyr commented Aug 6, 2024

Here is more information about steps to accomplish this:

In order to do so, we'd need to:

  1. For each stack program, check out the repo for that program at the correct version (for now, we can hard-code a list of program repos, but eventually we can also use origin information from the proxy (e.g., for gopls)). See also the aforementioned shallow clone logic.
  2. Use go/packages to load package information for the stack using the correct GOOS/GOARCH combination corresponding to the report.
  3. Look up symbols in in the package, and translate line offsets in function bodies to absolute line positions.
  4. Update the output of the stacks command to support html output, so that we can format nicely linkified stack information (perhaps an -html flag can generate an html summary which can be opened in the browser? I'm not sure here how best to serve the information.

@h9jiang this may be a good project to get some experience with go/packages. Tentatively assigning.

@adonovan
Copy link
Member

adonovan commented Aug 6, 2024

Use go/packages to load package information for the stack using the correct GOOS/GOARCH combination corresponding to the report.

...and GOLTOOCHAIN?

Look up symbols in the package, and translate line offsets in function bodies to absolute line positions.

Given that it appears we are not going to change the format produced by telemetry, the challenge here is to accurately reverse-engineer the symbol names actually produced by the compiler; see #64654 (comment) for some of the challenges. You may want to discuss with the compiler & runtime folks before spending too much time on heuristics.

@h9jiang this may be a good project to get some experience with go/packages.

I agree.

I think it would be best to consider the problem as a sequence of steps.

  1. Enrich the raw description of the stack using the git repository as an input (as described above). This is a deterministic and non-heuristic pure function.
  2. Improve the summary line extracted from the enriched data. For example, if the topmost stack frame is bug.Report("foo") then "foo" might be a good summary. This step is heuristic.
  3. Improved detection of near-duplicates. Currently the human user is required to remember and/or search for near duplicates. Ideally we would have an algorithm to find them; this could be based on machine learning, but the current... enthusiasm for LLMs should not blind us to simpler alternatives. Searching for near-duplicates requires interaction with GitHub Issues. This step is also heuristic.
  4. Recording the user's judgments about duplicates. This requires creating/updating GitHub Issues. We could maintain the canonical stack-to-issue mapping in some other data store if there are compelling reasons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. telemetry x/telemetry issues Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants