use frame pointer unwinding for Go 1.21+ on aarch64#422
Merged
fabled merged 2 commits intoopen-telemetry:mainfrom Apr 17, 2025
Merged
use frame pointer unwinding for Go 1.21+ on aarch64#422fabled merged 2 commits intoopen-telemetry:mainfrom
fabled merged 2 commits intoopen-telemetry:mainfrom
Conversation
This was referenced Mar 31, 2025
fabled
force-pushed
the
golang-arm64-framepointer-unwind
branch
from
0724bfe to
f95662e
Compare
fabled
commented
Apr 1, 2025
fabled
commented
Apr 1, 2025
fabled
force-pushed
the
golang-arm64-framepointer-unwind
branch
from
c9ccd1f to
30fb123
Compare
Introduce a mechanism for native frame pointer unwinding in the ebpf code, and use it for the V8 and Golang unwinders. This allows a slightly optimized unwinding to happen as less bpf_probe_read_user() calls are done with the expense of slightly larger native unwinder ebpf program. Enable frame pointer unwinding for Go1.21+ on aarch64. fixes open-telemetry#342
fabled
force-pushed
the
golang-arm64-framepointer-unwind
branch
from
30fb123 to
a3c7287
Compare
christos68k
approved these changes
Apr 7, 2025
Member
christos68k
left a comment
christos68k
left a comment
There was a problem hiding this comment.
LGTM but I can't test this atm.
florianl
approved these changes
Apr 10, 2025
Member
florianl
left a comment
florianl
left a comment
There was a problem hiding this comment.
Just two minor comments, that can be addressed in a follow up. Besides that 👍 thanks for the update!
| // Ambiguous regarding if frame pointer is kept correctly. | ||
| // Take the slow path of resolving Go version. | ||
| goVer, err := ee.file.GoVersion() | ||
| if err != nil || version.Compare(goVer, "go1.21rc1") < 0 { |
Member
There was a problem hiding this comment.
Nit: Release candidates are for temporary testing. Maybe we can just use the first release with this change in Go:
Suggested change
| if err != nil || version.Compare(goVer, "go1.21rc1") < 0 { | |
| if err != nil || version.Compare(goVer, "go1.21.0") < 0 { |
Contributor
Author
There was a problem hiding this comment.
I have a strong preference to keep the actual first rc tag here. The Go libraries support comparing versions with them, and this makes the code work with the prerelease tags. This also serves as note to remind what is the first tag when the Golang change was made.
| UnwindCommandStop int32 = C.UNWIND_COMMAND_STOP | ||
| UnwindCommandPLT int32 = C.UNWIND_COMMAND_PLT | ||
| UnwindCommandSignal int32 = C.UNWIND_COMMAND_SIGNAL | ||
| UnwindCommandInvalid int32 = C.UNWIND_COMMAND_INVALID |
Member
There was a problem hiding this comment.
Maybe we should bring these constants into support/types_def.go - then we don't need to manually keep them in sync. But this can be part of another PR.
Contributor
Author
There was a problem hiding this comment.
Sure, will do a follow up PR on this.
florianl
added a commit
that referenced
this pull request
Apr 23, 2025
Reduce the spread of CGO across the code base and move definitions to support package. Follow up to #422 (comment) Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
florianl
added a commit
that referenced
this pull request
Apr 23, 2025
Reduce the spread of CGO across the code base and move definitions to support package. Follow up to #422 (comment). Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
This was referenced Jul 8, 2025
This was referenced Aug 14, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduce a mechanism for native frame pointer unwinding in the ebpf code, and use it for the V8 and Golang unwinders. This allows a slightly optimized unwinding to happen as less bpf_probe_read_user() calls are done with the expense of slightly larger native unwinder ebpf program.
Enable frame pointer unwinding for Go1.21+ on aarch64 (this significantly reduces the kernel memory needed for Go executable stack delta maps).
fixes #342