refactor: split main otel config into multiple connections, add tests

Author: obs-gh-mattcotter

.goreleaser.yaml

@@ -74,14 +74,10 @@ archives:
       {{- else }}{{ .Arch }}{{ end }}
       {{- if .Arm }}v{{ .Arm }}{{ end }}
     files:
-      - src: "packaging/windows/config/otel-collector.yaml"
-        dst: "otel-collector.yaml"
       - src: "packaging/windows/observe-agent.yaml"
         dst: "observe-agent.yaml"
-      - src: "packaging/windows/connections/host_monitoring/*"
-        dst: "connections/host_monitoring"
-      - src: "packaging/windows/connections/self_monitoring/*"
-        dst: "connections/self_monitoring"
+      - src: "packaging/windows/connections/*"
+        dst: "connections"
 
   - id: macos
     formats: ["zip"]

go.mod

@@ -5,6 +5,7 @@ go 1.23.7
 require (
 	github.com/go-viper/mapstructure/v2 v2.2.1
 	github.com/jarcoal/httpmock v1.3.1
+	github.com/mcuadros/go-defaults v1.2.0
 	github.com/observeinc/observe-agent/observecol v0.0.0-00010101000000-000000000000
 	github.com/prometheus/client_model v0.6.1
 	github.com/prometheus/common v0.62.0

integration/scripts/install_windows.ps1

@@ -18,7 +18,7 @@ $temp_dir="C:\temp"
 #Create directories for temp & observe-agent installation ls
 New-Item -ItemType Directory -Force -Path $temp_dir
 New-Item -ItemType Directory -Force -Path $observeagent_install_dir
-New-Item -ItemType Directory -Force -Path $observeagent_install_dir\config
+New-Item -ItemType Directory -Force -Path $observeagent_install_dir\connections
 New-Item -ItemType Directory -Force -Path $program_data_filestorage
 
 # Stop the observe agent if its running so that we can copy the new .exe
@@ -36,7 +36,6 @@ Expand-Archive -Force -LiteralPath $local_installer  -DestinationPath "$temp_dir
 Write-Output "Copying files from $temp_dir\observe-agent_extract to $observeagent_install_dir"
 Copy-Item -Force -Path $temp_dir\observe-agent_extract\observe-agent.exe -Destination $observeagent_install_dir
 Copy-Item -Force -Path $temp_dir\observe-agent_extract\observe-agent.yaml -Destination $observeagent_install_dir
-Copy-Item -Force -Path $temp_dir\observe-agent_extract\otel-collector.yaml -Destination $observeagent_install_dir\config\otel-collector.yaml
 Copy-Item -Force -Path $temp_dir\observe-agent_extract\connections\ -Destination $observeagent_install_dir\connections -Recurse
 
 

internal/commands/config/config.go

@@ -89,6 +89,9 @@ func printAllConfigsIndividually(configFilePaths []string) error {
 }
 
 func printShortOtelConfig(ctx context.Context, configFilePaths []string) error {
+	if len(configFilePaths) == 0 {
+		return nil
+	}
 	settings := observecol.ConfigProviderSettings(configFilePaths)
 	resolver, err := confmap.NewResolver(settings.ResolverSettings)
 	if err != nil {
@@ -107,6 +110,9 @@ func printShortOtelConfig(ctx context.Context, configFilePaths []string) error {
 }
 
 func printFullOtelConfig(configFilePaths []string) error {
+	if len(configFilePaths) == 0 {
+		return nil
+	}
 	colSettings := observecol.GenerateCollectorSettingsWithConfigFiles(configFilePaths)
 	factories, err := colSettings.Factories()
 	if err != nil {

internal/commands/initconfig/initconfig_test.go

@@ -4,10 +4,11 @@ import (
 	"os"
 	"testing"
 
+	"github.com/go-viper/mapstructure/v2"
 	"github.com/observeinc/observe-agent/internal/config"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 )
 
 func Test_InitConfigCommand(t *testing.T) {
@@ -77,17 +78,31 @@ func Test_InitConfigCommand(t *testing.T) {
 		if err != nil {
 			if tc.expectErr == "" {
 				t.Errorf("Expected no error, got %v", err)
+			} else {
+				assert.ErrorContains(t, err, tc.expectErr)
 			}
 		}
-		var config config.AgentConfig
-		configFile, err := os.ReadFile("./test-config.yaml")
-		if err != nil {
-			t.Errorf("Expected no error, got %v", err)
-		}
-		err = yaml.Unmarshal(configFile, &config)
-		if err != nil {
-			t.Errorf("Expected no error, got %v", err)
-		}
-		assert.Equal(t, tc.expectedConfig, config)
+
+		var configYaml, configMapstructure config.AgentConfig
+
+		// Decode via mapstrucutre (which is how viper does it)
+		configFileContents, err := os.ReadFile("./test-config.yaml")
+		assert.NoError(t, err)
+		var yamlMap map[string]any
+		err = yaml.Unmarshal(configFileContents, &yamlMap)
+		assert.NoError(t, err)
+		err = mapstructure.Decode(yamlMap, &configMapstructure)
+		assert.NoError(t, err)
+		assert.Equal(t, tc.expectedConfig, configMapstructure)
+
+		// Decode via yaml in order to do strict field checks
+		configFile, err := os.Open("./test-config.yaml")
+		assert.NoError(t, err)
+		decoder := yaml.NewDecoder(configFile)
+		// Ensure that all fields in the output yaml are present in our struct
+		decoder.KnownFields(true)
+		err = decoder.Decode(&configYaml)
+		assert.NoError(t, err)
+		assert.Equal(t, tc.expectedConfig, configYaml)
 	}
 }

internal/config/configschema.go

@@ -6,6 +6,7 @@ import (
 	"net/url"
 	"strings"
 
+	"github.com/mcuadros/go-defaults"
 	"github.com/spf13/viper"
 )
 
@@ -38,17 +39,29 @@ type SelfMonitoringConfig struct {
 	Enabled bool `yaml:"enabled" mapstructure:"enabled"`
 }
 
+type HealthCheckConfig struct {
+	Enabled bool `yaml:"enabled" mapstructure:"enabled" default:"true"`
+}
+
+type ForwardingConfig struct {
+	Enabled bool `yaml:"enabled" mapstructure:"enabled" default:"true"`
+}
+
 type AgentConfig struct {
 	Token                  string               `yaml:"token" mapstructure:"token"`
 	ObserveURL             string               `yaml:"observe_url" mapstructure:"observe_url"`
 	CloudResourceDetectors []string             `yaml:"cloud_resource_detectors,omitempty" mapstructure:"cloud_resource_detectors"`
+	Debug                  bool                 `yaml:"debug,omitempty" mapstructure:"debug"`
+	HealthCheck            HealthCheckConfig    `yaml:"health_check" mapstructure:"health_check"`
+	Forwarding             ForwardingConfig     `yaml:"forwarding" mapstructure:"forwarding"`
 	SelfMonitoring         SelfMonitoringConfig `yaml:"self_monitoring,omitempty" mapstructure:"self_monitoring"`
 	HostMonitoring         HostMonitoringConfig `yaml:"host_monitoring,omitempty" mapstructure:"host_monitoring"`
 	OtelConfigOverrides    map[string]any       `yaml:"otel_config_overrides,omitempty" mapstructure:"otel_config_overrides"`
 }
 
 func AgentConfigFromViper(v *viper.Viper) (*AgentConfig, error) {
 	var config AgentConfig
+	defaults.SetDefaults(&config)
 	err := v.Unmarshal(&config)
 	if err != nil {
 		return nil, err

internal/config/configschema_test.go

@@ -3,6 +3,7 @@ package config
 import (
 	"testing"
 
+	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -43,3 +44,26 @@ func TestAgentConfigValidate(t *testing.T) {
 	}
 	assert.ErrorContains(t, invalidTokenConfig.Validate(), "invalid Token")
 }
+
+func TestAgentConfigFromViper(t *testing.T) {
+	v := viper.NewWithOptions(viper.KeyDelimiter("::"))
+	v.Set("token", "some:token")
+	v.Set("observe_url", "https://observeinc.com")
+	v.Set("host_monitoring::enabled", true)
+	config, err := AgentConfigFromViper(v)
+	assert.NoError(t, err)
+	assert.Equal(t, "some:token", config.Token)
+	assert.Equal(t, "https://observeinc.com", config.ObserveURL)
+	assert.Equal(t, true, config.HostMonitoring.Enabled)
+
+	// Validate that defaults are set when the value is not in the viper config
+	assert.Equal(t, true, config.HealthCheck.Enabled)
+	assert.Equal(t, true, config.Forwarding.Enabled)
+
+	// Validate that defaults are overridden by present values
+	v.Set("health_check::enabled", false)
+	config, err = AgentConfigFromViper(v)
+	assert.NoError(t, err)
+	assert.Equal(t, false, config.HealthCheck.Enabled)
+	assert.Equal(t, true, config.Forwarding.Enabled)
+}

internal/connections/allconnectiontypes.go

@@ -0,0 +1,89 @@
+package connections
+
+import "github.com/observeinc/observe-agent/internal/config"
+
+var AllConnectionTypes = []*ConnectionType{
+	// NB: whenever a new connection type is added here, it's enable check
+	// should be reflected in the base fragment enable check.
+	CommonConnectionType,
+	HostMonitoringConnectionType,
+	SelfMonitoringConnectionType,
+}
+
+var CommonConnectionType = MakeConnectionType(
+	"common",
+	func(_ *config.AgentConfig) bool {
+		return true
+	},
+	[]CollectorConfigFragment{
+		{
+			enabledCheck: func(agentConfig *config.AgentConfig) bool {
+				return (agentConfig.Forwarding.Enabled ||
+					agentConfig.HealthCheck.Enabled ||
+					agentConfig.SelfMonitoring.Enabled ||
+					agentConfig.HostMonitoring.Enabled)
+			},
+			colConfigFilePath: "base.yaml.tmpl",
+		},
+		{
+			enabledCheck: func(agentConfig *config.AgentConfig) bool {
+				return agentConfig.Forwarding.Enabled
+			},
+			colConfigFilePath: "forward.yaml.tmpl",
+		},
+		{
+			enabledCheck: func(agentConfig *config.AgentConfig) bool {
+				return agentConfig.HealthCheck.Enabled
+			},
+			colConfigFilePath: "health_check.yaml.tmpl",
+		},
+	},
+)
+
+var HostMonitoringConnectionType = MakeConnectionType(
+	"host_monitoring",
+	func(agentConfig *config.AgentConfig) bool {
+		return agentConfig.HostMonitoring.Enabled
+	},
+	[]CollectorConfigFragment{
+		{
+			enabledCheck: func(agentConfig *config.AgentConfig) bool {
+				return agentConfig.HostMonitoring.Enabled
+			},
+			colConfigFilePath: "host.yaml.tmpl",
+		},
+		{
+			enabledCheck: func(agentConfig *config.AgentConfig) bool {
+				return agentConfig.HostMonitoring.Metrics.Host.Enabled
+			},
+			colConfigFilePath: "host_metrics.yaml.tmpl",
+		},
+		{
+			enabledCheck: func(agentConfig *config.AgentConfig) bool {
+				return agentConfig.HostMonitoring.Metrics.Process.Enabled
+			},
+			colConfigFilePath: "process_metrics.yaml.tmpl",
+		},
+		{
+			enabledCheck: func(agentConfig *config.AgentConfig) bool {
+				return agentConfig.HostMonitoring.Logs.Enabled
+			},
+			colConfigFilePath: "logs.yaml.tmpl",
+		},
+	},
+)
+
+var SelfMonitoringConnectionType = MakeConnectionType(
+	"self_monitoring",
+	func(agentConfig *config.AgentConfig) bool {
+		return agentConfig.SelfMonitoring.Enabled
+	},
+	[]CollectorConfigFragment{
+		{
+			enabledCheck: func(agentConfig *config.AgentConfig) bool {
+				return agentConfig.SelfMonitoring.Enabled
+			},
+			colConfigFilePath: "logs_and_metrics.yaml.tmpl",
+		},
+	},
+)

internal/connections/confighandler.go

@@ -20,28 +20,17 @@ const (
 
 func GetAllOtelConfigFilePaths(ctx context.Context, tmpDir string) ([]string, error) {
 	configFilePaths := []string{}
-	// If the default otel-collector.yaml exists, add it to the list of config files
-	defaultOtelConfigPath := filepath.Join(GetDefaultConfigFolder(), "otel-collector.yaml")
-	if _, err := os.Stat(defaultOtelConfigPath); err == nil {
-		agentConf, err := config.AgentConfigFromViper(viper.GetViper())
-		if err != nil {
-			return nil, err
-		}
-		otelConfigRendered, err := RenderConfigTemplate(ctx, tmpDir, defaultOtelConfigPath, agentConf)
-		if err != nil {
-			return nil, err
-		}
-		configFilePaths = append(configFilePaths, otelConfigRendered)
-	}
 	// Get additional config paths based on connection configs
+	agentConfig, err := config.AgentConfigFromViper(viper.GetViper())
+	if err != nil {
+		return nil, err
+	}
 	for _, conn := range AllConnectionTypes {
-		if viper.IsSet(conn.Name) {
-			connectionPaths, err := conn.GetConfigFilePaths(ctx, tmpDir)
-			if err != nil {
-				return nil, err
-			}
-			configFilePaths = append(configFilePaths, connectionPaths...)
+		connectionPaths, err := conn.GetConfigFilePaths(ctx, tmpDir, agentConfig)
+		if err != nil {
+			return nil, err
 		}
+		configFilePaths = append(configFilePaths, connectionPaths...)
 	}
 	// Generate override file and include path if overrides provided
 	if viper.IsSet(OTEL_OVERRIDE_YAML_KEY) {
@@ -119,19 +108,6 @@ func GetOverrideConfigFile(tmpDir string, data map[string]any) (string, error) {
 	return f.Name(), nil
 }
 
-func GetDefaultConfigFolder() string {
-	switch currOS := runtime.GOOS; currOS {
-	case "darwin":
-		return filepath.Join(GetDefaultAgentPath(), "config")
-	case "windows":
-		return filepath.Join(GetDefaultAgentPath(), "config")
-	case "linux":
-		return GetDefaultAgentPath()
-	default:
-		return GetDefaultAgentPath()
-	}
-}
-
 func GetConfigFragmentFolderPath() string {
 	return filepath.Join(GetDefaultAgentPath(), "connections")
 }