From 31c679c799aefc2472238ffee47888778eeec390 Mon Sep 17 00:00:00 2001 From: Alexey Alexandrov Date: Thu, 26 Apr 2018 17:16:42 -0700 Subject: [PATCH] Add "trim path" option which can be used to relocate sources. When pprof is asked to show annotated source for a profile collected on another machine for a Go program, the profile contains absolute paths which may not exist on the local machine. In that case pprof currently fails to locate the source with no option to help it. The new option adds a way to specify one or several source path prefixes that should be trimmed from the source paths in the profile before applying the search using search path option. For example, taking the example from the issue where the source file path in the profile is /home/teamcitycpp/agent09/work/56cbaf90675b10ff/_gopath/src/badoo/lakafka/main.go and the local path is /home/marko/lakafka/main.go. The user may specify `-trim-path=/home/teamcitycpp/agent09/work/56cbaf90675b10ff/_gopath/src/badoo` to make pprof find the source. The source path doesn't need to be specified if the current working dir is anything at or under `/home/marko/`. When the trim path is not specified, it is guessed heuristically based on the basename of configured searched paths. In the example above, setting `-search-path=/home/marko/lakafka` would be sufficient to activate the heuristic successfully. Or having the current directory as `/home/marko/lakafka` since the search path is by default set to the current working directory. Note that the heuristic currently does not attempt to walk the configured search paths up like the search does. This is to keep it simple, use `-trim-path` explicitly in more complicated cases. Fixes #262. --- internal/driver/commands.go | 1 + internal/driver/driver.go | 1 + internal/report/report.go | 3 +- internal/report/report_test.go | 4 +- internal/report/source.go | 77 +++++++++++++++++++++------------- internal/report/source_test.go | 50 ++++++++++++++-------- 6 files changed, 89 insertions(+), 47 deletions(-) diff --git a/internal/driver/commands.go b/internal/driver/commands.go index 80658314..b2c0a9cd 100644 --- a/internal/driver/commands.go +++ b/internal/driver/commands.go @@ -153,6 +153,7 @@ var pprofVariables = variables{ "Using auto will scale each value independently to the most natural unit.")}, "compact_labels": &variable{boolKind, "f", "", "Show minimal headers"}, "source_path": &variable{stringKind, "", "", "Search path for source files"}, + "trim_path": &variable{stringKind, "", "", "Path to trim from source paths before search"}, // Filtering options "nodecount": &variable{intKind, "-1", "", helpText( diff --git a/internal/driver/driver.go b/internal/driver/driver.go index 0a14c67a..986f9a4a 100644 --- a/internal/driver/driver.go +++ b/internal/driver/driver.go @@ -272,6 +272,7 @@ func reportOptions(p *profile.Profile, numLabelUnits map[string]string, vars var OutputUnit: vars["unit"].value, SourcePath: vars["source_path"].stringValue(), + TrimPath: vars["trim_path"].stringValue(), } if len(p.Mapping) > 0 && p.Mapping[0].File != "" { diff --git a/internal/report/report.go b/internal/report/report.go index fa0ee3b9..15cadfb5 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -78,6 +78,7 @@ type Options struct { Symbol *regexp.Regexp // Symbols to include on disassembly report. SourcePath string // Search path for source files. + TrimPath string // Paths to trim from source file paths. } // Generate generates a report as directed by the Report. @@ -238,7 +239,7 @@ func (rpt *Report) newGraph(nodes graph.NodeSet) *graph.Graph { // Clean up file paths using heuristics. prof := rpt.prof for _, f := range prof.Function { - f.Filename = trimPath(f.Filename) + f.Filename = trimPath(f.Filename, o.TrimPath, o.SourcePath) } // Removes all numeric tags except for the bytes tag prior // to making graph. diff --git a/internal/report/report_test.go b/internal/report/report_test.go index c243e20c..49c6e493 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -46,6 +46,7 @@ func TestSource(t *testing.T) { &Options{ OutputFormat: List, Symbol: regexp.MustCompile(`.`), + TrimPath: "/some/path", SampleValue: sampleValue1, SampleUnit: testProfile.SampleType[1].Unit, @@ -60,6 +61,7 @@ func TestSource(t *testing.T) { OutputFormat: Dot, CallTree: true, Symbol: regexp.MustCompile(`.`), + TrimPath: "/some/path", SampleValue: sampleValue1, SampleUnit: testProfile.SampleType[1].Unit, @@ -119,7 +121,7 @@ var testF = []*profile.Function{ { ID: 4, Name: "tee", - Filename: "testdata/source2", + Filename: "/some/path/testdata/source2", }, } diff --git a/internal/report/source.go b/internal/report/source.go index 52958399..bd376e25 100644 --- a/internal/report/source.go +++ b/internal/report/source.go @@ -63,7 +63,7 @@ func printSource(w io.Writer, rpt *Report) error { } sourcePath = wd } - reader := newSourceReader(sourcePath) + reader := newSourceReader(sourcePath, o.TrimPath) fmt.Fprintf(w, "Total: %s\n", rpt.formatValue(rpt.total)) for _, fn := range functions { @@ -146,7 +146,7 @@ func PrintWebList(w io.Writer, rpt *Report, obj plugin.ObjTool, maxFiles int) er } sourcePath = wd } - reader := newSourceReader(sourcePath) + reader := newSourceReader(sourcePath, o.TrimPath) type fileFunction struct { fileName, functionName string @@ -263,7 +263,7 @@ func assemblyPerSourceLine(objSyms []*objSymbol, rs graph.Nodes, src string, obj // // E.g., suppose we are printing source code for F and this // instruction is from H where F called G called H and both - // of those calls were inlined. We want to use the line + // of those calls were inlined. We want to use the line // number from F, not from H (which is what Disasm gives us). // // So find the outer-most linenumber in the source file. @@ -391,8 +391,7 @@ func printFunctionSourceLine(w io.Writer, fn *graph.Node, assembly []assemblyIns continue } curCalls = nil - fname := trimPath(c.file) - fline, ok := reader.line(fname, c.line) + fline, ok := reader.line(c.file, c.line) if !ok { fline = "" } @@ -400,7 +399,7 @@ func printFunctionSourceLine(w io.Writer, fn *graph.Node, assembly []assemblyIns fmt.Fprintf(w, " %8s %10s %10s %8s %s %s:%d\n", "", "", "", "", template.HTMLEscapeString(fmt.Sprintf("%-80s", text)), - template.HTMLEscapeString(filepath.Base(fname)), c.line) + template.HTMLEscapeString(filepath.Base(c.file)), c.line) } curCalls = an.inlineCalls text := strings.Repeat(" ", srcIndent+4+4*len(curCalls)) + an.instruction @@ -426,7 +425,6 @@ func printPageClosing(w io.Writer) { // file and annotates it with the samples in fns. Returns the sources // as nodes, using the info.name field to hold the source code. func getSourceFromFile(file string, reader *sourceReader, fns graph.Nodes, start, end int) (graph.Nodes, string, error) { - file = trimPath(file) lineNodes := make(map[int]graph.Nodes) // Collect source coordinates from profile. @@ -516,20 +514,26 @@ func getMissingFunctionSource(filename string, asm map[int][]assemblyInstruction // sourceReader provides access to source code with caching of file contents. type sourceReader struct { + // searchPath is a filepath.ListSeparator-separated list of directories where + // source files should be searched. searchPath string + // trimPath is a filepath.ListSeparator-separated list of paths to trim. + trimPath string + // files maps from path name to a list of lines. // files[*][0] is unused since line numbering starts at 1. files map[string][]string - // errors collects errors encountered per file. These errors are + // errors collects errors encountered per file. These errors are // consulted before returning out of these module. errors map[string]error } -func newSourceReader(searchPath string) *sourceReader { +func newSourceReader(searchPath, trimPath string) *sourceReader { return &sourceReader{ searchPath, + trimPath, make(map[string][]string), make(map[string]error), } @@ -544,7 +548,7 @@ func (reader *sourceReader) line(path string, lineno int) (string, bool) { if !ok { // Read and cache file contents. lines = []string{""} // Skip 0th line - f, err := openSourceFile(path, reader.searchPath) + f, err := openSourceFile(path, reader.searchPath, reader.trimPath) if err != nil { reader.errors[path] = err } else { @@ -565,17 +569,20 @@ func (reader *sourceReader) line(path string, lineno int) (string, bool) { return lines[lineno], true } -// openSourceFile opens a source file from a name encoded in a -// profile. File names in a profile after often relative paths, so -// search them in each of the paths in searchPath (or CWD by default), -// and their parents. -func openSourceFile(path, searchPath string) (*os.File, error) { +// openSourceFile opens a source file from a name encoded in a profile. File +// names in a profile after can be relative paths, so search them in each of +// the paths in searchPath and their parents. In case the profile contains +// absolute paths, additional paths may be configured to trim from the source +// paths in the profile. This effectively turns the path into a relative path +// searching it using searchPath as usual). +func openSourceFile(path, searchPath, trim string) (*os.File, error) { + path = trimPath(path, trim, searchPath) + // If file is still absolute, require file to exist. if filepath.IsAbs(path) { f, err := os.Open(path) return f, err } - - // Scan each component of the path + // Scan each component of the path. for _, dir := range filepath.SplitList(searchPath) { // Search up for every parent of each possible path. for { @@ -595,18 +602,32 @@ func openSourceFile(path, searchPath string) (*os.File, error) { } // trimPath cleans up a path by removing prefixes that are commonly -// found on profiles. -func trimPath(path string) string { - basePaths := []string{ - "/proc/self/cwd/./", - "/proc/self/cwd/", +// found on profiles plus configured prefixes. +func trimPath(path, trimPath, searchPath string) string { + // Keep path variable intact as it's used below to form the return value. + sPath, searchPath := filepath.ToSlash(path), filepath.ToSlash(searchPath) + if trimPath == "" { + // If the trim path is not configured, try to guess it heuristically: + // search for basename of each search path in the original path and, if + // found, strip everything up to and including the basename. So, for + // example, given original path "/some/remote/path/my-project/foo/bar.c" + // and search path "/my/local/path/my-project" the heuristic will return + // "/my/local/path/my-project/foo/bar.c". + for _, dir := range filepath.SplitList(searchPath) { + want := "/" + filepath.Base(dir) + "/" + if found := strings.Index(sPath, want); found != -1 { + return path[found+len(want):] + } + } } - - sPath := filepath.ToSlash(path) - - for _, base := range basePaths { - if strings.HasPrefix(sPath, base) { - return filepath.FromSlash(sPath[len(base):]) + // Trim configured trim prefixes. + trimPaths := append(filepath.SplitList(filepath.ToSlash(trimPath)), "/proc/self/cwd/./", "/proc/self/cwd/") + for _, trimPath := range trimPaths { + if !strings.HasSuffix(trimPath, "/") { + trimPath += "/" + } + if strings.HasPrefix(sPath, trimPath) { + return path[len(trimPath):] } } return path diff --git a/internal/report/source_test.go b/internal/report/source_test.go index 682bfe0a..f1dd5c70 100644 --- a/internal/report/source_test.go +++ b/internal/report/source_test.go @@ -48,40 +48,56 @@ func TestOpenSourceFile(t *testing.T) { for _, tc := range []struct { desc string searchPath string + trimPath string fs []string path string wantPath string // If empty, error is wanted. }{ { desc: "exact absolute path is found", - fs: []string{"foo/bar.txt"}, - path: "$dir/foo/bar.txt", - wantPath: "$dir/foo/bar.txt", + fs: []string{"foo/bar.cc"}, + path: "$dir/foo/bar.cc", + wantPath: "$dir/foo/bar.cc", }, { desc: "exact relative path is found", searchPath: "$dir", - fs: []string{"foo/bar.txt"}, - path: "foo/bar.txt", - wantPath: "$dir/foo/bar.txt", + fs: []string{"foo/bar.cc"}, + path: "foo/bar.cc", + wantPath: "$dir/foo/bar.cc", }, { desc: "multiple search path", searchPath: "some/path" + lsep + "$dir", - fs: []string{"foo/bar.txt"}, - path: "foo/bar.txt", - wantPath: "$dir/foo/bar.txt", + fs: []string{"foo/bar.cc"}, + path: "foo/bar.cc", + wantPath: "$dir/foo/bar.cc", }, { desc: "relative path is found in parent dir", searchPath: "$dir/foo/bar", - fs: []string{"bar.txt", "foo/bar/baz.txt"}, - path: "bar.txt", - wantPath: "$dir/bar.txt", + fs: []string{"bar.cc", "foo/bar/baz.cc"}, + path: "bar.cc", + wantPath: "$dir/bar.cc", + }, + { + desc: "trims configured prefix", + searchPath: "$dir", + trimPath: "some-path" + lsep + "/some/remote/path", + fs: []string{"my-project/foo/bar.cc"}, + path: "/some/remote/path/my-project/foo/bar.cc", + wantPath: "$dir/my-project/foo/bar.cc", + }, + { + desc: "trims heuristically", + searchPath: "$dir/my-project", + fs: []string{"my-project/foo/bar.cc"}, + path: "/some/remote/path/my-project/foo/bar.cc", + wantPath: "$dir/my-project/foo/bar.cc", }, { desc: "error when not found", - path: "foo.txt", + path: "foo.cc", }, } { t.Run(tc.desc, func(t *testing.T) { @@ -103,15 +119,15 @@ func TestOpenSourceFile(t *testing.T) { tc.searchPath = filepath.FromSlash(strings.Replace(tc.searchPath, "$dir", tempdir, -1)) tc.path = filepath.FromSlash(strings.Replace(tc.path, "$dir", tempdir, 1)) tc.wantPath = filepath.FromSlash(strings.Replace(tc.wantPath, "$dir", tempdir, 1)) - if file, err := openSourceFile(tc.path, tc.searchPath); err != nil && tc.wantPath != "" { - t.Errorf("openSourceFile(%q, %q) = err %v, want path %q", tc.path, tc.searchPath, err, tc.wantPath) + if file, err := openSourceFile(tc.path, tc.searchPath, tc.trimPath); err != nil && tc.wantPath != "" { + t.Errorf("openSourceFile(%q, %q, %q) = err %v, want path %q", tc.path, tc.searchPath, tc.trimPath, err, tc.wantPath) } else if err == nil { defer file.Close() gotPath := file.Name() if tc.wantPath == "" { - t.Errorf("openSourceFile(%q, %q) = %q, want error", tc.path, tc.searchPath, gotPath) + t.Errorf("openSourceFile(%q, %q, %q) = %q, want error", tc.path, tc.searchPath, tc.trimPath, gotPath) } else if gotPath != tc.wantPath { - t.Errorf("openSourceFile(%q, %q) = %q, want path %q", tc.path, tc.searchPath, gotPath, tc.wantPath) + t.Errorf("openSourceFile(%q, %q, %q) = %q, want path %q", tc.path, tc.searchPath, tc.trimPath, gotPath, tc.wantPath) } } })