[configoptional] programmatic usage is uneasy

[atoulme]

Component(s)

No response

Describe the issue you're reporting

The configoptional.Optional struct does not allow programmatic setting of values. This is what we have to use instead:

rf := otlpreceiver.NewFactory()
cfg := rf.CreateDefaultConfig().(*otlpreceiver.Config)
_ = cfg.GRPC.Unmarshal(confmap.NewFromStringMap(map[string]any{"endpoint": fmt.Sprintf("%s:%d", listeningAddress, grpcPort)}))
_ = cfg.HTTP.Unmarshal(confmap.NewFromStringMap(map[string]any{"endpoint": fmt.Sprintf("%s:%d", listeningAddress, httpPort)}))

The same problem applies if say we try to explicitly the value to nil.

I seek that we add to the configoptional.Optional struct methods that allow to set values, including the case where the value is explicitly nil.

Tip

_{React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it. Learn more here.}

[mx-psi]

The configoptional.Optional struct does not allow programmatic setting of values. This is what we have to use instead: [...]

We have the following function in tests for that purpose:

opentelemetry-collector/receiver/otlpreceiver/config_test.go

Lines 28 to 38 in cb24802

	func GetOrInsertDefault[T any](t *testing.T, opt *configoptional.Optional[T]) *T {
	if opt.HasValue() {
	return opt.Get()
	}
	empty := confmap.NewFromStringMap(map[string]any{})
	require.NoError(t, empty.Unmarshal(opt))
	val := opt.Get()
	require.NotNil(t, "Expected a default value to be set for %T", val)
	return val
	}

.
Example usage in tests very similar to the one you are using:

opentelemetry-collector/receiver/otlpreceiver/factory_test.go

Lines 42 to 44 in cb24802

	cfg := factory.CreateDefaultConfig().(*Config)
	GetOrInsertDefault(t, &cfg.GRPC).NetAddr.Endpoint = testutil.GetAvailableLocalAddress(t)
	GetOrInsertDefault(t, &cfg.HTTP).ServerConfig.Endpoint = testutil.GetAvailableLocalAddress(t)

The same problem applies if say we try to explicitly the value to nil.

I don't understand that, why can't you just set it to configoptional.None()?

I seek that we add to the configoptional.Optional struct methods that allow to set values, including the case where the value is explicitly nil.

I decided to keep GetOrInsertDefault out from the API after comments from @yurishkuro, but it would be easy to add it if we want to. Would that solve your problem?

[atoulme]

I don't understand that, why can't you just set it to configoptional.None()?

that didn't occur to me :)

you're saying this would work:

cfg.GRPC = configoptional.None()

OK that'd work for my use case.

It's a bit weird to have this be a method without receiver?

Can we use:

cfg.GRPC.Get().NetAddr.Endpoint = testutil.GetAvailableLocalAddress(t) 

seems more idiomatic. Get() would indeed set the default as the local value if unset.

[mx-psi]

I don't understand that, why can't you just set it to configoptional.None()?

that didn't occur to me :)

you're saying this would work:

cfg.GRPC = configoptional.None()

OK that'd work for my use case.

Yep! That should work

It's a bit weird to have this be a method without receiver?

Can we use:

cfg.GRPC.Get().NetAddr.Endpoint = testutil.GetAvailableLocalAddress(t) 

seems more idiomatic. Get() would indeed set the default as the local value if unset.

Yeah originally we had

cfg.GRPC.GetOrInsertDefault().NetAddr.Endpoint = testutil.GetAvailableLocalAddress(t) 

That would be the thing I would propose adding for solving this problem.

One further questions: do you have a use case for this outside of tests?

[atoulme]

seems more idiomatic. Get() would indeed set the default as the local value if unset.

Yeah originally we had

cfg.GRPC.GetOrInsertDefault().NetAddr.Endpoint = testutil.GetAvailableLocalAddress(t)
That would be the thing I would propose adding for solving this problem.

Perfect.

One further questions: do you have a use case for this outside of tests?

Yes. In my custom application here we load configuration based on direct program input, there is no configuration file.

[mx-psi]

#13859 adds this method and has a couple of examples. PTAL