-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Replace "backward" stack trace with absl::Stacktrace. #5723
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
80a0685
Replace "backward" stack trace with absl::Stacktrace.
dnoe 5572868
Switch to new style constant, fix initialization bug.
dnoe 9698ea7
Merge branch 'master' of https://github.com/envoyproxy/envoy into rep…
dnoe cce16c0
Include addresses even if resolved, update README.
dnoe e826141
Merge branch 'master' of https://github.com/envoyproxy/envoy into rep…
dnoe 08fb320
Modify stack_decode.py to work again.
dnoe 58139d5
Re-fix-format.
dnoe 2887f00
Fix python format with Docker container.
dnoe f3a928e
Trim /proc/cwd stuff, update docs.
dnoe 706107a
Python nits and doc improvements.
dnoe 90e4908
Merge branch 'master' of https://github.com/envoyproxy/envoy into rep…
dnoe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -279,7 +279,6 @@ def envoy_dependencies(path = "@envoy_deps//", skip_targets = []): | |
| # semi-standard in the Bazel community, intended to avoid both duplicate | ||
| # dependencies and name conflicts. | ||
| _com_google_absl() | ||
| _com_github_bombela_backward() | ||
| _com_github_circonus_labs_libcircllhist() | ||
| _com_github_cyan4973_xxhash() | ||
| _com_github_eile_tclap() | ||
|
|
@@ -321,16 +320,6 @@ def _boringssl_fips(): | |
| build_file = "@envoy//bazel/external:boringssl_fips.BUILD", | ||
| ) | ||
|
|
||
| def _com_github_bombela_backward(): | ||
| _repository_impl( | ||
| name = "com_github_bombela_backward", | ||
| build_file = "@envoy//bazel/external:backward.BUILD", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just noticed that https://github.com/envoyproxy/envoy/blob/master/bazel/external/backward.BUILD is still in the codebase. This can be removed now I believe @dnoe |
||
| ) | ||
| native.bind( | ||
| name = "backward", | ||
| actual = "@com_github_bombela_backward//:backward", | ||
| ) | ||
|
|
||
| def _com_github_circonus_labs_libcircllhist(): | ||
| _repository_impl( | ||
| name = "com_github_circonus_labs_libcircllhist", | ||
|
|
@@ -529,6 +518,10 @@ def _com_google_absl(): | |
| name = "abseil_symbolize", | ||
| actual = "@com_google_absl//absl/debugging:symbolize", | ||
| ) | ||
| native.bind( | ||
| name = "abseil_stacktrace", | ||
| actual = "@com_google_absl//absl/debugging:stacktrace", | ||
| ) | ||
|
|
||
| # Require abseil_time as an indirect dependency as it is needed by the | ||
| # direct dependency jwt_verify_lib. | ||
|
|
||
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
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
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
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
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
This file was deleted.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a suggested
bazel --run_under=....snippet describing how we can get line numbers?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addr2lineisn't a pass through by itself, although I implemented such a functionality intostack_decode.py. If you think it'll actually be useful I can modifystack_decode.pyto work with the new format backtraces rather than eliminate it. Then you'll be able to usebazel --run_under=like before.Alternatively, I can just write instructions for manually invoking
addr2linewith the right args and the addresses of interest (it just won't be a--run_under=).I have a mild preference for the latter (and let
stack_decode.pydie), WDYT?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like run_under is easier to use unless addr2line is destructive to lines that are not part of stack traces. What's the reason you prefer to ask users to do this manually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let's revive
stack_decode.py. Sorry about the futz here, but I love my line numbers! :DThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries! I didn't think anyone was using it! Happy to fix it up to get the best of both worlds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, revert this doc change? The previous flow still works, yeah? So the additional detail is not strictly necessary.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flow is slightly different. Previously,
stack_decode.pyautodetected the object file name from the log, because backward had this information and we could log it. But we don't have it anymore and the executable file needs to be passed in all circumstances so the usage details ofstack_decode.pychanged slightly. I'm going to update the doc now that mac CI is confirmed to be passing.