[configoptional][WIP] Allow configoptional.Optional to wrap scalar values

.chloggen/configoptional-scalars-2.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: all
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Add support for marshaling and unmarshaling scalar values
+
+# One or more tracking issues or pull requests related to the change
+issues: [13421]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: This allows using fields like `Optional[int]`
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]

.chloggen/configoptional-scalars.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: pkg/confmap
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Add `WithScalarMarshaler` and `WithScalarUnmarshaler` options to support handling scalar values in confmap
+
+# One or more tracking issues or pull requests related to the change
+issues: [13421]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]

config/configopaque/maplist.go

@@ -4,12 +4,10 @@
 package configopaque // import "go.opentelemetry.io/collector/config/configopaque"
 
 import (
-	"cmp"
 	"fmt"
 	"iter"
 	"slices"
 
-	"go.opentelemetry.io/collector/confmap"
 	"go.opentelemetry.io/collector/confmap/xconfmap"
 )
 
@@ -28,27 +26,32 @@ type Pair struct {
 // Pairs are assumed to have distinct names. This is checked during config validation.
 type MapList []Pair
 
-var _ confmap.Unmarshaler = (*MapList)(nil)
+type MapFormat map[string]String
 
-// Unmarshal is called by the Collector when unmarshalling from a map.
-// When the input config is a slice, this will be skipped,
-// and mapstructure's default unmarshalling logic will be used.
-func (ml *MapList) Unmarshal(conf *confmap.Conf) error {
-	var m2 map[string]String
-	if err := conf.Unmarshal(&m2); err != nil {
-		return err
+var _ xconfmap.ConfigMigrator = MapFormat{}
+
+func (mm MapFormat) Migrate(cfg any) (bool, error) {
+	ml, ok := cfg.(*MapList)
+	if !ok {
+		return false, nil
+	}
+	if ml == nil {
+		return false, nil
+	}
+
+	for key, val := range mm {
+		ml.Set(key, val)
 	}
-	*ml = make(MapList, 0, len(m2))
-	for name, value := range m2 {
-		*ml = append(*ml, Pair{
-			Name:  name,
-			Value: value,
-		})
+
+	return true, nil
+}
+
+var _ xconfmap.MigrateableConfig = (*MapList)(nil)
+
+func (ml MapList) Migrations() []xconfmap.ConfigMigrator {
+	return []xconfmap.ConfigMigrator{
+		MapFormat{},
 	}
-	slices.SortFunc(*ml, func(p1, p2 Pair) int {
-		return cmp.Compare(p1.Name, p2.Name)
-	})
-	return nil
 }
 
 var _ xconfmap.Validator = MapList(nil)

config/configopaque/maplist_test.go

@@ -55,15 +55,15 @@ func TestMapListDuality(t *testing.T) {
 	conf1, err := retrieved1.AsConf()
 	require.NoError(t, err)
 	var tc1 testConfig
-	require.NoError(t, conf1.Unmarshal(&tc1))
+	require.NoError(t, conf1.Unmarshal(&tc1, xconfmap.WithScalarUnmarshaler()))
 	assert.NoError(t, xconfmap.Validate(&tc1))
 
 	retrieved2, err := confmap.NewRetrievedFromYAML([]byte(headersMap))
 	require.NoError(t, err)
 	conf2, err := retrieved2.AsConf()
 	require.NoError(t, err)
 	var tc2 testConfig
-	require.NoError(t, conf2.Unmarshal(&tc2))
+	require.NoError(t, conf2.Unmarshal(&tc2, xconfmap.WithScalarUnmarshaler()))
 	assert.NoError(t, xconfmap.Validate(&tc2))
 
 	assert.Equal(t, tc1, tc2)

config/configoptional/optional.go

@@ -36,6 +36,9 @@
 	// flavor indicates the flavor of the Optional.
 	// The zero value of flavor is noneFlavor.
 	flavor flavor
+
+	// Whether `enabled` was used
+	manuallySet bool
 }
 
 // deref a reflect.Type to its underlying type.
@@ -157,15 +160,18 @@
 	}
 
 	empty := confmap.NewFromStringMap(map[string]any{})
-	if err := empty.Unmarshal(o); err != nil {
+	if err := empty.Unmarshal(o, xconfmap.WithScalarUnmarshaler()); err != nil {
 		// This should never happen, if it happens it is a bug, so this panic is not documented.
 		panic(fmt.Errorf("failed to unmarshal empty map into %T type: %w. Please report this bug", o.value, err))
 	}
 
 	return o.Get()
 }
 
-var _ confmap.Unmarshaler = (*Optional[any])(nil)
+var (
+	// _ confmap.Unmarshaler        = (*Optional[any])(nil)
+	_ xconfmap.ScalarUnmarshaler = (*Optional[any])(nil)
+)
 
 var (
 	addEnabledFieldFeatureGateID = "configoptional.AddEnabledField"
@@ -193,41 +199,97 @@
 //
 // T must be derefenceable to a type with struct kind and not have an 'enabled' field.
 // Scalar values are not supported.
-func (o *Optional[T]) Unmarshal(conf *confmap.Conf) error {
-	if err := assertNoEnabledField[T](); err != nil {
-		return err
+// func (o *Optional[T]) Unmarshal(conf *confmap.Conf) error {
+// 	if err := assertNoEnabledField[T](); err != nil {
+// 		return err
+// 	}
+
+// 	if o.flavor == noneFlavor && conf.ToStringMap() == nil {
+// 		// If the Optional is None and the configuration is nil, we do nothing.
+// 		// This replicates the behavior of unmarshaling into a field with a nil pointer.
+// 		return nil
+// 	}
+
+// 	isEnabled := true
+// 	if addEnabledFieldFeatureGate.IsEnabled() && conf.IsSet("enabled") {
+// 		enabled := conf.Get("enabled")
+// 		conf.Delete("enabled")
+// 		var ok bool
+// 		if isEnabled, ok = enabled.(bool); !ok {
+// 			return fmt.Errorf("unexpected type %T for 'enabled': got '%v' value expected 'true' or 'false'", enabled, enabled)
+// 		}
+// 	}
+
+// 	if err := conf.Unmarshal(&o.value, xconfmap.WithScalarUnmarshaler()); err != nil {
+// 		return err
+// 	}
+
+// 	if isEnabled {
+// 		o.flavor = someFlavor
+// 	} else {
+// 		o.flavor = noneFlavor
+// 	}
+
+// 	return nil
+// }
+
+var _ xconfmap.ConfigMigrator = (*EnabledField[any])(nil)
+
+type EnabledField[T any] struct {
+	Enabled bool `mapstructure:"enabled"`
+}
+
+func (ef EnabledField[T]) Migrate(val any) (bool, error) {
+	o, ok := val.(*Optional[T])
+	if !ok {
+		return false, fmt.Errorf("expected Optional type but got %T", val)
 	}
 
-	if o.flavor == noneFlavor && conf.ToStringMap() == nil {
-		// If the Optional is None and the configuration is nil, we do nothing.
-		// This replicates the behavior of unmarshaling into a field with a nil pointer.
-		return nil
+	if ef.Enabled {
+		o.flavor = someFlavor
+	} else {
+		o.flavor = noneFlavor
 	}
 
-	isEnabled := true
-	if addEnabledFieldFeatureGate.IsEnabled() && conf.IsSet("enabled") {
-		enabled := conf.Get("enabled")
-		conf.Delete("enabled")
-		var ok bool
-		if isEnabled, ok = enabled.(bool); !ok {
-			return fmt.Errorf("unexpected type %T for 'enabled': got '%v' value expected 'true' or 'false'", enabled, enabled)
-		}
+	o.manuallySet = true
+
+	return true, nil
+}
+
+func (o *Optional[T]) UnmarshalScalar(val any) error {
+	if val == nil {
+		return nil
 	}
 
-	if err := conf.Unmarshal(&o.value); err != nil {
-		return err
+	v, ok := val.(T)
+	if !ok {
+		return fmt.Errorf("val is %T, not %T", val, v)
 	}
+	o.value = v
 
-	if isEnabled {
+	if !o.manuallySet {
 		o.flavor = someFlavor
-	} else {
-		o.flavor = noneFlavor
 	}
 
 	return nil
 }
 
-var _ confmap.Marshaler = (*Optional[any])(nil)
+func (o *Optional[T]) ScalarType() any {
+	return o.value
+}
+
+var _ xconfmap.MigrateableConfig = (*Optional[any])(nil)
+
+func (o *Optional[T]) Migrations() []xconfmap.ConfigMigrator {
+	return []xconfmap.ConfigMigrator{
+		EnabledField[T]{},
+	}
+}
+
+var (
+	_ confmap.Marshaler        = (*Optional[any])(nil)
+	_ xconfmap.ScalarMarshaler = (*Optional[any])(nil)
+)
 
 // Marshal the Optional value into the configuration.
 // If the Optional is None or Default, it does not marshal anything.
@@ -245,13 +307,21 @@
 		return conf.Marshal(map[string]any(nil))
 	}
 
-	if err := conf.Marshal(o.value); err != nil {
+	if err := conf.Marshal(o.value, xconfmap.WithScalarMarshaler()); err != nil {
 		return fmt.Errorf("configoptional: failed to marshal Optional value: %w", err)
 	}
 
 	return nil
 }
 
+func (o Optional[T]) GetScalarValue() (any, error) {
+	if o.flavor == noneFlavor || o.flavor == defaultFlavor {
+		return nil, nil
+	}
+
+	return o.value, nil
+}
+
 var _ xconfmap.Validator = (*Optional[any])(nil)
 
 // Validate implements [xconfmap.Validator]. This is required because the