From 2643e8256d9692e8582a7ca632f9b1d71199da0e Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Mon, 16 Sep 2024 14:00:15 -0700 Subject: [PATCH] [chore] Remove use of alias types in service/telemetry Signed-off-by: Bogdan Drutu --- service/telemetry/attributes_test.go | 3 +- service/telemetry/factory.go | 84 +++++++---- service/telemetry/factory_impl.go | 109 ++++++++++++++ .../{telemetry_test.go => factory_test.go} | 0 service/telemetry/internal/factory.go | 137 ------------------ service/telemetry/telemetry.go | 11 -- service/telemetry/tracer_test.go | 3 +- 7 files changed, 165 insertions(+), 182 deletions(-) create mode 100644 service/telemetry/factory_impl.go rename service/telemetry/{telemetry_test.go => factory_test.go} (100%) delete mode 100644 service/telemetry/internal/factory.go delete mode 100644 service/telemetry/telemetry.go diff --git a/service/telemetry/attributes_test.go b/service/telemetry/attributes_test.go index 6ace1a887af..c40a21f11f1 100644 --- a/service/telemetry/attributes_test.go +++ b/service/telemetry/attributes_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/service/telemetry/internal" ) func TestAttributes(t *testing.T) { @@ -50,7 +49,7 @@ func TestAttributes(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - attrs := attributes(internal.Settings{BuildInfo: tt.buildInfo}, tt.cfg) + attrs := attributes(Settings{BuildInfo: tt.buildInfo}, tt.cfg) require.Equal(t, tt.wantAttributes, attrs) }) } diff --git a/service/telemetry/factory.go b/service/telemetry/factory.go index 826c9cc8f74..474c245c22d 100644 --- a/service/telemetry/factory.go +++ b/service/telemetry/factory.go @@ -16,7 +16,6 @@ import ( "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/featuregate" "go.opentelemetry.io/collector/service/internal/resource" - "go.opentelemetry.io/collector/service/telemetry/internal" ) // disableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collector should enable @@ -27,46 +26,46 @@ var disableHighCardinalityMetricsfeatureGate = featuregate.GlobalRegistry().Must featuregate.WithRegisterDescription("controls whether the collector should enable potentially high"+ "cardinality metrics. The gate will be removed when the collector allows for view configuration.")) -func createDefaultConfig() component.Config { - return &Config{ - Logs: LogsConfig{ - Level: zapcore.InfoLevel, - Development: false, - Encoding: "console", - Sampling: &LogsSamplingConfig{ - Enabled: true, - Tick: 10 * time.Second, - Initial: 10, - Thereafter: 100, - }, - OutputPaths: []string{"stderr"}, - ErrorOutputPaths: []string{"stderr"}, - DisableCaller: false, - DisableStacktrace: false, - InitialFields: map[string]any(nil), - }, - Metrics: MetricsConfig{ - Level: configtelemetry.LevelNormal, - Address: ":8888", - }, - } +// Settings holds configuration for building Telemetry. +type Settings struct { + BuildInfo component.BuildInfo + AsyncErrorChannel chan error + ZapOptions []zap.Option } -// Factory is a telemetry factory. -type Factory = internal.Factory +// Factory is factory interface for telemetry. +// This interface cannot be directly implemented. Implementations must +// use the NewFactory to implement it. +type Factory interface { + // CreateDefaultConfig creates the default configuration for the telemetry. + // TODO: Should we just inherit from component.Factory? + CreateDefaultConfig() component.Config + + // CreateLogger creates a logger. + CreateLogger(ctx context.Context, set Settings, cfg component.Config) (*zap.Logger, error) + + // CreateTracerProvider creates a TracerProvider. + CreateTracerProvider(ctx context.Context, set Settings, cfg component.Config) (trace.TracerProvider, error) + + // CreateMeterProvider creates a MeterProvider. + CreateMeterProvider(ctx context.Context, set Settings, cfg component.Config) (metric.MeterProvider, error) + + // unexportedFactoryFunc is used to prevent external implementations of Factory. + unexportedFactoryFunc() +} // NewFactory creates a new Factory. func NewFactory() Factory { - return internal.NewFactory(createDefaultConfig, - internal.WithLogger(func(_ context.Context, set Settings, cfg component.Config) (*zap.Logger, error) { + return newFactory(createDefaultConfig, + withLogger(func(_ context.Context, set Settings, cfg component.Config) (*zap.Logger, error) { c := *cfg.(*Config) return newLogger(c.Logs, set.ZapOptions) }), - internal.WithTracerProvider(func(ctx context.Context, set Settings, cfg component.Config) (trace.TracerProvider, error) { + withTracerProvider(func(ctx context.Context, set Settings, cfg component.Config) (trace.TracerProvider, error) { c := *cfg.(*Config) return newTracerProvider(ctx, set, c) }), - internal.WithMeterProvider(func(_ context.Context, set Settings, cfg component.Config) (metric.MeterProvider, error) { + withMeterProvider(func(_ context.Context, set Settings, cfg component.Config) (metric.MeterProvider, error) { c := *cfg.(*Config) disableHighCard := disableHighCardinalityMetricsfeatureGate.IsEnabled() return newMeterProvider( @@ -80,3 +79,28 @@ func NewFactory() Factory { }), ) } + +func createDefaultConfig() component.Config { + return &Config{ + Logs: LogsConfig{ + Level: zapcore.InfoLevel, + Development: false, + Encoding: "console", + Sampling: &LogsSamplingConfig{ + Enabled: true, + Tick: 10 * time.Second, + Initial: 10, + Thereafter: 100, + }, + OutputPaths: []string{"stderr"}, + ErrorOutputPaths: []string{"stderr"}, + DisableCaller: false, + DisableStacktrace: false, + InitialFields: map[string]any(nil), + }, + Metrics: MetricsConfig{ + Level: configtelemetry.LevelNormal, + Address: ":8888", + }, + } +} diff --git a/service/telemetry/factory_impl.go b/service/telemetry/factory_impl.go new file mode 100644 index 00000000000..439012723a0 --- /dev/null +++ b/service/telemetry/factory_impl.go @@ -0,0 +1,109 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package telemetry // import "go.opentelemetry.io/collector/service/telemetry" + +import ( + "context" + + "go.opentelemetry.io/otel/metric" + metricnoop "go.opentelemetry.io/otel/metric/noop" + "go.opentelemetry.io/otel/trace" + tracenoop "go.opentelemetry.io/otel/trace/noop" + "go.uber.org/zap" + + "go.opentelemetry.io/collector/component" +) + +// factoryOption apply changes to Factory. +type factoryOption interface { + // applyTelemetryFactoryOption applies the option. + applyTelemetryFactoryOption(o *factory) +} + +var _ factoryOption = (*factoryOptionFunc)(nil) + +// factoryOptionFunc is an factoryOption created through a function. +type factoryOptionFunc func(*factory) + +func (f factoryOptionFunc) applyTelemetryFactoryOption(o *factory) { + f(o) +} + +var _ Factory = (*factory)(nil) + +// Factory is the implementation of Factory. +type factory struct { + createDefaultConfig component.CreateDefaultConfigFunc + createLoggerFunc + createTracerProviderFunc + createMeterProviderFunc +} + +func (f *factory) CreateDefaultConfig() component.Config { + return f.createDefaultConfig() +} + +// createLoggerFunc is the equivalent of Factory.CreateLogger. +type createLoggerFunc func(context.Context, Settings, component.Config) (*zap.Logger, error) + +// withLogger overrides the default no-op logger. +func withLogger(createLogger createLoggerFunc) factoryOption { + return factoryOptionFunc(func(o *factory) { + o.createLoggerFunc = createLogger + }) +} + +func (f *factory) CreateLogger(ctx context.Context, set Settings, cfg component.Config) (*zap.Logger, error) { + if f.createLoggerFunc == nil { + return zap.NewNop(), nil + } + return f.createLoggerFunc(ctx, set, cfg) +} + +// createTracerProviderFunc is the equivalent of Factory.CreateTracerProvider. +type createTracerProviderFunc func(context.Context, Settings, component.Config) (trace.TracerProvider, error) + +// withTracerProvider overrides the default no-op tracer provider. +func withTracerProvider(createTracerProvider createTracerProviderFunc) factoryOption { + return factoryOptionFunc(func(o *factory) { + o.createTracerProviderFunc = createTracerProvider + }) +} + +func (f *factory) CreateTracerProvider(ctx context.Context, set Settings, cfg component.Config) (trace.TracerProvider, error) { + if f.createTracerProviderFunc == nil { + return tracenoop.NewTracerProvider(), nil + } + return f.createTracerProviderFunc(ctx, set, cfg) +} + +// createMeterProviderFunc is the equivalent of Factory.CreateMeterProvider. +type createMeterProviderFunc func(context.Context, Settings, component.Config) (metric.MeterProvider, error) + +// withMeterProvider overrides the default no-op meter provider. +func withMeterProvider(createMeterProvider createMeterProviderFunc) factoryOption { + return factoryOptionFunc(func(o *factory) { + o.createMeterProviderFunc = createMeterProvider + }) +} + +func (f *factory) CreateMeterProvider(ctx context.Context, set Settings, cfg component.Config) (metric.MeterProvider, error) { + if f.createMeterProviderFunc == nil { + return metricnoop.NewMeterProvider(), nil + } + return f.createMeterProviderFunc(ctx, set, cfg) +} + +func (f *factory) unexportedFactoryFunc() {} + +// newFactory returns a new Factory. +func newFactory(createDefaultConfig component.CreateDefaultConfigFunc, options ...factoryOption) Factory { + f := &factory{ + createDefaultConfig: createDefaultConfig, + } + for _, op := range options { + op.applyTelemetryFactoryOption(f) + } + return f +} diff --git a/service/telemetry/telemetry_test.go b/service/telemetry/factory_test.go similarity index 100% rename from service/telemetry/telemetry_test.go rename to service/telemetry/factory_test.go diff --git a/service/telemetry/internal/factory.go b/service/telemetry/internal/factory.go deleted file mode 100644 index 5e8149d7401..00000000000 --- a/service/telemetry/internal/factory.go +++ /dev/null @@ -1,137 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package internal // import "go.opentelemetry.io/collector/service/telemetry/internal" - -import ( - "context" - - "go.opentelemetry.io/otel/metric" - metricnoop "go.opentelemetry.io/otel/metric/noop" - "go.opentelemetry.io/otel/trace" - tracenoop "go.opentelemetry.io/otel/trace/noop" - "go.uber.org/zap" - - "go.opentelemetry.io/collector/component" -) - -// Settings holds configuration for building Telemetry. -type Settings struct { - BuildInfo component.BuildInfo - AsyncErrorChannel chan error - ZapOptions []zap.Option -} - -// Factory is factory interface for telemetry. -// This interface cannot be directly implemented. Implementations must -// use the NewFactory to implement it. -type Factory interface { - // CreateDefaultConfig creates the default configuration for the telemetry. - // TODO: Should we just inherit from component.Factory? - CreateDefaultConfig() component.Config - - // CreateLogger creates a logger. - CreateLogger(ctx context.Context, set Settings, cfg component.Config) (*zap.Logger, error) - - // CreateTracerProvider creates a TracerProvider. - CreateTracerProvider(ctx context.Context, set Settings, cfg component.Config) (trace.TracerProvider, error) - - // CreateMeterProvider creates a MeterProvider. - CreateMeterProvider(ctx context.Context, set Settings, cfg component.Config) (metric.MeterProvider, error) - - // unexportedFactoryFunc is used to prevent external implementations of Factory. - unexportedFactoryFunc() -} - -// FactoryOption apply changes to Factory. -type FactoryOption interface { - // applyTelemetryFactoryOption applies the option. - applyTelemetryFactoryOption(o *factory) -} - -var _ FactoryOption = (*factoryOptionFunc)(nil) - -// factoryOptionFunc is an FactoryOption created through a function. -type factoryOptionFunc func(*factory) - -func (f factoryOptionFunc) applyTelemetryFactoryOption(o *factory) { - f(o) -} - -var _ Factory = (*factory)(nil) - -// factory is the implementation of Factory. -type factory struct { - createDefaultConfig component.CreateDefaultConfigFunc - CreateLoggerFunc - CreateTracerProviderFunc - CreateMeterProviderFunc -} - -func (f *factory) CreateDefaultConfig() component.Config { - return f.createDefaultConfig() -} - -// CreateLoggerFunc is the equivalent of Factory.CreateLogger. -type CreateLoggerFunc func(context.Context, Settings, component.Config) (*zap.Logger, error) - -// WithLogger overrides the default no-op logger. -func WithLogger(createLogger CreateLoggerFunc) FactoryOption { - return factoryOptionFunc(func(o *factory) { - o.CreateLoggerFunc = createLogger - }) -} - -func (f *factory) CreateLogger(ctx context.Context, set Settings, cfg component.Config) (*zap.Logger, error) { - if f.CreateLoggerFunc == nil { - return zap.NewNop(), nil - } - return f.CreateLoggerFunc(ctx, set, cfg) -} - -// CreateTracerProviderFunc is the equivalent of Factory.CreateTracerProvider. -type CreateTracerProviderFunc func(context.Context, Settings, component.Config) (trace.TracerProvider, error) - -// WithTracerProvider overrides the default no-op tracer provider. -func WithTracerProvider(createTracerProvider CreateTracerProviderFunc) FactoryOption { - return factoryOptionFunc(func(o *factory) { - o.CreateTracerProviderFunc = createTracerProvider - }) -} - -func (f *factory) CreateTracerProvider(ctx context.Context, set Settings, cfg component.Config) (trace.TracerProvider, error) { - if f.CreateTracerProviderFunc == nil { - return tracenoop.NewTracerProvider(), nil - } - return f.CreateTracerProviderFunc(ctx, set, cfg) -} - -// CreateMeterProviderFunc is the equivalent of Factory.CreateMeterProvider. -type CreateMeterProviderFunc func(context.Context, Settings, component.Config) (metric.MeterProvider, error) - -// WithMeterProvider overrides the default no-op meter provider. -func WithMeterProvider(createMeterProvider CreateMeterProviderFunc) FactoryOption { - return factoryOptionFunc(func(o *factory) { - o.CreateMeterProviderFunc = createMeterProvider - }) -} - -func (f *factory) CreateMeterProvider(ctx context.Context, set Settings, cfg component.Config) (metric.MeterProvider, error) { - if f.CreateMeterProviderFunc == nil { - return metricnoop.NewMeterProvider(), nil - } - return f.CreateMeterProviderFunc(ctx, set, cfg) -} - -func (f *factory) unexportedFactoryFunc() {} - -// NewFactory returns a new Factory. -func NewFactory(createDefaultConfig component.CreateDefaultConfigFunc, options ...FactoryOption) Factory { - f := &factory{ - createDefaultConfig: createDefaultConfig, - } - for _, op := range options { - op.applyTelemetryFactoryOption(f) - } - return f -} diff --git a/service/telemetry/telemetry.go b/service/telemetry/telemetry.go deleted file mode 100644 index 8ca29332924..00000000000 --- a/service/telemetry/telemetry.go +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package telemetry // import "go.opentelemetry.io/collector/service/telemetry" - -import ( - "go.opentelemetry.io/collector/service/telemetry/internal" -) - -// Settings holds configuration for building Telemetry. -type Settings = internal.Settings diff --git a/service/telemetry/tracer_test.go b/service/telemetry/tracer_test.go index 6c365351549..f2ed37c1b50 100644 --- a/service/telemetry/tracer_test.go +++ b/service/telemetry/tracer_test.go @@ -13,7 +13,6 @@ import ( "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/featuregate" "go.opentelemetry.io/collector/internal/globalgates" - "go.opentelemetry.io/collector/service/telemetry/internal" ) func TestNewTracerProvider(t *testing.T) { @@ -51,7 +50,7 @@ func TestNewTracerProvider(t *testing.T) { defer func() { require.NoError(t, featuregate.GlobalRegistry().Set(globalgates.NoopTracerProvider.ID(), previousValue)) }() - provider, err := newTracerProvider(context.TODO(), internal.Settings{}, tt.cfg) + provider, err := newTracerProvider(context.TODO(), Settings{}, tt.cfg) require.NoError(t, err) require.IsType(t, tt.wantTracerProvider, provider) })