From da1f7e9f2b251de696b28108b5a2b1c0bbce44ed Mon Sep 17 00:00:00 2001 From: Sanjay Ghemawat Date: Thu, 29 Aug 2024 09:03:00 -0700 Subject: [PATCH] Add doc URL to profile format and use it display help link. (#888) * Add doc URL to profile format and use it display help link. * Add test for Report.DocURL * Update new proto field comment --- internal/driver/html/common.css | 4 +++ internal/driver/html/header.html | 6 ++++ internal/driver/webui.go | 2 ++ internal/report/report.go | 17 +++++++++ internal/report/report_test.go | 29 +++++++++++++++ profile/encode.go | 5 +++ profile/merge.go | 5 +++ profile/merge_test.go | 61 ++++++++++++++++++++++++++++++++ profile/profile.go | 5 +++ profile/profile_test.go | 22 ++++++++++++ proto/profile.proto | 6 ++++ 11 files changed, 162 insertions(+) diff --git a/internal/driver/html/common.css b/internal/driver/html/common.css index 14f836ff1..0a897ce29 100644 --- a/internal/driver/html/common.css +++ b/internal/driver/html/common.css @@ -148,6 +148,10 @@ a { right: 2px; } +.help { + padding-left: 1em; +} + {{/* Used to disable events when a modal dialog is displayed */}} #dialog-overlay { display: none; diff --git a/internal/driver/html/header.html b/internal/driver/html/header.html index e946e6b88..5405a0be9 100644 --- a/internal/driver/html/header.html +++ b/internal/driver/html/header.html @@ -83,6 +83,12 @@

pprof

{{range .Legend}}
{{.}}
{{end}} + + {{if .DocURL}} +
+
Help ⤇
+
+ {{end}}
diff --git a/internal/driver/webui.go b/internal/driver/webui.go index 2a2d7fb1d..dd628f7c2 100644 --- a/internal/driver/webui.go +++ b/internal/driver/webui.go @@ -79,6 +79,7 @@ type webArgs struct { Total int64 SampleTypes []string Legend []string + DocURL string Standalone bool // True for command-line generation of HTML Help map[string]string Nodes []string @@ -290,6 +291,7 @@ func renderHTML(dst io.Writer, tmpl string, rpt *report.Report, errList, legend data.Title = file + " " + profile data.Errors = errList data.Total = rpt.Total() + data.DocURL = rpt.DocURL() data.Legend = legend return getHTMLTemplates().ExecuteTemplate(dst, tmpl, data) } diff --git a/internal/report/report.go b/internal/report/report.go index e21ce859d..7035e2313 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -19,6 +19,7 @@ package report import ( "fmt" "io" + "net/url" "path/filepath" "regexp" "sort" @@ -1331,6 +1332,22 @@ func (rpt *Report) Total() int64 { return rpt.total } // OutputFormat returns the output format for the report. func (rpt *Report) OutputFormat() int { return rpt.options.OutputFormat } +// DocURL returns the documentation URL for Report, or "" if not available. +func (rpt *Report) DocURL() string { + u := rpt.prof.DocURL + if u == "" || !absoluteURL(u) { + return "" + } + return u +} + +func absoluteURL(str string) bool { + // Avoid returning relative URLs to prevent unwanted local navigation + // within pprof server. + u, err := url.Parse(str) + return err == nil && (u.Scheme == "https" || u.Scheme == "http") +} + func abs64(i int64) int64 { if i < 0 { return -i diff --git a/internal/report/report_test.go b/internal/report/report_test.go index e21189b40..08ce9da1e 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -547,3 +547,32 @@ func TestPrintAssemblyErrorMessage(t *testing.T) { } } } + +func TestDocURL(t *testing.T) { + type testCase struct { + input string + want string + } + for name, c := range map[string]testCase{ + "empty": {"", ""}, + "http": {"http://example.com/pprof-help", "http://example.com/pprof-help"}, + "https": {"https://example.com/pprof-help", "https://example.com/pprof-help"}, + "relative": {"/foo", ""}, + "nonhttp": {"mailto:nobody@example.com", ""}, + } { + t.Run(name, func(t *testing.T) { + profile := testProfile.Copy() + profile.DocURL = c.input + rpt := New(profile, &Options{ + OutputFormat: Dot, + Symbol: regexp.MustCompile(`.`), + TrimPath: "/some/path", + SampleValue: func(v []int64) int64 { return v[1] }, + SampleUnit: testProfile.SampleType[1].Unit, + }) + if got := rpt.DocURL(); got != c.want { + t.Errorf("bad doc URL %q, expecting %q", got, c.want) + } + }) + } +} diff --git a/profile/encode.go b/profile/encode.go index 860bb304c..8ce9d3cf3 100644 --- a/profile/encode.go +++ b/profile/encode.go @@ -122,6 +122,7 @@ func (p *Profile) preEncode() { } p.defaultSampleTypeX = addString(strings, p.DefaultSampleType) + p.docURLX = addString(strings, p.DocURL) p.stringTable = make([]string, len(strings)) for s, i := range strings { @@ -156,6 +157,7 @@ func (p *Profile) encode(b *buffer) { encodeInt64Opt(b, 12, p.Period) encodeInt64s(b, 13, p.commentX) encodeInt64(b, 14, p.defaultSampleTypeX) + encodeInt64Opt(b, 15, p.docURLX) } var profileDecoder = []decoder{ @@ -237,6 +239,8 @@ var profileDecoder = []decoder{ func(b *buffer, m message) error { return decodeInt64s(b, &m.(*Profile).commentX) }, // int64 defaultSampleType = 14 func(b *buffer, m message) error { return decodeInt64(b, &m.(*Profile).defaultSampleTypeX) }, + // string doc_link = 15; + func(b *buffer, m message) error { return decodeInt64(b, &m.(*Profile).docURLX) }, } // postDecode takes the unexported fields populated by decode (with @@ -384,6 +388,7 @@ func (p *Profile) postDecode() error { p.commentX = nil p.DefaultSampleType, err = getString(p.stringTable, &p.defaultSampleTypeX, err) + p.DocURL, err = getString(p.stringTable, &p.docURLX, err) p.stringTable = nil return err } diff --git a/profile/merge.go b/profile/merge.go index eee0132e7..ba4d74640 100644 --- a/profile/merge.go +++ b/profile/merge.go @@ -476,6 +476,7 @@ func combineHeaders(srcs []*Profile) (*Profile, error) { var timeNanos, durationNanos, period int64 var comments []string seenComments := map[string]bool{} + var docURL string var defaultSampleType string for _, s := range srcs { if timeNanos == 0 || s.TimeNanos < timeNanos { @@ -494,6 +495,9 @@ func combineHeaders(srcs []*Profile) (*Profile, error) { if defaultSampleType == "" { defaultSampleType = s.DefaultSampleType } + if docURL == "" { + docURL = s.DocURL + } } p := &Profile{ @@ -509,6 +513,7 @@ func combineHeaders(srcs []*Profile) (*Profile, error) { Comments: comments, DefaultSampleType: defaultSampleType, + DocURL: docURL, } copy(p.SampleType, srcs[0].SampleType) return p, nil diff --git a/profile/merge_test.go b/profile/merge_test.go index 8c181f0c4..d2d05e63e 100644 --- a/profile/merge_test.go +++ b/profile/merge_test.go @@ -17,6 +17,7 @@ package profile import ( "bytes" "fmt" + "reflect" "testing" "github.com/google/pprof/internal/proftest" @@ -443,3 +444,63 @@ func TestCompatibilizeSampleTypes(t *testing.T) { }) } } + +func TestDocURLMerge(t *testing.T) { + const url1 = "http://example.com/url1" + const url2 = "http://example.com/url2" + type testCase struct { + name string + profiles []*Profile + want string + } + profile := func(url string) *Profile { + return &Profile{ + PeriodType: &ValueType{Type: "cpu", Unit: "seconds"}, + DocURL: url, + } + } + for _, test := range []testCase{ + { + name: "nolinks", + profiles: []*Profile{ + profile(""), + profile(""), + }, + want: "", + }, + { + name: "single", + profiles: []*Profile{ + profile(url1), + }, + want: url1, + }, + { + name: "mix", + profiles: []*Profile{ + profile(""), + profile(url1), + }, + want: url1, + }, + { + name: "different", + profiles: []*Profile{ + profile(url1), + profile(url2), + }, + want: url1, + }, + } { + t.Run(test.name, func(t *testing.T) { + merged, err := combineHeaders(test.profiles) + if err != nil { + t.Fatal(err) + } + got := merged.DocURL + if !reflect.DeepEqual(test.want, got) { + t.Errorf("unexpected links; want: %#v, got: %#v", test.want, got) + } + }) + } +} diff --git a/profile/profile.go b/profile/profile.go index 5551eb0bf..0983656c2 100644 --- a/profile/profile.go +++ b/profile/profile.go @@ -39,6 +39,7 @@ type Profile struct { Location []*Location Function []*Function Comments []string + DocURL string DropFrames string KeepFrames string @@ -53,6 +54,7 @@ type Profile struct { encodeMu sync.Mutex commentX []int64 + docURLX int64 dropFramesX int64 keepFramesX int64 stringTable []string @@ -555,6 +557,9 @@ func (p *Profile) String() string { for _, c := range p.Comments { ss = append(ss, "Comment: "+c) } + if url := p.DocURL; url != "" { + ss = append(ss, fmt.Sprintf("Doc: %s", url)) + } if pt := p.PeriodType; pt != nil { ss = append(ss, fmt.Sprintf("PeriodType: %s %s", pt.Type, pt.Unit)) } diff --git a/profile/profile_test.go b/profile/profile_test.go index c9e39a623..31adb66f7 100644 --- a/profile/profile_test.go +++ b/profile/profile_test.go @@ -1869,6 +1869,28 @@ func TestParseKernelRelocation(t *testing.T) { } } +func TestEncodeDecodeDocURL(t *testing.T) { + input := testProfile1.Copy() + input.DocURL = "http://example.comp/url" + + // Encode/decode. + var buf bytes.Buffer + if err := input.Write(&buf); err != nil { + t.Fatal("encode: ", err) + } + output, err := Parse(&buf) + if err != nil { + t.Fatal("decode: ", err) + } + if want, got := input.String(), output.String(); want != got { + d, err := proftest.Diff([]byte(want), []byte(got)) + if err != nil { + t.Fatal(err) + } + t.Errorf("wrong result of encode/decode (-want,+got):\n%s\n", string(d)) + } +} + // parallel runs n copies of fn in parallel. func parallel(n int, fn func()) { var wg sync.WaitGroup diff --git a/proto/profile.proto b/proto/profile.proto index ff987a617..9cb816e66 100644 --- a/proto/profile.proto +++ b/proto/profile.proto @@ -93,6 +93,12 @@ message Profile { // Index into the string table of the type of the preferred sample // value. If unset, clients should default to the last sample value. int64 default_sample_type = 14; + // Documentation link for this profile. The URL must be absolute, + // e.g., http://pprof.example.com/cpu-profile.html + // + // The URL may be missing if the profile was generated by older code or code + // that did not bother to supply a link. + int64 doc_url = 15; // Index into string table. } // ValueType describes the semantics and measurement units of a value.