Skip to content

Commit

Permalink
Add doc URL to profile format and use it display help link. (#888)
Browse files Browse the repository at this point in the history
* Add doc URL to profile format and use it display help link.

* Add test for Report.DocURL

* Update new proto field comment
  • Loading branch information
ghemawat authored Aug 29, 2024
1 parent fa2c70b commit da1f7e9
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 0 deletions.
4 changes: 4 additions & 0 deletions internal/driver/html/common.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions internal/driver/html/header.html
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ <h1><a href="./">pprof</a></h1>
{{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>
Expand Down
2 changes: 2 additions & 0 deletions internal/driver/webui.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
17 changes: 17 additions & 0 deletions internal/report/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package report
import (
"fmt"
"io"
"net/url"
"path/filepath"
"regexp"
"sort"
Expand Down Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions internal/report/report_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:[email protected]", ""},
} {
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)
}
})
}
}
5 changes: 5 additions & 0 deletions profile/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
5 changes: 5 additions & 0 deletions profile/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -494,6 +495,9 @@ func combineHeaders(srcs []*Profile) (*Profile, error) {
if defaultSampleType == "" {
defaultSampleType = s.DefaultSampleType
}
if docURL == "" {
docURL = s.DocURL
}
}

p := &Profile{
Expand All @@ -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
Expand Down
61 changes: 61 additions & 0 deletions profile/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package profile
import (
"bytes"
"fmt"
"reflect"
"testing"

"github.com/google/pprof/internal/proftest"
Expand Down Expand Up @@ -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)
}
})
}
}
5 changes: 5 additions & 0 deletions profile/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type Profile struct {
Location []*Location
Function []*Function
Comments []string
DocURL string

DropFrames string
KeepFrames string
Expand All @@ -53,6 +54,7 @@ type Profile struct {
encodeMu sync.Mutex

commentX []int64
docURLX int64
dropFramesX int64
keepFramesX int64
stringTable []string
Expand Down Expand Up @@ -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))
}
Expand Down
22 changes: 22 additions & 0 deletions profile/profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions proto/profile.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit da1f7e9

Please sign in to comment.