Skip to content

Commit

Permalink
profiler: don't upload full profiles if delta profiling is enabled (#…
Browse files Browse the repository at this point in the history
…1187)

The full heap, mutex, and block profiles are not needed when delta
profiling is enabled. Removing them (especially the full heap profile)
results in a significant bandwidth and storage cost reduction.
  • Loading branch information
nsrip-dd committed Mar 1, 2022
1 parent 55b07a6 commit ebf57ee
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 22 deletions.
16 changes: 9 additions & 7 deletions profiler/profile.go
Expand Up @@ -214,24 +214,26 @@ func (p *profiler) runProfile(pt ProfileType) ([]*profile, error) {
if err != nil {
return nil, err
}
profs := []*profile{{
name: t.Filename,
data: data,
}}
// Compute the deltaProf (will be nil if not enabled for this profile type).
deltaStart := time.Now()
deltaProf, err := p.deltaProfile(t, data)
if err != nil {
return nil, fmt.Errorf("delta profile error: %s", err)
}
// Report metrics and append deltaProf if not nil.
end := now()
tags := append(p.cfg.tags, pt.Tag())
// TODO(fg) stop uploading non-delta profiles in the next version of
// dd-trace-go after delta profiles are released.
var profs []*profile
if deltaProf != nil {
profs = append(profs, deltaProf)
p.cfg.statsd.Timing("datadog.profiler.go.delta_time", end.Sub(deltaStart), tags, 1)
} else {
// If the user has disabled delta profiles, or the profile type
// doesn't support delta profiles (like the CPU profile) then
// send the original profile unchanged.
profs = append(profs, &profile{
name: t.Filename,
data: data,
})
}
p.cfg.statsd.Timing("datadog.profiler.go.collect_time", end.Sub(start), tags, 1)
return profs, nil
Expand Down
16 changes: 6 additions & 10 deletions profiler/profile_test.go
Expand Up @@ -139,23 +139,19 @@ main;bar 0 0 8 16
// additional profile type to ease the transition)
profs, err := p.runProfile(profType)
require.NoError(t, err)
require.Equal(t, 2, len(profs))
require.Equal(t, profType.Filename(), profs[0].name)
require.Equal(t, 1, len(profs))
require.Equal(t, "delta-"+profType.Filename(), profs[0].name)
require.Equal(t, prof1, profs[0].data)
require.Equal(t, "delta-"+profType.Filename(), profs[1].name)
require.Equal(t, prof1, profs[1].data)

// second run, should produce p1 profile and delta profile
profs, err = p.runProfile(profType)
require.NoError(t, err)
require.Equal(t, 2, len(profs))
require.Equal(t, profType.Filename(), profs[0].name)
require.Equal(t, prof2, profs[0].data)
require.Equal(t, "delta-"+profType.Filename(), profs[1].name)
require.Equal(t, test.WantDelta.String(), protobufToText(profs[1].data))
require.Equal(t, 1, len(profs))
require.Equal(t, "delta-"+profType.Filename(), profs[0].name)
require.Equal(t, test.WantDelta.String(), protobufToText(profs[0].data))

// check delta prof details like timestamps and duration
deltaProf, err := pprofile.ParseData(profs[1].data)
deltaProf, err := pprofile.ParseData(profs[0].data)
require.NoError(t, err)
require.Equal(t, test.Prof2.Time.UnixNano(), deltaProf.TimeNanos)
require.Equal(t, deltaPeriod.Nanoseconds(), deltaProf.DurationNanos)
Expand Down
9 changes: 4 additions & 5 deletions profiler/profiler_test.go
Expand Up @@ -265,8 +265,8 @@ func TestProfilerInternal(t *testing.T) {
assert.EqualValues(1, startCPU)
assert.EqualValues(1, stopCPU)

// should contain cpu.pprof, metrics.json, heap.pprof, delta-heap.pprof
assert.Equal(4, len(bat.profiles))
// should contain cpu.pprof, metrics.json, delta-heap.pprof
assert.Equal(3, len(bat.profiles))

p.exit <- struct{}{}
<-wait
Expand Down Expand Up @@ -319,11 +319,10 @@ func TestProfilerPassthrough(t *testing.T) {
}

assert := assert.New(t)
// should contain cpu.pprof, heap.pprof, delta-heap.pprof
assert.Equal(3, len(bat.profiles))
// should contain cpu.pprof, delta-heap.pprof
assert.Equal(2, len(bat.profiles))
assert.NotEmpty(bat.profiles[0].data)
assert.NotEmpty(bat.profiles[1].data)
assert.NotEmpty(bat.profiles[2].data)
}

func unstartedProfiler(opts ...Option) (*profiler, error) {
Expand Down

0 comments on commit ebf57ee

Please sign in to comment.