symbolizer: Symbolization implemented in Go #314
Merged
Conversation
This file contains 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
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Add demangling support Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
kakkoyun
commented
Oct 14, 2021
pkg/symbolizer/symbolizer_test.go
Outdated
| @@ -277,7 +285,7 @@ func TestRealSymbolizer(t *testing.T) { | |||
| }) | |||
| require.Equal(t, 3, len(lines)) | |||
| require.Equal(t, "/home/brancz/src/github.com/polarsignals/pprof-labels-example/main.go", lines[0].Function.Filename) | |||
| require.Equal(t, int64(10), lines[0].Line) | |||
| require.Equal(t, int64(7), lines[0].Line) // llvm-addr2line gives 10 | |||
There was a problem hiding this comment.
@brancz My only concern is this case.
There was a problem hiding this comment.
as long as inlining works in general I'm ok with anything that works consistently and we can improve in the future
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
kakkoyun
changed the title
brancz
reviewed
Oct 18, 2021
| @@ -62,6 +63,8 @@ type Flags struct { | |||
|
|
|||
| StorageTSDBRetentionTime time.Duration `default:"6h" help:"How long to retain samples in storage."` | |||
| StorageTSDBExpensiveMetrics bool `default:"false" help:"Enable really heavy metrics. Only do this for debugging as the metrics are slowing Parca down by a lot." hidden:"true"` | |||
|
|
|||
| SymbolizerDemangleMode string `default:"simple" help:"Mode to demangle C++ symbols. Default mode is simplified: : no parameters, no templates, no return type" enum:"simple,full,none,templates"` | |||
There was a problem hiding this comment.
How do I as a user know what to choose here? Maybe we can at least open an issue on the docs repo to clarify what this is, what it means and how to configure it?
| ) | ||
|
|
||
| func DWARF(demangler *demangle.Demangler, _ *profile.Mapping, path string) (func(addr uint64) ([]profile.Line, error), error) { | ||
| // TODO(kakkoyun): Handle offset, start and limit? |
There was a problem hiding this comment.
Can we open an issue for this? I think we will need this to symbolize dynamically linked libraries.
This was referenced Oct 18, 2021
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
kakkoyun
commented
Oct 18, 2021
| @@ -59,6 +59,8 @@ func (s *Symbolizer) NewLiner(m *profile.Mapping, file string) (liner, error) { | |||
| } | |||
| if hasDWARF { | |||
| level.Debug(s.logger).Log("msg", "using DWARF to resolve symbols", "file", file) | |||
| // TODO(kakkoyun): Add cache per file. | |||
kakkoyun
commented
Oct 18, 2021
| @@ -84,6 +84,7 @@ func (s *Symbolizer) symbolize(ctx context.Context, locations []*profile.Locatio | |||
| var result *multierror.Error | |||
| for id, mapping := range mappings { | |||
| level.Debug(s.logger).Log("msg", "storage symbolization request started", "buildid", mapping.BuildID) | |||
| // TODO(kakkoyun): Cache failed symbolization attempts per location. | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #276, #213
There's a lot of room for improvement (which will be addressed in subsequent PRs)