Fix config parsing for structs and pointers to structs

internal/extension/smartagentextension/config.go

@@ -18,12 +18,13 @@ import (
 	"fmt"
 	"path/filepath"
 	"reflect"
-	"strings"
 
 	"github.com/signalfx/defaults"
 	saconfig "github.com/signalfx/signalfx-agent/pkg/core/config"
 	"go.opentelemetry.io/collector/config"
 	"gopkg.in/yaml.v2"
+
+	"github.com/signalfx/splunk-otel-collector/internal/utils"
 )
 
 // SmartAgentConfigProvider exposes global saconfig.Config to other components
@@ -77,15 +78,7 @@ func (cfg *Config) Unmarshal(componentParser *config.Parser) error {
 
 func smartAgentConfigFromSettingsMap(settings map[string]interface{}) (*saconfig.Config, error) {
 	var config saconfig.Config
-	yamlTags := yamlTagsFromStruct(reflect.TypeOf(config))
-
-	for key, val := range settings {
-		updatedKey := yamlTags[key]
-		if updatedKey != "" {
-			delete(settings, key)
-			settings[updatedKey] = val
-		}
-	}
+	utils.RespectYamlTagsInAllSettings(reflect.TypeOf(config), settings)
 
 	var collectdSettings map[string]interface{}
 	var ok bool
@@ -94,14 +87,7 @@ func smartAgentConfigFromSettingsMap(settings map[string]interface{}) (*saconfig
 	}
 
 	var collectdConfig saconfig.CollectdConfig
-	yamlTags = yamlTagsFromStruct(reflect.TypeOf(collectdConfig))
-	for key, val := range collectdSettings {
-		updatedKey := yamlTags[key]
-		if updatedKey != "" {
-			delete(collectdSettings, key)
-			collectdSettings[updatedKey] = val
-		}
-	}
+	utils.RespectYamlTagsInAllSettings(reflect.TypeOf(collectdConfig), collectdSettings)
 
 	settings["collectd"] = collectdSettings
 
@@ -128,18 +114,3 @@ func smartAgentConfigFromSettingsMap(settings map[string]interface{}) (*saconfig
 	config.Collectd.BundleDir = config.BundleDir
 	return &config, nil
 }
-
-func yamlTagsFromStruct(s reflect.Type) map[string]string {
-	yamlTags := map[string]string{}
-	for i := 0; i < s.NumField(); i++ {
-		field := s.Field(i)
-		tag := field.Tag
-		yamlTag := strings.Split(tag.Get("yaml"), ",")[0]
-		lowerTag := strings.ToLower(yamlTag)
-		if yamlTag != lowerTag {
-			yamlTags[lowerTag] = yamlTag
-		}
-	}
-
-	return yamlTags
-}

internal/receiver/smartagentreceiver/config.go

@@ -27,6 +27,8 @@ import (
 	"github.com/signalfx/signalfx-agent/pkg/monitors"
 	"go.opentelemetry.io/collector/config"
 	"gopkg.in/yaml.v2"
+
+	"github.com/signalfx/splunk-otel-collector/internal/utils"
 )
 
 const defaultIntervalSeconds = 10
@@ -94,18 +96,7 @@ func (cfg *Config) Unmarshal(componentParser *config.Parser) error {
 	}
 	monitorConfigType := reflect.TypeOf(customMonitorConfig).Elem()
 	monitorConfig := reflect.New(monitorConfigType).Interface()
-
-	// Viper is case insensitive and doesn't preserve a record of actual yaml map key cases from the provided config,
-	// which is a problem when unmarshalling custom agent monitor configs.  Here we use a map of lowercase to supported
-	// case tag key names and update the keys where applicable.
-	yamlTags := yamlTagsFromStruct(monitorConfigType)
-	for key, val := range allSettings {
-		updatedKey := yamlTags[key]
-		if updatedKey != "" {
-			delete(allSettings, key)
-			allSettings[updatedKey] = val
-		}
-	}
+	utils.RespectYamlTagsInAllSettings(monitorConfigType, allSettings)
 
 	asBytes, err := yaml.Marshal(allSettings)
 	if err != nil {
@@ -151,31 +142,6 @@ func getStringSliceFromAllSettings(allSettings map[string]interface{}, key strin
 	return items, nil
 }
 
-// Walks through a custom monitor config struct type, creating a map of
-// lowercase to supported yaml struct tag name cases.
-func yamlTagsFromStruct(s reflect.Type) map[string]string {
-	yamlTags := map[string]string{}
-	for i := 0; i < s.NumField(); i++ {
-		field := s.Field(i)
-		tag := field.Tag
-		yamlTag := strings.Split(tag.Get("yaml"), ",")[0]
-		lowerTag := strings.ToLower(yamlTag)
-		if yamlTag != lowerTag {
-			yamlTags[lowerTag] = yamlTag
-		}
-
-		fieldType := field.Type
-		if fieldType.Kind() == reflect.Struct {
-			otherFields := yamlTagsFromStruct(fieldType)
-			for k, v := range otherFields {
-				yamlTags[k] = v
-			}
-		}
-	}
-
-	return yamlTags
-}
-
 // If using the receivercreator, observer-provided endpoints should be used to set
 // the Host and Port fields of monitor config structs.  This can only be done by reflection without
 // making type assertions over all possible monitor types.

internal/receiver/smartagentreceiver/config_test.go

@@ -20,14 +20,19 @@ import (
 	"time"
 
 	"github.com/signalfx/signalfx-agent/pkg/core/common/httpclient"
+	"github.com/signalfx/signalfx-agent/pkg/core/common/kubelet"
+	"github.com/signalfx/signalfx-agent/pkg/core/common/kubernetes"
 	saconfig "github.com/signalfx/signalfx-agent/pkg/core/config"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/collectd/consul"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/collectd/hadoop"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/collectd/python"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/collectd/redis"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/filesystems"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/haproxy"
+	"github.com/signalfx/signalfx-agent/pkg/monitors/kubernetes/volumes"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/prometheusexporter"
+	"github.com/signalfx/signalfx-agent/pkg/monitors/telegraf/common/parser"
+	"github.com/signalfx/signalfx-agent/pkg/monitors/telegraf/monitors/exec"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/telegraf/monitors/ntpq"
 	"github.com/signalfx/signalfx-agent/pkg/utils/timeutil"
 	"github.com/stretchr/testify/assert"
@@ -488,3 +493,65 @@ func TestInvalidFilteringConfig(t *testing.T) {
 	require.Error(t, err)
 	require.EqualError(t, err, "unexpected end of input")
 }
+
+func TestLoadConfigWithNestedMonitorConfig(t *testing.T) {
+	factories, err := componenttest.NopFactories()
+	assert.Nil(t, err)
+
+	factory := NewFactory()
+	factories.Receivers[config.Type(typeStr)] = factory
+	cfg, err := configtest.LoadConfigFile(
+		t, path.Join(".", "testdata", "nested_monitor_config.yaml"), factories,
+	)
+
+	require.NoError(t, err)
+	require.NotNil(t, cfg)
+
+	assert.Equal(t, len(cfg.Receivers), 2)
+
+	telegrafExecCfg := cfg.Receivers["smartagent/exec"].(*Config)
+	require.Equal(t, &Config{
+		ReceiverSettings: config.ReceiverSettings{
+			TypeVal: typeStr,
+			NameVal: typeStr + "/exec",
+		},
+		monitorConfig: &exec.Config{
+			MonitorConfig: saconfig.MonitorConfig{
+				Type:                "telegraf/exec",
+				DatapointsToExclude: []saconfig.MetricFilter{},
+			},
+			Commands: []string{
+				`powershell.exe -Command "\Monitoring\Get_Directory.ps1"`,
+			},
+			TelegrafParser: &parser.Config{
+				DataFormat: "influx",
+			},
+		},
+	}, telegrafExecCfg)
+	require.NoError(t, telegrafExecCfg.validate())
+
+	k8sVolumesCfg := cfg.Receivers["smartagent/kubernetes_volumes"].(*Config)
+	tru := true
+	require.Equal(t, &Config{
+		ReceiverSettings: config.ReceiverSettings{
+			TypeVal: typeStr,
+			NameVal: typeStr + "/kubernetes_volumes",
+		},
+		monitorConfig: &volumes.Config{
+			MonitorConfig: saconfig.MonitorConfig{
+				Type:                "kubernetes-volumes",
+				DatapointsToExclude: []saconfig.MetricFilter{},
+			},
+			KubeletAPI: kubelet.APIConfig{
+				URL:        "https://192.168.99.103:10250",
+				AuthType:   "serviceAccount",
+				SkipVerify: &tru,
+			},
+			KubernetesAPI: &kubernetes.APIConfig{
+				AuthType:   "serviceAccount",
+				SkipVerify: false,
+			},
+		},
+	}, k8sVolumesCfg)
+	require.NoError(t, k8sVolumesCfg.validate())
+}

internal/receiver/smartagentreceiver/testdata/nested_monitor_config.yaml

@@ -0,0 +1,26 @@
+receivers:
+  smartagent/exec:
+    type: telegraf/exec
+    commands: [ 'powershell.exe -Command "\Monitoring\Get_Directory.ps1"' ]
+    telegrafParser:
+      dataFormat: "influx"
+  smartagent/kubernetes_volumes:
+    type: kubernetes-volumes
+    kubeletAPI:
+      authType: serviceAccount
+      url: https://192.168.99.103:10250
+
+processors:
+  nop:
+
+exporters:
+  nop:
+
+service:
+  pipelines:
+    metrics:
+      receivers:
+        - smartagent/exec
+        - smartagent/kubernetes_volumes
+      processors: [nop]
+      exporters: [nop]

internal/utils/config.go

@@ -0,0 +1,77 @@
+// Copyright OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"reflect"
+	"strings"
+)
+
+// RespectYamlTagsInAllSettings recursively walks through all tags and fixes them.
+// Viper is case insensitive and doesn't preserve a record of actual yaml map key
+// cases from the provided config, which is a problem when unmarshalling custom
+// agent monitor configs.  Here we use a map of lowercase to supported case tag key
+// names and update the keys where applicable.
+func RespectYamlTagsInAllSettings(s reflect.Type, settings map[string]interface{}) {
+	yamlTags := yamlTagsFromStruct(s)
+	recursivelyCapitalizeConfigKeys(settings, yamlTags)
+}
+
+// yamlTagsFromStruct Walks through a custom monitor config struct type,
+// creating a map of lowercase to supported yaml struct tag name cases.
+func yamlTagsFromStruct(s reflect.Type) map[string]string {
+	yamlTags := map[string]string{}
+	for i := 0; i < s.NumField(); i++ {
+		field := s.Field(i)
+		tag := field.Tag
+		yamlTag := strings.Split(tag.Get("yaml"), ",")[0]
+		lowerTag := strings.ToLower(yamlTag)
+		if yamlTag != lowerTag {
+			yamlTags[lowerTag] = yamlTag
+		}
+
+		fieldType := field.Type
+		switch fieldType.Kind() {
+		case reflect.Struct:
+			otherFields := yamlTagsFromStruct(fieldType)
+			for k, v := range otherFields {
+				yamlTags[k] = v
+			}
+		case reflect.Ptr:
+			fieldTypeElem := fieldType.Elem()
+			if fieldTypeElem.Kind() == reflect.Struct {
+				otherFields := yamlTagsFromStruct(fieldTypeElem)
+				for k, v := range otherFields {
+					yamlTags[k] = v
+				}
+			}
+		}
+	}
+
+	return yamlTags
+}
+
+func recursivelyCapitalizeConfigKeys(settings map[string]interface{}, yamlTags map[string]string) {
+	for key, val := range settings {
+		updatedKey := yamlTags[key]
+		if updatedKey != "" {
+			delete(settings, key)
+			settings[updatedKey] = val
+			if m, ok := val.(map[string]interface{}); ok {
+				recursivelyCapitalizeConfigKeys(m, yamlTags)
+			}
+		}
+	}
+}