qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-23 18:20:04 -07:00
Code Releases Activity

runtime/pprof: improve output of TestLabelSystemstack

Browse Source 
The current output of TestLabelSystemstack is a bit cryptic. This CL
improves various messages and hopefully simplifies the logic in the
test.

Simplifying the logic leads to three changes in possible outcomes,
which I verified by running the logic before and after this change
through all 2^4 possibilities (https://go.dev/play/p/bnfb-OQCT4j):

1. If a sample both must be labeled and must not be labeled, the test
now reports that explicitly rather than giving other confusing output.

2. If a sample must not be labeled but is, the current logic will
print two identical error messages. The new logic prints only one.

3. If the test finds no frames at all that it recognizes, but the
sample is labeled, it will currently print a confusing "Sample labeled
got true want false" message. The new logic prints nothing. We've seen
this triggered by empty stacks in profiles.

Fixes #51550. This bug was caused by case 3 above, where it was
triggered by a profile label on an empty stack. It's valid for empty
stacks to appear in a profile if we sample a goroutine just as it's
exiting (and that goroutine may have a profile label), so the test
shouldn't fail in this case.

Change-Id: I1593ec4ac33eced5bb89572a3ba7623e56f2fb3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/460516
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
Austin Clements 2023-01-04 12:31:23 -05:00
parent 8232a09e3e
commit d9f23cfe78
1 changed files with 26 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
src/runtime/pprof/pprof_test.go
View File

@ -607,7 +607,7 @@ func matchAndAvoidStacks(matches sampleMatchFunc, need []string, avoid []string)
var total uintptr
for i, name := range need {
total += have[i]
t.Logf("%s: %d\n", name, have[i])
t.Logf("found %d samples in expected function %s\n", have[i], name)
}
if total == 0 {
t.Logf("no samples in expected functions")
@ -727,6 +727,9 @@ func TestGoroutineSwitch(t *testing.T) {
}
func fprintStack(w io.Writer, stk []*profile.Location) {
if len(stk) == 0 {
fmt.Fprintf(w, " (stack empty)")
}
for _, loc := range stk {
fmt.Fprintf(w, " %#x", loc.Address)
fmt.Fprintf(w, " (")
@ -1867,14 +1870,14 @@ func TestLabelSystemstack(t *testing.T) {
isLabeled := s.Label != nil && contains(s.Label["key"], "value")
var (
mayBeLabeled bool
mustBeLabeled bool
mustNotBeLabeled bool
mustBeLabeled string
mustNotBeLabeled string
)
for _, loc := range s.Location {
for _, l := range loc.Line {
switch l.Function.Name {
case "runtime/pprof.labelHog", "runtime/pprof.parallelLabelHog", "runtime/pprof.parallelLabelHog.func1":
mustBeLabeled = true
mustBeLabeled = l.Function.Name
case "runtime/pprof.Do":
// Do sets the labels, so samples may
// or may not be labeled depending on
@ -1886,7 +1889,7 @@ func TestLabelSystemstack(t *testing.T) {
// (such as those identified by
// runtime.isSystemGoroutine). These
// should never be labeled.
mustNotBeLabeled = true
mustNotBeLabeled = l.Function.Name
case "gogo", "gosave_systemstack_switch", "racecall":
// These are context switch/race
// critical that we can't do a full
@ -1908,25 +1911,28 @@ func TestLabelSystemstack(t *testing.T) {
}
}
}
if mustNotBeLabeled {
// If this must not be labeled, then mayBeLabeled hints
// are not relevant.
errorStack := func(f string, args ...any) {
var buf strings.Builder
fprintStack(&buf, s.Location)
t.Errorf("%s: %s", fmt.Sprintf(f, args...), buf.String())
}
if mustBeLabeled != "" && mustNotBeLabeled != "" {
errorStack("sample contains both %s, which must be labeled, and %s, which must not be labeled", mustBeLabeled, mustNotBeLabeled)
continue
}
if mustBeLabeled != "" || mustNotBeLabeled != "" {
// We found a definitive frame, so mayBeLabeled hints are not relevant.
mayBeLabeled = false
}
if mustBeLabeled && !isLabeled {
var buf strings.Builder
fprintStack(&buf, s.Location)
t.Errorf("Sample labeled got false want true: %s", buf.String())
if mayBeLabeled {
// This sample may or may not be labeled, so there's nothing we can check.
continue
}
if mustNotBeLabeled && isLabeled {
var buf strings.Builder
fprintStack(&buf, s.Location)
t.Errorf("Sample labeled got true want false: %s", buf.String())
if mustBeLabeled != "" && !isLabeled {
errorStack("sample must be labeled because of %s, but is not", mustBeLabeled)
}
if isLabeled && !(mayBeLabeled || mustBeLabeled) {
var buf strings.Builder
fprintStack(&buf, s.Location)
t.Errorf("Sample labeled got true want false: %s", buf.String())
if mustNotBeLabeled != "" && isLabeled {
errorStack("sample must not be labeled because of %s, but is", mustNotBeLabeled)
}
}
}