Picking tags with tag focus and ignore (#183)

* Modified variable type to have []string value

* added repeatableStringKind

* modify repeatableStringKind to actually handle multiple values

* Modify applyFocus to work with multiple values for tagfocus/tagignore

* Modify applyFocus to work with multiple values for tagfocus/tagignore

* Modified CompileTagFilter to handle tag filters of the form -tagfocus=key1=val1

* Modified StringList

* add testing for compileTagFilter

* fixed typo

* fixed code style error

* Revert "fixed code style error"

This reverts commit 0b1826c163.

* Revert "fixed typo"

This reverts commit 2593652bd0.

* Revert "add testing for compileTagFilter"

This reverts commit 152d7b0363.

* Revert "Modified StringList"

This reverts commit b57d35a762.

* Revert "Modified CompileTagFilter to handle tag filters of the form -tagfocus=key1=val1"

This reverts commit ebe1e344bf.

* Revert "Modify applyFocus to work with multiple values for tagfocus/tagignore"

This reverts commit 3b62091b67.

* Revert "Modify applyFocus to work with multiple values for tagfocus/tagignore"

This reverts commit 7172159cd3.

* Revert "modify repeatableStringKind to actually handle multiple values"

This reverts commit abb1e3ec4b.

* Revert "added repeatableStringKind"

This reverts commit 8b8e4e76fc.

* Revert "Modified variable type to have []string value"

This reverts commit 5f213dc4d1.

* added support for filtering by specific tags

* Fixed formatting problems

* Updated variable name for clarity

* Added test cases for tag filtering

* Update tagfocus and tagignore description

* Fixed formatting error

* Updated docs to explain new tag filtering

* Updated docs to clarify tag filtering

* Updated docs to clarify tag filtering

* Updated docs and help comments for tag filtering

* Updated docs for tag filtering

* Update pprof docs for tag filtering

* Update tagfocus and tagignore help descriptions

* Update tagfocus and tagignore descriptions in docs

* Update tagfocus and tagignore descriptions in docs and help

* Updated tagfocus and tagignore descriptions in docs; added TODO for regexp matching

* Updated tagfocus and tagignore descriptions in docs

* Modified docs fo tagfocus for clarity

appveyor.yml

@@ -2,7 +2,7 @@ clone_folder: c:\go\src\github.com\google\pprof
 
 install:
  - cinst graphviz.portable
- 
+
 before_build:
  - go get github.com/ianlancetaylor/demangle
 

doc/pprof.md

@@ -105,10 +105,56 @@ number of values - 1) or the name of the sample value.
 
 Sample values are numeric values associated to a unit. If pprof can recognize
 these units, it will attempt to scale the values to a suitable unit for
-visualization. The `unite=` option will force the use of a specific unit. For
-example, `sample_index=sec` will force any time values to be reported in
+visualization. The `unit=` option will force the use of a specific unit. For
+example, `unit=sec` will force any time values to be reported in
 seconds. pprof recognizes most common time and memory size units.
 
+## Tag filtering
+
+Samples in a profile may have tags. These tags have a name and a value; this
+value can be either numeric or a string. pprof can select samples from a
+profile based on these tags using the `-tagfocus` and `-tagignore` options.
+
+Generally, these options work as follows:
+* **-tagfocus=_regex_** or **-tagfocus=_range_:** Restrict to samples with tags
+  matched by regexp or in range.
+* **-tagignore=_regex_** or **-tagignore=_range_:** Discard samples with tags
+  matched by regexp or in range.
+
+When using `-tagfocus=regex` and `-tagignore=regex`, the regex will be compared
+to each value associated with each tag. If one specifies a value
+like `regex1,regex2`, then only samples with a tag value matching `regex1`
+and a tag value matching `regex2` will be kept.
+
+In addition to being able to filter on tag values, one can specify the name of
+the tag which a certain value must be associated with using the notation
+`-tagfocus=tagName=value`. Here, the `tagName` must match the tag's name
+exactly, and the value can be either a regex or a range. If one specifies
+a value like `regex1,regex2`, then samples with a tag value (paired with the
+specified tag name) matching either `regex1` or matching `regex2` will match.
+
+Here are examples explaining how `tagfocus` can be used:
+
+* `-tagfocus 128kb:512kb` accepts a sample iff it has any numeric tag with
+  memory value in the specified range.
+* `-tagfocus mytag=128kb:512kb` accepts a sample iff it has a numeric tag
+  `mytag` with memory value in the specified range. There isn't a way to say
+   `-tagfocus mytag=128kb:512kb,16kb:32kb`
+   or `-tagfocus mytag=128kb:512kb,mytag2=128kb:512kb`. Just single value or
+   range for numeric tags.
+* `-tagfocus someregex` accepts a sample iff it has any string tag with
+  `tagName:tagValue` string matching specified regexp. In the future, this
+  will change to accept sample iff it has any string tag with `tagValue` string
+  matching specified regexp.
+* `-tagfocus mytag=myvalue1,myvalue2` matches if either of the two tag values
+  are present.
+
+`-tagignore` works similarly, except that it discards matching samples, instead
+of keeping them.
+
+If both the `-tagignore` and `-tagfocus` expressions (either a regexp or a
+range) match a given sample, then the sample will be discarded.
+
 ## Text reports
 
 pprof text reports show the location hierarchy in text format.

internal/driver/commands.go

@@ -195,11 +195,15 @@ var pprofVariables = variables{
 		"If set, only show nodes that match this location.",
 		"Matching includes the function name, filename or object name.")},
 	"tagfocus": &variable{stringKind, "", "", helpText(
-		"Restrict to samples with tags in range or matched by regexp",
-		"Discard samples that do not include a node with a tag matching this regexp.")},
+		"Restricts to samples with tags in range or matched by regexp",
+		"Use name=value syntax to limit the matching to a specific tag.",
+		"Numeric tag filter examples: 1kb, 1kb:10kb, memory=32mb:",
+		"String tag filter examples: foo, foo.*bar, mytag=foo.*bar")},
 	"tagignore": &variable{stringKind, "", "", helpText(
 		"Discard samples with tags in range or matched by regexp",
-		"Discard samples that do include a node with a tag matching this regexp.")},
+		"Use name=value syntax to limit the matching to a specific tag.",
+		"Numeric tag filter examples: 1kb, 1kb:10kb, memory=32mb:",
+		"String tag filter examples: foo, foo.*bar, mytag=foo.*bar")},
 	"tagshow": &variable{stringKind, "", "", helpText(
 		"Only consider tags matching this regexp",
 		"Discard tags that do not match this regexp")},

internal/driver/driver_focus.go

@@ -77,19 +77,44 @@ func compileTagFilter(name, value string, ui plugin.UI, err error) (func(*profil
 	if value == "" || err != nil {
 		return nil, err
 	}
+
+	tagValuePair := strings.SplitN(value, "=", 2)
+	var wantKey string
+	if len(tagValuePair) == 2 {
+		wantKey = tagValuePair[0]
+		value = tagValuePair[1]
+	} else {
+		wantKey = ""
+	}
+
 	if numFilter := parseTagFilterRange(value); numFilter != nil {
 		ui.PrintErr(name, ":Interpreted '", value, "' as range, not regexp")
-		return func(s *profile.Sample) bool {
-			for key, vals := range s.NumLabel {
-				for _, val := range vals {
-					if numFilter(val, key) {
+		labelFilter := func(vals []int64, key string) bool {
+			for _, val := range vals {
+				if numFilter(val, key) {
+					return true
+				}
+			}
+			return false
+		}
+		if wantKey == "" {
+			return func(s *profile.Sample) bool {
+				for key, vals := range s.NumLabel {
+					if labelFilter(vals, key) {
 						return true
 					}
 				}
+				return false
+			}, nil
+		}
+		return func(s *profile.Sample) bool {
+			if vals, ok := s.NumLabel[wantKey]; ok {
+				return labelFilter(vals, wantKey)
 			}
 			return false
 		}, nil
 	}
+
 	var rfx []*regexp.Regexp
 	for _, tagf := range strings.Split(value, ",") {
 		fx, err := regexp.Compile(tagf)
@@ -98,19 +123,34 @@ func compileTagFilter(name, value string, ui plugin.UI, err error) (func(*profil
 		}
 		rfx = append(rfx, fx)
 	}
+	if wantKey == "" {
+		return func(s *profile.Sample) bool {
+		matchedrx:
+			for _, rx := range rfx {
+				for key, vals := range s.Label {
+					for _, val := range vals {
+						// TODO: Match against val, not key:val in future
+						if rx.MatchString(key + ":" + val) {
+							continue matchedrx
+						}
+					}
+				}
+				return false
+			}
+			return true
+		}, nil
+	}
 	return func(s *profile.Sample) bool {
-	matchedrx:
-		for _, rx := range rfx {
-			for key, vals := range s.Label {
+		if vals, ok := s.Label[wantKey]; ok {
+			for _, rx := range rfx {
 				for _, val := range vals {
-					if rx.MatchString(key + ":" + val) {
-						continue matchedrx
+					if rx.MatchString(val) {
+						return true
 					}
 				}
 			}
-			return false
 		}
-		return true
+		return false
 	}, nil
 }
 

internal/driver/driver_test.go

@@ -998,8 +998,12 @@ func TestTagFilter(t *testing.T) {
 		{"test3", "tag1,tag3", map[string][]string{"value1": {"tag1", "tag2"}, "value2": {"tag3"}}, true},
 		{"test4", "t..[12],t..3", map[string][]string{"value1": {"tag1", "tag2"}, "value2": {"tag3"}}, true},
 		{"test5", "tag2,tag3", map[string][]string{"value1": {"tag1", "tag2"}}, false},
+		{"test6", "key1=tag1,tag2", map[string][]string{"key1": {"tag1", "tag2"}}, true},
+		{"test7", "key1=tag1,tag2", map[string][]string{"key1": {"tag1"}}, true},
+		{"test8", "key1:tag1,tag2", map[string][]string{"key1": {"tag1", "tag2"}}, true},
+		{"test9", "key1:tag1,tag2", map[string][]string{"key1": {"tag2"}}, false},
+		{"test10", "key1:tag1,tag2", map[string][]string{"key1": {"tag1"}}, false},
 	}
-
 	for _, test := range tagFilterTests {
 		filter, err := compileTagFilter(test.name, test.value, &proftest.TestUI{T: t}, nil)
 		if err != nil {
@@ -1016,6 +1020,48 @@ func TestTagFilter(t *testing.T) {
 	}
 }
 
+func TestNumericTagFilter(t *testing.T) {
+	var tagFilterTests = []struct {
+		name, value string
+		tags        map[string][]int64
+		want        bool
+	}{
+		{"test1", "128kb", map[string][]int64{"bytes": {131072}, "kilobytes": {128}}, true},
+		{"test2", "512kb", map[string][]int64{"bytes": {512}, "kilobytes": {128}}, false},
+		{"test3", "10b", map[string][]int64{"bytes": {10, 20}, "kilobytes": {128}}, true},
+		{"test4", ":10b", map[string][]int64{"bytes": {8}}, true},
+		{"test5", ":10kb", map[string][]int64{"bytes": {8}}, true},
+		{"test6", "10b:", map[string][]int64{"kilobytes": {8}}, true},
+		{"test7", "10b:", map[string][]int64{"bytes": {12}}, true},
+		{"test8", "10b:", map[string][]int64{"bytes": {8}}, false},
+		{"test9", "10kb:", map[string][]int64{"bytes": {8}}, false},
+		{"test10", ":10b", map[string][]int64{"kilobytes": {8}}, false},
+		{"test11", ":10b", map[string][]int64{"bytes": {12}}, false},
+		{"test12", "bytes=5b", map[string][]int64{"bytes": {5, 10}}, true},
+		{"test13", "bytes=1024b", map[string][]int64{"kilobytes": {1, 1024}}, false},
+		{"test14", "bytes=1024b", map[string][]int64{"kilobytes": {5}, "bytes": {1024}}, true},
+		{"test15", "bytes=512b:1024b", map[string][]int64{"bytes": {780}}, true},
+		{"test16", "bytes=1kb:2kb", map[string][]int64{"bytes": {4096}}, false},
+		{"test17", "bytes=512b:1024b", map[string][]int64{"bytes": {256}}, false},
+	}
+	for _, test := range tagFilterTests {
+		expectedErrMsg := fmt.Sprint([]string{test.name, ":Interpreted '", test.value, "' as range, not regexp"})
+		filter, err := compileTagFilter(test.name, test.value, &proftest.TestUI{T: t,
+			IgnoreRx: expectedErrMsg}, nil)
+		if err != nil {
+			t.Errorf("tagFilter %s:%v", test.name, err)
+			continue
+		}
+		s := profile.Sample{
+			NumLabel: test.tags,
+		}
+
+		if got := filter(&s); got != test.want {
+			t.Errorf("tagFilter %s: got %v, want %v", test.name, got, test.want)
+		}
+	}
+}
+
 type testSymbolzMergeFetcher struct{}
 
 func (testSymbolzMergeFetcher) Fetch(s string, d, t time.Duration) (*profile.Profile, string, error) {