[Process Agent] Add container_collection.enabled and `process_colle…

…ction.enabled` (#10555)

* Add container_collection.enabled and process_collection.enabled

* Set up env key transformer for process_config.enabled

* Fixed linting error

* Fixed initProcessDiscoveryCheck to use new settings

* Added release note and updated the config_template.yaml

* Fixed LoadProcessYamlConfig to use new settings

* Fixed deprecation message not showing up for environment variable

* Fixed "could not convert 'disabled' to bool" error

* Updated other usages of `process_config.enabled` to check `process_config.process_collection.enabled` instead

* Reverted system probe changes

* Fix import tests

* Fix agent6DisabledMessage

* Apply suggestions from code review

Co-authored-by: Moisés Botarro <45740044+mbotarro@users.noreply.github.com>
Co-authored-by: Ivan Ilichev <ivan.ilichev@datadoghq.com>

* Applied code review suggestions

* Renamed result to enabled

* Removed unused function `isAffirmative`

* Update releasenotes/notes/deprecate-process_config.enabled-89e9ab7b0193cc04.yaml

Co-authored-by: maxime mouial <hush-hush@users.noreply.github.com>

* Removed `TestDisplayProcConfigEnabledDeprecationWarning`

* Removed helpers

* Removed helpers

* Fixed some documentation

* Fixed process discovery test

* Moved config loading from LoadProcessYamlConfig and into global config

* Add override func

* Fixed problems with merge

* Fixed failing test in `pkg/process/config`

* Fix process agent check in core agent

* Made LoadProcessTransforms private

* Fixed deprecation info note, and changed release note to explain what the new setting translates to

* Updated comment in config_test.go

* Added container collection to inventories payload

* Applied maxime's suggestions

* Updated inventories documentation

* Updated inventories.go naming

Co-authored-by: Moisés Botarro <45740044+mbotarro@users.noreply.github.com>
Co-authored-by: Ivan Ilichev <ivan.ilichev@datadoghq.com>
Co-authored-by: maxime mouial <hush-hush@users.noreply.github.com>

cmd/agent/app/dependent_services_windows.go

@@ -34,8 +34,15 @@ var subservices = []Servicedef{
 		serviceInit: apmInit,
 	},
 	{
-		name:        "process",
-		configKeys:  []string{"process_config.enabled", "process_config.process_discovery.enabled", "network_config.enabled", "system_probe_config.enabled"},
+		name: "process",
+		configKeys: []string{
+			"process_config.enabled",
+			"process_config.process_collection.enabled",
+			"process_config.container_collection.enabled",
+			"process_config.process_discovery.enabled",
+			"network_config.enabled",
+			"system_probe_config.enabled",
+		},
 		serviceName: "datadog-process-agent",
 		serviceInit: processInit,
 	},

cmd/agent/common/import_test.go

@@ -159,7 +159,8 @@ func validateSelectedParameters(t *testing.T, migratedConfigFile, oldConfigFile
 
 	// Some second level parameters
 	migratedProcessConfig := migratedConf["process_config"].(map[interface{}]interface{})
-	assert.Equal(t, oldConfig["process_agent_enabled"], migratedProcessConfig["enabled"])
+	processConfigProcessCollection := migratedProcessConfig["process_collection"].(map[interface{}]interface{})
+	assert.Equal(t, oldConfig["process_agent_enabled"], strconv.FormatBool(processConfigProcessCollection["enabled"].(bool)))
 
 	migratedApmConfig := migratedConf["apm_config"].(map[interface{}]interface{})
 	assert.Equal(t, toBool(oldConfig["apm_enabled"]), migratedApmConfig["enabled"])

cmd/process-agent/main_common.go

@@ -151,9 +151,10 @@ func versionString(sep string) string {
 
 const (
 	agent6DisabledMessage = `process-agent not enabled.
-Set env var DD_PROCESS_AGENT_ENABLED=true or add
+Set env var DD_PROCESS_CONFIG_PROCESS_COLLECTION_ENABLED=true or add
 process_config:
-  enabled: "true"
+  process_collection:
+    enabled: true
 to your datadog.yaml file.
 Exiting.`
 )

pkg/collector/corechecks/embed/process_agent.go

@@ -131,7 +131,7 @@ func (c *ProcessAgentCheck) run() error {
 func (c *ProcessAgentCheck) Configure(data integration.Data, initConfig integration.Data, source string) error {
 	// only log whether process check is enabled or not but don't return early, because we still need to initialize "binPath", "source" and
 	// start up process-agent. Ultimately it's up to process-agent to decide whether to run or not based on the config
-	if enabled := config.Datadog.GetBool("process_config.enabled"); !enabled {
+	if enabled := config.Datadog.GetBool("process_config.process_collection.enabled"); !enabled {
 		log.Info("live process monitoring is disabled through main configuration file")
 	}
 

pkg/config/config_template.yaml

@@ -1182,6 +1182,21 @@ api_key:
 #
 # process_config:
 
+  ## @param process_collection - custom object - optional
+  ## Specifies settings for collecting processes.
+  # process_collection:
+    ## @param enabled - boolean - optional - default: false
+    ## Enables collection of information about running processes.
+    # enabled: false
+
+  ## @param container_collection - custom object - optional
+  ## Specifies settings for collecting containers.
+  # container_collection:
+    ## @param enabled - boolean - optional - default: true
+    ## Enables collection of information about running containers.
+    # enabled: true
+
+  ## Deprecated - use `process_collection.enabled` and `container_collection.enabled` instead
   ## @param enabled - string - optional - default: "false"
   ## @env DD_PROCESS_CONFIG_ENABLED - string - optional - default: "false"
   ##  A string indicating the enabled state of the Process Agent:

pkg/config/legacy/converter.go

@@ -39,10 +39,10 @@ func FromAgentConfig(agentConfig Config, converter *config.LegacyConfigConverter
 
 	if enabled, err := isAffirmative(agentConfig["process_agent_enabled"]); enabled {
 		// process agent is explicitly enabled
-		converter.Set("process_config.enabled", "true")
+		converter.Set("process_config.process_collection.enabled", true)
 	} else if err == nil && !enabled {
 		// process agent is explicitly disabled
-		converter.Set("process_config.enabled", "disabled")
+		converter.Set("process_config.container_collection.enabled", false)
 	}
 
 	tags := strings.Split(agentConfig["tags"], ",")

pkg/config/process.go

@@ -6,15 +6,23 @@
 package config
 
 import (
+	"strconv"
 	"strings"
+	"sync"
 	"time"
+
+	"github.com/DataDog/datadog-agent/pkg/util/log"
 )
 
 const (
 	// DefaultGRPCConnectionTimeoutSecs sets the default value for timeout when connecting to the agent
 	DefaultGRPCConnectionTimeoutSecs = 60
 )
 
+// setupProcesses is meant to be called multiple times for different configs, but overrides apply to all configs, so
+// we need to make sure it is only applied once
+var processesAddOverrideOnce sync.Once
+
 // procBindEnvAndSetDefault is a helper function that generates both "DD_PROCESS_CONFIG_" and "DD_PROCESS_AGENT_" prefixes from a key.
 // We need this helper function because the standard BindEnvAndSetDefault can only generate one prefix from a key.
 func procBindEnvAndSetDefault(config Config, key string, val interface{}) {
@@ -27,10 +35,31 @@ func procBindEnvAndSetDefault(config Config, key string, val interface{}) {
 	config.BindEnvAndSetDefault(key, val, envs...)
 }
 
+// procBindEnv is a helper function that generates both "DD_PROCESS_CONFIG_" and "DD_PROCESS_AGENT_" prefixes from a key, but does not set a default.
+// We need this helper function because the standard BindEnv can only generate one prefix from a key.
+func procBindEnv(config Config, key string) {
+	processConfigKey := "DD_" + strings.Replace(strings.ToUpper(key), ".", "_", -1)
+	processAgentKey := strings.Replace(processConfigKey, "PROCESS_CONFIG", "PROCESS_AGENT", 1)
+
+	config.BindEnv(key, processConfigKey, processAgentKey)
+}
+
 func setupProcesses(config Config) {
-	// process_config.enabled is only used on Windows by the core agent to start the process agent service.
-	// it can be set from file, but not from env. Override it with value from DD_PROCESS_AGENT_ENABLED.
-	procBindEnvAndSetDefault(config, "process_config.enabled", "false")
+	// "process_config.enabled" is deprecated. We must still be able to detect if it is present, to know if we should use it
+	// or container_collection.enabled and process_collection.enabled.
+	procBindEnv(config, "process_config.enabled")
+	config.SetEnvKeyTransformer("process_config.enabled", func(val string) interface{} {
+		// DD_PROCESS_AGENT_ENABLED: true - Process + Container checks enabled
+		//                           false - No checks enabled
+		//                           (unset) - Defaults are used, only container check is enabled
+		if enabled, _ := strconv.ParseBool(val); enabled {
+			return "true"
+		}
+		return "disabled"
+	})
+	procBindEnvAndSetDefault(config, "process_config.container_collection.enabled", true)
+	procBindEnvAndSetDefault(config, "process_config.process_collection.enabled", false)
+
 	config.BindEnv("process_config.process_dd_url", "")
 	config.SetKnown("process_config.dd_agent_env")
 	config.SetKnown("process_config.enabled")
@@ -69,4 +98,28 @@ func setupProcesses(config Config) {
 		"DD_PROCESS_AGENT_DISCOVERY_ENABLED",
 	)
 	procBindEnvAndSetDefault(config, "process_config.process_discovery.interval", 4*time.Hour)
+
+	processesAddOverrideOnce.Do(func() {
+		AddOverrideFunc(loadProcessTransforms)
+	})
+}
+
+// loadProcessTransforms loads transforms associated with process config settings.
+func loadProcessTransforms(config Config) {
+	if config.IsSet("process_config.enabled") {
+		log.Info("process_config.enabled is deprecated, use process_config.container_collection.enabled " +
+			"and process_config.process_collection.enabled instead, " +
+			"see https://docs.datadoghq.com/infrastructure/process#installation for more information")
+		procConfigEnabled := strings.ToLower(config.GetString("process_config.enabled"))
+		if procConfigEnabled == "disabled" {
+			config.Set("process_config.process_collection.enabled", false)
+			config.Set("process_config.container_collection.enabled", false)
+		} else if enabled, _ := strconv.ParseBool(procConfigEnabled); enabled { // "true"
+			config.Set("process_config.process_collection.enabled", true)
+			config.Set("process_config.container_collection.enabled", false)
+		} else { // "false"
+			config.Set("process_config.process_collection.enabled", false)
+			config.Set("process_config.container_collection.enabled", true)
+		}
+	}
 }

pkg/config/process_test.go

@@ -46,6 +46,14 @@ func TestProcessDefaultConfig(t *testing.T) {
 			key:          "process_config.process_discovery.interval",
 			defaultValue: 4 * time.Hour,
 		},
+		{
+			key:          "process_config.process_collection.enabled",
+			defaultValue: false,
+		},
+		{
+			key:          "process_config.container_collection.enabled",
+			defaultValue: true,
+		},
 	} {
 		t.Run(tc.key+" default", func(t *testing.T) {
 			assert.Equal(t, tc.defaultValue, cfg.Get(tc.key))
@@ -158,6 +166,30 @@ func TestEnvVarOverride(t *testing.T) {
 			value:    "true",
 			expected: true,
 		},
+		{
+			key:      "process_config.enabled",
+			env:      "DD_PROCESS_CONFIG_ENABLED",
+			value:    "true",
+			expected: "true",
+		},
+		{
+			key:      "process_config.process_collection.enabled",
+			env:      "DD_PROCESS_CONFIG_PROCESS_COLLECTION_ENABLED",
+			value:    "true",
+			expected: true,
+		},
+		{
+			key:      "process_config.container_collection.enabled",
+			env:      "DD_PROCESS_CONFIG_CONTAINER_COLLECTION_ENABLED",
+			value:    "true",
+			expected: true,
+		},
+		{
+			key:      "process_config.enabled",
+			env:      "DD_PROCESS_CONFIG_ENABLED",
+			value:    "false",
+			expected: "disabled",
+		},
 	} {
 		t.Run(tc.env, func(t *testing.T) {
 			reset := setEnvForTest(tc.env, tc.value)
@@ -195,3 +227,61 @@ func TestProcBindEnvAndSetDefault(t *testing.T) {
 	// Make sure the default is set properly
 	assert.Equal(t, "asdf", cfg.GetString("process_config.foo.bar"))
 }
+
+func TestProcBindEnv(t *testing.T) {
+	cfg := setupConf()
+	procBindEnv(cfg, "process_config.foo.bar")
+
+	envs := map[string]struct{}{}
+	for _, env := range cfg.GetEnvVars() {
+		envs[env] = struct{}{}
+	}
+
+	_, ok := envs["DD_PROCESS_CONFIG_FOO_BAR"]
+	assert.True(t, ok)
+
+	_, ok = envs["DD_PROCESS_AGENT_FOO_BAR"]
+	assert.True(t, ok)
+
+	// Make sure that DD_PROCESS_CONFIG_FOO_BAR shows up as unset by default
+	assert.False(t, cfg.IsSet("process_config.foo.bar"))
+
+	// Try and set DD_PROCESS_CONFIG_FOO_BAR and make sure it shows up in the config
+	reset := setEnvForTest("DD_PROCESS_CONFIG_FOO_BAR", "baz")
+	assert.True(t, cfg.IsSet("process_config.foo.bar"))
+	assert.Equal(t, "baz", cfg.GetString("process_config.foo.bar"))
+	reset()
+}
+
+func TestProcConfigEnabledTransform(t *testing.T) {
+	for _, tc := range []struct {
+		procConfigEnabled                                      string
+		expectedContainerCollection, expectedProcessCollection bool
+	}{
+		{
+			procConfigEnabled:           "true",
+			expectedContainerCollection: false,
+			expectedProcessCollection:   true,
+		},
+		{
+			procConfigEnabled:           "false",
+			expectedContainerCollection: true,
+			expectedProcessCollection:   false,
+		},
+		{
+			procConfigEnabled:           "disabled",
+			expectedContainerCollection: false,
+			expectedProcessCollection:   false,
+		},
+	} {
+		t.Run("process_config.enabled="+tc.procConfigEnabled, func(t *testing.T) {
+			cfg := setupConf()
+			cfg.Set("process_config.enabled", tc.procConfigEnabled)
+			loadProcessTransforms(cfg)
+
+			assert.Equal(t, tc.expectedContainerCollection, cfg.GetBool("process_config.container_collection.enabled"))
+			assert.Equal(t, tc.expectedProcessCollection, cfg.GetBool("process_config.process_collection.enabled"))
+		})
+	}
+
+}

pkg/metadata/inventories/README.md

@@ -1,3 +1,4 @@
+
 # Inventory Payload
 
 This package populates some of the agent-related fields in the `inventories` product in DataDog.
@@ -61,8 +62,10 @@ The payload is a JSON dict with the following fields
     enabled, otherwise the field is omitted.
   - `feature_cws_enabled` - **bool**: True if the Cloud Workload Security is enabled (see: `runtime_security_config.enabled`
     config option).
-  - `feature_process_enabled` - **bool**: True if the Process Agent is enabled (see: `process_config.enabled` config
-    option).
+  - `feature_process_enabled` - **bool**: True if the Process Agent has process collection enabled
+     (see: `process_config.process_collection.enabled` config option).
+  - `feature_container_processes_enabled` - **bool**: True if the Process Agent has container collection enabled
+     (see: `process_config.container_collection.enabled`)
   - `feature_networks_enabled` - **bool**: True if the Network Performance Monitoring is enabled (see:
     `network_config.enabled` config option in `system-probe.yaml`).
   - `feature_logs_enabled` - **bool**: True if the logs collection is enabled (see: `logs_enabled` config option).

pkg/metadata/inventories/inventories.go

@@ -88,6 +88,7 @@ const (
 	AgentLogsTransport                 AgentMetadataName = "logs_transport"
 	AgentCWSEnabled                    AgentMetadataName = "feature_cws_enabled"
 	AgentProcessEnabled                AgentMetadataName = "feature_process_enabled"
+	AgentProcessesContainerEnabled     AgentMetadataName = "feature_processes_container_enabled"
 	AgentNetworksEnabled               AgentMetadataName = "feature_networks_enabled"
 	AgentLogsEnabled                   AgentMetadataName = "feature_logs_enabled"
 	AgentCSPMEnabled                   AgentMetadataName = "feature_cspm_enabled"
@@ -280,7 +281,8 @@ func initializeConfig(cfg config.Config) {
 	SetAgentMetadata(AgentConfigProxyHTTP, clean(cfg.GetString("proxy.http")))
 	SetAgentMetadata(AgentConfigProxyHTTPS, clean(cfg.GetString("proxy.https")))
 	SetAgentMetadata(AgentCWSEnabled, config.Datadog.GetBool("runtime_security_config.enabled"))
-	SetAgentMetadata(AgentProcessEnabled, config.Datadog.GetBool("process_config.enabled"))
+	SetAgentMetadata(AgentProcessEnabled, config.Datadog.GetBool("process_config.process_collection.enabled"))
+	SetAgentMetadata(AgentProcessesContainerEnabled, config.Datadog.GetBool("process_config.container_collection.enabled"))
 	SetAgentMetadata(AgentNetworksEnabled, config.Datadog.GetBool("network_config.enabled"))
 	SetAgentMetadata(AgentLogsEnabled, config.Datadog.GetBool("logs_enabled"))
 	SetAgentMetadata(AgentCSPMEnabled, config.Datadog.GetBool("compliance_config.enabled"))

pkg/process/config/config.go

@@ -445,14 +445,6 @@ func IsBlacklisted(cmdline []string, blacklist []*regexp.Regexp) bool {
 	return false
 }
 
-func isAffirmative(value string) (bool, error) {
-	if value == "" {
-		return false, fmt.Errorf("value is empty")
-	}
-	v := strings.ToLower(value)
-	return v == "true" || v == "yes" || v == "1", nil
-}
-
 // getHostname attempts to resolve the hostname in the following order: the main datadog agent via grpc, the main agent
 // via cli and lastly falling back to os.Hostname() if it is unavailable
 func getHostname(ctx context.Context, ddAgentBin string, grpcConnectionTimeout time.Duration) (string, error) {

pkg/process/config/config_test.go

@@ -137,6 +137,7 @@ func TestOnlyEnvConfig(t *testing.T) {
 
 	os.Setenv("DD_PROCESS_AGENT_ENABLED", "false")
 	os.Setenv("DD_PROCESS_AGENT_PROCESS_DISCOVERY_ENABLED", "false")
+	_, _ = config.Load() // Deprecated environment variables won't have their transforms applied until the config is reloaded
 	agentConfig, _ = NewAgentConfig("test", "", syscfg, true)
 	assert.Equal(t, "apikey_from_env", agentConfig.APIEndpoints[0].APIKey)
 	assert.False(t, agentConfig.Enabled)
@@ -577,27 +578,6 @@ func TestSystemProbeNoNetwork(t *testing.T) {
 	assert.ElementsMatch(t, []string{OOMKillCheckName, ProcessCheckName, RTProcessCheckName}, agentConfig.EnabledChecks)
 }
 
-func TestIsAffirmative(t *testing.T) {
-	value, err := isAffirmative("yes")
-	assert.Nil(t, err)
-	assert.True(t, value)
-
-	value, err = isAffirmative("True")
-	assert.Nil(t, err)
-	assert.True(t, value)
-
-	value, err = isAffirmative("1")
-	assert.Nil(t, err)
-	assert.True(t, value)
-
-	_, err = isAffirmative("")
-	assert.NotNil(t, err)
-
-	value, err = isAffirmative("ok")
-	assert.Nil(t, err)
-	assert.False(t, value)
-}
-
 func TestGetHostnameFromGRPC(t *testing.T) {
 	ctx := context.Background()
 	ctrl := gomock.NewController(t)
@@ -700,19 +680,17 @@ func TestGetHostnameShellCmd(t *testing.T) {
 func TestProcessDiscoveryConfig(t *testing.T) {
 	assert := assert.New(t)
 
-	procConfigEnabledValues := []string{"true", "false", "disabled"}
-	procDiscoveryEnabledValues := []bool{true, false}
-	for _, procConfigEnabled := range procConfigEnabledValues {
-		for _, procDiscoveryEnabled := range procDiscoveryEnabledValues {
-			config.Datadog.Set("process_config.enabled", procConfigEnabled)
+	for _, procCollectionEnabled := range []bool{true, false} {
+		for _, procDiscoveryEnabled := range []bool{true, false} {
+			config.Datadog.Set("process_config.process_collection.enabled", procCollectionEnabled)
 			config.Datadog.Set("process_config.process_discovery.enabled", procDiscoveryEnabled)
 			config.Datadog.Set("process_config.process_discovery.interval", time.Hour)
 			cfg := AgentConfig{EnabledChecks: []string{}, CheckIntervals: map[string]time.Duration{}}
 			cfg.initProcessDiscoveryCheck()
 
-			// Make sure that the process discovery check is only enabled when both the process-agent is not set to true,
+			// Make sure that the process discovery check is only enabled when process collection is disabled,
 			// and procDiscoveryEnabled isn't overridden.
-			if procDiscoveryEnabled == true && procConfigEnabled != "true" {
+			if procDiscoveryEnabled && !procCollectionEnabled {
 				assert.ElementsMatch([]string{DiscoveryCheckName}, cfg.EnabledChecks)
 
 				// Interval Tests: