Skip to content

Commit 901c395

Browse files
evan-bradleymx-psi
andauthored
[chore] Create RFC for Optional config types (open-telemetry#12596)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This RFC will help us explore the use cases Optional types solve and move us toward a resolution for whether to adopt these in our config. Related to open-telemetry#10266 --------- Co-authored-by: Pablo Baeyens <[email protected]> Co-authored-by: Pablo Baeyens <[email protected]>
1 parent bac08e2 commit 901c395

File tree

2 files changed

+278
-0
lines changed

2 files changed

+278
-0
lines changed

.github/workflows/utils/cspell.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"Andrzej",
77
"Anoshin",
88
"Appy",
9+
"atombender",
910
"Backpressure",
1011
"Baeyens",
1112
"Bebbington",

docs/rfcs/optional-config-type.md

Lines changed: 277 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,277 @@
1+
# Optional[T] type for use in configuration structs
2+
3+
## Overview
4+
5+
In Go, types are by default set to a "zero-value", a supported value for the
6+
respective type that is semantically equivalent or similar to "empty", but which
7+
is also a valid value for the type. For many config fields, the zero value is
8+
not a valid configuration value and can be taken to mean that the option is
9+
disabled, but in certain cases it can indicate a default value, necessitating a
10+
way to represent the presence of a value without using a valid value for the
11+
type.
12+
13+
Using standard Go without inventing any new types, the two most straightforward
14+
ways to accomplish this are:
15+
16+
1. Using a separate boolean field to indicate whether the field is enabled or
17+
disabled.
18+
2. Making the type a pointer, which makes a `nil` pointer represent that a
19+
value is not present, and a valid pointer represent the desired value.
20+
21+
Each of these approaches has deficiencies: Using a separate boolean field
22+
requires the user to set the boolean field to `true` in addition to setting the
23+
actual config option, leading to suboptimal UX. Using a pointer value has a few
24+
drawbacks:
25+
26+
1. It may not be immediately obvious to a new user that a pointer type indicates
27+
a field is optional.
28+
2. The distinction between values that are conventionally pointers (e.g. gRPC
29+
configs) and optional values is lost.
30+
3. Setting a default value for a pointer field when decoding will set the field
31+
on the resulting config struct, and additional logic must be done to unset
32+
the default if the user has not specified a value.
33+
4. The potential for null pointer exceptions is created.
34+
5. Config structs are generally intended to be immutable and may be passed
35+
around a lot, which makes the mutability property of pointer fields
36+
an undesirable property.
37+
38+
## Optional types
39+
40+
Go does not include any form of Optional type in the standard library, but other
41+
popular languages like Rust and Java do. We could implement something similar in
42+
our config packages that allows us to address the downsides of using pointers
43+
for optional config fields.
44+
45+
## Basic definition
46+
47+
A production-grade implementation will not be discussed or shown here, but the
48+
basic implementation for an Optional type in Go could look something like:
49+
50+
```golang
51+
type Optional[T any] struct {
52+
hasValue bool
53+
value T
54+
55+
defaultVal T
56+
}
57+
58+
func Some[T any](value T) Optional[T] {
59+
return Optional[T]{value: value, hasValue: true}
60+
}
61+
62+
func None[T any](value T) Optional[T] {
63+
return Optional[T]{}
64+
}
65+
66+
func WithDefault[T any](defaultVal T) Optional[T] {
67+
return Optional[T]{defaultVal: defaultVal}
68+
}
69+
70+
func (o Optional[T]) HasValue() bool {
71+
return o.hasValue
72+
}
73+
74+
func (o Optional[T]) Value() T {
75+
return o.value
76+
}
77+
78+
func (o Optional[T]) Default() T {
79+
return o.defaultVal
80+
}
81+
```
82+
83+
## Use cases
84+
85+
Optional types can fulfill the following use cases we have when decoding config.
86+
87+
### Representing optional config fields
88+
89+
To use the optional type to mark a config field as optional, the type can simply be used as a type
90+
parameter to `Optional[T]`. The YAML representation of `Optional[T]` is the same as that of `T`,
91+
except that the type records whether the value is present. The following config struct shows how
92+
this may look, both in definition and in usage:
93+
94+
```golang
95+
type Protocols struct {
96+
GRPC Optional[configgrpc.ServerConfig] `mapstructure:"grpc"`
97+
HTTP Optional[HTTPConfig] `mapstructure:"http"`
98+
}
99+
100+
func (cfg *Config) Validate() error {
101+
if !cfg.GRPC.HasValue() && !cfg.HTTP.HasValue() {
102+
return errors.New("must specify at least one protocol when using the OTLP receiver")
103+
}
104+
return nil
105+
}
106+
107+
func createDefaultConfig() component.Config {
108+
return &Config{
109+
Protocols: Protocols{
110+
GRPC: WithDefault(configgrpc.ServerConfig{
111+
// ...
112+
}),
113+
HTTP: WithDefault(HTTPConfig{
114+
// ...
115+
}),
116+
},
117+
}
118+
}
119+
```
120+
121+
For something like `confighttp.ServerConfig`, using an `Optional[T]` type for
122+
optional fields would look like this:
123+
124+
```golang
125+
type ServerConfig struct {
126+
TLSSetting Optional[configtls.ServerConfig] `mapstructure:"tls"`
127+
128+
CORS Optional[CORSConfig] `mapstructure:"cors"`
129+
130+
Auth Optional[AuthConfig] `mapstructure:"auth,omitempty"`
131+
132+
ResponseHeaders Optional[map[string]configopaque.String] `mapstructure:"response_headers"`
133+
}
134+
135+
func NewDefaultServerConfig() ServerConfig {
136+
return ServerConfig{
137+
TLSSetting: WithDefault(configtls.NewDefaultServerConfig()),
138+
CORS: WithDefault(NewDefaultCORSConfig()),
139+
WriteTimeout: 30 * time.Second,
140+
ReadHeaderTimeout: 1 * time.Minute,
141+
IdleTimeout: 1 * time.Minute,
142+
}
143+
}
144+
145+
func (hss *ServerConfig) ToListener(ctx context.Context) (net.Listener, error) {
146+
listener, err := net.Listen("tcp", hss.Endpoint)
147+
if err != nil {
148+
return nil, err
149+
}
150+
151+
if hss.TLSSetting.HasValue() {
152+
var tlsCfg *tls.Config
153+
tlsCfg, err = hss.TLSSetting.Value().LoadTLSConfig(ctx)
154+
if err != nil {
155+
return nil, err
156+
}
157+
tlsCfg.NextProtos = []string{http2.NextProtoTLS, "http/1.1"}
158+
listener = tls.NewListener(listener, tlsCfg)
159+
}
160+
161+
return listener, nil
162+
}
163+
164+
func (hss *ServerConfig) ToServer(_ context.Context, host component.Host, settings component.TelemetrySettings, handler http.Handler, opts ...ToServerOption) (*http.Server, error) {
165+
// ...
166+
167+
handler = httpContentDecompressor(
168+
handler,
169+
hss.MaxRequestBodySize,
170+
serverOpts.ErrHandler,
171+
hss.CompressionAlgorithms,
172+
serverOpts.Decoders,
173+
)
174+
175+
// ...
176+
177+
if hss.Auth.HasValue() {
178+
server, err := hss.Auth.Value().GetServerAuthenticator(context.Background(), host.GetExtensions())
179+
if err != nil {
180+
return nil, err
181+
}
182+
183+
handler = authInterceptor(handler, server, hss.Auth.Value().RequestParameters)
184+
}
185+
186+
corsValue := hss.CORS.Value()
187+
if hss.CORS.HasValue() && len(hss.CORS.AllowedOrigins) > 0 {
188+
co := cors.Options{
189+
AllowedOrigins: corsValue.AllowedOrigins,
190+
AllowCredentials: true,
191+
AllowedHeaders: corsValue.AllowedHeaders,
192+
MaxAge: corsValue.MaxAge,
193+
}
194+
handler = cors.New(co).Handler(handler)
195+
}
196+
if hss.CORS.HasValue() && len(hss.CORS.AllowedOrigins) == 0 && len(hss.CORS.AllowedHeaders) > 0 {
197+
settings.Logger.Warn("The CORS configuration specifies allowed headers but no allowed origins, and is therefore ignored.")
198+
}
199+
200+
if hss.ResponseHeaders.HasValue() {
201+
handler = responseHeadersHandler(handler, hss.ResponseHeaders.Value())
202+
}
203+
204+
// ...
205+
}
206+
```
207+
208+
### Proper unmarshaling of empty values when a default is set
209+
210+
Currently, the OTLP receiver requires a workaround to make enabling each
211+
protocol optional while providing defaults if a key for the protocol has been
212+
set:
213+
214+
```golang
215+
type Protocols struct {
216+
GRPC *configgrpc.ServerConfig `mapstructure:"grpc"`
217+
HTTP *HTTPConfig `mapstructure:"http"`
218+
}
219+
220+
// Config defines configuration for OTLP receiver.
221+
type Config struct {
222+
// Protocols is the configuration for the supported protocols, currently gRPC and HTTP (Proto and JSON).
223+
Protocols `mapstructure:"protocols"`
224+
}
225+
226+
func createDefaultConfig() component.Config {
227+
return &Config{
228+
Protocols: Protocols{
229+
GRPC: configgrpc.NewDefaultServerConfig(),
230+
HTTP: &HTTPConfig{
231+
// ...
232+
},
233+
},
234+
}
235+
}
236+
237+
func (cfg *Config) Unmarshal(conf *confmap.Conf) error {
238+
// first load the config normally
239+
err := conf.Unmarshal(cfg)
240+
if err != nil {
241+
return err
242+
}
243+
244+
// gRPC will be enabled if this line is not run
245+
if !conf.IsSet(protoGRPC) {
246+
cfg.GRPC = nil
247+
}
248+
249+
// HTTP will be enabled if this line is not run
250+
if !conf.IsSet(protoHTTP) {
251+
cfg.HTTP = nil
252+
}
253+
254+
return nil
255+
}
256+
```
257+
258+
With an Optional type, the checks in `Unmarshal` become unnecessary, and it's
259+
possible the entire `Unmarshal` function may no longer be needed. Instead, when
260+
the config is unmarshaled, no value would be put into the default Optional
261+
values and `HasValue` would return false when using this config object.
262+
263+
This situation is something of an edge case with our current unmarshaling
264+
facilities and while not common, could be a rough edge for users looking to
265+
implement similar config structures.
266+
267+
## Disadvantages of an Optional type
268+
269+
There is one noteworthy disadvantage of introducing an Optional type:
270+
271+
1. Since the type isn't standard, external packages working with config may
272+
require additional adaptations to work with our config structs. For example,
273+
if we wanted to generate our types from a JSON schema using a package like
274+
[github.com/atombender/go-jsonschema][go-jsonschema], we would need some way
275+
to ensure compatibility with an Optional type.
276+
277+
[go-jsonschema]: https://github.com/omissis/go-jsonschema

0 commit comments

Comments
 (0)