Skip to content

runtime: add 'created by goroutine number' to stack traces #38651

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

Closed
firelizzard18 opened this issue Apr 24, 2020 · 13 comments
Closed

runtime: add 'created by goroutine number' to stack traces #38651

firelizzard18 opened this issue Apr 24, 2020 · 13 comments

Comments

@firelizzard18
Copy link
Contributor

Per @ianlancetaylor's comment on #35178 (comment), I propose improving the debugging experience by including goroutine tree information in stack dumps and providing that information to debuggers (delve).

I propose that created by file.go:col in stack dumps is expanded to created by goroutine # at file.go:col, and that goroutine tree information is provided to debuggers like delve.

Ian's original comment:

If the problem we are trying to address is better support for understanding large numbers of goroutines in stack dumps and when debugging, then let's discuss that problem. Let's not jump to the idea of goroutine names, which have many drawbacks in a language like Go where goroutines are started casually. Maybe goroutine names are the best idea we can come up with, but that conclusion seems premature given that we haven't even started talking about the actual problem.

For example, one thing that might help is giving the stack dump, and debuggers, access to the goroutine tree, so that you can see clearly that goroutine N was started by goroutine M. You can see this a bit today by using GODEBUG=gotracebackancestors=N for various integer N. Or I'm sure there are other better ideas out there.

In general debuggers do not do well when there are many separate threads of executions, because most languages do not make it trivial to start many separate threads of executions. We need to do better in this area.

@gopherbot gopherbot added this to the Proposal milestone Apr 24, 2020
@rsc rsc moved this to Incoming in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@rsc rsc changed the title proposal: runtime: allow stack dump and debuggers access to the goroutine tree. proposal: runtime: add 'created by goroutine number' to stack traces Aug 10, 2022
@rsc
Copy link
Contributor

rsc commented Aug 10, 2022

We'd need to add this to the g struct and print it.
Debuggers can then fetch it because they look at the g structs.
Sounds like a great idea.

@rsc rsc moved this from Incoming to Active in Proposals Aug 12, 2022
@rsc
Copy link
Contributor

rsc commented Aug 12, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@prattmic prattmic added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 12, 2022
@rsc
Copy link
Contributor

rsc commented Aug 17, 2022

Does anyone object to adding this?

@kaey
Copy link

kaey commented Aug 17, 2022

I'd rather want to see pprof.Labels in stacktraces, as I suggested in linked issue.

But if adding parent goroutine ID is easy to do then why not I guess.

@kortschak
Copy link
Contributor

It seems to me that this would not give access to a full tree view into the goroutine's origins since when we have the creator's ID, we cannot easily step beyond that to its creator since we have an ID, not a g.

@firelizzard18
Copy link
Contributor Author

@kortschak The only time I can think of that you'd be building a tree view of goroutines is during a debug session. In that case the debugger could build a map from ID to g and the performance impact on a human scale should be negligible. But that's only relevant if there's a downside to adding parent *g to g instead of parentId int (I have no idea).

@kaey
Copy link

kaey commented Aug 19, 2022

Parent goroutine can very well be gone at any point while the child is still running. Will knowing ID be even useful in that case?

@firelizzard18
Copy link
Contributor Author

@kaey Good point and probably a good reason why we couldn't add parent *g to g.

It's been a while since I created this and I don't honestly remember what my motivation was. I think the main issue is figuring out what caused a goroutine to be spawned. If the parent is dead and I don't know the parent's parent, then knowing the parent's ID is probably useless. However if I am able to reconstruct the goroutine chain, I might be able to figure out the chain of events that lead to the last goroutine getting spawned.

The cases I can think of are things like an event loop spawning goroutines to process events, or a socket accept loop spawning goroutines to handle connections. In those cases the parent goroutine should be long-lived.

@rsc
Copy link
Contributor

rsc commented Aug 31, 2022

This is very simple to implement. It has limitations that more comprehensive schemes might not, but it's still very likely useful on its own, far in excess of its cost. So it sounds like we should do this, and we can leave any suggested heavier schemes for other proposals.

@rsc rsc moved this from Active to Likely Accept in Proposals Sep 7, 2022
@rsc
Copy link
Contributor

rsc commented Sep 7, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@julieqiu julieqiu removed this from Go Security Sep 8, 2022
@rsc rsc moved this from Likely Accept to Accepted in Proposals Sep 21, 2022
@rsc
Copy link
Contributor

rsc commented Sep 21, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: runtime: add 'created by goroutine number' to stack traces runtime: add 'created by goroutine number' to stack traces Sep 21, 2022
@rsc rsc modified the milestones: Proposal, Backlog Sep 21, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/435337 mentions this issue: runtime: record parent goroutine ID, and print it in stack traces

@github-project-automation github-project-automation bot moved this from Todo to Done in Go Compiler / Runtime Feb 21, 2023
johanbrandhorst pushed a commit to Pryz/go that referenced this issue Feb 22, 2023
Fixes golang#38651

Change-Id: Id46d684ee80e208c018791a06c26f304670ed159
Reviewed-on: https://go-review.googlesource.com/c/go/+/435337
Run-TryBot: Nick Ripley <[email protected]>
Reviewed-by: Ethan Reesor <[email protected]>
Auto-Submit: Michael Pratt <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
@dmitshur dmitshur modified the milestones: Backlog, Go1.21 Jun 4, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/501336 mentions this issue: doc/go1.21: add release notes for parent goroutine in stack traces

gopherbot pushed a commit that referenced this issue Jun 7, 2023
For #38651.

Change-Id: Ie73c1da0629287efda7f0c617e94a7f3a881eee7
Reviewed-on: https://go-review.googlesource.com/c/go/+/501336
Reviewed-by: Eli Bendersky <[email protected]>
TryBot-Bypass: Michael Knyszek <[email protected]>
Auto-Submit: Michael Knyszek <[email protected]>
Reviewed-by: Austin Clements <[email protected]>
@rsc rsc removed this from Proposals Mar 1, 2024
@golang golang locked and limited conversation to collaborators Jun 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants