From 8de544b176179a21637baf094b833969786f4479 Mon Sep 17 00:00:00 2001 From: Kristina Pathak Date: Mon, 8 Jul 2024 12:06:53 -0700 Subject: [PATCH] [builder] more information for missing gomod error (#10475) #### Description improving an error message - a missing gomod field would be reported but without informing where in the config the field is missing. #### Link to tracking issue Fixes #10474 #### Testing Unit tests pass, added some tests for missing cases, manually tested new error looks like: ``` ../../bin/ocb_darwin_amd64 --config=./default.yaml 2024-06-27T11:39:10.316-0700 INFO internal/command.go:125 OpenTelemetry Collector Builder {"version": "", "date": "unknown"} 2024-06-27T11:39:10.319-0700 INFO internal/command.go:161 Using config file {"path": "./default.yaml"} Error: invalid configuration: receiver module at index 0: missing gomod specification for module; provider module at index 2: missing gomod specification for module ``` --- .chloggen/builder-empty-gomod-field.yaml | 25 ++++++++++++++++++ cmd/builder/internal/builder/config.go | 22 ++++++++-------- cmd/builder/internal/builder/config_test.go | 28 +++++++++++++++++---- 3 files changed, 59 insertions(+), 16 deletions(-) create mode 100644 .chloggen/builder-empty-gomod-field.yaml diff --git a/.chloggen/builder-empty-gomod-field.yaml b/.chloggen/builder-empty-gomod-field.yaml new file mode 100644 index 00000000000..2ebba3ce204 --- /dev/null +++ b/.chloggen/builder-empty-gomod-field.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: builder + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: provide context when a module in the config is missing its gomod value + +# One or more tracking issues or pull requests related to the change +issues: [10474] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/cmd/builder/internal/builder/config.go b/cmd/builder/internal/builder/config.go index 8ce3316ea72..45636cd5fdf 100644 --- a/cmd/builder/internal/builder/config.go +++ b/cmd/builder/internal/builder/config.go @@ -19,8 +19,8 @@ import ( const defaultOtelColVersion = "0.104.0" -// ErrInvalidGoMod indicates an invalid gomod -var ErrInvalidGoMod = errors.New("invalid gomod specification for module") +// ErrMissingGoMod indicates an empty gomod field +var ErrMissingGoMod = errors.New("missing gomod specification for module") // Config holds the builder's configuration type Config struct { @@ -115,14 +115,14 @@ func NewDefaultConfig() Config { func (c *Config) Validate() error { var providersError error if c.Providers != nil { - providersError = validateModules(*c.Providers) + providersError = validateModules("provider", *c.Providers) } return multierr.Combine( - validateModules(c.Extensions), - validateModules(c.Receivers), - validateModules(c.Exporters), - validateModules(c.Processors), - validateModules(c.Connectors), + validateModules("extension", c.Extensions), + validateModules("receiver", c.Receivers), + validateModules("exporter", c.Exporters), + validateModules("processor", c.Processors), + validateModules("connector", c.Connectors), providersError, ) } @@ -235,10 +235,10 @@ func (c *Config) ParseModules() error { return nil } -func validateModules(mods []Module) error { - for _, mod := range mods { +func validateModules(name string, mods []Module) error { + for i, mod := range mods { if mod.GoMod == "" { - return fmt.Errorf("module %q: %w", mod.GoMod, ErrInvalidGoMod) + return fmt.Errorf("%s module at index %v: %w", name, i, ErrMissingGoMod) } } return nil diff --git a/cmd/builder/internal/builder/config_test.go b/cmd/builder/internal/builder/config_test.go index cd81b9f2887..9daf158cb8e 100644 --- a/cmd/builder/internal/builder/config_test.go +++ b/cmd/builder/internal/builder/config_test.go @@ -79,13 +79,22 @@ func TestModuleFromCore(t *testing.T) { assert.True(t, strings.HasPrefix(cfg.Extensions[0].Name, "otlpreceiver")) } -func TestInvalidModule(t *testing.T) { +func TestMissingModule(t *testing.T) { type invalidModuleTest struct { cfg Config err error } // prepare configurations := []invalidModuleTest{ + { + cfg: Config{ + Logger: zap.NewNop(), + Providers: &[]Module{{ + Import: "invalid", + }}, + }, + err: ErrMissingGoMod, + }, { cfg: Config{ Logger: zap.NewNop(), @@ -93,7 +102,7 @@ func TestInvalidModule(t *testing.T) { Import: "invalid", }}, }, - err: ErrInvalidGoMod, + err: ErrMissingGoMod, }, { cfg: Config{ @@ -102,7 +111,7 @@ func TestInvalidModule(t *testing.T) { Import: "invalid", }}, }, - err: ErrInvalidGoMod, + err: ErrMissingGoMod, }, { cfg: Config{ @@ -111,7 +120,7 @@ func TestInvalidModule(t *testing.T) { Import: "invali", }}, }, - err: ErrInvalidGoMod, + err: ErrMissingGoMod, }, { cfg: Config{ @@ -120,7 +129,16 @@ func TestInvalidModule(t *testing.T) { Import: "invalid", }}, }, - err: ErrInvalidGoMod, + err: ErrMissingGoMod, + }, + { + cfg: Config{ + Logger: zap.NewNop(), + Connectors: []Module{{ + Import: "invalid", + }}, + }, + err: ErrMissingGoMod, }, }