appsec: allow enabling sca from configuration (#2634)

internal/appsec/appsec.go

@@ -12,6 +12,7 @@ import (
 	"github.com/DataDog/appsec-internal-go/limiter"
 	appsecLog "github.com/DataDog/appsec-internal-go/log"
 	waf "github.com/DataDog/go-libddwaf/v2"
+
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/config"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/log"

internal/appsec/config/config.go

@@ -12,11 +12,42 @@ import (
 	"time"
 
 	internal "github.com/DataDog/appsec-internal-go/appsec"
+
+	"gopkg.in/DataDog/dd-trace-go.v1/internal/log"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/remoteconfig"
+	"gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry"
 )
 
-// EnvEnabled is the env var used to enable/disable appsec
-const EnvEnabled = "DD_APPSEC_ENABLED"
+func init() {
+	registerAppConfigTelemetry()
+}
+
+// Register the global app telemetry configuration.
+func registerAppConfigTelemetry() {
+	registerSCAAppConfigTelemetry(telemetry.GlobalClient)
+}
+
+// Register the global app telemetry configuration related to the Software Composition Analysis (SCA) product.
+// Report over telemetry whether SCA's enablement env var was set or not along with its value. Nothing is reported in
+// case of an error or if the env var is not set.
+func registerSCAAppConfigTelemetry(client telemetry.Client) {
+	val, defined, err := parseBoolEnvVar(EnvSCAEnabled)
+	if err != nil {
+		log.Error("appsec: %v", err)
+		return
+	}
+	if defined {
+		client.RegisterAppConfig(EnvSCAEnabled, val, "env_var")
+	}
+}
+
+// The following environment variables dictate the enablement of different the ASM products.
+const (
+	// EnvEnabled controls ASM Threats Protection's enablement.
+	EnvEnabled = "DD_APPSEC_ENABLED"
+	// EnvSCAEnabled controls ASM Software Composition Analysis (SCA)'s enablement.
+	EnvSCAEnabled = "DD_APPSEC_SCA_ENABLED"
+)
 
 // StartOption is used to customize the AppSec configuration when invoked with appsec.Start()
 type StartOption func(c *Config)
@@ -45,15 +76,22 @@ func WithRCConfig(cfg remoteconfig.ClientConfig) StartOption {
 	}
 }
 
-// IsEnabled returns true when appsec is enabled when the environment variable
-// DD_APPSEC_ENABLED is set to true.
-// It also returns whether the env var is actually set in the env or not.
+// IsEnabled returns true when appsec is enabled by the environment variable DD_APPSEC_ENABLED (as of strconv's boolean
+// parsing rules). When false, it also returns whether the env var was actually set or not.
+// In case of a parsing error, it returns a detailed error.
 func IsEnabled() (enabled bool, set bool, err error) {
-	enabledStr, set := os.LookupEnv(EnvEnabled)
-	if enabledStr == "" {
+	return parseBoolEnvVar(EnvEnabled)
+}
+
+// Return true when the given environment variable is defined and set to true (as of strconv's
+// parsing rules). When false, it also returns whether the env var was actually set or not.
+// In case of a parsing error, it returns a detailed error.
+func parseBoolEnvVar(env string) (enabled bool, set bool, err error) {
+	str, set := os.LookupEnv(env)
+	if str == "" {
 		return false, set, nil
-	} else if enabled, err = strconv.ParseBool(enabledStr); err != nil {
-		return false, set, fmt.Errorf("could not parse %s value `%s` as a boolean value", EnvEnabled, enabledStr)
+	} else if enabled, err = strconv.ParseBool(str); err != nil {
+		return false, set, fmt.Errorf("could not parse %s value `%s` as a boolean value", env, str)
 	}
 
 	return enabled, set, nil

internal/appsec/config/config_test.go

@@ -0,0 +1,64 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2016 Datadog, Inc.
+
+package config
+
+import (
+	"testing"
+
+	"gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry/telemetrytest"
+)
+
+func TestSCAEnabled(t *testing.T) {
+	for _, tc := range []struct {
+		name              string
+		envVarVal         string
+		telemetryExpected bool
+		expectedValue     bool
+	}{
+		{
+			name:              "true",
+			envVarVal:         "true",
+			telemetryExpected: true,
+			expectedValue:     true,
+		},
+		{
+			name:              "false",
+			envVarVal:         "false",
+			telemetryExpected: true,
+			expectedValue:     false,
+		},
+		{
+			name:              "undefined",
+			envVarVal:         "", // special case for undefined
+			telemetryExpected: false,
+			expectedValue:     false,
+		},
+		{
+			name:              "parsing error",
+			envVarVal:         "not a boolean string representation [at {all!}]",
+			telemetryExpected: false,
+			expectedValue:     false,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.envVarVal != "" {
+				t.Setenv(EnvSCAEnabled, tc.envVarVal)
+			}
+
+			telemetryClient := new(telemetrytest.MockClient)
+			telemetryClient.On("RegisterAppConfig", EnvSCAEnabled, tc.expectedValue, "env_var").Return()
+
+			registerSCAAppConfigTelemetry(telemetryClient)
+
+			if tc.telemetryExpected {
+				telemetryClient.AssertCalled(t, "RegisterAppConfig", EnvSCAEnabled, tc.expectedValue, "env_var")
+				telemetryClient.AssertNumberOfCalls(t, "RegisterAppConfig", 1)
+			} else {
+				telemetryClient.AssertNumberOfCalls(t, "RegisterAppConfig", 0)
+			}
+		})
+	}
+}

internal/telemetry/client.go

@@ -30,6 +30,7 @@ import (
 // Client buffers and sends telemetry messages to Datadog (possibly through an
 // agent).
 type Client interface {
+	RegisterAppConfig(name string, val interface{}, origin string)
 	ProductChange(namespace Namespace, enabled bool, configuration []Configuration)
 	ConfigChange(configuration []Configuration)
 	Record(namespace Namespace, metric MetricKind, name string, value float64, tags []string, common bool)
@@ -44,7 +45,7 @@ var (
 	GlobalClient Client
 	globalClient sync.Mutex
 
-	// integrations tracks the the integrations enabled
+	// integrations tracks the integrations enabled
 	contribPackages []Integration
 	contrib         sync.Mutex
 
@@ -144,21 +145,35 @@ type client struct {
 	// metrics are sent
 	metrics    map[Namespace]map[string]*metric
 	newMetrics bool
+
+	// Globally registered application configuration sent in the app-started request, along with the locally-defined
+	// configuration of the  event.
+	globalAppConfig []Configuration
 }
 
 func log(msg string, args ...interface{}) {
 	// Debug level so users aren't spammed with telemetry info.
 	logger.Debug(fmt.Sprintf(LogPrefix+msg, args...))
 }
 
+// RegisterAppConfig allows to register a globally-defined application configuration.
+// This configuration will be sent when the telemetry client is started and over related configuration updates.
+func (c *client) RegisterAppConfig(name string, value interface{}, origin string) {
+	c.globalAppConfig = append(c.globalAppConfig, Configuration{
+		Name:   name,
+		Value:  value,
+		Origin: origin,
+	})
+}
+
 // start registers that the app has begun running with the app-started event.
 // Must be called with c.mu locked.
 // start also configures the telemetry client based on the following telemetry
 // environment variables: DD_INSTRUMENTATION_TELEMETRY_ENABLED,
 // DD_TELEMETRY_HEARTBEAT_INTERVAL, DD_INSTRUMENTATION_TELEMETRY_DEBUG,
 // and DD_TELEMETRY_DEPENDENCY_COLLECTION_ENABLED.
 // TODO: implement passing in error information about tracer start
-func (c *client) start(configuration []Configuration, namespace Namespace) {
+func (c *client) start(configuration []Configuration, namespace Namespace, flush bool) {
 	if Disabled() {
 		return
 	}
@@ -191,8 +206,13 @@ func (c *client) start(configuration []Configuration, namespace Namespace) {
 			Enabled: namespace == NamespaceProfilers,
 		},
 	}
+
+	var cfg []Configuration
+	cfg = append(cfg, c.globalAppConfig...)
+	cfg = append(cfg, configuration...)
+
 	payload := &AppStarted{
-		Configuration: configuration,
+		Configuration: cfg,
 		Products:      productInfo,
 	}
 	appStarted := c.newRequest(RequestTypeAppStarted)
@@ -222,7 +242,9 @@ func (c *client) start(configuration []Configuration, namespace Namespace) {
 		c.scheduleSubmit(req)
 	}
 
-	c.flush()
+	if flush {
+		c.flush()
+	}
 	c.heartbeatInterval = heartbeatInterval()
 	c.heartbeatT = time.AfterFunc(c.heartbeatInterval, c.backgroundHeartbeat)
 }

internal/telemetry/client_test.go

@@ -41,8 +41,8 @@ func TestClient(t *testing.T) {
 		URL: server.URL,
 	}
 	client.mu.Lock()
-	client.start(nil, NamespaceTracers)
-	client.start(nil, NamespaceTracers) // test idempotence
+	client.start(nil, NamespaceTracers, true)
+	client.start(nil, NamespaceTracers, true) // test idempotence
 	client.mu.Unlock()
 	defer client.Stop()
 
@@ -106,7 +106,7 @@ func TestMetrics(t *testing.T) {
 		client := &client{
 			URL: server.URL,
 		}
-		client.start(nil, NamespaceTracers)
+		client.start(nil, NamespaceTracers, true)
 
 		// Records should have the most recent value
 		client.Record(NamespaceTracers, MetricKindGauge, "foobar", 1, nil, false)
@@ -182,7 +182,7 @@ func TestDistributionMetrics(t *testing.T) {
 		client := &client{
 			URL: server.URL,
 		}
-		client.start(nil, NamespaceTracers)
+		client.start(nil, NamespaceTracers, true)
 		// Records should have the most recent value
 		client.Record(NamespaceTracers, MetricKindDist, "soobar", 1, nil, false)
 		client.Record(NamespaceTracers, MetricKindDist, "soobar", 3, nil, false)
@@ -214,7 +214,7 @@ func TestDisabledClient(t *testing.T) {
 	client := &client{
 		URL: server.URL,
 	}
-	client.start(nil, NamespaceTracers)
+	client.start(nil, NamespaceTracers, true)
 	client.Record(NamespaceTracers, MetricKindGauge, "foobar", 1, nil, false)
 	client.Count(NamespaceTracers, "bonk", 4, []string{"org:1"}, false)
 	client.Stop()
@@ -280,7 +280,7 @@ func TestConcurrentClient(t *testing.T) {
 		client := &client{
 			URL: server.URL,
 		}
-		client.start(nil, NamespaceTracers)
+		client.start(nil, NamespaceTracers, true)
 		defer client.Stop()
 
 		var wg sync.WaitGroup
@@ -355,7 +355,7 @@ func TestAgentlessRetry(t *testing.T) {
 	client := &client{
 		URL: brokenServer.URL,
 	}
-	client.start(nil, NamespaceTracers)
+	client.start(nil, NamespaceTracers, true)
 	waitAgentlessEndpoint()
 }
 
@@ -382,7 +382,7 @@ func TestCollectDependencies(t *testing.T) {
 	client := &client{
 		URL: server.URL,
 	}
-	client.start(nil, NamespaceTracers)
+	client.start(nil, NamespaceTracers, true)
 	select {
 	case <-received:
 	case <-ctx.Done():

internal/telemetry/telemetry.go

@@ -28,10 +28,15 @@ func (c *client) ProductChange(namespace Namespace, enabled bool, configuration
 			// Namespace is not enabled & telemetry isn't started, won't start it now.
 			return
 		}
-		c.start(configuration, namespace)
+		c.start(configuration, namespace, true)
 		return
 	}
-	c.configChange(configuration)
+
+	var cfg []Configuration
+	cfg = append(cfg, c.globalAppConfig...)
+	cfg = append(cfg, configuration...)
+	c.configChange(cfg)
+
 	switch namespace {
 	case NamespaceTracers, NamespaceProfilers, NamespaceAppSec:
 		c.productChange(namespace, enabled)

internal/telemetry/telemetry_test.go

@@ -22,7 +22,7 @@ import (
 
 func TestProductEnabled(t *testing.T) {
 	client := new(client)
-	client.start(nil, NamespaceTracers)
+	client.start(nil, NamespaceTracers, true)
 	client.productChange(NamespaceProfilers, true)
 	// should just contain app-product-change
 	require.Len(t, client.requests, 1)
@@ -35,7 +35,7 @@ func TestProductEnabled(t *testing.T) {
 
 func TestConfigChange(t *testing.T) {
 	client := new(client)
-	client.start(nil, NamespaceTracers)
+	client.start(nil, NamespaceTracers, true)
 	client.configChange([]Configuration{BoolConfig("delta_profiles", true)})
 	require.Len(t, client.requests, 1)
 
@@ -138,3 +138,31 @@ func TestProductChange(t *testing.T) {
 		})
 	}
 }
+
+// Test that globally registered app config is sent in telemetry requests including the configuration state.
+func TestRegisterAppConfig(t *testing.T) {
+	client := new(client)
+	client.RegisterAppConfig("key1", "val1", "origin1")
+
+	// Test that globally registered app config is sent in app-started payloads
+	client.start([]Configuration{{Name: "key2", Value: "val2", Origin: "origin2"}}, NamespaceTracers, false)
+
+	req := client.requests[0].Body
+	require.Equal(t, RequestTypeAppStarted, req.RequestType)
+	appStarted := req.Payload.(*AppStarted)
+	cfg := appStarted.Configuration
+	require.Len(t, cfg, 2)
+	require.Contains(t, cfg, Configuration{Name: "key1", Value: "val1", Origin: "origin1"})
+	require.Contains(t, cfg, Configuration{Name: "key2", Value: "val2", Origin: "origin2"})
+
+	// Test that globally registered app config is sent in app-client-configuration-change payloads
+	client.ProductChange(NamespaceTracers, true, []Configuration{{Name: "key3", Value: "val3", Origin: "origin3"}})
+
+	req = client.requests[2].Body
+	require.Equal(t, RequestTypeAppClientConfigurationChange, req.RequestType)
+	appConfigChange := req.Payload.(*ConfigurationChange)
+	cfg = appConfigChange.Configuration
+	require.Len(t, cfg, 2)
+	require.Contains(t, cfg, Configuration{Name: "key1", Value: "val1", Origin: "origin1"})
+	require.Contains(t, cfg, Configuration{Name: "key3", Value: "val3", Origin: "origin3"})
+}

internal/telemetry/telemetrytest/telemetrytest.go

@@ -27,6 +27,10 @@ type MockClient struct {
 	Metrics         map[telemetry.Namespace]map[string]float64
 }
 
+func (c *MockClient) RegisterAppConfig(name string, val interface{}, origin string) {
+	_ = c.Called(name, val, origin)
+}
+
 // ProductChange starts and adds configuration data to the mock client.
 func (c *MockClient) ProductChange(namespace telemetry.Namespace, enabled bool, configuration []telemetry.Configuration) {
 	c.mu.Lock()