From 29a1cd5041c2bbf1d43fd9858de7cd5dbb3c9592 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 would 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/`. TODO: Heuristical default value for the trim path. 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 | 62 +++++++++++++++++++--------------- internal/report/source_test.go | 43 +++++++++++++---------- 6 files changed, 67 insertions(+), 47 deletions(-) diff --git a/internal/driver/commands.go b/internal/driver/commands.go index 16b0b0a3b..c920a5ba7 100644 --- a/internal/driver/commands.go +++ b/internal/driver/commands.go @@ -161,6 +161,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 3b7439fc9..d032bfc81 100644 --- a/internal/driver/driver.go +++ b/internal/driver/driver.go @@ -273,6 +273,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 e127f7fce..2b84241eb 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -79,6 +79,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. @@ -239,7 +240,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) } // 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 c243e20c2..49c6e4934 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 529583997..cc46098cc 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) + // 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,17 @@ 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 string) string { + // Trim configured trim prefixes. + trimPaths := append(filepath.SplitList(filepath.ToSlash(trimPath)), "/proc/self/cwd/./", "/proc/self/cwd/") sPath := filepath.ToSlash(path) - - for _, base := range basePaths { - if strings.HasPrefix(sPath, base) { - return filepath.FromSlash(sPath[len(base):]) + 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 682bfe0a1..7dab6b91d 100644 --- a/internal/report/source_test.go +++ b/internal/report/source_test.go @@ -48,40 +48,49 @@ 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: "error when not found", - path: "foo.txt", + path: "foo.cc", }, } { t.Run(tc.desc, func(t *testing.T) { @@ -103,15 +112,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) } } })