profiler: Disable agentless mode for WithAPIKey (#906)

This is a backwards incompatible change, but deemed neccessary by the
profiling team. Since it's been analyzed to only impact a tiny minority
of users, it's done without a major version change.

UPGRADING:

If you are currently using profiler.WithAPIKey() or DD_API_KEY to upload
profiling data, please remove this configuration. dd-trace-go will now
default to upload through the datadog agent running at localhost:8126.
If your agent is running on another port, please use
profiler.WithAgentAddr() to configure the correct agent address.

The new profiler.WithAgentlessUpload() is currently not supported for
anything other than debugging/testing when hitting integration issues.
If you think you need it for your production environment, please contact
our support.

BACKGROUND:

Historically the profiler only supported direct uploading to the backend
without using an agent, aka "agentless mode". This integration method
was succeeded by agent based uploading which should always be the
default behavior by now. The old agentless mode is not officially
supported anymore and only meant for testing and debugging at this
point. It may make a comeback in the future, but for now it's a
second-class citizen.

until this patch setting an API Key via WithAPIKey or DD_API_KEY still
allowed users to opt into the agentless mode, but this was problematic
for two reasons:

1. A DD_API_KEY might be set in the environment for unrelated reasons or
   by accident.
2. The user may have forgotten switch their integration to agent
   uploading.

Users were warned against agentless uploading via a banner that was
shown from July 2020 to February 2021 and various email campaigns. It is
now time to make sure nobody is uploading via agentless mode in
production anymore.

Author: felixge

profiler/options.go

@@ -18,6 +18,7 @@ import (
 	"time"
 	"unicode"
 
+	"gopkg.in/DataDog/dd-trace-go.v1/internal"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/log"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/version"
@@ -77,7 +78,8 @@ var defaultClient = &http.Client{
 var defaultProfileTypes = []ProfileType{MetricsProfile, CPUProfile, HeapProfile}
 
 type config struct {
-	apiKey string
+	apiKey    string
+	agentless bool
 	// targetURL is the upload destination URL. It will be set by the profiler on start to either apiURL or agentURL
 	// based on the other options.
 	targetURL     string
@@ -158,6 +160,9 @@ func defaultConfig() (*config, error) {
 	if v := os.Getenv("DD_API_KEY"); v != "" {
 		WithAPIKey(v)(&c)
 	}
+	if internal.BoolEnv("DD_PROFILING_AGENTLESS", false) {
+		WithAgentlessUpload()(&c)
+	}
 	if v := os.Getenv("DD_SITE"); v != "" {
 		WithSite(v)(&c)
 	}
@@ -209,15 +214,29 @@ func WithAgentAddr(hostport string) Option {
 	}
 }
 
-// WithAPIKey is deprecated and might be removed in future versions of this
-// package. It allows to skip the agent and talk to the Datadog API directly
-// using the provided API key.
+// 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
+	}
+}
+
 // WithURL specifies the HTTP URL for the Datadog Profiling API.
 func WithURL(url string) Option {
 	return func(cfg *config) {

profiler/profiler.go

@@ -78,12 +78,25 @@ func newProfiler(opts ...Option) (*profiler, error) {
 	if os.Getenv("DD_PROFILING_WAIT_PROFILE") != "" {
 		cfg.addProfileType(expGoroutineWaitProfile)
 	}
-	if cfg.apiKey != "" {
+	// Agentless upload is disabled by default as of v1.30.0, but
+	// WithAgentlessUpload can be used to enable it for testing and debugging.
+	if cfg.agentless {
 		if !isAPIKeyValid(cfg.apiKey) {
-			return nil, errors.New("API key has incorrect format")
+			return nil, errors.New("profiler.WithAgentlessUpload requires a valid API key. Use profiler.WithAPIKey or 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.")
 		cfg.targetURL = cfg.apiURL
 	} else {
+		// Historically people could use an API Key to enable agentless uploading.
+		// As of v1.30.0 customers the default behavior is to use agent based
+		// uploading regardless of the presence of an API key. So if we see an API
+		// 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.")
+		}
 		cfg.targetURL = cfg.agentURL
 	}
 	if cfg.hostname == "" {

profiler/profiler_test.go

@@ -58,14 +58,25 @@ func TestStart(t *testing.T) {
 		mu.Unlock()
 	})
 
-	t.Run("options/GoodAPIKey", func(t *testing.T) {
+	t.Run("options/GoodAPIKey/Agent", func(t *testing.T) {
 		err := Start(WithAPIKey("12345678901234567890123456789012"))
 		defer Stop()
 		assert.Nil(t, err)
+		assert.Equal(t, activeProfiler.cfg.agentURL, activeProfiler.cfg.targetURL)
+	})
+
+	t.Run("options/GoodAPIKey/Agentless", func(t *testing.T) {
+		err := Start(
+			WithAPIKey("12345678901234567890123456789012"),
+			WithAgentlessUpload(),
+		)
+		defer Stop()
+		assert.Nil(t, err)
+		assert.Equal(t, activeProfiler.cfg.apiURL, activeProfiler.cfg.targetURL)
 	})
 
 	t.Run("options/BadAPIKey", func(t *testing.T) {
-		err := Start(WithAPIKey("aaaa"))
+		err := Start(WithAPIKey("aaaa"), WithAgentlessUpload())
 		defer Stop()
 		assert.NotNil(t, err)