Repository: google/pprof
Pull request #888 (2024-08-27 20:46:07)

Add doc URL to profile format and use it display help link.
![image](https://github.com/user-attachments/assets/b1bccc9d-ca60-46ef-9bce-e39f1cdaa42e)


codecov-commenter: ## [Codecov](https://app.codecov.io/gh/google/pprof/pull/888?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) Report
Attention: Patch coverage is `54.16667%` with `11 lines` in your changes missing coverage. Please review.
> Project coverage is 66.77%. Comparing base [(`0ed6a68`)](https://app.codecov.io/gh/google/pprof/commit/0ed6a68c8d9e6d5df69a1f2694c7b5c3297fdf4f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) to head [(`4934c08`)](https://app.codecov.io/gh/google/pprof/commit/4934c0863d5b91b588e8c1244f8c101282963d6b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).
> Report is 33 commits behind head on main.

| [Files](https://app.codecov.io/gh/google/pprof/pull/888?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) | Patch % | Lines |
|---|---|---|
| [internal/report/report.go](https://app.codecov.io/gh/google/pprof/pull/888?src=pr&el=tree&filepath=internal%2Freport%2Freport.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google#diff-aW50ZXJuYWwvcmVwb3J0L3JlcG9ydC5nbw==) | 0.00% | [11 Missing :warning: ](https://app.codecov.io/gh/google/pprof/pull/888?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #888      +/-   ##
==========================================
- Coverage   66.86%   66.77%   -0.09%     
==========================================
  Files          44       44              
  Lines        9824     9822       -2     
==========================================
- Hits         6569     6559      -10     
- Misses       2794     2815      +21     
+ Partials      461      448      -13     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/google/pprof/pull/888?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).

ghemawat: This is now ready to review. Changes from prior commit:
1. Rebased to pick up Go version changes.
2. Added a small test to cover new code.

ghemawat: Will need another approval because of the comment change, sorry.

Patch:
diff --git a/internal/driver/html/common.css b/internal/driver/html/common.css
index 14f836f..0a897ce 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 e946e6b..5405a0b 100644
--- a/internal/driver/html/header.html
+++ b/internal/driver/html/header.html
@@ -83,6 +83,12 @@
       {{range .Legend}}<div>{{.}}</div>{{end}}
     </div>
   </div>
+
+  {{if .DocURL}}
+     <div class="menu-item">
+       <div class="help menu-name"><a title="Profile documentation" href="{{.DocURL}}" target="_blank">Help&nbsp;⤇</a></div>
+     </div>
+  {{end}}
 </div>
 
 <div id="dialog-overlay"></div>
diff --git a/internal/driver/webui.go b/internal/driver/webui.go
index 2a2d7fb..dd628f7 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 e21ce85..7035e23 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/profile/encode.go b/profile/encode.go
index 860bb30..8ce9d3c 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 eee0132..ba4d746 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/profile.go b/profile/profile.go
index 5551eb0..0983656 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/proto/profile.proto b/proto/profile.proto
index ff987a6..9cb816e 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.


Test patch:
diff --git a/internal/report/report_test.go b/internal/report/report_test.go
index e21189b..08ce9da 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/merge_test.go b/profile/merge_test.go
index 8c181f0..d2d05e6 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_test.go b/profile/profile_test.go
index c9e39a6..31adb66 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