Skip to content

Commit

Permalink
refactor: add default true type (#998)
Browse files Browse the repository at this point in the history 
## Which problem is this PR solving?

- Closes #970 

## Short description of the changes

- adds a `DefaultTrue` type to use when specifying values with a default
of boolean `true` this improves some getter functions by abstracting
away the `*bool` behavior and allows boolean `true` be a true default
  • Loading branch information
@fchikwekwe
fchikwekwe authored Feb 21, 2024
1 parent 430fcbf commit 23fac40
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 66 deletions.
2 changes: 1 addition & 1 deletion config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ func TestHoneycombGRPCConfigDefaults(t *testing.T) {
assert.Equal(t, "localhost:4343", a)

grpcConfig := c.GetGRPCConfig()
assert.Equal(t, true, *grpcConfig.Enabled)
assert.Equal(t, DefaultTrue(true), *grpcConfig.Enabled)
assert.Equal(t, "localhost:4343", grpcConfig.ListenAddr)
assert.Equal(t, 1*time.Minute, time.Duration(grpcConfig.MaxConnectionIdle))
assert.Equal(t, 3*time.Minute, time.Duration(grpcConfig.MaxConnectionAge))
Expand Down
108 changes: 49 additions & 59 deletions config/file_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"math/rand"
"net"
"os"
"strconv"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -89,11 +90,35 @@ type AccessKeyConfig struct {
keymap generics.Set[string]
}

type DefaultTrue bool

func (dt *DefaultTrue) Get() (enabled bool) {
if dt == nil {
return true
}
return bool(*dt)
}

func (dt *DefaultTrue) MarshalText() ([]byte, error) {
return []byte(strconv.FormatBool(bool(*dt))), nil
}

func (dt *DefaultTrue) UnmarshalText(text []byte) error {
trueBool, err := strconv.ParseBool(string(text))
if err != nil {
return err
}

*dt = DefaultTrue(trueBool)

return nil
}

type RefineryTelemetryConfig struct {
AddRuleReasonToTrace bool `yaml:"AddRuleReasonToTrace"`
AddSpanCountToRoot *bool `yaml:"AddSpanCountToRoot" default:"true"` // Avoid pointer woe on access, use GetAddSpanCountToRoot() instead.
AddCountsToRoot bool `yaml:"AddCountsToRoot"`
AddHostMetadataToTrace *bool `yaml:"AddHostMetadataToTrace" default:"true"` // Avoid pointer woe on access, use GetAddHostMetadataToTrace() instead.
AddRuleReasonToTrace bool `yaml:"AddRuleReasonToTrace"`
AddSpanCountToRoot *DefaultTrue `yaml:"AddSpanCountToRoot" default:"true"` // Avoid pointer woe on access, use GetAddSpanCountToRoot() instead.
AddCountsToRoot bool `yaml:"AddCountsToRoot"`
AddHostMetadataToTrace *DefaultTrue `yaml:"AddHostMetadataToTrace" default:"true"` // Avoid pointer woe on access, use GetAddHostMetadataToTrace() instead.
}

type TracesConfig struct {
Expand All @@ -117,22 +142,17 @@ type LoggerConfig struct {
}

type HoneycombLoggerConfig struct {
APIHost string `yaml:"APIHost" default:"https://api.honeycomb.io"`
APIKey string `yaml:"APIKey" cmdenv:"HoneycombLoggerAPIKey,HoneycombAPIKey"`
Dataset string `yaml:"Dataset" default:"Refinery Logs"`
SamplerEnabled *bool `yaml:"SamplerEnabled" default:"true"` // Avoid pointer woe on access, use GetSamplerEnabled() instead.
SamplerThroughput int `yaml:"SamplerThroughput" default:"10"`
APIHost string `yaml:"APIHost" default:"https://api.honeycomb.io"`
APIKey string `yaml:"APIKey" cmdenv:"HoneycombLoggerAPIKey,HoneycombAPIKey"`
Dataset string `yaml:"Dataset" default:"Refinery Logs"`
SamplerEnabled *DefaultTrue `yaml:"SamplerEnabled" default:"true"` // Avoid pointer woe on access, use GetSamplerEnabled() instead.
SamplerThroughput int `yaml:"SamplerThroughput" default:"10"`
}

// GetSamplerEnabled returns whether configuration has enabled sampling of
// Refinery's own logs destined for Honeycomb.
func (c *HoneycombLoggerConfig) GetSamplerEnabled() (enabled bool) {
if c.SamplerEnabled == nil {
enabled = true
} else {
enabled = *c.SamplerEnabled
}
return enabled
return c.SamplerEnabled.Get()
}

type StdoutLoggerConfig struct {
Expand Down Expand Up @@ -229,7 +249,7 @@ type BufferSizeConfig struct {

type SpecializedConfig struct {
EnvironmentCacheTTL Duration `yaml:"EnvironmentCacheTTL" default:"1h"`
CompressPeerCommunication *bool `yaml:"CompressPeerCommunication" default:"true"` // Avoid pointer woe on access, use GetCompressPeerCommunication() instead.
CompressPeerCommunication *DefaultTrue `yaml:"CompressPeerCommunication" default:"true"` // Avoid pointer woe on access, use GetCompressPeerCommunication() instead.
AdditionalAttributes map[string]string `yaml:"AdditionalAttributes" default:"{}"`
}

Expand All @@ -242,15 +262,15 @@ type IDFieldsConfig struct {
// by refinery's own GRPC server:
// https://pkg.go.dev/google.golang.org/grpc/keepalive#ServerParameters
type GRPCServerParameters struct {
Enabled *bool `yaml:"Enabled" default:"true"` // Avoid pointer woe on access, use GetGRPCEnabled() instead.
ListenAddr string `yaml:"ListenAddr" cmdenv:"GRPCListenAddr"`
MaxConnectionIdle Duration `yaml:"MaxConnectionIdle" default:"1m"`
MaxConnectionAge Duration `yaml:"MaxConnectionAge" default:"3m"`
MaxConnectionAgeGrace Duration `yaml:"MaxConnectionAgeGrace" default:"1m"`
KeepAlive Duration `yaml:"KeepAlive" default:"1m"`
KeepAliveTimeout Duration `yaml:"KeepAliveTimeout" default:"20s"`
MaxSendMsgSize MemorySize `yaml:"MaxSendMsgSize" default:"5MB"`
MaxRecvMsgSize MemorySize `yaml:"MaxRecvMsgSize" default:"5MB"`
Enabled *DefaultTrue `yaml:"Enabled" default:"true"` // Avoid pointer woe on access, use GetGRPCEnabled() instead.
ListenAddr string `yaml:"ListenAddr" cmdenv:"GRPCListenAddr"`
MaxConnectionIdle Duration `yaml:"MaxConnectionIdle" default:"1m"`
MaxConnectionAge Duration `yaml:"MaxConnectionAge" default:"3m"`
MaxConnectionAgeGrace Duration `yaml:"MaxConnectionAgeGrace" default:"1m"`
KeepAlive Duration `yaml:"KeepAlive" default:"1m"`
KeepAliveTimeout Duration `yaml:"KeepAliveTimeout" default:"20s"`
MaxSendMsgSize MemorySize `yaml:"MaxSendMsgSize" default:"5MB"`
MaxRecvMsgSize MemorySize `yaml:"MaxRecvMsgSize" default:"5MB"`
}

type SampleCacheConfig struct {
Expand Down Expand Up @@ -489,28 +509,14 @@ func (f *fileConfig) GetCompressPeerCommunication() bool {
f.mux.RLock()
defer f.mux.RUnlock()

var compressPeerCommunication bool
if f.mainConfig.Specialized.CompressPeerCommunication == nil {
compressPeerCommunication = true
} else {
compressPeerCommunication = *f.mainConfig.Specialized.CompressPeerCommunication
}

return compressPeerCommunication
return f.mainConfig.Specialized.CompressPeerCommunication.Get()
}

func (f *fileConfig) GetGRPCEnabled() bool {
f.mux.RLock()
defer f.mux.RUnlock()

var enabled bool
if f.mainConfig.GRPCServerParameters.Enabled == nil {
enabled = true
} else {
enabled = *f.mainConfig.GRPCServerParameters.Enabled
}

return enabled
return f.mainConfig.GRPCServerParameters.Enabled.Get()
}

func (f *fileConfig) GetGRPCListenAddr() (string, error) {
Expand Down Expand Up @@ -808,15 +814,7 @@ func (f *fileConfig) GetAddHostMetadataToTrace() bool {
f.mux.RLock()
defer f.mux.RUnlock()

var addHostMetadataToTrace bool

if f.mainConfig.Telemetry.AddHostMetadataToTrace == nil {
addHostMetadataToTrace = true // TODO: the default, but maybe look that up on the struct?
} else {
addHostMetadataToTrace = *f.mainConfig.Telemetry.AddHostMetadataToTrace
}

return addHostMetadataToTrace
return f.mainConfig.Telemetry.AddHostMetadataToTrace.Get()
}

func (f *fileConfig) GetAddRuleReasonToTrace() bool {
Expand Down Expand Up @@ -865,15 +863,7 @@ func (f *fileConfig) GetAddSpanCountToRoot() bool {
f.mux.RLock()
defer f.mux.RUnlock()

var addSpanCountToRoot bool

if f.mainConfig.Telemetry.AddSpanCountToRoot == nil {
addSpanCountToRoot = true // TODO: lookup default from struct
} else {
addSpanCountToRoot = *f.mainConfig.Telemetry.AddSpanCountToRoot
}

return addSpanCountToRoot
return f.mainConfig.Telemetry.AddSpanCountToRoot.Get()
}

func (f *fileConfig) GetAddCountsToRoot() bool {
Expand Down
12 changes: 6 additions & 6 deletions config/metadata/configMeta.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ groups:
the behavior of your Refinery installation.
- name: AddSpanCountToRoot
type: bool
type: defaulttrue
valuetype: nondefault
default: true
reload: true
Expand Down Expand Up @@ -243,7 +243,7 @@ groups:
- `meta.event_count`: the number of honeycomb events on the trace
- name: AddHostMetadataToTrace
type: bool
type: defaulttrue
valuetype: nondefault
default: true
reload: true
Expand Down Expand Up @@ -490,7 +490,7 @@ groups:
- name: SamplerEnabled
v1group: HoneycombLogger
v1name: LoggerSamplerEnabled
type: bool
type: defaulttrue
valuetype: nondefault
default: true
reload: false
Expand Down Expand Up @@ -1152,7 +1152,7 @@ groups:
increase this value.
- name: CompressPeerCommunication
type: bool
type: defaulttrue
default: true
valuetype: nondefault
reload: false
Expand Down Expand Up @@ -1257,10 +1257,10 @@ groups:
receive OpenTelemetry data in gRPC format.
fields:
- name: Enabled
type: bool
type: defaulttrue
valuetype: conditional
extra: "nonempty GRPCListenAddr"
default: false
default: true
reload: false
summary: specifies whether the gRPC server is enabled.
description: >
Expand Down
13 changes: 13 additions & 0 deletions config/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,19 @@ func validateDatatype(k string, v any, typ string) string {
if u.Scheme != "http" && u.Scheme != "https" {
return fmt.Sprintf("field %s (%v) must use an http or https scheme", k, v)
}
case "defaulttrue":
switch val := v.(type) {
case bool:
return ""
case string:
switch strings.ToLower(val) {
case "t", "true", "f", "false":
default:
return fmt.Sprintf("field %s (%v) must be 'true', 'false', 't', or 'f'", k, v)
}
default:
return fmt.Sprintf("field %s (%v) must be a bool or string with value true/false or 'true'/'false'/'t'/'f'", k, v)
}
default:
panic("unknown data type " + typ)
}
Expand Down

0 comments on commit 23fac40

Please sign in to comment.