Skip to content

Commit 8704c5e

Browse files
mx-psimx-psi
committed
[PoC] Show how the factory approach would work
1 parent aee7405 commit 8704c5e

File tree

7 files changed

+106
-84
lines changed

7 files changed

+106
-84
lines changed

‎config/configoptional/optional.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ type Optional[T any] struct {
1313
hasValue bool
1414
value T
1515

16-
defaultFn DefaultFunc[T]
16+
defaultFn *DefaultFunc[T]
1717
}
1818

1919
type DefaultFunc[T any] func() T
@@ -30,9 +30,19 @@ func None[T any]() Optional[T] {
3030
return Optional[T]{}
3131
}
3232

33-
// WithDefault creates an Optional which has no value
33+
type Factory[T any] struct {
34+
defaultFn *DefaultFunc[T]
35+
}
36+
37+
// NewFactory creates a new Factory with the given default function.
38+
// Factories should be package variables
39+
func NewFactory[T any](defaultFn DefaultFunc[T]) Factory[T] {
40+
return Factory[T]{defaultFn: &defaultFn}
41+
}
42+
43+
// WithFactory creates an Optional which has no value
3444
// unless user config provides some, in which case
35-
// the defaultFn is used to create the initial value,
45+
// the factory is used to create the initial value,
3646
// which may be overridden by the user provided config.
3747
//
3848
// The reason we pass a function instead of T directly
@@ -41,8 +51,8 @@ func None[T any]() Optional[T] {
4151
// since it might reuse (and override) some shared state.
4252
//
4353
// On unmarshal, the defaultFn is removed.
44-
func WithDefault[T any](defaultFn DefaultFunc[T]) Optional[T] {
45-
return Optional[T]{defaultFn: defaultFn}
54+
func WithFactory[T any](factory Factory[T]) Optional[T] {
55+
return Optional[T]{defaultFn: factory.defaultFn}
4656
}
4757

4858
func (o Optional[T]) HasValue() bool {
@@ -53,13 +63,9 @@ func (o Optional[T]) Value() T {
5363
return o.value
5464
}
5565

56-
func (o Optional[T]) Ref() *T {
57-
return &o.value
58-
}
59-
6066
func (o *Optional[T]) Unmarshal(conf *confmap.Conf) error {
6167
if o.defaultFn != nil {
62-
o.value = o.defaultFn()
68+
o.value = (*o.defaultFn)()
6369
o.hasValue = true
6470
o.defaultFn = nil
6571
}

‎config/configoptional/optional_test.go

Lines changed: 47 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ type Sub struct {
1919
Foo string `mapstructure:"foo"`
2020
}
2121

22+
var subFactory = NewFactory(func() Sub {
23+
return Sub{
24+
Foo: "foobar",
25+
}
26+
})
27+
2228
func TestOptional(t *testing.T) {
2329
tests := []struct {
2430
name string
@@ -27,54 +33,46 @@ func TestOptional(t *testing.T) {
2733
expectedSub bool
2834
expectedFoo string
2935
}{
30-
// {
31-
// name: "no_default_no_config",
32-
// defaultCfg: Config{
33-
// Sub1: None[Sub](),
34-
// },
35-
// expectedSub: false,
36-
// },
37-
// {
38-
// name: "no_default_with_config",
39-
// config: map[string]any{
40-
// "sub": map[string]any{
41-
// "foo": "bar",
42-
// },
43-
// },
44-
// defaultCfg: Config{
45-
// Sub1: None[Sub](),
46-
// },
47-
// expectedSub: true,
48-
// expectedFoo: "bar",
49-
// },
50-
// {
51-
// name: "with_default_no_config",
52-
// defaultCfg: Config{
53-
// Sub1: WithDefault(func() Sub {
54-
// return Sub{
55-
// Foo: "foobar",
56-
// }
57-
// }),
58-
// },
59-
// expectedSub: false,
60-
// },
61-
// {
62-
// name: "with_default_with_config",
63-
// config: map[string]any{
64-
// "sub": map[string]any{
65-
// "foo": "bar",
66-
// },
67-
// },
68-
// defaultCfg: Config{
69-
// Sub1: WithDefault(func() Sub {
70-
// return Sub{
71-
// Foo: "foobar",
72-
// }
73-
// }),
74-
// },
75-
// expectedSub: true,
76-
// expectedFoo: "bar", // input overrides default
77-
// },
36+
{
37+
name: "no_default_no_config",
38+
defaultCfg: Config{
39+
Sub1: None[Sub](),
40+
},
41+
expectedSub: false,
42+
},
43+
{
44+
name: "no_default_with_config",
45+
config: map[string]any{
46+
"sub": map[string]any{
47+
"foo": "bar",
48+
},
49+
},
50+
defaultCfg: Config{
51+
Sub1: None[Sub](),
52+
},
53+
expectedSub: true,
54+
expectedFoo: "bar",
55+
},
56+
{
57+
name: "with_default_no_config",
58+
defaultCfg: Config{
59+
Sub1: WithFactory(subFactory),
60+
},
61+
expectedSub: false,
62+
},
63+
{
64+
name: "with_default_with_config",
65+
config: map[string]any{
66+
"sub": map[string]any{
67+
"foo": "bar",
68+
},
69+
},
70+
defaultCfg: Config{
71+
Sub1: WithFactory(subFactory),
72+
},
73+
expectedSub: true,
74+
expectedFoo: "bar", // input overrides default
75+
},
7876
{
7977
// this test fails, because "sub:" is considered null value by mapstructure
8078
// and no additional processing happens for it, including custom unmarshaler.
@@ -84,11 +82,7 @@ func TestOptional(t *testing.T) {
8482
"sub": nil,
8583
},
8684
defaultCfg: Config{
87-
Sub1: WithDefault(func() Sub {
88-
return Sub{
89-
Foo: "foobar",
90-
}
91-
}),
85+
Sub1: WithFactory(subFactory),
9286
},
9387
expectedSub: true,
9488
expectedFoo: "foobar", // default applies

‎receiver/otlpreceiver/config_test.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ func TestUnmarshalDefaultConfig(t *testing.T) {
3030
factory := NewFactory()
3131
cfg := factory.CreateDefaultConfig()
3232
require.NoError(t, cm.Unmarshal(&cfg))
33-
assert.Equal(t, factory.CreateDefaultConfig(), cfg)
33+
defaultCfg := factory.CreateDefaultConfig()
34+
defaultCfg.(*Config).GRPC = configoptional.Some(defaultGRPCServerConfig())
35+
defaultCfg.(*Config).HTTP = configoptional.Some(defaultHTTPConfig())
36+
assert.Equal(t, defaultCfg, cfg)
3437
}
3538

3639
func TestUnmarshalConfigOnlyGRPC(t *testing.T) {
@@ -41,7 +44,7 @@ func TestUnmarshalConfigOnlyGRPC(t *testing.T) {
4144
require.NoError(t, cm.Unmarshal(&cfg))
4245

4346
defaultOnlyGRPC := factory.CreateDefaultConfig().(*Config)
44-
defaultOnlyGRPC.HTTP = configoptional.None[HTTPConfig]()
47+
defaultOnlyGRPC.GRPC = configoptional.Some(defaultGRPCServerConfig())
4548
assert.Equal(t, defaultOnlyGRPC, cfg)
4649
}
4750

@@ -53,7 +56,7 @@ func TestUnmarshalConfigOnlyHTTP(t *testing.T) {
5356
require.NoError(t, cm.Unmarshal(&cfg))
5457

5558
defaultOnlyHTTP := factory.CreateDefaultConfig().(*Config)
56-
defaultOnlyHTTP.GRPC = configoptional.None[configgrpc.ServerConfig]()
59+
defaultOnlyHTTP.HTTP = configoptional.Some(defaultHTTPConfig())
5760
assert.Equal(t, defaultOnlyHTTP, cfg)
5861
}
5962

@@ -65,7 +68,7 @@ func TestUnmarshalConfigOnlyHTTPNull(t *testing.T) {
6568
require.NoError(t, cm.Unmarshal(&cfg))
6669

6770
defaultOnlyHTTP := factory.CreateDefaultConfig().(*Config)
68-
defaultOnlyHTTP.GRPC = configoptional.None[configgrpc.ServerConfig]()
71+
defaultOnlyHTTP.HTTP = configoptional.Some(defaultHTTPConfig())
6972
assert.Equal(t, defaultOnlyHTTP, cfg)
7073
}
7174

@@ -78,7 +81,6 @@ func TestUnmarshalConfigOnlyHTTPEmptyMap(t *testing.T) {
7881

7982
defaultOnlyHTTP := factory.CreateDefaultConfig().(*Config)
8083
defaultOnlyHTTP.HTTP = configoptional.Some(defaultHTTPConfig())
81-
defaultOnlyHTTP.GRPC = configoptional.WithDefault(defaultGRPCServerConfig)
8284
assert.Equal(t, defaultOnlyHTTP.GRPC, cfg.(*Config).GRPC)
8385
}
8486

‎receiver/otlpreceiver/factory.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,15 @@ func defaultGRPCServerConfig() configgrpc.ServerConfig {
6464
return *grpcCfg
6565
}
6666

67+
var httpConfigFactory = configoptional.NewFactory(defaultHTTPConfig)
68+
var grpcConfigFactory = configoptional.NewFactory(defaultGRPCServerConfig)
69+
6770
// createDefaultConfig creates the default configuration for receiver.
6871
func createDefaultConfig() component.Config {
6972
return &Config{
7073
Protocols: Protocols{
71-
GRPC: configoptional.WithDefault(defaultGRPCServerConfig),
72-
HTTP: configoptional.WithDefault(defaultHTTPConfig),
74+
GRPC: configoptional.WithFactory(grpcConfigFactory),
75+
HTTP: configoptional.WithFactory(httpConfigFactory),
7376
},
7477
}
7578
}

‎receiver/otlpreceiver/factory_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,13 @@ func TestCreateDefaultConfig(t *testing.T) {
4040
func TestCreateSameReceiver(t *testing.T) {
4141
factory := NewFactory()
4242
cfg := factory.CreateDefaultConfig().(*Config)
43-
cfg.GRPC.Ref().NetAddr.Endpoint = testutil.GetAvailableLocalAddress(t)
44-
cfg.HTTP.Ref().ServerConfig.Endpoint = testutil.GetAvailableLocalAddress(t)
43+
grpcCfg := defaultGRPCServerConfig()
44+
grpcCfg.NetAddr.Endpoint = testutil.GetAvailableLocalAddress(t)
45+
cfg.GRPC = configoptional.Some(grpcCfg)
46+
47+
httpCfg := defaultHTTPConfig()
48+
httpCfg.ServerConfig.Endpoint = testutil.GetAvailableLocalAddress(t)
49+
cfg.HTTP = configoptional.Some(httpCfg)
4550

4651
core, observer := observer.New(zapcore.DebugLevel)
4752
attrs := attribute.NewSet(

‎receiver/otlpreceiver/otlp.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ func (r *otlpReceiver) startGRPCServer(host component.Host) error {
9393
}
9494

9595
var err error
96-
if r.serverGRPC, err = r.cfg.GRPC.Ref().ToServer(context.Background(), host, r.settings.TelemetrySettings); err != nil {
96+
grpcServerConfig := r.cfg.GRPC.Value()
97+
if r.serverGRPC, err = grpcServerConfig.ToServer(context.Background(), host, r.settings.TelemetrySettings); err != nil {
9798
return err
9899
}
99100

@@ -115,7 +116,7 @@ func (r *otlpReceiver) startGRPCServer(host component.Host) error {
115116

116117
r.settings.Logger.Info("Starting GRPC server", zap.String("endpoint", r.cfg.GRPC.Value().NetAddr.Endpoint))
117118
var gln net.Listener
118-
if gln, err = r.cfg.GRPC.Ref().NetAddr.Listen(context.Background()); err != nil {
119+
if gln, err = grpcServerConfig.NetAddr.Listen(context.Background()); err != nil {
119120
return err
120121
}
121122

@@ -166,13 +167,14 @@ func (r *otlpReceiver) startHTTPServer(ctx context.Context, host component.Host)
166167
}
167168

168169
var err error
169-
if r.serverHTTP, err = r.cfg.HTTP.Ref().ServerConfig.ToServer(ctx, host, r.settings.TelemetrySettings, httpMux, confighttp.WithErrorHandler(errorHandler)); err != nil {
170+
httpConfig := r.cfg.HTTP.Value()
171+
if r.serverHTTP, err = httpConfig.ServerConfig.ToServer(ctx, host, r.settings.TelemetrySettings, httpMux, confighttp.WithErrorHandler(errorHandler)); err != nil {
170172
return err
171173
}
172174

173-
r.settings.Logger.Info("Starting HTTP server", zap.String("endpoint", r.cfg.HTTP.Value().ServerConfig.Endpoint))
175+
r.settings.Logger.Info("Starting HTTP server", zap.String("endpoint", httpConfig.ServerConfig.Endpoint))
174176
var hln net.Listener
175-
if hln, err = r.cfg.HTTP.Ref().ServerConfig.ToListener(ctx); err != nil {
177+
if hln, err = httpConfig.ServerConfig.ToListener(ctx); err != nil {
176178
return err
177179
}
178180

‎receiver/otlpreceiver/otlp_test.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -696,8 +696,7 @@ func TestGRPCInvalidTLSCredentials(t *testing.T) {
696696
},
697697
TLSSetting: &configtls.ServerConfig{
698698
Config: configtls.Config{
699-
CertFile: "/path/to/valid/cert.pem",
700-
KeyFile: "/path/to/valid/key.pem",
699+
CertFile: "willfail",
701700
},
702701
},
703702
}),
@@ -722,8 +721,9 @@ func TestGRPCMaxRecvSize(t *testing.T) {
722721
sink := newErrOrSinkConsumer()
723722

724723
cfg := createDefaultConfig().(*Config)
725-
cfg.GRPC.Ref().NetAddr.Endpoint = addr
726-
cfg.HTTP = configoptional.None[HTTPConfig]()
724+
grpcCfg := defaultGRPCServerConfig()
725+
grpcCfg.NetAddr.Endpoint = addr
726+
cfg.GRPC = configoptional.Some(grpcCfg)
727727
recv := newReceiver(t, componenttest.NewNopTelemetrySettings(), cfg, otlpReceiverID, sink)
728728
require.NoError(t, recv.Start(context.Background(), componenttest.NewNopHost()))
729729

@@ -736,7 +736,10 @@ func TestGRPCMaxRecvSize(t *testing.T) {
736736
assert.NoError(t, cc.Close())
737737
require.NoError(t, recv.Shutdown(context.Background()))
738738

739-
cfg.GRPC.Ref().MaxRecvMsgSizeMiB = 1
739+
grpcCfg = defaultGRPCServerConfig()
740+
grpcCfg.NetAddr.Endpoint = addr
741+
grpcCfg.MaxRecvMsgSizeMiB = 100
742+
cfg.GRPC = configoptional.Some(grpcCfg)
740743

741744
recv = newReceiver(t, componenttest.NewNopTelemetrySettings(), cfg, otlpReceiverID, sink)
742745
require.NoError(t, recv.Start(context.Background(), componenttest.NewNopHost()))
@@ -1021,8 +1024,15 @@ func TestShutdown(t *testing.T) {
10211024
// Create OTLP receiver with gRPC and HTTP protocols.
10221025
factory := NewFactory()
10231026
cfg := factory.CreateDefaultConfig().(*Config)
1024-
cfg.GRPC.Ref().NetAddr.Endpoint = endpointGrpc
1025-
cfg.HTTP.Ref().ServerConfig.Endpoint = endpointHTTP
1027+
1028+
grpcCfg := defaultGRPCServerConfig()
1029+
grpcCfg.NetAddr.Endpoint = endpointGrpc
1030+
cfg.GRPC = configoptional.Some(grpcCfg)
1031+
1032+
httpCfg := defaultHTTPConfig()
1033+
httpCfg.ServerConfig.Endpoint = endpointHTTP
1034+
cfg.HTTP = configoptional.Some(httpCfg)
1035+
10261036
set := receivertest.NewNopSettings(metadata.Type)
10271037
set.ID = otlpReceiverID
10281038
r, err := NewFactory().CreateTraces(

0 commit comments

Comments
 (0)