Skip to content

Commit

Permalink
Try WithDefault approach suggested by @mx-psi
Browse files Browse the repository at this point in the history
  • Loading branch information
yurishkuro committed Jul 14, 2024
1 parent 6934b6e commit c8acf84
Show file tree
Hide file tree
Showing 9 changed files with 247 additions and 79 deletions.
32 changes: 0 additions & 32 deletions confmap/optional.go

This file was deleted.

55 changes: 55 additions & 0 deletions confmap/optional/optional.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package optional

import "go.opentelemetry.io/collector/confmap"

// Optional is a type that can be used to represent a value that may or may not be present.
// It supports three flavors: Some(value), None(), and WithDefault(defaultValue).
type Optional[T any] struct {
hasValue bool
value T

defaultFn DefaultFunc[T]
}

type DefaultFunc[T any] func() T

var _ confmap.Unmarshaler = (*Optional[any])(nil)

// Some creates an Optional with a value.
func Some[T any](value T) Optional[T] {
return Optional[T]{value: value, hasValue: true}
}

// None creates an Optional with no value.
func None[T any]() Optional[T] {
return Optional[T]{}
}

// WithDefault creates an Optional which has no value
// unless user config provides some, in which case
// the defaultValue is used as a starting point,
// which may be overridden by the user provided values.
func WithDefault[T any](defaultFn DefaultFunc[T]) Optional[T] {
return Optional[T]{defaultFn: defaultFn}
}

func (o Optional[T]) HasValue() bool {
return o.hasValue
}

func (o Optional[T]) Value() T {
return o.value
}

func (o *Optional[T]) Unmarshal(conf *confmap.Conf) error {
// we assume that Unmarshal will not be called if conf has no value.
if o.defaultFn != nil {
o.value = o.defaultFn()
o.hasValue = true
}
if err := conf.Unmarshal(&o.value); err != nil {
return err
}
o.hasValue = true
return nil
}
150 changes: 150 additions & 0 deletions confmap/optional/optional_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
package optional

import (
"os"
"path/filepath"
"strings"
"testing"
"unicode"

"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/confmap/confmaptest"
)

type Config struct {
Sub1 Optional[Sub] `mapstructure:"sub"`
}

type Sub struct {
Foo string `mapstructure:"foo"`
}

func TestConfigValidateNoBackends(t *testing.T) {
tests := []struct {
name string
config string
defaultCfg Config
expectedSub bool
expectedFoo string
}{
{
name: "no_default_no_config",
defaultCfg: Config{
Sub1: None[Sub](),
},
expectedSub: false,
},
{
name: "no_default_with_config",
config: `
sub:
foo: bar
`,
defaultCfg: Config{
Sub1: None[Sub](),
},
expectedSub: true,
expectedFoo: "bar",
},
{
name: "with_default_no_config",
defaultCfg: Config{
Sub1: WithDefault(func() Sub {
return Sub{
Foo: "foobar",
}
}),
},
expectedSub: false,
},
{
name: "with_default_with_config",
config: `
sub:
foo: bar
`,
defaultCfg: Config{
Sub1: WithDefault(func() Sub {
return Sub{
Foo: "foobar",
}
}),
},
expectedSub: true,
expectedFoo: "bar", // input overrides default
},
{
// this test fails, because "sub:" is considered null value by mapstructure
// and no additional processing happens for it, including custom unmarshaler.
// https://github.com/go-viper/mapstructure/blob/0382e5b7e3987443c91311b7fdb60b92c69a47bf/mapstructure.go#L445
name: "with_default_with_config_no_foo",
config: `
sub:
`,
defaultCfg: Config{
Sub1: WithDefault(func() Sub {
return Sub{
Foo: "foobar",
}
}),
},
expectedSub: true,
expectedFoo: "barbar", // default applies
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
cfg := test.defaultCfg
conf := strToConf(t, test.config)
require.NoError(t, conf.Unmarshal(&cfg))
require.Equal(t, test.expectedSub, cfg.Sub1.HasValue())
if test.expectedSub {
require.Equal(t, test.expectedFoo, cfg.Sub1.Value().Foo)
}
})
}
}

func strToConf(t *testing.T, config string) *confmap.Conf {
config = stripWhitespacePrefix(config)
d := t.TempDir()
f := filepath.Join(d, "config.yaml")
require.NoError(t, os.WriteFile(f, []byte(config), 0o644))
cm, err := confmaptest.LoadConf(f)
require.NoError(t, err)
return cm
}

// stripWhitespacePrefix finds the first non-blank line,
// detects how much whitespace is in front of it, and removes
// that much whitespace from the beginning of all lines.
func stripWhitespacePrefix(s string) string {
lines := strings.Split(s, "\n")
var prefix string

// Find the first non-blank line
for _, line := range lines {
if strings.TrimSpace(line) == "" {
continue
}
nonSpace := strings.IndexFunc(line, func(r rune) bool {
return !unicode.IsSpace(r)
})
prefix = string([]rune(line)[:nonSpace])
break
}

// Remove the prefix from all lines
var result []string
for _, line := range lines {
if strings.TrimSpace(line) == "" {
continue
}
result = append(result, strings.TrimPrefix(line, prefix))
}

return strings.Join(result, "\n")
}
16 changes: 7 additions & 9 deletions receiver/otlpreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ type HTTPConfig struct {
// Protocols is the configuration for the supported protocols.
type Protocols struct {
GRPC *configgrpc.ServerConfig `mapstructure:"grpc"`

// TODO For now using Optional with pointer, because some tests modify config in place
HTTP confmap.Optional[*HTTPConfig] `mapstructure:"http"`
HTTP *HTTPConfig `mapstructure:"http"`
}

// Config defines configuration for OTLP receiver.
Expand All @@ -53,7 +51,7 @@ var _ confmap.Unmarshaler = (*Config)(nil)

// Validate checks the receiver configuration is valid
func (cfg *Config) Validate() error {
if cfg.GRPC == nil && !cfg.HTTP.HasValue() {
if cfg.GRPC == nil && cfg.HTTP == nil {
return errors.New("must specify at least one protocol when using the OTLP receiver")
}
return nil
Expand All @@ -72,17 +70,17 @@ func (cfg *Config) Unmarshal(conf *confmap.Conf) error {
}

if !conf.IsSet(protoHTTP) {
cfg.HTTP = confmap.None[*HTTPConfig]()
cfg.HTTP = nil
} else {
var err error
httpPtr := cfg.HTTP.Value()
if httpPtr.TracesURLPath, err = sanitizeURLPath(httpPtr.TracesURLPath); err != nil {

if cfg.HTTP.TracesURLPath, err = sanitizeURLPath(cfg.HTTP.TracesURLPath); err != nil {
return err
}
if httpPtr.MetricsURLPath, err = sanitizeURLPath(httpPtr.MetricsURLPath); err != nil {
if cfg.HTTP.MetricsURLPath, err = sanitizeURLPath(cfg.HTTP.MetricsURLPath); err != nil {
return err
}
if httpPtr.LogsURLPath, err = sanitizeURLPath(httpPtr.LogsURLPath); err != nil {
if cfg.HTTP.LogsURLPath, err = sanitizeURLPath(cfg.HTTP.LogsURLPath); err != nil {
return err
}
}
Expand Down
10 changes: 5 additions & 5 deletions receiver/otlpreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestUnmarshalConfigOnlyGRPC(t *testing.T) {
assert.NoError(t, cm.Unmarshal(&cfg))

defaultOnlyGRPC := factory.CreateDefaultConfig().(*Config)
defaultOnlyGRPC.HTTP = confmap.None[*HTTPConfig]()
defaultOnlyGRPC.HTTP = nil
assert.Equal(t, defaultOnlyGRPC, cfg)
}

Expand Down Expand Up @@ -115,7 +115,7 @@ func TestUnmarshalConfig(t *testing.T) {
},
},
},
HTTP: confmap.Some(&HTTPConfig{
HTTP: &HTTPConfig{
ServerConfig: &confighttp.ServerConfig{
Endpoint: "localhost:4318",
TLSSetting: &configtls.ServerConfig{
Expand All @@ -132,7 +132,7 @@ func TestUnmarshalConfig(t *testing.T) {
TracesURLPath: "/traces",
MetricsURLPath: "/v2/metrics",
LogsURLPath: "/log/ingest",
}),
},
},
}, cfg)

Expand All @@ -154,14 +154,14 @@ func TestUnmarshalConfigUnix(t *testing.T) {
},
ReadBufferSize: 512 * 1024,
},
HTTP: confmap.Some(&HTTPConfig{
HTTP: &HTTPConfig{
ServerConfig: &confighttp.ServerConfig{
Endpoint: "/tmp/http_otlp.sock",
},
TracesURLPath: defaultTracesURLPath,
MetricsURLPath: defaultMetricsURLPath,
LogsURLPath: defaultLogsURLPath,
}),
},
},
}, cfg)
}
Expand Down
5 changes: 2 additions & 3 deletions receiver/otlpreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/internal/localhostgate"
"go.opentelemetry.io/collector/internal/sharedcomponent"
Expand Down Expand Up @@ -50,14 +49,14 @@ func createDefaultConfig() component.Config {
// We almost write 0 bytes, so no need to tune WriteBufferSize.
ReadBufferSize: 512 * 1024,
},
HTTP: confmap.Some(&HTTPConfig{
HTTP: &HTTPConfig{
ServerConfig: &confighttp.ServerConfig{
Endpoint: localhostgate.EndpointForPort(httpPort),
},
TracesURLPath: defaultTracesURLPath,
MetricsURLPath: defaultMetricsURLPath,
LogsURLPath: defaultLogsURLPath,
}),
},
},
}
}
Expand Down
Loading

0 comments on commit c8acf84

Please sign in to comment.