Skip to content

Commit e94b44e

Browse files
authored
fix: drop negative samples (#3955)
1 parent 17ec44e commit e94b44e

File tree

2 files changed

+87
-12
lines changed

2 files changed

+87
-12
lines changed

‎pkg/pprof/pprof.go

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -367,16 +367,31 @@ func (p *Profile) Normalize() {
367367
})
368368
}
369369

370+
// Remove samples.
371+
var removedSamples []*profilev1.Sample
372+
p.Sample = slices.RemoveInPlace(p.Sample, func(s *profilev1.Sample, i int) bool {
373+
for j := 0; j < len(s.Value); j++ {
374+
if s.Value[j] < 0 {
375+
removedSamples = append(removedSamples, s)
376+
return true
377+
}
378+
}
379+
for j := 0; j < len(s.Value); j++ {
380+
if s.Value[j] > 0 {
381+
return false
382+
}
383+
}
384+
removedSamples = append(removedSamples, s)
385+
return true
386+
})
387+
370388
// first we sort the samples.
371389
hashes := p.hasher.Hashes(p.Sample)
372390
ss := &sortedSample{samples: p.Sample, hashes: hashes}
373391
sort.Sort(ss)
374392
p.Sample = ss.samples
375393
hashes = ss.hashes
376394

377-
// Remove samples.
378-
var removedSamples []*profilev1.Sample
379-
380395
p.Sample = slices.RemoveInPlace(p.Sample, func(s *profilev1.Sample, i int) bool {
381396
// if the next sample has the same hash and labels, we can remove this sample but add the value to the next sample.
382397
if i < len(p.Sample)-1 && hashes[i] == hashes[i+1] {
@@ -387,15 +402,7 @@ func (p *Profile) Normalize() {
387402
removedSamples = append(removedSamples, s)
388403
return true
389404
}
390-
for j := 0; j < len(s.Value); j++ {
391-
if s.Value[j] > 0 {
392-
// we found a non-zero value, so we can keep this sample.
393-
return false
394-
}
395-
}
396-
// all values are 0, remove the sample.
397-
removedSamples = append(removedSamples, s)
398-
return true
405+
return false
399406
})
400407

401408
// Remove references to removed samples.

‎pkg/pprof/pprof_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,74 @@ func TestNormalizeProfile(t *testing.T) {
105105
}, pf.Profile)
106106
}
107107

108+
func TestNormalizeProfile_NegativeSample(t *testing.T) {
109+
currentTime = func() time.Time {
110+
t, _ := time.Parse(time.RFC3339, "2020-01-01T00:00:00Z")
111+
return t
112+
}
113+
defer func() {
114+
currentTime = time.Now
115+
}()
116+
117+
p := &profilev1.Profile{
118+
SampleType: []*profilev1.ValueType{
119+
{Type: 1, Unit: 2},
120+
},
121+
Sample: []*profilev1.Sample{
122+
{LocationId: []uint64{2, 1}, Value: []int64{10}, Label: []*profilev1.Label{{Str: 5, Key: 5}, {Str: 5, Key: 6}}},
123+
{LocationId: []uint64{2, 1}, Value: []int64{10}, Label: []*profilev1.Label{{Str: 5, Key: 5}, {Str: 5, Key: 7}}},
124+
{LocationId: []uint64{2, 1}, Value: []int64{10}, Label: []*profilev1.Label{{Str: 5, Key: 5}, {Str: 5, Key: 7}}},
125+
{LocationId: []uint64{2, 1}, Value: []int64{-10}, Label: []*profilev1.Label{{Str: 5, Key: 5}, {Str: 5, Key: 7}}},
126+
{LocationId: []uint64{2, 1}, Value: []int64{0}, Label: []*profilev1.Label{{Str: 5, Key: 5}, {Str: 5, Key: 7}}},
127+
},
128+
Mapping: []*profilev1.Mapping{{Id: 1, HasFunctions: true, MemoryStart: 100, MemoryLimit: 200, FileOffset: 200}},
129+
Location: []*profilev1.Location{
130+
{Id: 1, MappingId: 1, Address: 5, Line: []*profilev1.Line{{FunctionId: 1, Line: 1}}},
131+
{Id: 2, MappingId: 1, Address: 2, Line: []*profilev1.Line{{FunctionId: 2, Line: 1}}},
132+
},
133+
Function: []*profilev1.Function{
134+
{Id: 1, Name: 3, SystemName: 3, Filename: 4, StartLine: 1},
135+
{Id: 2, Name: 5, SystemName: 5, Filename: 4, StartLine: 1},
136+
},
137+
StringTable: []string{
138+
"",
139+
"cpu", "nanoseconds",
140+
"main", "main.go",
141+
"foo", "bar", "baz",
142+
},
143+
PeriodType: &profilev1.ValueType{Type: 1, Unit: 2},
144+
}
145+
146+
pf := &Profile{Profile: p}
147+
pf.Normalize()
148+
require.Equal(t, pf.Profile, &profilev1.Profile{
149+
SampleType: []*profilev1.ValueType{
150+
{Type: 1, Unit: 2},
151+
},
152+
Sample: []*profilev1.Sample{
153+
{LocationId: []uint64{2, 1}, Value: []int64{10}, Label: []*profilev1.Label{{Str: 5, Key: 5}, {Str: 5, Key: 6}}},
154+
{LocationId: []uint64{2, 1}, Value: []int64{20}, Label: []*profilev1.Label{{Str: 5, Key: 5}, {Str: 5, Key: 7}}},
155+
},
156+
Mapping: []*profilev1.Mapping{{Id: 1, HasFunctions: true}},
157+
Location: []*profilev1.Location{
158+
{Id: 1, MappingId: 1, Line: []*profilev1.Line{{FunctionId: 1, Line: 1}}},
159+
{Id: 2, MappingId: 1, Line: []*profilev1.Line{{FunctionId: 2, Line: 1}}},
160+
},
161+
Function: []*profilev1.Function{
162+
{Id: 1, Name: 3, SystemName: 3, Filename: 4, StartLine: 1},
163+
{Id: 2, Name: 5, SystemName: 5, Filename: 4, StartLine: 1},
164+
},
165+
StringTable: []string{
166+
"",
167+
"cpu", "nanoseconds",
168+
"main", "main.go",
169+
"foo", "bar", "baz",
170+
},
171+
PeriodType: &profilev1.ValueType{Type: 1, Unit: 2},
172+
TimeNanos: 1577836800000000000,
173+
})
174+
}
175+
108176
func TestNormalizeProfile_SampleLabels(t *testing.T) {
109177
currentTime = func() time.Time {
110178
t, _ := time.Parse(time.RFC3339, "2020-01-01T00:00:00Z")

0 commit comments

Comments
 (0)