Skip to content

Commit

Permalink
Remove reference to defaultcomponents in core and deprecate `includ…
Browse files Browse the repository at this point in the history
…e_core` flag (#4087)

* remove defaultComponents from core and make it internal for tests

* remove package-lock.json file

* remove checkdoc from GitHub actions workflow

* add CHANGELOG entry

* add checkdoc to github workflow

* testing lint check

* remove-default-components-from-core

* add import statement

* fix Makefile

* add changelog entry

* lint check

* fix lint test

* run integration tests

* fix lint

* Modify changelog entry

* deprecate include_core flag

* modify include_core flag comment

* deprecate include_core flag revised

* deprecate include_core flag revised with log messages

* commit initial test for codecov

* fixed test

* add changelog entry
  • Loading branch information
JamesJHPark authored Dec 8, 2021
1 parent d505629 commit 29db3d3
Show file tree
Hide file tree
Showing 16 changed files with 88 additions and 559 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- Remove reference to `defaultcomponents` in core and deprecate `include_core` flag (#4087)

## 🛑 Breaking changes 🛑

- Remove `config.NewConfigMapFrom[File|Buffer]`, add testonly version (#4502)
Expand Down
1 change: 0 additions & 1 deletion cmd/builder/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ dist:
module: github.com/open-telemetry/opentelemetry-collector # the module name for the new distribution, following Go mod conventions. Optional, but recommended.
name: otelcol-custom # the binary name. Optional.
description: "Custom OpenTelemetry Collector distribution" # a long name for the application. Optional.
include_core: true # whether the core components should be included in the distribution. Optional.
otelcol_version: "0.40.0" # the OpenTelemetry Collector version to use as base for the distribution. Optional.
output_path: /tmp/otelcol-distributionNNN # the path to write the output (sources and binary). Optional.
version: "1.0.0" # the version for your custom OpenTelemetry Collector. Optional.
Expand Down
15 changes: 12 additions & 3 deletions cmd/builder/internal/builder/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@ type Distribution struct {
Go string `mapstructure:"go"`
Description string `mapstructure:"description"`
OtelColVersion string `mapstructure:"otelcol_version"`
IncludeCore bool `mapstructure:"include_core"`
OutputPath string `mapstructure:"output_path"`
Version string `mapstructure:"version"`
// IncludeCore is deprecated and note that if this is being used, it will be removed in a subsequent release
IncludeCore bool `mapstructure:"include_core"`
OutputPath string `mapstructure:"output_path"`
Version string `mapstructure:"version"`
}

// Module represents a receiver, exporter, processor or extension for the distribution
Expand Down Expand Up @@ -100,6 +101,14 @@ func (c *Config) Validate() error {
}
c.Distribution.Go = path
}

// create a warning message that include_core is deprecated and will be removed in a subsequent release
if c.Distribution.IncludeCore {
c.Logger.Warn("IncludeCore is deprecated. Starting from v0.41.0, you need to include all components explicitly.")
} else {
c.Logger.Info("IncludeCore is deprecated, starting from v0.41.0. The new behavior won't affect your distribution, just remove the option from the manifest.")
}

c.Logger.Info("Using go", zap.String("go-executable", c.Distribution.Go))

return nil
Expand Down
6 changes: 6 additions & 0 deletions cmd/builder/internal/builder/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,9 @@ func TestDefaultConfig(t *testing.T) {
require.NoError(t, cfg.ParseModules())
require.NoError(t, cfg.Validate())
}

func TestValidateDeprecatedIncludeCoreWarnLog(t *testing.T) {
cfg := DefaultConfig()
cfg.Distribution.IncludeCore = true
require.NoError(t, cfg.Validate())
}
7 changes: 6 additions & 1 deletion cmd/builder/internal/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,12 @@ func Command() (*cobra.Command, error) {
cmd.Flags().StringVar(&cfg.Distribution.Name, "name", "otelcol-custom", "The executable name for the OpenTelemetry Collector distribution")
cmd.Flags().StringVar(&cfg.Distribution.Description, "description", "Custom OpenTelemetry Collector distribution", "A descriptive name for the OpenTelemetry Collector distribution")
cmd.Flags().StringVar(&cfg.Distribution.Version, "version", "1.0.0", "The version for the OpenTelemetry Collector distribution")
cmd.Flags().BoolVar(&cfg.Distribution.IncludeCore, "include-core", true, "Whether the core components should be included in the distribution")
// IncludeCore is deprecated and will be removed in a subsequent release
cmd.Flags().BoolVar(&cfg.Distribution.IncludeCore, "include-core", true, "Deprecated: starting from v0.41.0, core components are not going to be implicitly included.")
if err := cmd.Flags().MarkDeprecated("include-core", "IncludeCore is deprecated, please explicitly list all the components required"); err != nil {
cfg.Logger.Error("failed to mark the IncludeCore flag is deprecated", zap.Error(err))
return nil, err
}
cmd.Flags().StringVar(&cfg.Distribution.OtelColVersion, "otelcol-version", cfg.Distribution.OtelColVersion, "Which version of OpenTelemetry Collector to use as base")
cmd.Flags().StringVar(&cfg.Distribution.OutputPath, "output-path", cfg.Distribution.OutputPath, "Where to write the resulting files")
cmd.Flags().StringVar(&cfg.Distribution.Go, "go", "", "The Go binary to use during the compilation phase. Default: go from the PATH")
Expand Down
8 changes: 4 additions & 4 deletions cmd/builder/test/nocore.builder.yaml
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
dist:
module: go.opentelemetry.io/collector/builder/test/nocore
otelcol_version: 0.38.0
otelcol_version: 0.40.0
include_core: false

receivers:
- import: go.opentelemetry.io/collector/receiver/otlpreceiver
gomod: go.opentelemetry.io/collector v0.38.0
gomod: go.opentelemetry.io/collector v0.40.0
exporters:
- import: go.opentelemetry.io/collector/exporter/loggingexporter
gomod: go.opentelemetry.io/collector v0.38.0
gomod: go.opentelemetry.io/collector v0.40.0
extensions:
- import: go.opentelemetry.io/collector/extension/zpagesextension
gomod: go.opentelemetry.io/collector v0.38.0
gomod: go.opentelemetry.io/collector v0.40.0
8 changes: 4 additions & 4 deletions cmd/builder/test/replaces.builder.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
dist:
module: go.opentelemetry.io/collector/builder/test/replaces
otelcol_version: 0.38.0
otelcol_version: 0.40.0

processors:
- gomod: github.com/open-telemetry/opentelemetry-collector-contrib/processor/routingprocessor v0.38.0
- gomod: github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor v0.38.0
- gomod: github.com/open-telemetry/opentelemetry-collector-contrib/processor/routingprocessor v0.40.0
- gomod: github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor v0.40.0

replaces:
- github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => github.com/open-telemetry/opentelemetry-collector-contrib/internal/common v0.38.0
- github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => github.com/open-telemetry/opentelemetry-collector-contrib/internal/common v0.40.0
43 changes: 43 additions & 0 deletions internal/testcomponents/example_factories.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@
package testcomponents // import "go.opentelemetry.io/collector/internal/testcomponents"

import (
"go.uber.org/multierr"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/exporter/otlpexporter"
"go.opentelemetry.io/collector/extension/zpagesextension"
"go.opentelemetry.io/collector/processor/batchprocessor"
"go.opentelemetry.io/collector/receiver/otlpreceiver"
)

// ExampleComponents registers example factories. This is only used by tests.
Expand All @@ -39,3 +45,40 @@ func ExampleComponents() (

return
}

// DefaultFactories returns the set of components in "testdata/otelcol-config.yaml". This is only used by tests.
func DefaultFactories() (
component.Factories,
error,
) {
var errs error

extensions, err := component.MakeExtensionFactoryMap(
zpagesextension.NewFactory(),
)
errs = multierr.Append(errs, err)

receivers, err := component.MakeReceiverFactoryMap(
otlpreceiver.NewFactory(),
)
errs = multierr.Append(errs, err)

processors, err := component.MakeProcessorFactoryMap(
batchprocessor.NewFactory(),
)
errs = multierr.Append(errs, err)

exporters, err := component.MakeExporterFactoryMap(
otlpexporter.NewFactory(),
)
errs = multierr.Append(errs, err)

factories := component.Factories{
Extensions: extensions,
Receivers: receivers,
Processors: processors,
Exporters: exporters,
}

return factories, errs
}
8 changes: 4 additions & 4 deletions service/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configmapprovider"
"go.opentelemetry.io/collector/internal/testcomponents"
"go.opentelemetry.io/collector/internal/testutil"
"go.opentelemetry.io/collector/service/defaultcomponents"
)

// TestCollector_StartAsGoRoutine must be the first unit test on the file,
Expand All @@ -48,7 +48,7 @@ func TestCollector_StartAsGoRoutine(t *testing.T) {
collectorTelemetry = &colTelemetry{}
defer func() { collectorTelemetry = preservedAppTelemetry }()

factories, err := defaultcomponents.Components()
factories, err := testcomponents.DefaultFactories()
require.NoError(t, err)

set := CollectorSettings{
Expand Down Expand Up @@ -81,7 +81,7 @@ func TestCollector_StartAsGoRoutine(t *testing.T) {
}

func TestCollector_Start(t *testing.T) {
factories, err := defaultcomponents.Components()
factories, err := testcomponents.DefaultFactories()
require.NoError(t, err)

loggingHookCalled := false
Expand Down Expand Up @@ -153,7 +153,7 @@ func TestCollector_ReportError(t *testing.T) {
collectorTelemetry = &mockColTelemetry{}
defer func() { collectorTelemetry = preservedAppTelemetry }()

factories, err := defaultcomponents.Components()
factories, err := testcomponents.DefaultFactories()
require.NoError(t, err)

col, err := New(CollectorSettings{
Expand Down
4 changes: 2 additions & 2 deletions service/collector_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ import (
"golang.org/x/sys/windows/svc"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/service/defaultcomponents"
"go.opentelemetry.io/collector/internal/testcomponents"
)

func TestWindowsService_Execute(t *testing.T) {
os.Args = []string{"otelcol", "--config", path.Join("testdata", "otelcol-config.yaml")}

factories, err := defaultcomponents.Components()
factories, err := testcomponents.DefaultFactories()
require.NoError(t, err)

s := NewWindowsService(CollectorSettings{BuildInfo: component.NewDefaultBuildInfo(), Factories: factories})
Expand Down
4 changes: 2 additions & 2 deletions service/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/service/defaultcomponents"
"go.opentelemetry.io/collector/internal/testcomponents"
)

func TestNewCommand(t *testing.T) {
Expand All @@ -40,7 +40,7 @@ func TestNewCommandMapProviderIsNil(t *testing.T) {
}

func TestNewCommandInvalidFactories(t *testing.T) {
factories, err := defaultcomponents.Components()
factories, err := testcomponents.ExampleComponents()
require.NoError(t, err)
f := &badConfigExtensionFactory{}
factories.Extensions[f.Type()] = f
Expand Down
6 changes: 3 additions & 3 deletions service/configcheck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,19 @@ import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/internal/internalinterface"
"go.opentelemetry.io/collector/service/defaultcomponents"
"go.opentelemetry.io/collector/internal/testcomponents"
)

func TestValidateConfigFromFactories_Success(t *testing.T) {
factories, err := defaultcomponents.Components()
factories, err := testcomponents.ExampleComponents()
require.NoError(t, err)

err = validateConfigFromFactories(factories)
require.NoError(t, err)
}

func TestValidateConfigFromFactories_Failure(t *testing.T) {
factories, err := defaultcomponents.Components()
factories, err := testcomponents.ExampleComponents()
require.NoError(t, err)

// Add a factory returning config not following pattern to force error.
Expand Down
Loading

0 comments on commit 29db3d3

Please sign in to comment.