From 6eca9a01afbd486b8b04b3f578f21bdd616d59c6 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Mon, 23 Sep 2024 14:57:57 +0200 Subject: [PATCH 1/5] feat: add error prefixer Signed-off-by: Mark Sagi-Kazar --- pkg/errorsx/errorsx.go | 32 ++++++++++++++++++ pkg/errorsx/errorsx_test.go | 65 +++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 pkg/errorsx/errorsx.go create mode 100644 pkg/errorsx/errorsx_test.go diff --git a/pkg/errorsx/errorsx.go b/pkg/errorsx/errorsx.go new file mode 100644 index 000000000..9e55362d1 --- /dev/null +++ b/pkg/errorsx/errorsx.go @@ -0,0 +1,32 @@ +package errorsx + +import ( + "errors" + "fmt" +) + +// WithPrefix annotates an error with a prefix. +func WithPrefix(err error, prefix string) error { + if err == nil { + return nil + } + + type unwrapper interface { + Unwrap() []error + } + + // Deliberately checking for the unwrapper interface instead of the errors.Is function. + // We only want to check the top-level error otherwise we may accidentally drop other wrappers from the error chain. + e, ok := err.(unwrapper) + if !ok { + return fmt.Errorf("%s: %w", prefix, err) + } + + errs := e.Unwrap() + + for i, err := range errs { + errs[i] = WithPrefix(err, prefix) + } + + return errors.Join(errs...) +} diff --git a/pkg/errorsx/errorsx_test.go b/pkg/errorsx/errorsx_test.go new file mode 100644 index 000000000..5b4246b1a --- /dev/null +++ b/pkg/errorsx/errorsx_test.go @@ -0,0 +1,65 @@ +package errorsx + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestWithPrefix(t *testing.T) { + const prefix = "prefix" + + tests := []struct { + name string + err error + expected string + }{ + { + name: "single error", + err: errors.New("error"), + expected: "prefix: error", + }, + { + name: "multiple errors", + err: errors.Join( + errors.New("error 1"), + errors.New("error 2"), + ), + expected: "prefix: error 1\nprefix: error 2", + }, + + { + name: "multiple errors (not top-level)", + err: fmt.Errorf("%w", errors.Join( + errors.New("error 1"), + errors.New("error 2"), + )), + expected: "prefix: error 1\nerror 2", + }, + { + name: "multiple errors", + err: errors.Join( + WithPrefix( + errors.Join( + + errors.New("error 1.1"), + errors.New("error 1.2"), + ), + "subprefix", + ), + errors.New("error 2"), + ), + expected: "prefix: subprefix: error 1.1\nprefix: subprefix: error 1.2\nprefix: error 2", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := WithPrefix(test.err, prefix).Error() + + assert.Equal(t, test.expected, actual) + }) + } +} From 73360f37629f37a543e6cda703d23bb2afe3edc6 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Mon, 23 Sep 2024 15:52:07 +0200 Subject: [PATCH 2/5] chore: return all errors during config validation Signed-off-by: Mark Sagi-Kazar --- config/aggregation.go | 16 +++--- config/balanceworker.go | 10 +++- config/config.go | 34 +++++++------ config/dedupe.go | 17 ++++--- config/events.go | 68 +++++++++++++++++--------- config/ingest.go | 27 ++++++---- config/kafka.go | 8 ++- config/namespace.go | 6 ++- config/notification.go | 14 ++++-- config/portal.go | 16 ++++-- config/postgres.go | 9 ++-- config/sink.go | 24 +++++---- config/telemetry.go | 49 ++++++++++++------- openmeter/notification/webhook/svix.go | 8 +-- 14 files changed, 197 insertions(+), 109 deletions(-) diff --git a/config/aggregation.go b/config/aggregation.go index 5e80cb8db..dca483983 100644 --- a/config/aggregation.go +++ b/config/aggregation.go @@ -45,31 +45,33 @@ type ClickHouseAggregationConfiguration struct { } func (c ClickHouseAggregationConfiguration) Validate() error { + var errs []error + if c.Address == "" { - return errors.New("address is required") + errs = append(errs, errors.New("address is required")) } if c.DialTimeout <= 0 { - return errors.New("dial timeout must be greater than 0") + errs = append(errs, errors.New("dial timeout must be greater than 0")) } if c.MaxOpenConns <= 0 { - return errors.New("max open connections must be greater than 0") + errs = append(errs, errors.New("max open connections must be greater than 0")) } if c.MaxIdleConns <= 0 { - return errors.New("max idle connections must be greater than 0") + errs = append(errs, errors.New("max idle connections must be greater than 0")) } if c.ConnMaxLifetime <= 0 { - return errors.New("connection max lifetime must be greater than 0") + errs = append(errs, errors.New("connection max lifetime must be greater than 0")) } if c.BlockBufferSize <= 0 { - return errors.New("block buffer size must be greater than 0") + errs = append(errs, errors.New("block buffer size must be greater than 0")) } - return nil + return errors.Join(errs...) } func (c ClickHouseAggregationConfiguration) GetClientOptions() *clickhouse.Options { diff --git a/config/balanceworker.go b/config/balanceworker.go index c3508993d..0da40fd2e 100644 --- a/config/balanceworker.go +++ b/config/balanceworker.go @@ -1,7 +1,11 @@ package config import ( + "errors" + "github.com/spf13/viper" + + "github.com/openmeterio/openmeter/pkg/errorsx" ) type BalanceWorkerConfiguration struct { @@ -9,11 +13,13 @@ type BalanceWorkerConfiguration struct { } func (c BalanceWorkerConfiguration) Validate() error { + var errs []error + if err := c.ConsumerConfiguration.Validate(); err != nil { - return err + errs = append(errs, errorsx.WithPrefix(err, "consumer")) } - return nil + return errors.Join(errs...) } func ConfigureBalanceWorker(v *viper.Viper) { diff --git a/config/config.go b/config/config.go index ac321bf9f..0b0489518 100644 --- a/config/config.go +++ b/config/config.go @@ -3,12 +3,12 @@ package config import ( "errors" - "fmt" "strings" "github.com/spf13/pflag" "github.com/spf13/viper" + "github.com/openmeterio/openmeter/pkg/errorsx" "github.com/openmeterio/openmeter/pkg/models" ) @@ -37,44 +37,46 @@ type Configuration struct { // Validate validates the configuration. func (c Configuration) Validate() error { + var errs []error + if c.Address == "" { - return errors.New("server address is required") + errs = append(errs, errors.New("server address is required")) } if err := c.Telemetry.Validate(); err != nil { - return fmt.Errorf("telemetry: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "telemetry")) } if err := c.Namespace.Validate(); err != nil { - return fmt.Errorf("namespace: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "namespace")) } if err := c.Ingest.Validate(); err != nil { - return fmt.Errorf("ingest: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "ingest")) } if err := c.Aggregation.Validate(); err != nil { - return fmt.Errorf("aggregation: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "aggregation")) } if err := c.Sink.Validate(); err != nil { - return fmt.Errorf("sink: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "sink")) } if err := c.Dedupe.Validate(); err != nil { - return fmt.Errorf("dedupe: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "dedupe")) } if err := c.Portal.Validate(); err != nil { - return fmt.Errorf("portal: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "portal")) } if err := c.Entitlements.Validate(); err != nil { - return fmt.Errorf("entitlements: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "entitlements")) } if len(c.Meters) == 0 { - return errors.New("no meters configured: add meter to configuration file") + errs = append(errs, errors.New("no meters configured: add meter to configuration file")) } for _, m := range c.Meters { @@ -87,25 +89,25 @@ func (c Configuration) Validate() error { } if err := m.Validate(); err != nil { - return err + errs = append(errs, err) } } if err := c.BalanceWorker.Validate(); err != nil { - return fmt.Errorf("balance worker: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "balance worker")) } if c.Notification.Enabled { if err := c.Notification.Validate(); err != nil { - return fmt.Errorf("notification: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "notification")) } if err := c.Svix.Validate(); err != nil { - return fmt.Errorf("svix: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "svix")) } } - return nil + return errors.Join(errs...) } func SetViperDefaults(v *viper.Viper, flags *pflag.FlagSet) { diff --git a/config/dedupe.go b/config/dedupe.go index d50518ada..3d19d1864 100644 --- a/config/dedupe.go +++ b/config/dedupe.go @@ -11,6 +11,7 @@ import ( "github.com/openmeterio/openmeter/openmeter/dedupe" "github.com/openmeterio/openmeter/openmeter/dedupe/memorydedupe" "github.com/openmeterio/openmeter/openmeter/dedupe/redisdedupe" + "github.com/openmeterio/openmeter/pkg/errorsx" "github.com/openmeterio/openmeter/pkg/redis" ) @@ -43,7 +44,7 @@ func (c DedupeConfiguration) Validate() error { } if err := c.DedupeDriverConfiguration.Validate(); err != nil { - return fmt.Errorf("driver(%s): %w", c.DedupeDriverConfiguration.DriverName(), err) + return errorsx.WithPrefix(err, fmt.Sprintf("driver(%s)", c.DriverName())) } return nil @@ -149,11 +150,13 @@ func (c DedupeDriverMemoryConfiguration) NewDeduplicator() (dedupe.Deduplicator, } func (c DedupeDriverMemoryConfiguration) Validate() error { + var errs []error + if c.Size == 0 { - return errors.New("size is required") + errs = append(errs, errors.New("size is required")) } - return nil + return errors.Join(errs...) } // Dedupe redis driver configuration @@ -181,17 +184,19 @@ func (c DedupeDriverRedisConfiguration) NewDeduplicator() (dedupe.Deduplicator, } func (c DedupeDriverRedisConfiguration) Validate() error { + var errs []error + if c.Address == "" { - return errors.New("address is required") + errs = append(errs, errors.New("address is required")) } if c.Sentinel.Enabled { if c.Sentinel.MasterName == "" { - return errors.New("sentinel: master name is required") + errs = append(errs, errors.New("sentinel: master name is required")) } } - return nil + return errors.Join(errs...) } // ConfigureDedupe configures some defaults in the Viper instance. diff --git a/config/events.go b/config/events.go index 77d3be8a1..655983628 100644 --- a/config/events.go +++ b/config/events.go @@ -2,10 +2,11 @@ package config import ( "errors" - "fmt" "time" "github.com/spf13/viper" + + "github.com/openmeterio/openmeter/pkg/errorsx" ) type EventsConfiguration struct { @@ -30,10 +31,17 @@ func (c EventSubsystemConfiguration) Validate() error { return nil } + var errs []error + if c.Topic == "" { - return errors.New("topic name is required") + errs = append(errs, errors.New("topic name is required")) + } + + if err := c.AutoProvision.Validate(); err != nil { + errs = append(errs, errorsx.WithPrefix(err, "auto provision")) } - return c.AutoProvision.Validate() + + return errors.Join(errs...) } type AutoProvisionConfiguration struct { @@ -43,10 +51,17 @@ type AutoProvisionConfiguration struct { } func (c AutoProvisionConfiguration) Validate() error { - if c.Enabled && c.Partitions < 1 { - return errors.New("partitions must be greater than 0") + if !c.Enabled { + return nil + } + + var errs []error + + if c.Partitions < 1 { + errs = append(errs, errors.New("partitions must be greater than 0")) } - return nil + + return errors.Join(errs...) } type ConsumerConfiguration struct { @@ -64,23 +79,25 @@ type ConsumerConfiguration struct { } func (c ConsumerConfiguration) Validate() error { + var errs []error + if c.ProcessingTimeout < 0 { - return errors.New("processing timeout must be positive or 0") + errs = append(errs, errors.New("processing timeout must be positive or 0")) } if c.ConsumerGroupName == "" { - return errors.New("consumer group name is required") + errs = append(errs, errors.New("consumer group name is required")) } if err := c.Retry.Validate(); err != nil { - return fmt.Errorf("retry configuration is invalid: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "retry")) } if err := c.DLQ.Validate(); err != nil { - return fmt.Errorf("dlq configuration is invalid: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "dlq")) } - return nil + return errors.Join(errs...) } type DLQConfiguration struct { @@ -94,15 +111,17 @@ func (c DLQConfiguration) Validate() error { return nil } + var errs []error + if c.Topic == "" { - return errors.New("topic name is required") + errs = append(errs, errors.New("topic name is required")) } if err := c.AutoProvision.Validate(); err != nil { - return fmt.Errorf("auto provision configuration is invalid: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "auto provision")) } - return nil + return errors.Join(errs...) } type DLQAutoProvisionConfiguration struct { @@ -116,14 +135,17 @@ func (c DLQAutoProvisionConfiguration) Validate() error { return nil } + var errs []error + if c.Partitions < 1 { - return errors.New("partitions must be greater than 0") + errs = append(errs, errors.New("partitions must be greater than 0")) } if c.Retention <= 0 { - return errors.New("retention must be greater than 0") + errs = append(errs, errors.New("retention must be greater than 0")) } - return nil + + return errors.Join(errs...) } type RetryConfiguration struct { @@ -138,23 +160,25 @@ type RetryConfiguration struct { } func (c RetryConfiguration) Validate() error { + var errs []error + if c.MaxRetries < 0 { - return errors.New("max retries must be positive or 0") + errs = append(errs, errors.New("max retries must be positive or 0")) } if c.MaxElapsedTime < 0 { - return errors.New("max elapsed time must be positive or 0") + errs = append(errs, errors.New("max elapsed time must be positive or 0")) } if c.InitialInterval <= 0 { - return errors.New("initial interval must be greater than 0") + errs = append(errs, errors.New("initial interval must be greater than 0")) } if c.MaxInterval <= 0 { - return errors.New("max interval must be greater than 0") + errs = append(errs, errors.New("max interval must be greater than 0")) } - return nil + return errors.Join(errs...) } func ConfigureConsumer(v *viper.Viper, prefix string) { diff --git a/config/ingest.go b/config/ingest.go index 740d1a930..390710e84 100644 --- a/config/ingest.go +++ b/config/ingest.go @@ -2,13 +2,13 @@ package config import ( "errors" - "fmt" "strings" "time" "github.com/confluentinc/confluent-kafka-go/v2/kafka" "github.com/spf13/viper" + "github.com/openmeterio/openmeter/pkg/errorsx" pkgkafka "github.com/openmeterio/openmeter/pkg/kafka" ) @@ -18,11 +18,13 @@ type IngestConfiguration struct { // Validate validates the configuration. func (c IngestConfiguration) Validate() error { + var errs []error + if err := c.Kafka.Validate(); err != nil { - return fmt.Errorf("kafka: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "kafka")) } - return nil + return errors.Join(errs...) } type KafkaIngestConfiguration struct { @@ -34,14 +36,17 @@ type KafkaIngestConfiguration struct { // Validate validates the configuration. func (c KafkaIngestConfiguration) Validate() error { + var errs []error + if c.EventsTopicTemplate == "" { - return errors.New("events topic template is required") + errs = append(errs, errors.New("events topic template is required")) } if err := c.KafkaConfiguration.Validate(); err != nil { - return err + errs = append(errs, err) } - return nil + + return errors.Join(errs...) } type KafkaConfiguration struct { @@ -71,19 +76,21 @@ type KafkaConfiguration struct { } func (c KafkaConfiguration) Validate() error { + var errs []error + if c.Broker == "" { - return errors.New("broker is required") + errs = append(errs, errors.New("broker is required")) } if c.StatsInterval > 0 && c.StatsInterval.Duration() < 5*time.Second { - return errors.New("StatsInterval must be >=5s") + errs = append(errs, errors.New("StatsInterval must be >=5s")) } if c.TopicMetadataRefreshInterval > 0 && c.TopicMetadataRefreshInterval.Duration() < 10*time.Second { - return errors.New("topic metadata refresh interval must be >=10s") + errs = append(errs, errors.New("topic metadata refresh interval must be >=10s")) } - return nil + return errors.Join(errs...) } // CreateKafkaConfig creates a Kafka config map. diff --git a/config/kafka.go b/config/kafka.go index 0386b5cd4..6558f157c 100644 --- a/config/kafka.go +++ b/config/kafka.go @@ -1,6 +1,8 @@ package config import ( + "errors" + "github.com/spf13/viper" pkgkafka "github.com/openmeterio/openmeter/pkg/kafka" @@ -35,13 +37,15 @@ func (c KafkaConfig) Validate() error { c.ProducerConfigParams, } + var errs []error + for _, validator := range validators { if err := validator.Validate(); err != nil { - return err + errs = append(errs, err) } } - return nil + return errors.Join(errs...) } // ConfigureKafkaConfiguration sets defaults in the Viper instance. diff --git a/config/namespace.go b/config/namespace.go index 14cce904b..efbc3b322 100644 --- a/config/namespace.go +++ b/config/namespace.go @@ -13,11 +13,13 @@ type NamespaceConfiguration struct { } func (c NamespaceConfiguration) Validate() error { + var errs []error + if c.Default == "" { - return errors.New("default namespace is required") + errs = append(errs, errors.New("default namespace is required")) } - return nil + return errors.Join(errs...) } // ConfigureNamespace configures some defaults in the Viper instance. diff --git a/config/notification.go b/config/notification.go index 47d34cfd5..b44243cc1 100644 --- a/config/notification.go +++ b/config/notification.go @@ -1,9 +1,10 @@ package config import ( - "fmt" + "errors" "time" + "github.com/openmeterio/openmeter/pkg/errorsx" "github.com/spf13/viper" "github.com/openmeterio/openmeter/openmeter/notification/webhook" @@ -24,10 +25,17 @@ type NotificationConfiguration struct { } func (c NotificationConfiguration) Validate() error { + if !c.Enabled { + return nil + } + + var errs []error + if err := c.Consumer.Validate(); err != nil { - return fmt.Errorf("consumer: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "consumer")) } - return nil + + return errors.Join(errs...) } func ConfigureNotification(v *viper.Viper) { diff --git a/config/portal.go b/config/portal.go index 129e51217..50adacf87 100644 --- a/config/portal.go +++ b/config/portal.go @@ -20,15 +20,21 @@ type PortalConfiguration struct { // Validate validates the configuration. func (c PortalConfiguration) Validate() error { - if c.Enabled && c.TokenSecret == "" { - return errors.New("token secret is required") + if !c.Enabled { + return nil } - if c.Enabled && c.TokenExpiration.Seconds() == 0 { - return errors.New("token duration is required") + var errs []error + + if c.TokenSecret == "" { + errs = append(errs, errors.New("token secret is required")) + } + + if c.TokenExpiration.Seconds() == 0 { + errs = append(errs, errors.New("token duration is required")) } - return nil + return errors.Join(errs...) } // ConfigurePortal configures some defaults in the Viper instance. diff --git a/config/postgres.go b/config/postgres.go index 16a9bf76e..afe837032 100644 --- a/config/postgres.go +++ b/config/postgres.go @@ -19,14 +19,17 @@ type PostgresConfig struct { // Validate validates the configuration. func (c PostgresConfig) Validate() error { + var errs []error + if c.URL == "" { - return errors.New("database URL is required") + errs = append(errs, errors.New("database URL is required")) } + if err := c.AutoMigrate.Validate(); err != nil { - return err + errs = append(errs, err) } - return nil + return errors.Join(errs...) } func ConfigurePostgres(v *viper.Viper) { diff --git a/config/sink.go b/config/sink.go index c65a0d897..40041c6c2 100644 --- a/config/sink.go +++ b/config/sink.go @@ -2,9 +2,9 @@ package config import ( "errors" - "fmt" "time" + "github.com/openmeterio/openmeter/pkg/errorsx" "github.com/spf13/viper" ) @@ -21,27 +21,29 @@ type SinkConfiguration struct { } func (c SinkConfiguration) Validate() error { + var errs []error + if c.MinCommitCount < 1 { - return errors.New("MinCommitCount must be greater than 0") + errs = append(errs, errors.New("MinCommitCount must be greater than 0")) } if c.MaxCommitWait < 1 { - return errors.New("MaxCommitWait must be greater than 0") + errs = append(errs, errors.New("MaxCommitWait must be greater than 0")) } if c.NamespaceRefetch < 1 { - return errors.New("NamespaceRefetch must be greater than 0") + errs = append(errs, errors.New("NamespaceRefetch must be greater than 0")) } if err := c.IngestNotifications.Validate(); err != nil { - return fmt.Errorf("ingest notifications: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "ingest notifications")) } if err := c.Kafka.Validate(); err != nil { - return fmt.Errorf("kafka: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "kafka")) } - return nil + return errors.Join(errs...) } type IngestNotificationsConfiguration struct { @@ -49,15 +51,17 @@ type IngestNotificationsConfiguration struct { } func (c IngestNotificationsConfiguration) Validate() error { + var errs []error + if c.MaxEventsInBatch < 0 { - return errors.New("ChunkSize must not be negative") + errs = append(errs, errors.New("ChunkSize must not be negative")) } if c.MaxEventsInBatch > 1000 { - return errors.New("ChunkSize must not be greater than 1000") + errs = append(errs, errors.New("ChunkSize must not be greater than 1000")) } - return nil + return errors.Join(errs...) } // ConfigureSink setup Sink specific configuration defaults for provided *viper.Viper instance. diff --git a/config/telemetry.go b/config/telemetry.go index e0a52546e..b6e1468e3 100644 --- a/config/telemetry.go +++ b/config/telemetry.go @@ -13,6 +13,7 @@ import ( "github.com/golang-cz/devslog" "github.com/lmittmann/tint" + "github.com/openmeterio/openmeter/pkg/errorsx" "github.com/spf13/pflag" "github.com/spf13/viper" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" @@ -31,11 +32,13 @@ type OTLPExporterTelemetryConfig struct { // Validate validates the configuration. func (c OTLPExporterTelemetryConfig) Validate() error { + var errs []error + if c.Address == "" { - return errors.New("address is required") + errs = append(errs, errors.New("address is required")) } - return nil + return errors.Join(errs...) } func (c OTLPExporterTelemetryConfig) DialExporter(ctx context.Context) (*grpc.ClientConn, error) { @@ -64,23 +67,25 @@ type TelemetryConfig struct { // Validate validates the configuration. func (c TelemetryConfig) Validate() error { + var errs []error + if c.Address == "" { - return errors.New("http server address is required") + errs = append(errs, errors.New("http server address is required")) } if err := c.Trace.Validate(); err != nil { - return fmt.Errorf("trace: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "trace")) } if err := c.Metrics.Validate(); err != nil { - return fmt.Errorf("metrics: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "metrics")) } if err := c.Log.Validate(); err != nil { - return fmt.Errorf("log: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "log")) } - return nil + return errors.Join(errs...) } type TraceTelemetryConfig struct { @@ -90,15 +95,17 @@ type TraceTelemetryConfig struct { // Validate validates the configuration. func (c TraceTelemetryConfig) Validate() error { + var errs []error + if _, err := strconv.ParseFloat(c.Sampler, 64); err != nil && !slices.Contains([]string{"always", "never"}, c.Sampler) { - return fmt.Errorf("sampler either needs to be always|never or a ration, got: %s", c.Sampler) + errs = append(errs, fmt.Errorf("sampler either needs to be always|never or a ration, got: %s", c.Sampler)) } if err := c.Exporters.Validate(); err != nil { - return fmt.Errorf("exporter: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "exporters")) } - return nil + return errors.Join(errs...) } func (c TraceTelemetryConfig) GetSampler() sdktrace.Sampler { @@ -143,11 +150,13 @@ type ExportersTraceTelemetryConfig struct { // Validate validates the configuration. func (c ExportersTraceTelemetryConfig) Validate() error { + var errs []error + if err := c.OTLP.Validate(); err != nil { - return fmt.Errorf("otlp: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "otlp")) } - return nil + return errors.Join(errs...) } type OTLPExportersTraceTelemetryConfig struct { @@ -234,15 +243,17 @@ type ExportersMetricsTelemetryConfig struct { // Validate validates the configuration. func (c ExportersMetricsTelemetryConfig) Validate() error { + var errs []error + if err := c.Prometheus.Validate(); err != nil { - return fmt.Errorf("prometheus: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "prometheus")) } if err := c.OTLP.Validate(); err != nil { - return fmt.Errorf("otlp: %w", err) + errs = append(errs, errorsx.WithPrefix(err, "otlp")) } - return nil + return errors.Join(errs...) } type PrometheusExportersMetricsTelemetryConfig struct { @@ -319,15 +330,17 @@ type LogTelemetryConfiguration struct { // Validate validates the configuration. func (c LogTelemetryConfiguration) Validate() error { + var errs []error + if !slices.Contains([]string{"json", "text", "tint", "prettydev"}, c.Format) { - return fmt.Errorf("invalid format: %q", c.Format) + errs = append(errs, fmt.Errorf("invalid format: %q", c.Format)) } if !slices.Contains([]slog.Level{slog.LevelDebug, slog.LevelInfo, slog.LevelWarn, slog.LevelError}, c.Level) { - return fmt.Errorf("invalid level: %q", c.Level) + errs = append(errs, fmt.Errorf("invalid level: %q", c.Level)) } - return nil + return errors.Join(errs...) } // NewHandler creates a new [slog.Handler]. diff --git a/openmeter/notification/webhook/svix.go b/openmeter/notification/webhook/svix.go index 0d4061b7a..a2e2d8601 100644 --- a/openmeter/notification/webhook/svix.go +++ b/openmeter/notification/webhook/svix.go @@ -38,17 +38,19 @@ type SvixConfig struct { } func (c SvixConfig) Validate() error { + var errs []error + if c.APIKey == "" { - return errors.New("API key is required") + errs = append(errs, errors.New("API key is required")) } if c.ServerURL != "" { if _, err := url.Parse(c.ServerURL); err != nil { - return fmt.Errorf("invalid server URL: %w", err) + errs = append(errs, fmt.Errorf("invalid server URL: %w", err)) } } - return nil + return errors.Join(errs...) } var _ Handler = (*svixWebhookHandler)(nil) From 923f358890671362fb76c4bc10fc3f4b81f10088 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Mon, 23 Sep 2024 15:53:09 +0200 Subject: [PATCH 3/5] feat: print configuration errors Signed-off-by: Mark Sagi-Kazar --- cmd/balance-worker/main.go | 4 +++- cmd/notification-service/main.go | 4 +++- cmd/server/main.go | 4 +++- cmd/sink-worker/main.go | 3 ++- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/cmd/balance-worker/main.go b/cmd/balance-worker/main.go index 2d9425a84..8ce0c9df2 100644 --- a/cmd/balance-worker/main.go +++ b/cmd/balance-worker/main.go @@ -87,7 +87,9 @@ func main() { err = conf.Validate() if err != nil { - panic(err) + println("configuration error:") + println(err.Error()) + os.Exit(1) } extraResources, _ := resource.New( diff --git a/cmd/notification-service/main.go b/cmd/notification-service/main.go index 2278aa82d..98f2dd171 100644 --- a/cmd/notification-service/main.go +++ b/cmd/notification-service/main.go @@ -89,7 +89,9 @@ func main() { err = conf.Validate() if err != nil { - panic(err) + println("configuration error:") + println(err.Error()) + os.Exit(1) } extraResources, _ := resource.New( diff --git a/cmd/server/main.go b/cmd/server/main.go index 88b649449..ba0ad67c6 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -112,7 +112,9 @@ func main() { err = conf.Validate() if err != nil { - panic(err) + println("configuration error:") + println(err.Error()) + os.Exit(1) } extraResources, _ := resource.New( diff --git a/cmd/sink-worker/main.go b/cmd/sink-worker/main.go index ffbf7446b..19f8d5f14 100644 --- a/cmd/sink-worker/main.go +++ b/cmd/sink-worker/main.go @@ -83,7 +83,8 @@ func main() { err = conf.Validate() if err != nil { - slog.Error("invalid configuration", slog.String("error", err.Error())) + println("configuration error:") + println(err.Error()) os.Exit(1) } From eb1e2de34bc96c40ff83016dd2de661512f59fd8 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 2 Oct 2024 14:43:49 +0200 Subject: [PATCH 4/5] fix: lint violations Signed-off-by: Mark Sagi-Kazar --- config/notification.go | 2 +- config/sink.go | 3 ++- config/telemetry.go | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/config/notification.go b/config/notification.go index b44243cc1..363dcc263 100644 --- a/config/notification.go +++ b/config/notification.go @@ -4,10 +4,10 @@ import ( "errors" "time" - "github.com/openmeterio/openmeter/pkg/errorsx" "github.com/spf13/viper" "github.com/openmeterio/openmeter/openmeter/notification/webhook" + "github.com/openmeterio/openmeter/pkg/errorsx" ) type WebhookConfiguration struct { diff --git a/config/sink.go b/config/sink.go index 40041c6c2..19aea7bb5 100644 --- a/config/sink.go +++ b/config/sink.go @@ -4,8 +4,9 @@ import ( "errors" "time" - "github.com/openmeterio/openmeter/pkg/errorsx" "github.com/spf13/viper" + + "github.com/openmeterio/openmeter/pkg/errorsx" ) type SinkConfiguration struct { diff --git a/config/telemetry.go b/config/telemetry.go index b6e1468e3..81f6ed3e4 100644 --- a/config/telemetry.go +++ b/config/telemetry.go @@ -13,7 +13,6 @@ import ( "github.com/golang-cz/devslog" "github.com/lmittmann/tint" - "github.com/openmeterio/openmeter/pkg/errorsx" "github.com/spf13/pflag" "github.com/spf13/viper" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" @@ -24,6 +23,8 @@ import ( sdktrace "go.opentelemetry.io/otel/sdk/trace" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + + "github.com/openmeterio/openmeter/pkg/errorsx" ) type OTLPExporterTelemetryConfig struct { From ede9550581484c9de27b461a0c39fefea7e812a9 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 2 Oct 2024 14:46:39 +0200 Subject: [PATCH 5/5] feat: add config validation flag Signed-off-by: Mark Sagi-Kazar --- cmd/balance-worker/main.go | 5 +++++ cmd/notification-service/main.go | 5 +++++ cmd/server/main.go | 5 +++++ cmd/sink-worker/main.go | 5 +++++ 4 files changed, 20 insertions(+) diff --git a/cmd/balance-worker/main.go b/cmd/balance-worker/main.go index 8ce0c9df2..3a3274c67 100644 --- a/cmd/balance-worker/main.go +++ b/cmd/balance-worker/main.go @@ -61,6 +61,7 @@ func main() { flags.String("config", "", "Configuration file") flags.Bool("version", false, "Show version information") + flags.Bool("validate", false, "Validate configuration and exit") _ = flags.Parse(os.Args[1:]) @@ -92,6 +93,10 @@ func main() { os.Exit(1) } + if v, _ := flags.GetBool("validate"); v { + os.Exit(0) + } + extraResources, _ := resource.New( context.Background(), resource.WithContainer(), diff --git a/cmd/notification-service/main.go b/cmd/notification-service/main.go index 98f2dd171..de8e3f302 100644 --- a/cmd/notification-service/main.go +++ b/cmd/notification-service/main.go @@ -63,6 +63,7 @@ func main() { flags.String("config", "", "Configuration file") flags.Bool("version", false, "Show version information") + flags.Bool("validate", false, "Validate configuration and exit") _ = flags.Parse(os.Args[1:]) @@ -94,6 +95,10 @@ func main() { os.Exit(1) } + if v, _ := flags.GetBool("validate"); v { + os.Exit(0) + } + extraResources, _ := resource.New( context.Background(), resource.WithContainer(), diff --git a/cmd/server/main.go b/cmd/server/main.go index ba0ad67c6..3b18215c2 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -86,6 +86,7 @@ func main() { flags.String("config", "", "Configuration file") flags.Bool("version", false, "Show version information") + flags.Bool("validate", false, "Validate configuration and exit") _ = flags.Parse(os.Args[1:]) @@ -117,6 +118,10 @@ func main() { os.Exit(1) } + if v, _ := flags.GetBool("validate"); v { + os.Exit(0) + } + extraResources, _ := resource.New( context.Background(), resource.WithContainer(), diff --git a/cmd/sink-worker/main.go b/cmd/sink-worker/main.go index 19f8d5f14..2bed643c3 100644 --- a/cmd/sink-worker/main.go +++ b/cmd/sink-worker/main.go @@ -55,6 +55,7 @@ func main() { flags.String("config", "", "Configuration file") flags.Bool("version", false, "Show version information") + flags.Bool("validate", false, "Validate configuration and exit") _ = flags.Parse(os.Args[1:]) @@ -88,6 +89,10 @@ func main() { os.Exit(1) } + if v, _ := flags.GetBool("validate"); v { + os.Exit(0) + } + extraResources, _ := resource.New( ctx, resource.WithContainer(),