From 6ff43dc073011648f3024db678c9c31a0ef6fe16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Geisend=C3=B6rfer?= Date: Thu, 28 Oct 2021 17:46:19 +0200 Subject: [PATCH] profiler: Add WithDeltaProfiles() option Delta profiles require a certain amount of heap memory that is proportional to MemStats.BuckHashSys (typically ~2x). For applications that make heavy use of code generation or recursion the memory usage can be several hundred MB. Add WithDeltaProfiles() that allows to disable delta profiles for applications where this is a deal breaker. Those applications will still see allocation profiles, but they won't be able to aggregate or compare them. The default value for WithDeltaProfiles() is true since applications with high BuckHashSys seem to be rare. See https://dtdg.co/go-delta-profile-docs for more information about delta profiles. Mitigates #1025 and implements PROF-4305. --- profiler/options.go | 12 ++++++ profiler/options_test.go | 17 ++++++++ profiler/profile.go | 7 ++-- profiler/profile_test.go | 84 +++++++++++++++++++++++++--------------- 4 files changed, 84 insertions(+), 36 deletions(-) diff --git a/profiler/options.go b/profiler/options.go index 2092c41304..b30ee145fe 100644 --- a/profiler/options.go +++ b/profiler/options.go @@ -99,6 +99,7 @@ type config struct { mutexFraction int blockRate int outputDir string + deltaProfiles bool } func urlForSite(site string) (string, error) { @@ -141,6 +142,7 @@ func defaultConfig() (*config, error) { uploadTimeout: DefaultUploadTimeout, maxGoroutinesWait: 1000, // arbitrary value, should limit STW to ~30ms tags: []string{fmt.Sprintf("pid:%d", os.Getpid())}, + deltaProfiles: internal.BoolEnv("DD_PROFILING_DELTA", true), } for _, t := range defaultProfileTypes { c.addProfileType(t) @@ -252,6 +254,16 @@ func WithAgentlessUpload() Option { } } +// WithDeltaProfiles specifies if delta profiles are enabled. The default value +// is true. This option takes precedence over the DD_PROFILING_DELTA +// environment variable that can be set to "true" or "false" as well. See +// https://dtdg.co/go-delta-profile-docs for more information. +func WithDeltaProfiles(enabled bool) Option { + return func(cfg *config) { + cfg.deltaProfiles = enabled + } +} + // WithURL specifies the HTTP URL for the Datadog Profiling API. func WithURL(url string) Option { return func(cfg *config) { diff --git a/profiler/options_test.go b/profiler/options_test.go index f782c652c1..c9693856a6 100644 --- a/profiler/options_test.go +++ b/profiler/options_test.go @@ -207,6 +207,14 @@ func TestOptions(t *testing.T) { assert.Contains(t, cfg.tags, "env1:tag1") assert.Contains(t, cfg.tags, "env2:tag2") }) + + t.Run("WithDeltaProfiles", func(t *testing.T) { + var cfg config + WithDeltaProfiles(true)(&cfg) + assert.Equal(t, true, cfg.deltaProfiles) + WithDeltaProfiles(false)(&cfg) + assert.Equal(t, false, cfg.deltaProfiles) + }) } func TestEnvVars(t *testing.T) { @@ -293,6 +301,14 @@ func TestEnvVars(t *testing.T) { assert.Contains(t, cfg.tags, "b:2") assert.Contains(t, cfg.tags, "c:3") }) + + t.Run("DD_PROFILING_DELTA", func(t *testing.T) { + os.Setenv("DD_PROFILING_DELTA", "false") + defer os.Unsetenv("DD_PROFILING_DELTA") + cfg, err := defaultConfig() + require.NoError(t, err) + assert.Equal(t, cfg.deltaProfiles, false) + }) } func TestDefaultConfig(t *testing.T) { @@ -317,6 +333,7 @@ func TestDefaultConfig(t *testing.T) { assert.Equal(DefaultMutexFraction, cfg.mutexFraction) assert.Equal(DefaultBlockRate, cfg.blockRate) assert.Contains(cfg.tags, "runtime-id:"+globalconfig.RuntimeID()) + assert.Equal(true, cfg.deltaProfiles) }) } diff --git a/profiler/profile.go b/profiler/profile.go index eb51a52a77..9fc3b108ad 100644 --- a/profiler/profile.go +++ b/profiler/profile.go @@ -238,11 +238,10 @@ func (p *profiler) runProfile(pt ProfileType) ([]*profile, error) { } // deltaProfile derives the delta profile between curData and the previous -// profile. For profile types that don't have delta profiling enabled, it -// simply returns nil, nil. +// profile. For profile types that don't have delta profiling enabled, or +// WithDeltaProfiles(false), it simply returns nil, nil. func (p *profiler) deltaProfile(t profileType, curData []byte) (*profile, error) { - // Not all profile types use delta profiling, return nil if this one doesn't. - if t.Delta == nil { + if !p.cfg.deltaProfiles || t.Delta == nil { return nil, nil } curProf, err := pprofile.ParseData(curData) diff --git a/profiler/profile_test.go b/profiler/profile_test.go index b279cde6cf..fc599c7a67 100644 --- a/profiler/profile_test.go +++ b/profiler/profile_test.go @@ -113,44 +113,64 @@ main;bar 0 0 8 16 for _, test := range tests { for _, profType := range test.Types { - t.Run(profType.String(), func(t *testing.T) { - prof1 := test.Prof1.Protobuf() - prof2 := test.Prof2.Protobuf() - + // deltaProfiler returns an unstarted profiler that is fed prof1 + // followed by prof2 when calling runProfile(). + deltaProfiler := func(prof1, prof2 []byte, opts ...Option) (*profiler, func()) { returnProfs := [][]byte{prof1, prof2} - defer func(old func(_ string, _ io.Writer, _ int) error) { lookupProfile = old }(lookupProfile) - lookupProfile = func(name string, w io.Writer, _ int) error { + old := lookupProfile + lookupProfile = func(_ string, w io.Writer, _ int) error { _, err := w.Write(returnProfs[0]) returnProfs = returnProfs[1:] return err } - p, err := unstartedProfiler() - - // first run, should produce the current profile twice (a bit - // awkward, but makes sense since we try to add delta profiles as an - // 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, 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) + p, err := unstartedProfiler(opts...) 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)) - - // check delta prof details like timestamps and duration - deltaProf, err := pprofile.ParseData(profs[1].data) - require.NoError(t, err) - require.Equal(t, test.Prof2.Time.UnixNano(), deltaProf.TimeNanos) - require.Equal(t, deltaPeriod.Nanoseconds(), deltaProf.DurationNanos) + return p, func() { lookupProfile = old } + } + + t.Run(profType.String(), func(t *testing.T) { + t.Run("enabled", func(t *testing.T) { + prof1 := test.Prof1.Protobuf() + prof2 := test.Prof2.Protobuf() + p, cleanup := deltaProfiler(prof1, prof2) + defer cleanup() + // first run, should produce the current profile twice (a bit + // awkward, but makes sense since we try to add delta profiles as an + // 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, 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)) + + // check delta prof details like timestamps and duration + deltaProf, err := pprofile.ParseData(profs[1].data) + require.NoError(t, err) + require.Equal(t, test.Prof2.Time.UnixNano(), deltaProf.TimeNanos) + require.Equal(t, deltaPeriod.Nanoseconds(), deltaProf.DurationNanos) + }) + + t.Run("disabled", func(t *testing.T) { + prof1 := test.Prof1.Protobuf() + prof2 := test.Prof2.Protobuf() + p, cleanup := deltaProfiler(prof1, prof2, WithDeltaProfiles(false)) + defer cleanup() + + profs, err := p.runProfile(profType) + require.NoError(t, err) + require.Equal(t, 1, len(profs)) + }) }) } }