Skip to content

Commit

Permalink
[cmd/builder] Fix otelcol_version being ignored (open-telemetry#8713)
Browse files Browse the repository at this point in the history
Adds `go.opentelemetry.io/collector/otelcol` to `go.mod` template for
the builder. This fixes open-telemetry#8692.

Since open-telemetry#8443, the `otelcol` folder is its own component. Before this
change, `otelcol_version` was enforced by the `collector` module
dependency:

https://github.com/open-telemetry/opentelemetry-collector/blob/287b98f6973fd6baa278150b9fca8c83abea0af4/cmd/builder/internal/builder/templates/go.mod.tmpl#L23

After this change, the `go mod tidy` step here:

https://github.com/open-telemetry/opentelemetry-collector/blob/287b98f6973fd6baa278150b9fca8c83abea0af4/cmd/builder/internal/builder/main.go#L115

will add the latest available version for the `otelcol` module since
none of the components actually depend on it. For example, with the
`v0.86.0` builder config the output is as follows:

```
❯ go mod tidy -v -compat=1.20
go: finding module for package go.opentelemetry.io/collector/otelcol
go: found go.opentelemetry.io/collector/otelcol in go.opentelemetry.io/collector/otelcol v0.87.0
```

Explicitly adding `otelcol` makes it so the `otelcol_version` is
correctly honored.

**Link to tracking Issue:** Fixes open-telemetry#8692
  • Loading branch information
mx-psi authored Oct 20, 2023
1 parent 287b98f commit 7c33b71
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 11 deletions.
25 changes: 25 additions & 0 deletions .chloggen/mx-psi_fix-builder.yaml
Original file line number Diff line number Diff line change
@@ -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: cmd/builder

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Fix ocb ignoring `otelcol_version` when set to v0.86.0 or later"

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

# (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: []
1 change: 1 addition & 0 deletions cmd/builder/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module go.opentelemetry.io/collector/cmd/builder
go 1.20

require (
github.com/hashicorp/go-version v1.6.0
github.com/knadh/koanf/parsers/yaml v0.1.0
github.com/knadh/koanf/providers/env v0.1.0
github.com/knadh/koanf/providers/file v0.1.0
Expand Down
2 changes: 2 additions & 0 deletions cmd/builder/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek=
github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/knadh/koanf/maps v0.1.1 h1:G5TjmUh2D7G2YWf5SQQqSiHRJEjaicvU0KpypqB3NIs=
Expand Down
35 changes: 26 additions & 9 deletions cmd/builder/internal/builder/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"path/filepath"
"strings"

"github.com/hashicorp/go-version"
"go.uber.org/multierr"
"go.uber.org/zap"
)
Expand Down Expand Up @@ -40,15 +41,16 @@ type Config struct {

// Distribution holds the parameters for the final binary
type Distribution struct {
Module string `mapstructure:"module"`
Name string `mapstructure:"name"`
Go string `mapstructure:"go"`
Description string `mapstructure:"description"`
OtelColVersion string `mapstructure:"otelcol_version"`
OutputPath string `mapstructure:"output_path"`
Version string `mapstructure:"version"`
BuildTags string `mapstructure:"build_tags"`
DebugCompilation bool `mapstructure:"debug_compilation"`
Module string `mapstructure:"module"`
Name string `mapstructure:"name"`
Go string `mapstructure:"go"`
Description string `mapstructure:"description"`
OtelColVersion string `mapstructure:"otelcol_version"`
RequireOtelColModule bool `mapstructure:"-"` // required for backwards-compatibility with builds older than 0.86.0
OutputPath string `mapstructure:"output_path"`
Version string `mapstructure:"version"`
BuildTags string `mapstructure:"build_tags"`
DebugCompilation bool `mapstructure:"debug_compilation"`
}

// Module represents a receiver, exporter, processor or extension for the distribution
Expand Down Expand Up @@ -108,6 +110,21 @@ func (c *Config) SetGoPath() error {
return nil
}

func (c *Config) SetRequireOtelColModule() error {
constraint, err := version.NewConstraint(">= 0.86.0")
if err != nil {
return err
}

otelColVersion, err := version.NewVersion(c.Distribution.OtelColVersion)
if err != nil {
return err
}

c.Distribution.RequireOtelColModule = constraint.Check(otelColVersion)
return nil
}

// ParseModules will parse the Modules entries and populate the missing values
func (c *Config) ParseModules() error {
var err error
Expand Down
33 changes: 33 additions & 0 deletions cmd/builder/internal/builder/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,36 @@ func TestDebugOptionSetConfig(t *testing.T) {
assert.NoError(t, cfg.Validate())
assert.True(t, cfg.Distribution.DebugCompilation)
}

func TestRequireOtelColModule(t *testing.T) {
tests := []struct {
Version string
ExpectedRequireOtelColModule bool
}{
{
Version: "0.85.0",
ExpectedRequireOtelColModule: false,
},
{
Version: "0.86.0",
ExpectedRequireOtelColModule: true,
},
{
Version: "0.86.1",
ExpectedRequireOtelColModule: true,
},
{
Version: "1.0.0",
ExpectedRequireOtelColModule: true,
},
}

for _, tt := range tests {
t.Run(tt.Version, func(t *testing.T) {
cfg := NewDefaultConfig()
cfg.Distribution.OtelColVersion = tt.Version
require.NoError(t, cfg.SetRequireOtelColModule())
assert.Equal(t, tt.ExpectedRequireOtelColModule, cfg.Distribution.RequireOtelColModule)
})
}
}
2 changes: 1 addition & 1 deletion cmd/builder/internal/builder/templates/go.mod.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ require (
{{- range .Processors}}
{{if .GoMod}}{{.GoMod}}{{end}}
{{- end}}
go.opentelemetry.io/collector v{{.Distribution.OtelColVersion}}
go.opentelemetry.io/collector{{if .Distribution.RequireOtelColModule}}/otelcol{{end}} v{{.Distribution.OtelColVersion}}
)

require (
Expand Down
4 changes: 4 additions & 0 deletions cmd/builder/internal/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ configuration is provided, ocb will generate a default Collector.
return fmt.Errorf("go not found: %w", err)
}

if err := cfg.SetRequireOtelColModule(); err != nil {
return fmt.Errorf("unable to compare otelcol version: %w", err)
}

if err := cfg.ParseModules(); err != nil {
return fmt.Errorf("invalid module configuration: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/otelcorecol/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ require (
go.opentelemetry.io/collector/extension v0.87.0
go.opentelemetry.io/collector/extension/ballastextension v0.87.0
go.opentelemetry.io/collector/extension/zpagesextension v0.87.0
go.opentelemetry.io/collector/otelcol v0.0.0-00010101000000-000000000000
go.opentelemetry.io/collector/otelcol v0.87.0
go.opentelemetry.io/collector/processor v0.87.0
go.opentelemetry.io/collector/processor/batchprocessor v0.87.0
go.opentelemetry.io/collector/processor/memorylimiterprocessor v0.87.0
Expand Down

0 comments on commit 7c33b71

Please sign in to comment.