Skip to content

Commit

Permalink
profiler: add enable flag to control profiler activation (#2840)
Browse files Browse the repository at this point in the history
This PR introduces a new environment variable DD_PROFILING_ENABLED.
If DD_PROFILING_ENABLED is set to false, profiling will be disabled even if
profiler.Start() is called. This allows the application code to always call
profiler.Start() while dynamically adjusting profiling through the environment
variable. Note that DD_PROFILING_ENABLED=true is not sufficient by itself
to enable profiling; you must still call profiler.Start().

Co-authored-by: Nick Ripley <nick.ripley@datadoghq.com>
Co-authored-by: Felix Geisendörfer <felix@datadoghq.com>
Co-authored-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
  • Loading branch information
4 people authored Oct 25, 2024
1 parent bca85bc commit f126bcf
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 0 deletions.
9 changes: 9 additions & 0 deletions profiler/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ type config struct {
logStartup bool
traceConfig executionTraceConfig
endpointCountEnabled bool
enabled bool
}

// logStartup records the configuration to the configured logger in JSON format
Expand Down Expand Up @@ -146,6 +147,7 @@ func logStartup(c *config) {
"execution_trace_size_limit": c.traceConfig.Limit,
"endpoint_count_enabled": c.endpointCountEnabled,
"custom_profiler_label_keys": c.customProfilerLabels,
"enabled": c.enabled,
}
b, err := json.Marshal(info)
if err != nil {
Expand Down Expand Up @@ -208,6 +210,13 @@ func defaultConfig() (*config, error) {
} else {
c.agentURL = url.String() + "/profiling/v1/input"
}
// If DD_PROFILING_ENABLED is set to "auto", the profiler's activation will be determined by
// the Datadog admission controller, so we set it to true.
if os.Getenv("DD_PROFILING_ENABLED") == "auto" {
c.enabled = true
} else {
c.enabled = internal.BoolEnv("DD_PROFILING_ENABLED", true)
}
if v := os.Getenv("DD_PROFILING_UPLOAD_TIMEOUT"); v != "" {
d, err := time.ParseDuration(v)
if err != nil {
Expand Down
15 changes: 15 additions & 0 deletions profiler/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,21 @@ func TestEnvVars(t *testing.T) {
assert.Equal(t, "http://localhost:6218/profiling/v1/input", cfg.agentURL)
})

t.Run("DD_PROFILING_ENABLED", func(t *testing.T) {
t.Run("default", func(t *testing.T) {
cfg, err := defaultConfig()
require.NoError(t, err)
assert.Equal(t, true, cfg.enabled)
})

t.Run("override", func(t *testing.T) {
t.Setenv("DD_PROFILING_ENABLED", "false")
cfg, err := defaultConfig()
require.NoError(t, err)
assert.Equal(t, false, cfg.enabled)
})
})

t.Run("DD_PROFILING_UPLOAD_TIMEOUT", func(t *testing.T) {
t.Setenv("DD_PROFILING_UPLOAD_TIMEOUT", "3s")
cfg, err := defaultConfig()
Expand Down
6 changes: 6 additions & 0 deletions profiler/profiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ var (
//
// It may return an error if an API key is not provided by means of the
// WithAPIKey option, or if a hostname is not found.
//
// If DD_PROFILING_ENABLED=false is set in the process environment, it will
// prevent the profiler from starting.
func Start(opts ...Option) error {
mu.Lock()
defer mu.Unlock()
Expand All @@ -58,6 +61,9 @@ func Start(opts ...Option) error {
if err != nil {
return err
}
if !p.cfg.enabled {
return nil
}
activeProfiler = p
activeProfiler.run()
traceprof.SetProfilerEnabled(true)
Expand Down
27 changes: 27 additions & 0 deletions profiler/profiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,20 @@ func TestStart(t *testing.T) {
mu.Unlock()
})

t.Run("dd_profiling_not_enabled", func(t *testing.T) {
t.Setenv("DD_PROFILING_ENABLED", "false")
if err := Start(); err != nil {
t.Fatal(err)
}
defer Stop()

mu.Lock()
// if DD_PROFILING_ENABLED is false, the profiler should not be started even if Start() is called
// So we should not have an activeProfiler
assert.Nil(t, activeProfiler)
mu.Unlock()
})

t.Run("options", func(t *testing.T) {
if err := Start(); err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -449,6 +463,19 @@ func TestImmediateProfile(t *testing.T) {
}
}

func TestEnabledFalse(t *testing.T) {
t.Setenv("DD_PROFILING_ENABLED", "false")
ch := startTestProfiler(t, 1, WithPeriod(10*time.Millisecond), WithProfileTypes())
select {
case <-ch:
t.Fatal("received profile when profiler should have been disabled")
case <-time.After(time.Second):
// This test might succeed incorrectly on an overloaded
// CI server, but is very likely to fail locally given a
// buggy implementation
}
}

func TestExecutionTraceCPUProfileRate(t *testing.T) {
// cpuProfileRate is picked randomly so we can check for it in the trace
// data to reduce the chance that it occurs in the trace data for some other
Expand Down
1 change: 1 addition & 0 deletions profiler/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func startTelemetry(c *config) {
{Name: "execution_trace_size_limit", Value: c.traceConfig.Limit},
{Name: "endpoint_count_enabled", Value: c.endpointCountEnabled},
{Name: "num_custom_profiler_label_keys", Value: len(c.customProfilerLabels)},
{Name: "enabled", Value: c.enabled},
},
)
}

0 comments on commit f126bcf

Please sign in to comment.