diff --git a/profiler/options.go b/profiler/options.go index e40593a5f1..7287d23add 100644 --- a/profiler/options.go +++ b/profiler/options.go @@ -246,11 +246,9 @@ func defaultConfig() (*config, error) { WithUploadTimeout(d)(&c) } if v := os.Getenv("DD_API_KEY"); v != "" { - WithAPIKey(v)(&c) - } - if internal.BoolEnv("DD_PROFILING_AGENTLESS", false) { - WithAgentlessUpload()(&c) + c.apiKey = v } + c.agentless = internal.BoolEnv("DD_PROFILING_AGENTLESS", false) if v := os.Getenv("DD_SITE"); v != "" { WithSite(v)(&c) } @@ -319,29 +317,6 @@ func WithAgentAddr(hostport string) Option { } } -// WithAPIKey sets the Datadog API Key and takes precedence over the DD_API_KEY -// env variable. Historically this option was used to enable agentless -// uploading, but as of dd-trace-go v1.30.0 the behavior has changed to always -// default to agent based uploading which doesn't require an API key. So if you -// currently don't have an agent running on the default localhost:8126 hostport -// you need to set it up, or use WithAgentAddr to specify the hostport location -// of the agent. See WithAgentlessUpload for more information. -func WithAPIKey(key string) Option { - return func(cfg *config) { - cfg.apiKey = key - } -} - -// WithAgentlessUpload is currently for internal usage only and not officially -// supported. You should not enable it unless somebody at Datadog instructed -// you to do so. It allows to skip the agent and talk to the Datadog API -// directly using the provided API key. -func WithAgentlessUpload() Option { - return func(cfg *config) { - cfg.agentless = true - } -} - // 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 diff --git a/profiler/options_test.go b/profiler/options_test.go index 141ad03020..26fe306d16 100644 --- a/profiler/options_test.go +++ b/profiler/options_test.go @@ -95,21 +95,6 @@ func TestOptions(t *testing.T) { assert.Equal(t, 5*time.Second, cfg.uploadTimeout) }) - t.Run("WithAPIKey", func(t *testing.T) { - var cfg config - WithAPIKey(testAPIKey)(&cfg) - assert.Equal(t, testAPIKey, cfg.apiKey) - assert.Equal(t, cfg.apiURL, cfg.targetURL) - }) - - t.Run("WithAPIKey/override", func(t *testing.T) { - t.Setenv("DD_API_KEY", "apikey") - var testAPIKey = "12345678901234567890123456789012" - var cfg config - WithAPIKey(testAPIKey)(&cfg) - assert.Equal(t, testAPIKey, cfg.apiKey) - }) - t.Run("WithURL", func(t *testing.T) { var cfg config WithURL("my-url")(&cfg) diff --git a/profiler/profiler.go b/profiler/profiler.go index ae65560b82..a48ed9d979 100644 --- a/profiler/profiler.go +++ b/profiler/profiler.go @@ -33,8 +33,7 @@ var ( // Start starts the profiler. If the profiler is already running, it will be // stopped and restarted with the given options. // -// 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. +// It may return an error if a hostname is not found. func Start(opts ...Option) error { mu.Lock() defer mu.Unlock() @@ -140,14 +139,14 @@ func newProfiler(opts ...Option) (*profiler, error) { cfg.addProfileType(expGoroutineWaitProfile) } // Agentless upload is disabled by default as of v1.30.0, but - // WithAgentlessUpload can be used to enable it for testing and debugging. + // DD_PROFILING_AGENTLESS can be set to enable it for testing and debugging. if cfg.agentless { if !isAPIKeyValid(cfg.apiKey) { - return nil, errors.New("profiler.WithAgentlessUpload requires a valid API key. Use profiler.WithAPIKey or the DD_API_KEY env variable to set it") + return nil, errors.New("Agentless upload requires a valid API key. Use the DD_API_KEY env variable to set it") } // Always warn people against using this mode for now. All customers should // use agent based uploading at this point. - log.Warn("profiler.WithAgentlessUpload is currently for internal usage only and not officially supported.") + log.Warn("Agentless upload is currently for internal usage only and not officially supported.") cfg.targetURL = cfg.apiURL } else { // Historically people could use an API Key to enable agentless uploading. @@ -156,7 +155,7 @@ func newProfiler(opts ...Option) (*profiler, error) { // key configured, we warn the customers that this is probably a // misconfiguration. if cfg.apiKey != "" { - log.Warn("You are currently setting profiler.WithAPIKey or the DD_API_KEY env variable, but as of dd-trace-go v1.30.0 this value is getting ignored by the profiler. Please see the profiler.WithAPIKey go docs and verify that your integration is still working. If you can't remove DD_API_KEY from your environment, you can use WithAPIKey(\"\") to silence this warning.") + log.Warn("You are currently setting the DD_API_KEY env variable, but as of dd-trace-go v1.30.0 this value is getting ignored by the profiler. Please verify that your integration is still working.") } cfg.targetURL = cfg.agentURL } diff --git a/profiler/profiler_test.go b/profiler/profiler_test.go index 592dc7a5b1..9620646e45 100644 --- a/profiler/profiler_test.go +++ b/profiler/profiler_test.go @@ -86,41 +86,56 @@ func TestStart(t *testing.T) { mu.Unlock() }) - t.Run("options/GoodAPIKey/Agent", func(t *testing.T) { + t.Run("Agent/GoodAPIKey", func(t *testing.T) { + t.Setenv("DD_API_KEY", "12345678901234567890123456789012") rl := &log.RecordLogger{} defer log.UseLogger(rl)() - err := Start(WithAPIKey("12345678901234567890123456789012")) + err := Start() defer Stop() assert.Nil(t, err) assert.Equal(t, activeProfiler.cfg.agentURL, activeProfiler.cfg.targetURL) // The package should log a warning that using an API has no // effect unless uploading directly to Datadog (i.e. agentless) assert.LessOrEqual(t, 1, len(rl.Logs())) - assert.Contains(t, strings.Join(rl.Logs(), " "), "profiler.WithAPIKey") + assert.Contains(t, strings.Join(rl.Logs(), " "), "DD_API_KEY") }) - t.Run("options/GoodAPIKey/Agentless", func(t *testing.T) { + t.Run("Agentless/GoodAPIKey", func(t *testing.T) { + t.Setenv("DD_PROFILING_AGENTLESS", "True") + t.Setenv("DD_API_KEY", "12345678901234567890123456789012") rl := &log.RecordLogger{} defer log.UseLogger(rl)() - err := Start( - WithAPIKey("12345678901234567890123456789012"), - WithAgentlessUpload(), - ) + err := Start() defer Stop() assert.Nil(t, err) assert.Equal(t, activeProfiler.cfg.apiURL, activeProfiler.cfg.targetURL) // The package should log a warning that agentless upload is not // officially supported, so prefer not to use it assert.LessOrEqual(t, 1, len(rl.Logs())) - assert.Contains(t, strings.Join(rl.Logs(), " "), "profiler.WithAgentlessUpload") + assert.Contains(t, strings.Join(rl.Logs(), " "), "Agentless") + }) + + t.Run("Agentless/NoAPIKey", func(t *testing.T) { + t.Setenv("DD_PROFILING_AGENTLESS", "True") + err := Start() + defer Stop() + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "Agentless upload requires a valid API key") + + // Check that mu gets unlocked, even if newProfiler() returns an error. + mu.Lock() + mu.Unlock() }) - t.Run("options/BadAPIKey", func(t *testing.T) { - err := Start(WithAPIKey("aaaa"), WithAgentlessUpload()) + t.Run("Agentless/BadAPIKey", func(t *testing.T) { + t.Setenv("DD_PROFILING_AGENTLESS", "True") + t.Setenv("DD_API_KEY", "aaaa") + err := Start() defer Stop() assert.NotNil(t, err) + assert.Contains(t, err.Error(), "Agentless upload requires a valid API key") // Check that mu gets unlocked, even if newProfiler() returns an error. mu.Lock()