support splunk.discovery mapping in properties.discovery.yaml

internal/confmapprovider/discovery/README.md

@@ -125,6 +125,25 @@ splunk.discovery.extensions.k8s_observer.enabled: false
 
 These properties can be in `config.d/properties.discovery.yaml` or specified at run time with `--set` command line options.
 
+The `config.d/properties.discovery.yaml` file supports specifying the property values directly as well within a mapped form:
+
+```yaml
+# --set form will take priority to mapped values
+splunk.discovery.receivers.prometheus_simple.config.labels::my_label: my_label_value
+splunk.discovery.receivers.prometheus_simple.enabled: true
+
+# mapped property form
+splunk.discovery:
+  extensions:
+    docker_observer:
+      enabled: false
+      config:
+        endpoint: tcp://localhost:54321
+  receivers:
+    prometheus_simple:
+      enabled: false # will be overwritten by above --set form (discovery is attempted for the receiver)
+```
+
 Each discovery property also has an equivalent environment variable form using `_x<hex pair>_` encoded delimiters for
 non-word characters `[^a-zA-Z0-9_]`:
 
@@ -142,9 +161,11 @@ SPLUNK_DISCOVERY_EXTENSIONS_docker_observer_CONFIG_endpoint="tcp://localhost:808
 SPLUNK_DISCOVERY_EXTENSIONS_k8s_observer_ENABLED=false
 ```
 
-The priority order for discovery config content from lowest to highest is:
+The priority order for discovery config values from lowest to highest is:
 
-1. `config.d/<receivers or extensions>/*.discovery.yaml` file content (lowest).
-2. `config.d/properties.discovery.yaml` file content.
-3. `SPLUNK_DISCOVERY_<xyz>` environment variables available to the collector process.
-4. `--set splunk.discovery.<xyz>` commandline options (highest).
+1. Pre-made `bundle.d` component config content (lowest).
+2. `config.d/<receivers or extensions>/*.discovery.yaml` component config file content.
+3. `config.d/properties.discovery.yaml` properties file mapped form content.
+4. `config.d/properties.discovery.yaml` properties file --set form content.
+5. `SPLUNK_DISCOVERY_<xyz>` property environment variables available to the collector process.
+6. `--set splunk.discovery.<xyz>` property commandline options (highest).

internal/confmapprovider/discovery/discoverer.go

@@ -44,6 +44,7 @@ import (
 
 	"github.com/signalfx/splunk-otel-collector/internal/common/discovery"
 	"github.com/signalfx/splunk-otel-collector/internal/components"
+	"github.com/signalfx/splunk-otel-collector/internal/confmapprovider/discovery/internal"
 	"github.com/signalfx/splunk-otel-collector/internal/confmapprovider/discovery/properties"
 	"github.com/signalfx/splunk-otel-collector/internal/receiver/discoveryreceiver"
 	"github.com/signalfx/splunk-otel-collector/internal/version"
@@ -401,7 +402,7 @@ func (d *discoverer) updateReceiverForObserver(receiverID component.ID, receiver
 	}
 	if hasObserverConfigBlock {
 		if hasDefault {
-			if err := mergeMaps(defaultConfig, observerConfigBlock); err != nil {
+			if err := internal.MergeMaps(defaultConfig, observerConfigBlock); err != nil {
 				return false, fmt.Errorf("failed merging %q config for %q: %w", receiverID, observerID, err)
 			}
 		} else {
@@ -709,19 +710,17 @@ func (d *discoverer) ConsumeLogs(_ context.Context, ld plog.Logs) error {
 // Priority is discovery.properties.yaml < env var properties < --set properties. --set and env var properties
 // are already resolved at this point.
 func (d *discoverer) mergeDiscoveryPropertiesEntry(cfg *Config) error {
-	props := map[string]any{}
-	for k, v := range cfg.DiscoveryProperties.Entry {
-		if prop, err := properties.NewProperty(k, fmt.Sprintf("%s", v)); err != nil {
-			d.logger.Warn(fmt.Sprintf("invalid discovery property %q", k), zap.Error(err))
-		} else {
-			mergeMaps(props, prop.ToStringMap())
-		}
+	conf, warning, fatal := properties.LoadConf(cfg.DiscoveryProperties.ToStringMap())
+	if fatal != nil {
+		return fatal
+	}
+	if warning != nil {
+		d.logger.Warn("invalid discovery properties will be disregarded", zap.Error(warning))
 	}
-	fileProps := confmap.NewFromStringMap(props)
-	if err := fileProps.Merge(d.propertiesConf); err != nil {
+	if err := conf.Merge(d.propertiesConf); err != nil {
 		return err
 	}
-	d.propertiesConf = fileProps
+	d.propertiesConf = conf
 	return nil
 }
 
@@ -738,15 +737,3 @@ func determineCurrentStatus(current, observed discovery.StatusType) discovery.St
 	}
 	return current
 }
-
-func mergeMaps(dst, src map[string]any) error {
-	dstMap := confmap.NewFromStringMap(dst)
-	srcMap := confmap.NewFromStringMap(src)
-	if err := dstMap.Merge(srcMap); err != nil {
-		return err
-	}
-	for k, v := range dstMap.ToStringMap() {
-		dst[k] = v
-	}
-	return nil
-}

internal/confmapprovider/discovery/discoverer_test.go

@@ -21,9 +21,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"go.opentelemetry.io/collector/confmap"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zaptest/observer"
 
@@ -69,81 +67,6 @@ func TestDiscovererDurationFromEnv(t *testing.T) {
 	}, 2*time.Second, time.Millisecond)
 }
 
-func TestMergeEntries(t *testing.T) {
-	a := confmap.NewFromStringMap(map[string]any{})
-	b := confmap.NewFromStringMap(map[string]any{})
-	a.Merge(b)
-
-	first := map[string]any{
-		"one.key": "one.val",
-		"two.key": "two.val",
-	}
-	second := map[string]any{
-		"three.key": "three.val",
-		"four.key": map[any]any{
-			"four.a.key": "four.a.val",
-			"four.b.key": "four.b.val",
-		},
-	}
-	err := mergeMaps(first, second)
-	assert.NoError(t, err)
-	require.Equal(t, map[string]any{
-		"one.key":   "one.val",
-		"two.key":   "two.val",
-		"three.key": "three.val",
-		"four.key": map[string]any{
-			"four.a.key": "four.a.val",
-			"four.b.key": "four.b.val",
-		},
-	}, first)
-
-	third := map[string]any{
-		"three.key": "three.val^",
-		"four.key": map[any]any{
-			"four.b.key": "four.b.val^",
-			"four.c.key": "four.c.val",
-		},
-	}
-	err = mergeMaps(first, third)
-	assert.NoError(t, err)
-	require.Equal(t, map[string]any{
-		"one.key":   "one.val",
-		"two.key":   "two.val",
-		"three.key": "three.val^",
-		"four.key": map[string]any{
-			"four.a.key": "four.a.val",
-			"four.b.key": "four.b.val^",
-			"four.c.key": "four.c.val",
-		},
-	}, first)
-
-	fourth := map[string]any{
-		"four.key": map[any]any{
-			"four.c.key": map[any]any{
-				"six.key": "six.val",
-			},
-			"four.d.key": "four.d.val",
-		},
-		"five.key": "five.val",
-	}
-	err = mergeMaps(first, fourth)
-	assert.NoError(t, err)
-	require.Equal(t, map[string]any{
-		"one.key":   "one.val",
-		"two.key":   "two.val",
-		"three.key": "three.val^",
-		"four.key": map[string]any{
-			"four.a.key": "four.a.val",
-			"four.b.key": "four.b.val^",
-			"four.c.key": map[string]any{
-				"six.key": "six.val",
-			},
-			"four.d.key": "four.d.val",
-		},
-		"five.key": "five.val",
-	}, first)
-}
-
 func TestDetermineCurrentStatus(t *testing.T) {
 	for _, test := range []struct {
 		current, observed, expected discovery.StatusType

internal/confmapprovider/discovery/internal/utils.go

@@ -0,0 +1,29 @@
+// Copyright Splunk, Inc.
+//
+// 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 internal
+
+import "go.opentelemetry.io/collector/confmap"
+
+func MergeMaps(dst, src map[string]any) error {
+	dstMap := confmap.NewFromStringMap(dst)
+	srcMap := confmap.NewFromStringMap(src)
+	if err := dstMap.Merge(srcMap); err != nil {
+		return err
+	}
+	for k, v := range dstMap.ToStringMap() {
+		dst[k] = v
+	}
+	return nil
+}

internal/confmapprovider/discovery/internal/utils_test.go

@@ -0,0 +1,98 @@
+// Copyright Splunk, Inc.
+//
+// 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 internal
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.opentelemetry.io/collector/confmap"
+)
+
+func TestMergeMaps(t *testing.T) {
+	a := confmap.NewFromStringMap(map[string]any{})
+	b := confmap.NewFromStringMap(map[string]any{})
+	a.Merge(b)
+
+	first := map[string]any{
+		"one.key": "one.val",
+		"two.key": "two.val",
+	}
+	second := map[string]any{
+		"three.key": "three.val",
+		"four.key": map[any]any{
+			"four.a.key": "four.a.val",
+			"four.b.key": "four.b.val",
+		},
+	}
+	err := MergeMaps(first, second)
+	assert.NoError(t, err)
+	require.Equal(t, map[string]any{
+		"one.key":   "one.val",
+		"two.key":   "two.val",
+		"three.key": "three.val",
+		"four.key": map[string]any{
+			"four.a.key": "four.a.val",
+			"four.b.key": "four.b.val",
+		},
+	}, first)
+
+	third := map[string]any{
+		"three.key": "three.val^",
+		"four.key": map[any]any{
+			"four.b.key": "four.b.val^",
+			"four.c.key": "four.c.val",
+		},
+	}
+	err = MergeMaps(first, third)
+	assert.NoError(t, err)
+	require.Equal(t, map[string]any{
+		"one.key":   "one.val",
+		"two.key":   "two.val",
+		"three.key": "three.val^",
+		"four.key": map[string]any{
+			"four.a.key": "four.a.val",
+			"four.b.key": "four.b.val^",
+			"four.c.key": "four.c.val",
+		},
+	}, first)
+
+	fourth := map[string]any{
+		"four.key": map[any]any{
+			"four.c.key": map[any]any{
+				"six.key": "six.val",
+			},
+			"four.d.key": "four.d.val",
+		},
+		"five.key": "five.val",
+	}
+	err = MergeMaps(first, fourth)
+	assert.NoError(t, err)
+	require.Equal(t, map[string]any{
+		"one.key":   "one.val",
+		"two.key":   "two.val",
+		"three.key": "three.val^",
+		"four.key": map[string]any{
+			"four.a.key": "four.a.val",
+			"four.b.key": "four.b.val^",
+			"four.c.key": map[string]any{
+				"six.key": "six.val",
+			},
+			"four.d.key": "four.d.val",
+		},
+		"five.key": "five.val",
+	}, first)
+}