Skip to content

Commit

Permalink
add confmap.Converter an option for ocb build manifest/template (#11584)
Browse files Browse the repository at this point in the history
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Adds ability to add confmap.Converter as a module to build/import with
ocb

<!-- Issue number if applicable -->
#### Link to tracking issue
Fixes
#11582

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Added validation tests for pulling the converter go module in
cmd/builder template/build process.
Additionally, I took the default otelcorecol manifest, added a
`converters` section, and added the `expandconverter` and verified that
a custom collector both built, started up, and exited successfully.

<!--Describe the documentation added.-->
#### Documentation
Changelog file added. I can also update the documentation in cmd/builder
README if community thinks that is necessary. Since `confmap.Converter`
is not yet registered in the OTel registry and does not have any
existing components published save for the deprecated `expandconverter`
I was not certain if anything further than a changelog was necessary.
<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Andrzej Stencel <[email protected]>
  • Loading branch information
jackgopack4 and andrzej-stencel authored Nov 6, 2024
1 parent e76145a commit 9cc15c5
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 4 deletions.
36 changes: 36 additions & 0 deletions .chloggen/builder-confmap-converters.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: cmd/builder

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Allow configuring `confmap.Converter` components in ocb.

# One or more tracking issues or pull requests related to the change
issues: [11582]

# (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: |
If no converters are specified, there will be no converters added.
Currently, the only published converter is `expandconverter` which is
deprecated as of v0.107.0, but can still be added for testing purposes.
To configure a custom converter, make sure your converter implements the converter
interface and is published as a go module (or replaced locally if not published).
You may then use the `converters` key in your OCB build manifest with a list of
Go modules (and replaces as necessary) to include your converter.
Please note that converters are order-dependent. The confmap will apply converters
in order of which they are listed in your manifest if there is more than one.
# 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: [user]
11 changes: 10 additions & 1 deletion cmd/builder/internal/builder/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type Config struct {
Processors []Module `mapstructure:"processors"`
Connectors []Module `mapstructure:"connectors"`
Providers []Module `mapstructure:"providers"`
Converters []Module `mapstructure:"converters"`
Replaces []string `mapstructure:"replaces"`
Excludes []string `mapstructure:"excludes"`

Expand Down Expand Up @@ -142,6 +143,7 @@ func (c *Config) Validate() error {
validateModules("processor", c.Processors),
validateModules("connector", c.Connectors),
validateModules("provider", c.Providers),
validateModules("converter", c.Converters),
)
}

Expand Down Expand Up @@ -194,7 +196,10 @@ func (c *Config) ParseModules() error {
if err != nil {
return err
}

c.Converters, err = parseModules(c.Converters)
if err != nil {
return err
}
return nil
}

Expand Down Expand Up @@ -226,6 +231,10 @@ func parseModules(mods []Module) ([]Module, error) {
if err != nil {
return mods, fmt.Errorf("module has a relative \"path\" element, but we couldn't resolve the current working dir: %w", err)
}
// Check if the path exists
if _, err := os.Stat(mod.Path); os.IsNotExist(err) {
return mods, fmt.Errorf("filepath does not exist: %s", mod.Path)
}
}

parsedModules = append(parsedModules, mod)
Expand Down
29 changes: 27 additions & 2 deletions cmd/builder/internal/builder/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,27 @@ func TestParseModules(t *testing.T) {
assert.Equal(t, "repo", cfg.Extensions[0].Name)
}

func TestInvalidConverter(t *testing.T) {
// Create a Config instance with invalid Converters
config := &Config{
Converters: []Module{
{
Path: "./invalid/module/path", // Invalid module path to trigger an error
},
},
}

// Call the method and expect an error
err := config.ParseModules()
require.Error(t, err, "expected an error when parsing invalid modules")
}

func TestRelativePath(t *testing.T) {
// prepare
cfg := Config{
Extensions: []Module{{
GoMod: "some-module",
Path: "./some-module",
Path: "./templates",
}},
}

Expand Down Expand Up @@ -116,7 +131,7 @@ func TestMissingModule(t *testing.T) {
cfg: Config{
Logger: zap.NewNop(),
Exporters: []Module{{
Import: "invali",
Import: "invalid",
}},
},
err: errMissingGoMod,
Expand All @@ -139,6 +154,15 @@ func TestMissingModule(t *testing.T) {
},
err: errMissingGoMod,
},
{
cfg: Config{
Logger: zap.NewNop(),
Converters: []Module{{
Import: "invalid",
}},
},
err: errMissingGoMod,
},
}

for _, test := range configurations {
Expand Down Expand Up @@ -235,6 +259,7 @@ func TestSkipsNilFieldValidation(t *testing.T) {
cfg, err := NewDefaultConfig()
require.NoError(t, err)
cfg.Providers = nil
cfg.Converters = nil
assert.NoError(t, cfg.Validate())
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/builder/internal/builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func processAndWrite(cfg *Config, tmpl *template.Template, outFile string, tmplP
}

func (c *Config) allComponents() []Module {
return slices.Concat[[]Module](c.Exporters, c.Receivers, c.Processors, c.Extensions, c.Connectors, c.Providers)
return slices.Concat[[]Module](c.Exporters, c.Receivers, c.Processors, c.Extensions, c.Connectors, c.Providers, c.Converters)
}

func (c *Config) readGoModFile() (string, map[string]string, error) {
Expand Down
3 changes: 3 additions & 0 deletions cmd/builder/internal/builder/templates/go.mod.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ require (
{{- range .Providers}}
{{if .GoMod}}{{.GoMod}}{{end}}
{{- end}}
{{- range .Converters}}
{{if .GoMod}}{{.GoMod}}{{end}}
{{- end}}
{{- range .Connectors}}
{{if .GoMod}}{{.GoMod}}{{end}}
{{- end}}
Expand Down
10 changes: 10 additions & 0 deletions cmd/builder/internal/builder/templates/main.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/confmap"
{{- range .Converters}}
{{.Name}} "{{.Import}}"
{{- end}}
{{- range .Providers}}
{{.Name}} "{{.Import}}"
{{- end}}
Expand All @@ -31,6 +34,13 @@ func main() {
{{.Name}}.NewFactory(),
{{- end}}
},
{{- if .Converters }}
ConverterFactories: []confmap.ConverterFactory{
{{- range .Converters}}
{{.Name}}.NewFactory(),
{{- end}}
},
{{- end }}
{{- if .ConfResolver.DefaultURIScheme }}
DefaultScheme: "{{ .ConfResolver.DefaultURIScheme }}",
{{- end }}
Expand Down

0 comments on commit 9cc15c5

Please sign in to comment.