-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[chore] Create RFC for Optional config types #12596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
mx-psi
merged 8 commits into
open-telemetry:main
from
evan-bradley:rfc-optional-config-types
Apr 21, 2025
Merged
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
4afe23f
[chore] Create RFC for Optional config types
evan-bradley 295b93b
Update docs/rfcs/optional-config-type.md
evan-bradley 806f3ba
Address feedback
evan-bradley 94e8a78
Update docs/rfcs/optional-config-type.md
evan-bradley 640ec50
Merge branch 'main' into rfc-optional-config-types
evan-bradley 9ee9aad
Add word to spellchecking list
mx-psi 5d5f652
Clarify that there is no impact in YAML representation besides record…
mx-psi 939fa0b
Merge branch 'main' into rfc-optional-config-types
mx-psi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,184 @@ | ||
# Optional[T] type for use in configuration structs | ||
|
||
## Overview | ||
|
||
In Go, types are by default set to a "zero-value", a supported value for the | ||
respective type that is semantically equivalent or similar to "empty", but which | ||
is also a valid value for the type. For many config fields, the zero value is | ||
not a valid configuration value and can be taken to mean that the option is | ||
disabled, but in certain cases it can indicate a default value, necessitating a | ||
way to represent the presence of a value without using a valid value for the | ||
type. | ||
|
||
Using standard Go without inventing any new types, the two most straightforward | ||
ways to accomplish this are: | ||
|
||
1. Using a separate boolean field to indicate whether the field is enabled or | ||
disabled. | ||
2. Making the type a pointer, which makes a `nil` pointer represent that a | ||
value is not present, and a valid pointer represent the desired value. | ||
|
||
Each of these approaches has deficiencies: Using a separate boolean field | ||
requires the user to set the boolean field to `true` in addition to setting the | ||
actual config option, leading to suboptimal UX. Using a pointer value has a few | ||
drawbacks: | ||
|
||
1. It may not be immediately obvious to a new user that a pointer type indicates | ||
a field is optional. | ||
2. The distinction between values that are conventionally pointers (e.g. gRPC | ||
configs) and optional values is lost. | ||
3. Setting a default value for a pointer field when decoding will set the field | ||
on the resulting config struct, and additional logic must be done to unset | ||
the default if the user has not specified a value. | ||
4. The potential for null pointer exceptions is created. | ||
|
||
## Optional types | ||
|
||
Go does not include any form of Optional type in the standard library, but other | ||
popular languages like Rust and Java do. We could implement something similar in | ||
our config packages that allows us to address the downsides of using pointers | ||
for optional config fields. | ||
|
||
## Basic definition | ||
|
||
A production-grade implementation will not be discussed or shown here, but the | ||
basic implementation for an Optional type in Go could look something like: | ||
|
||
```golang | ||
type Optional[T any] struct { | ||
hasValue bool | ||
value T | ||
|
||
defaultVal T | ||
} | ||
|
||
func Some[T any](value T) Optional[T] { | ||
return Optional[T]{value: value, hasValue: true} | ||
} | ||
|
||
func None[T any](value T) Optional[T] { | ||
return Optional[T]{} | ||
} | ||
|
||
func WithDefault[T any](defaultVal T) Optional[T] { | ||
evan-bradley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return Optional[T]{defaultVal: defaultVal} | ||
} | ||
|
||
func (o Optional[T]) HasValue() bool { | ||
return o.hasValue | ||
} | ||
|
||
func (o Optional[T]) Value() T { | ||
return o.value | ||
} | ||
|
||
func (o Optional[T]) Default() T { | ||
return o.defaultVal | ||
} | ||
``` | ||
|
||
## Use cases | ||
|
||
Optional types can fulfill the following use cases we have when decoding config. | ||
|
||
### Representing optional config fields | ||
evan-bradley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
To use the optional type to mark a config field as optional, the type can simply | ||
be used as a type parameter to `Optional[T]`. The following config struct shows | ||
how this may look, both in definition and in usage: | ||
|
||
```golang | ||
mx-psi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
type Protocols struct { | ||
GRPC Optional[configgrpc.ServerConfig] `mapstructure:"grpc"` | ||
HTTP Optional[HTTPConfig] `mapstructure:"http"` | ||
} | ||
|
||
func (cfg *Config) Validate() error { | ||
if !cfg.GRPC.HasValue() && !cfg.HTTP.HasValue() { | ||
return errors.New("must specify at least one protocol when using the OTLP receiver") | ||
} | ||
return nil | ||
} | ||
|
||
func createDefaultConfig() component.Config { | ||
return &Config{ | ||
Protocols: Protocols{ | ||
GRPC: WithDefault(configgrpc.ServerConfig{ | ||
// ... | ||
}), | ||
HTTP: WithDefault(HTTPConfig{ | ||
// ... | ||
}), | ||
}, | ||
} | ||
} | ||
``` | ||
|
||
### Proper unmarshaling of empty values when a default is set | ||
|
||
Currently, the OTLP receiver requires a workaround to make enabling each | ||
protocol optional while providing defaults if a key for the protocol has been | ||
set: | ||
|
||
```golang | ||
type Protocols struct { | ||
GRPC *configgrpc.ServerConfig `mapstructure:"grpc"` | ||
HTTP *HTTPConfig `mapstructure:"http"` | ||
} | ||
|
||
// Config defines configuration for OTLP receiver. | ||
type Config struct { | ||
// Protocols is the configuration for the supported protocols, currently gRPC and HTTP (Proto and JSON). | ||
Protocols `mapstructure:"protocols"` | ||
} | ||
|
||
func createDefaultConfig() component.Config { | ||
return &Config{ | ||
Protocols: Protocols{ | ||
GRPC: configgrpc.NewDefaultServerConfig(), | ||
HTTP: &HTTPConfig{ | ||
// ... | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func (cfg *Config) Unmarshal(conf *confmap.Conf) error { | ||
// first load the config normally | ||
err := conf.Unmarshal(cfg) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// gRPC will be enabled if this line is not run | ||
if !conf.IsSet(protoGRPC) { | ||
cfg.GRPC = nil | ||
} | ||
|
||
// HTTP will be enabled if this line is not run | ||
if !conf.IsSet(protoHTTP) { | ||
cfg.HTTP = nil | ||
} | ||
|
||
return nil | ||
} | ||
``` | ||
|
||
With an Optional type, the checks in `Unmarshal` become unnecessary, and it's | ||
evan-bradley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
possible the entire `Unmarshal` function may no longer be needed. Instead, when | ||
the the config is unmarshaled, no value would be put into the default Optional | ||
evan-bradley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
values and `HasValue` would return false when using this config object. | ||
|
||
This situation is something of an edge case with our current unmarshaling | ||
facilities and while not common, could be a rough edge for users looking to | ||
implement similar config structures. | ||
|
||
## Disadvantages of an Optional type | ||
|
||
There are two noteworthy disadvantages of introducing an Optional type: | ||
evan-bradley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
1. Since the type isn't standard, external packages working with config may | ||
mx-psi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
require additional adaptations to work with our config structs. | ||
mx-psi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
2. It will still be possible to use pointer types for optional config fields, | ||
evan-bradley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
and as a result there may be some fragmentation in which type is used to | ||
represent an optional field. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.