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

symbolizer: shell out to addr2line #5299

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

danipozo
Copy link

Use addr2line instead of custom addr2line implementation which doesn't symbolize addresses properly for some binaries (see #5291). Even with a fix for go-delve/delve#3861, which is the root cause of the failure in #5291, the current implementation can't symbolize ~50 % of the addresses in the stacks of the ClickHouse binary in my tests. Using gimli-rs/addr2line as the addr2line implementation works fast and well:
image

Please take this PR as a rough proposal, there are of course details to be fleshed out:

  • Probably you want to keep the current implementation under a setting
  • If not, probably some code would need to be deleted
  • This introduces another dependency, docs and Docker images at least would have to be updated
  • Also there are likely code issues, I'm far from a Go expert

@danipozo danipozo requested a review from a team as a code owner November 19, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant