Skip to content

Commit

Permalink
Deprecate all types and funcs in config package
Browse files Browse the repository at this point in the history
The main reason is to remove the circular dependency between the config (including sub-packages) and component. Here is the current state:
* component depends on config
* config/sub-package[grpc, http, etc.] depends on config & component

Because of this "circular" dependency, we cannot split for example "config" into its own module, only if all the other config sub-packages are also split.

Signed-off-by: Bogdan <bogdandrutu@gmail.com>
  • Loading branch information
bogdandrutu committed Oct 27, 2022
1 parent 07603a0 commit bee1ea8
Show file tree
Hide file tree
Showing 146 changed files with 1,350 additions and 1,316 deletions.
30 changes: 30 additions & 0 deletions .chloggen/deprecateallconfig.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: deprecation

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Deprecate all types and funcs in `config` package

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

# (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: |
- config.Type => component.Type
- config.DataType => component.DataType
- config.Receiver => component.ReceiverConfig
- config.UnmarshalReceiver => component.UnmarshalReceiverConfig
- config.[New]ReceiverSettings => component.[New]ReceiverConfigSettings
- config.Processor => component.ProcessorConfig
- config.UnmarshalProcessor => component.UnmarshalProcessorConfig
- config.[New]ProcessorSettings => component.[New]ProcessorConfigSettings
- config.Exporter => component.ExporterConfig
- config.UnmarshalExporter => component.UnmarshalExporterConfig
- config.[New]ExporterSettings => component.[New]ExporterConfigSettings
- config.Extension => component.ExtensionConfig
- config.UnmarshalExtension => component.UnmarshalExtensionConfig
- config.[New]ExtensionSettings => component.[New]ExtensionConfigSettings
10 changes: 5 additions & 5 deletions cmd/otelcorecol/components_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 3 additions & 5 deletions component/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ package component // import "go.opentelemetry.io/collector/component"
import (
"context"
"errors"

"go.opentelemetry.io/collector/config"
)

var (
Expand Down Expand Up @@ -162,17 +160,17 @@ func (sl StabilityLevel) LogMessage() string {
// use the factory helpers for the appropriate component type.
type Factory interface {
// Type gets the type of the component created by this factory.
Type() config.Type
Type() Type

unexportedFactoryFunc()
}

type baseFactory struct {
cfgType config.Type
cfgType Type
}

func (baseFactory) unexportedFactoryFunc() {}

func (bf baseFactory) Type() config.Type {
func (bf baseFactory) Type() Type {
return bf.cfgType
}
147 changes: 147 additions & 0 deletions component/componenttest/configtest.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package componenttest // import "go.opentelemetry.io/collector/component/componenttest"

import (
"fmt"
"reflect"
"regexp"
"strings"

"go.uber.org/multierr"
)

// The regular expression for valid config field tag.
var configFieldTagRegExp = regexp.MustCompile("^[a-z0-9][a-z0-9_]*$")

// CheckConfigStruct enforces that given configuration object is following the patterns
// used by the collector. This ensures consistency between different implementations
// of components and extensions. It is recommended for implementers of components
// to call this function on their tests passing the default configuration of the
// component factory.
func CheckConfigStruct(config interface{}) error {
t := reflect.TypeOf(config)
if t.Kind() == reflect.Ptr {
t = t.Elem()
}

if t.Kind() != reflect.Struct {
return fmt.Errorf("config must be a struct or a pointer to one, the passed object is a %s", t.Kind())
}

return validateConfigDataType(t)
}

// validateConfigDataType performs a descending validation of the given type.
// If the type is a struct it goes to each of its fields to check for the proper
// tags.
func validateConfigDataType(t reflect.Type) error {
var errs error

switch t.Kind() {
case reflect.Ptr:
errs = multierr.Append(errs, validateConfigDataType(t.Elem()))
case reflect.Struct:
// Reflect on the pointed data and check each of its fields.
nf := t.NumField()
for i := 0; i < nf; i++ {
f := t.Field(i)
errs = multierr.Append(errs, checkStructFieldTags(f))
}
default:
// The config object can carry other types but they are not used when
// reading the configuration via koanf so ignore them. Basically ignore:
// reflect.Uintptr, reflect.Chan, reflect.Func, reflect.Interface, and
// reflect.UnsafePointer.
}

if errs != nil {
return fmt.Errorf("type %q from package %q has invalid config settings: %w", t.Name(), t.PkgPath(), errs)
}

return nil
}

// checkStructFieldTags inspects the tags of a struct field.
func checkStructFieldTags(f reflect.StructField) error {

tagValue := f.Tag.Get("mapstructure")
if tagValue == "" {

// Ignore special types.
switch f.Type.Kind() {
case reflect.Interface, reflect.Chan, reflect.Func, reflect.Uintptr, reflect.UnsafePointer:
// Allow the config to carry the types above, but since they are not read
// when loading configuration, just ignore them.
return nil
}

// Public fields of other types should be tagged.
chars := []byte(f.Name)
if len(chars) > 0 && chars[0] >= 'A' && chars[0] <= 'Z' {
return fmt.Errorf("mapstructure tag not present on field %q", f.Name)
}

// Not public field, no need to have a tag.
return nil
}

tagParts := strings.Split(tagValue, ",")
if tagParts[0] != "" {
if tagParts[0] == "-" {
// Nothing to do, as mapstructure decode skips this field.
return nil
}
}

// Check if squash is specified.
squash := false
for _, tag := range tagParts[1:] {
if tag == "squash" {
squash = true
break
}
}

if squash {
// Field was squashed.
if (f.Type.Kind() != reflect.Struct) && (f.Type.Kind() != reflect.Ptr || f.Type.Elem().Kind() != reflect.Struct) {
return fmt.Errorf(
"attempt to squash non-struct type on field %q", f.Name)
}
}

switch f.Type.Kind() {
case reflect.Struct:
// It is another struct, continue down-level.
return validateConfigDataType(f.Type)

case reflect.Map, reflect.Slice, reflect.Array:
// The element of map, array, or slice can be itself a configuration object.
return validateConfigDataType(f.Type.Elem())

default:
fieldTag := tagParts[0]
if !configFieldTagRegExp.MatchString(fieldTag) {
return fmt.Errorf(
"field %q has config tag %q which doesn't satisfy %q",
f.Name,
fieldTag,
configFieldTagRegExp.String())
}
}

return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package configtest
package componenttest

import (
"io"
Expand Down
13 changes: 6 additions & 7 deletions component/componenttest/nop_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer/consumertest"
)

Expand All @@ -31,16 +30,16 @@ func NewNopExporterCreateSettings() component.ExporterCreateSettings {
}

type nopExporterConfig struct {
config.ExporterSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
component.ExporterConfigSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
}

// NewNopExporterFactory returns a component.ExporterFactory that constructs nop exporters.
func NewNopExporterFactory() component.ExporterFactory {
return component.NewExporterFactory(
"nop",
func() config.Exporter {
func() component.ExporterConfig {
return &nopExporterConfig{
ExporterSettings: config.NewExporterSettings(config.NewComponentID("nop")),
ExporterConfigSettings: component.NewExporterConfigSettings(component.NewID("nop")),
}
},
component.WithTracesExporter(createTracesExporter, component.StabilityLevelStable),
Expand All @@ -49,15 +48,15 @@ func NewNopExporterFactory() component.ExporterFactory {
)
}

func createTracesExporter(context.Context, component.ExporterCreateSettings, config.Exporter) (component.TracesExporter, error) {
func createTracesExporter(context.Context, component.ExporterCreateSettings, component.ExporterConfig) (component.TracesExporter, error) {
return nopExporterInstance, nil
}

func createMetricsExporter(context.Context, component.ExporterCreateSettings, config.Exporter) (component.MetricsExporter, error) {
func createMetricsExporter(context.Context, component.ExporterCreateSettings, component.ExporterConfig) (component.MetricsExporter, error) {
return nopExporterInstance, nil
}

func createLogsExporter(context.Context, component.ExporterCreateSettings, config.Exporter) (component.LogsExporter, error) {
func createLogsExporter(context.Context, component.ExporterCreateSettings, component.ExporterConfig) (component.LogsExporter, error) {
return nopExporterInstance, nil
}

Expand Down
6 changes: 3 additions & 3 deletions component/componenttest/nop_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/pdata/plog"
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/pdata/ptrace"
Expand All @@ -30,9 +30,9 @@ import (
func TestNewNopExporterFactory(t *testing.T) {
factory := NewNopExporterFactory()
require.NotNil(t, factory)
assert.Equal(t, config.Type("nop"), factory.Type())
assert.Equal(t, component.Type("nop"), factory.Type())
cfg := factory.CreateDefaultConfig()
assert.Equal(t, &nopExporterConfig{ExporterSettings: config.NewExporterSettings(config.NewComponentID("nop"))}, cfg)
assert.Equal(t, &nopExporterConfig{ExporterConfigSettings: component.NewExporterConfigSettings(component.NewID("nop"))}, cfg)

traces, err := factory.CreateTracesExporter(context.Background(), NewNopExporterCreateSettings(), cfg)
require.NoError(t, err)
Expand Down
9 changes: 4 additions & 5 deletions component/componenttest/nop_extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
)

// NewNopExtensionCreateSettings returns a new nop settings for Create*Extension functions.
Expand All @@ -30,19 +29,19 @@ func NewNopExtensionCreateSettings() component.ExtensionCreateSettings {
}

type nopExtensionConfig struct {
config.ExtensionSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
component.ExtensionConfigSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
}

// NewNopExtensionFactory returns a component.ExtensionFactory that constructs nop extensions.
func NewNopExtensionFactory() component.ExtensionFactory {
return component.NewExtensionFactory(
"nop",
func() config.Extension {
func() component.ExtensionConfig {
return &nopExtensionConfig{
ExtensionSettings: config.NewExtensionSettings(config.NewComponentID("nop")),
ExtensionConfigSettings: component.NewExtensionConfigSettings(component.NewID("nop")),
}
},
func(context.Context, component.ExtensionCreateSettings, config.Extension) (component.Extension, error) {
func(context.Context, component.ExtensionCreateSettings, component.ExtensionConfig) (component.Extension, error) {
return nopExtensionInstance, nil
},
component.StabilityLevelStable)
Expand Down
6 changes: 3 additions & 3 deletions component/componenttest/nop_extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/component"
)

func TestNewNopExtensionFactory(t *testing.T) {
factory := NewNopExtensionFactory()
require.NotNil(t, factory)
assert.Equal(t, config.Type("nop"), factory.Type())
assert.Equal(t, component.Type("nop"), factory.Type())
cfg := factory.CreateDefaultConfig()
assert.Equal(t, &nopExtensionConfig{ExtensionSettings: config.NewExtensionSettings(config.NewComponentID("nop"))}, cfg)
assert.Equal(t, &nopExtensionConfig{ExtensionConfigSettings: component.NewExtensionConfigSettings(component.NewID("nop"))}, cfg)

traces, err := factory.CreateExtension(context.Background(), NewNopExtensionCreateSettings(), cfg)
require.NoError(t, err)
Expand Down
7 changes: 3 additions & 4 deletions component/componenttest/nop_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package componenttest // import "go.opentelemetry.io/collector/component/compone

import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
)

// nopHost mocks a receiver.ReceiverHost for test purposes.
Expand All @@ -29,14 +28,14 @@ func NewNopHost() component.Host {

func (nh *nopHost) ReportFatalError(_ error) {}

func (nh *nopHost) GetFactory(_ component.Kind, _ config.Type) component.Factory {
func (nh *nopHost) GetFactory(_ component.Kind, _ component.Type) component.Factory {
return nil
}

func (nh *nopHost) GetExtensions() map[config.ComponentID]component.Extension {
func (nh *nopHost) GetExtensions() map[component.ID]component.Extension {
return nil
}

func (nh *nopHost) GetExporters() map[config.DataType]map[config.ComponentID]component.Exporter {
func (nh *nopHost) GetExporters() map[component.DataType]map[component.ID]component.Exporter {
return nil
}
Loading

0 comments on commit bee1ea8

Please sign in to comment.