Allow multiple comments with add_comment

internal/driver/cli.go

@@ -37,7 +37,7 @@ type source struct {
 	Symbolize          string
 	HTTPHostport       string
 	HTTPDisableBrowser bool
-	Comment            string
+	Comments           []*string
 }
 
 // parseFlags parses the command lines through the specified flags package
@@ -52,7 +52,7 @@ func parseFlags(o *plugin.Options) (*source, []string, error) {
 	flagSymbolize := flag.String("symbolize", "", "Options for profile symbolization")
 	flagBuildID := flag.String("buildid", "", "Override build id for first mapping")
 	flagTimeout := flag.Int("timeout", -1, "Timeout in seconds for fetching a profile")
-	flagAddComment := flag.String("add_comment", "", "Annotation string to record in the profile")
+	flagAddComment := flag.StringList("add_comment", "", "Annotation string to record in the profile")
 	// CPU profile options
 	flagSeconds := flag.Int("seconds", -1, "Length of time for dynamic profiles")
 	// Heap profile options
@@ -148,7 +148,7 @@ func parseFlags(o *plugin.Options) (*source, []string, error) {
 		Symbolize:          *flagSymbolize,
 		HTTPHostport:       *flagHTTP,
 		HTTPDisableBrowser: *flagNoBrowser,
-		Comment:            *flagAddComment,
+		Comments:           *flagAddComment,
 	}
 
 	if err := source.addBaseProfiles(*flagBase, *flagDiffBase); err != nil {

internal/driver/driver_test.go

@@ -78,7 +78,8 @@ func TestParse(t *testing.T) {
 		{"dot,unit=minimum", "heap_sizetags"},
 		{"dot,addresses,flat,ignore=[X3]002,focus=[X1]000", "contention"},
 		{"dot,files,cum", "contention"},
-		{"comments,add_comment=some-comment", "cpu"},
+		{"comments,add_comment=some-comment=", "cpu"},
+		{"comments,add_comment=comment1=,add_comment=comment2=", "cpu"},
 		{"comments", "heap"},
 		{"tags", "cpu"},
 		{"tags,tagignore=tag[13],tagfocus=key[12]", "cpu"},
@@ -156,15 +157,17 @@ func TestParse(t *testing.T) {
 			f.args = []string{protoTempFile.Name()}
 
 			delete(f.bools, "proto")
-			addFlags(&f, flags)
-			solution := solutionFilename(tc.source, &f)
+			solution := ""
 			// Apply the flags for the second pprof run, and identify name of
 			// the file containing expected results
 			if flags[0] == "topproto" {
-				addFlags(&f, flags)
+				addFlags(t, &f, flags)
 				solution = solutionFilename(tc.source, &f)
 				delete(f.bools, "topproto")
 				f.bools["text"] = true
+			} else {
+				addFlags(t, &f, flags)
+				solution = solutionFilename(tc.source, &f)
 			}
 
 			// Second pprof invocation to read the profile from profile.proto
@@ -238,18 +241,33 @@ func removeScripts(in []byte) []byte {
 }
 
 // addFlags parses flag descriptions and adds them to the testFlags
-func addFlags(f *testFlags, flags []string) {
+func addFlags(t *testing.T, f *testFlags, flags []string) {
 	for _, flag := range flags {
-		fields := strings.SplitN(flag, "=", 2)
+		fields := strings.SplitN(flag, "=", 3)
 		switch len(fields) {
 		case 1:
 			f.bools[fields[0]] = true
 		case 2:
+			fallthrough
+		case 3:
+			flagName := fields[0]
 			if i, err := strconv.Atoi(fields[1]); err == nil {
-				f.ints[fields[0]] = i
+				f.ints[flagName] = i
 			} else {
-				f.strings[fields[0]] = fields[1]
+				// String flag
+				if len(fields) == 3 {
+					// Assume stringLists
+					if strings, found := f.stringLists[flagName]; found {
+						f.stringLists[flagName] = append(strings, fields[1])
+					} else {
+						f.stringLists[flagName] = []string{fields[1]}
+					}
+				} else {
+					f.strings[flagName] = fields[1]
+				}
 			}
+		default:
+			t.Errorf("Invalid flag: %s", flag)
 		}
 	}
 }
@@ -263,6 +281,7 @@ func solutionFilename(source string, f *testFlags) string {
 	name := []string{"pprof", strings.TrimPrefix(source, testSourceURL(8000))}
 	name = addString(name, f, []string{"flat", "cum"})
 	name = addString(name, f, []string{"functions", "filefunctions", "files", "lines", "addresses"})
+	name = addString(name, f, []string{"add_comment"})
 	name = addString(name, f, []string{"noinlines"})
 	name = addString(name, f, []string{"inuse_space", "inuse_objects", "alloc_space", "alloc_objects"})
 	name = addString(name, f, []string{"relative_percentages"})
@@ -290,6 +309,9 @@ func addString(name []string, f *testFlags, components []string) []string {
 		if f.bools[c] || f.strings[c] != "" || f.ints[c] != 0 {
 			return append(name, c)
 		}
+		if values, found := f.stringLists[c]; found {
+			return append(name, c, fmt.Sprint(len(values)))
+		}
 	}
 	return name
 }
@@ -340,8 +362,8 @@ func (f testFlags) StringList(s, d, c string) *[]*string {
 	if t, ok := f.stringLists[s]; ok {
 		// convert slice of strings to slice of string pointers before returning.
 		tp := make([]*string, len(t))
-		for i, v := range t {
-			tp[i] = &v
+		for i, _ := range t {
+			tp[i] = &t[i]
 		}
 		return &tp
 	}
@@ -370,6 +392,7 @@ func baseFlags() testFlags {
 		strings: map[string]string{
 			"unit": "minimum",
 		},
+		stringLists: map[string][]string{},
 	}
 }
 

internal/driver/fetch.go

@@ -84,8 +84,10 @@ func fetchProfiles(s *source, o *plugin.Options) (*profile.Profile, error) {
 	p.RemoveUninteresting()
 	unsourceMappings(p)
 
-	if s.Comment != "" {
-		p.Comments = append(p.Comments, s.Comment)
+	if len(s.Comments) > 0 {
+		for _, comment := range s.Comments {
+			p.Comments = append(p.Comments, *comment)
+		}
 	}
 
 	// Save a copy of the merged profile if there is at least one remote source.

internal/driver/testdata/pprof.cpu.add_comment.2.comments

@@ -0,0 +1,2 @@
+comment1
+comment2