Skip to content

Commit 2710536

Browse files
axwaxw
authored
[chore] service/telemetry: split providers interface back apart (#13739)
#### Description Split the monolithic Providers interface back into signal-specific interfaces and associated factory methods. This will enable the Factory interface to be expanded over time, without breaking changes. This should be the last major interface refactor for telemetry :crossed_fingers: #### Link to tracking issue Fixes #13722 #### Testing N/A #### Documentation N/A
1 parent 37a3ace commit 2710536

File tree

14 files changed

+449
-385
lines changed

14 files changed

+449
-385
lines changed

‎service/service.go‎

Lines changed: 74 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,17 @@ type Settings struct {
9898

9999
// Service represents the implementation of a component.Host.
100100
type Service struct {
101-
buildInfo component.BuildInfo
102-
telemetrySettings component.TelemetrySettings
103-
host *graph.Host
104-
collectorConf *confmap.Conf
105-
telemetryProviders telemetry.Providers
101+
buildInfo component.BuildInfo
102+
telemetrySettings component.TelemetrySettings
103+
host *graph.Host
104+
collectorConf *confmap.Conf
105+
loggerProvider telemetry.LoggerProvider
106+
meterProvider telemetry.MeterProvider
107+
tracerProvider telemetry.TracerProvider
106108
}
107109

108110
// New creates a new Service, its telemetry, and Components.
109-
func New(ctx context.Context, set Settings, cfg Config) (*Service, error) {
111+
func New(ctx context.Context, set Settings, cfg Config) (_ *Service, resultErr error) {
110112
srv := &Service{
111113
buildInfo: set.BuildInfo,
112114
host: &graph.Host{
@@ -123,37 +125,30 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) {
123125
collectorConf: set.CollectorConf,
124126
}
125127

126-
telFactory := otelconftelemetry.NewFactory()
127-
telset := telemetry.Settings{
128-
BuildInfo: set.BuildInfo,
129-
ZapOptions: set.LoggingOptions,
130-
DefaultViews: configureViews,
131-
}
128+
telemetryFactory := otelconftelemetry.NewFactory()
129+
telemetrySettings := telemetry.Settings{BuildInfo: set.BuildInfo}
132130

133-
telProviders, err := telFactory.CreateProviders(ctx, telset, &cfg.Telemetry)
131+
// Create the logger & LoggerProvider first. These may be used
132+
// when creating the other telemetry providers.
133+
loggerSettings := telemetry.LoggerSettings{
134+
Settings: telemetrySettings,
135+
ZapOptions: set.LoggingOptions,
136+
}
137+
logger, loggerProvider, err := telemetryFactory.CreateLogger(ctx, loggerSettings, &cfg.Telemetry)
134138
if err != nil {
135-
return nil, fmt.Errorf("failed to create telemetry providers: %w", err)
139+
return nil, fmt.Errorf("failed to create logger: %w", err)
136140
}
137-
srv.telemetryProviders = telProviders
138141
defer func() {
139-
if err != nil {
140-
err = multierr.Append(err, telProviders.Shutdown(ctx))
141-
}
142-
}()
143-
144-
// Use initialized logger to handle any subsequent errors
145-
// https://github.com/open-telemetry/opentelemetry-collector/pull/13081
146-
logger := telProviders.Logger()
147-
defer func() {
148-
if err != nil {
149-
logger.Error("error found during service initialization", zap.Error(err))
142+
if resultErr != nil {
143+
logger.Error("error found during service initialization", zap.Error(resultErr))
144+
resultErr = multierr.Append(resultErr, loggerProvider.Shutdown(ctx))
150145
}
151146
}()
147+
srv.loggerProvider = loggerProvider
152148

153149
// Wrap the zap.Logger with componentattribute so scope attributes
154150
// can be added and removed dynamically, and tee logs to the
155151
// LoggerProvider.
156-
loggerProvider := telProviders.LoggerProvider()
157152
logger = logger.WithOptions(zap.WrapCore(func(core zapcore.Core) zapcore.Core {
158153
core = componentattribute.NewConsoleCoreWithAttributes(core, attribute.NewSet())
159154
core = componentattribute.NewOTelTeeCoreWithAttributes(
@@ -165,11 +160,50 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) {
165160
return core
166161
}))
167162

163+
meterSettings := telemetry.MeterSettings{
164+
Settings: telemetrySettings,
165+
Logger: logger,
166+
}
167+
mpConfig := &cfg.Telemetry.Metrics.MeterProvider
168+
if mpConfig.Views == nil {
169+
mpConfig.Views = configureViews(cfg.Telemetry.Metrics.Level)
170+
}
171+
meterProvider, err := telemetryFactory.CreateMeterProvider(ctx, meterSettings, &cfg.Telemetry)
172+
if err != nil {
173+
return nil, fmt.Errorf("failed to create meter provider: %w", err)
174+
}
175+
defer func() {
176+
if resultErr != nil {
177+
resultErr = multierr.Append(resultErr, meterProvider.Shutdown(ctx))
178+
}
179+
}()
180+
srv.meterProvider = meterProvider
181+
182+
tracerSettings := telemetry.TracerSettings{
183+
Settings: telemetrySettings,
184+
Logger: logger,
185+
}
186+
tracerProvider, err := telemetryFactory.CreateTracerProvider(ctx, tracerSettings, &cfg.Telemetry)
187+
if err != nil {
188+
return nil, fmt.Errorf("failed to create tracer provider: %w", err)
189+
}
190+
defer func() {
191+
if resultErr != nil {
192+
resultErr = multierr.Append(resultErr, tracerProvider.Shutdown(ctx))
193+
}
194+
}()
195+
srv.tracerProvider = tracerProvider
196+
197+
resource, err := telemetryFactory.CreateResource(ctx, telemetrySettings, &cfg.Telemetry)
198+
if err != nil {
199+
return nil, fmt.Errorf("failed to create resource: %w", err)
200+
}
201+
168202
srv.telemetrySettings = component.TelemetrySettings{
169203
Logger: logger,
170-
MeterProvider: telProviders.MeterProvider(),
171-
TracerProvider: telProviders.TracerProvider(),
172-
Resource: telProviders.Resource(),
204+
MeterProvider: meterProvider,
205+
TracerProvider: tracerProvider,
206+
Resource: resource,
173207
}
174208
srv.host.Reporter = status.NewReporter(srv.host.NotifyComponentStatusChange, func(err error) {
175209
if errors.Is(err, status.ErrStatusNotReady) {
@@ -255,8 +289,16 @@ func (srv *Service) Shutdown(ctx context.Context) error {
255289

256290
srv.telemetrySettings.Logger.Info("Shutdown complete.")
257291

258-
if err := srv.telemetryProviders.Shutdown(ctx); err != nil {
259-
errs = multierr.Append(errs, fmt.Errorf("failed to shutdown telemetry providers: %w", err))
292+
// Shut down telemetry providers in the reverse order of creation,
293+
// since the tracer and meter providers may use the logger.
294+
if err := srv.tracerProvider.Shutdown(ctx); err != nil {
295+
errs = multierr.Append(errs, fmt.Errorf("failed to shutdown tracer provider: %w", err))
296+
}
297+
if err := srv.meterProvider.Shutdown(ctx); err != nil {
298+
errs = multierr.Append(errs, fmt.Errorf("failed to shutdown meter provider: %w", err))
299+
}
300+
if err := srv.loggerProvider.Shutdown(ctx); err != nil {
301+
errs = multierr.Append(errs, fmt.Errorf("failed to shutdown logger provider: %w", err))
260302
}
261303

262304
return errs

‎service/service_test.go‎

Lines changed: 72 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,17 @@ func TestServiceTelemetryShutdownError(t *testing.T) {
545545
},
546546
},
547547
}}
548+
cfg.Telemetry.Metrics.Level = configtelemetry.LevelDetailed
549+
cfg.Telemetry.Metrics.Readers = []config.MetricReader{{
550+
Periodic: &config.PeriodicMetricReader{
551+
Exporter: config.PushMetricExporter{
552+
OTLP: &config.OTLPMetric{
553+
Protocol: ptr("http/protobuf"),
554+
Endpoint: ptr("http://testing.invalid"),
555+
},
556+
},
557+
},
558+
}}
548559

549560
// Create and start a service
550561
srv, err := New(context.Background(), newNopSettings(), cfg)
@@ -553,7 +564,8 @@ func TestServiceTelemetryShutdownError(t *testing.T) {
553564

554565
// Shutdown the service
555566
err = srv.Shutdown(context.Background())
556-
assert.ErrorContains(t, err, `failed to shutdown telemetry`)
567+
require.ErrorContains(t, err, `failed to shutdown logger provider`)
568+
require.ErrorContains(t, err, `failed to shutdown meter provider`)
557569
}
558570

559571
func TestExtensionNotificationFailure(t *testing.T) {
@@ -635,40 +647,74 @@ func TestServiceFatalError(t *testing.T) {
635647
func TestServiceInvalidTelemetryConfiguration(t *testing.T) {
636648
tests := []struct {
637649
name string
638-
wantErr error
650+
wantErr string
639651
cfg otelconftelemetry.Config
640652
}{
641653
{
642654
name: "log config with processors and invalid config",
643-
cfg: otelconftelemetry.Config{
644-
Logs: otelconftelemetry.LogsConfig{
645-
Encoding: "console",
646-
Processors: []config.LogRecordProcessor{
647-
{
648-
Batch: &config.BatchLogRecordProcessor{
649-
Exporter: config.LogRecordExporter{
650-
OTLP: &config.OTLP{},
651-
},
652-
},
655+
cfg: func() otelconftelemetry.Config {
656+
cfg := otelconftelemetry.NewFactory().CreateDefaultConfig().(*otelconftelemetry.Config)
657+
cfg.Logs.Level = zapcore.FatalLevel
658+
cfg.Logs.Processors = []config.LogRecordProcessor{{
659+
Batch: &config.BatchLogRecordProcessor{
660+
Exporter: config.LogRecordExporter{
661+
OTLP: &config.OTLP{},
653662
},
654663
},
655-
},
656-
},
657-
wantErr: errors.New("no valid log exporter"),
664+
}}
665+
return *cfg
666+
}(),
667+
wantErr: "failed to create logger: no valid log exporter",
668+
},
669+
{
670+
name: "invalid metric reader",
671+
cfg: func() otelconftelemetry.Config {
672+
cfg := otelconftelemetry.NewFactory().CreateDefaultConfig().(*otelconftelemetry.Config)
673+
cfg.Logs.Level = zapcore.FatalLevel
674+
cfg.Metrics.Level = configtelemetry.LevelDetailed
675+
cfg.Metrics.Readers = []config.MetricReader{{
676+
Periodic: &config.PeriodicMetricReader{
677+
Exporter: config.PushMetricExporter{
678+
OTLP: &config.OTLPMetric{},
679+
},
680+
},
681+
}}
682+
return *cfg
683+
}(),
684+
wantErr: "failed to create meter provider: no valid metric exporter",
685+
},
686+
{
687+
name: "invalid trace exporter",
688+
cfg: func() otelconftelemetry.Config {
689+
cfg := otelconftelemetry.NewFactory().CreateDefaultConfig().(*otelconftelemetry.Config)
690+
cfg.Logs.Level = zapcore.FatalLevel
691+
cfg.Traces.Level = configtelemetry.LevelDetailed
692+
cfg.Traces.Processors = []config.SpanProcessor{{
693+
Batch: &config.BatchSpanProcessor{
694+
Exporter: config.SpanExporter{
695+
OTLP: &config.OTLP{},
696+
},
697+
},
698+
}}
699+
return *cfg
700+
}(),
701+
wantErr: "failed to create tracer provider: no valid span exporter",
658702
},
659703
}
660704
for _, tt := range tests {
661-
set := newNopSettings()
662-
set.AsyncErrorChannel = make(chan error)
663-
664-
cfg := newNopConfig()
665-
cfg.Telemetry = tt.cfg
666-
_, err := New(context.Background(), set, cfg)
667-
if tt.wantErr != nil {
668-
require.ErrorContains(t, err, tt.wantErr.Error())
669-
} else {
670-
require.NoError(t, err)
671-
}
705+
t.Run(tt.name, func(t *testing.T) {
706+
set := newNopSettings()
707+
set.AsyncErrorChannel = make(chan error)
708+
709+
cfg := newNopConfig()
710+
cfg.Telemetry = tt.cfg
711+
_, err := New(context.Background(), set, cfg)
712+
if tt.wantErr != "" {
713+
require.EqualError(t, err, tt.wantErr)
714+
} else {
715+
require.NoError(t, err)
716+
}
717+
})
672718
}
673719
}
674720

‎service/telemetry/otelconftelemetry/factory.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,13 @@ var useLocalHostAsDefaultMetricsAddressFeatureGate = featuregate.GlobalRegistry(
2525
// NewFactory creates a new telemetry.Factory that uses otelconf
2626
// to configure opentelemetry-go SDK telemetry providers.
2727
func NewFactory() telemetry.Factory {
28-
return telemetry.NewFactory(createDefaultConfig, createProviders)
28+
return telemetry.NewFactory(
29+
createDefaultConfig,
30+
telemetry.WithCreateResource(createResource),
31+
telemetry.WithCreateLogger(createLogger),
32+
telemetry.WithCreateMeterProvider(createMeterProvider),
33+
telemetry.WithCreateTracerProvider(createTracerProvider),
34+
)
2935
}
3036

3137
func createDefaultConfig() component.Config {

‎service/telemetry/otelconftelemetry/logger.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ import (
1717
// createLogger creates a Logger and a LoggerProvider from Config.
1818
func createLogger(
1919
ctx context.Context,
20-
set telemetry.Settings,
20+
set telemetry.LoggerSettings,
2121
componentConfig component.Config,
2222
) (*zap.Logger, telemetry.LoggerProvider, error) {
2323
cfg := componentConfig.(*Config)
24-
res := newResource(set, cfg)
24+
res := newResource(set.Settings, cfg)
2525

2626
// Copied from NewProductionConfig.
2727
ec := zap.NewProductionEncoderConfig()

‎service/telemetry/otelconftelemetry/logger_test.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,8 @@ func TestCreateLogger(t *testing.T) {
128128
for _, tt := range tests {
129129
t.Run(tt.name, func(t *testing.T) {
130130
buildInfo := component.BuildInfo{}
131-
132-
_, provider, err := createLogger(context.Background(), telemetry.Settings{
133-
BuildInfo: buildInfo,
131+
_, provider, err := NewFactory().CreateLogger(context.Background(), telemetry.LoggerSettings{
132+
Settings: telemetry.Settings{BuildInfo: buildInfo},
134133
}, &tt.cfg)
135134
if tt.wantErr != nil {
136135
require.ErrorContains(t, err, tt.wantErr.Error())
@@ -222,8 +221,8 @@ func TestCreateLoggerWithResource(t *testing.T) {
222221
for _, tt := range tests {
223222
t.Run(tt.name, func(t *testing.T) {
224223
core, observedLogs := observer.New(zapcore.DebugLevel)
225-
set := telemetry.Settings{
226-
BuildInfo: tt.buildInfo,
224+
set := telemetry.LoggerSettings{
225+
Settings: telemetry.Settings{BuildInfo: tt.buildInfo},
227226
ZapOptions: []zap.Option{
228227
// Redirect logs to the observer core
229228
zap.WrapCore(func(zapcore.Core) zapcore.Core { return core }),
@@ -348,7 +347,7 @@ func newOTLPLoggerProvider(t *testing.T, level zapcore.Level, handler http.Handl
348347
},
349348
}
350349

351-
_, loggerProvider, err := createLogger(t.Context(), telemetry.Settings{}, cfg)
350+
_, loggerProvider, err := createLogger(t.Context(), telemetry.LoggerSettings{}, cfg)
352351
require.NoError(t, err)
353352
return loggerProvider
354353
}

‎service/telemetry/otelconftelemetry/metrics.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88

99
config "go.opentelemetry.io/contrib/otelconf/v0.3.0"
1010
noopmetric "go.opentelemetry.io/otel/metric/noop"
11-
"go.uber.org/zap"
1211

1312
"go.opentelemetry.io/collector/component"
1413
"go.opentelemetry.io/collector/config/configtelemetry"
@@ -17,19 +16,18 @@ import (
1716

1817
func createMeterProvider(
1918
ctx context.Context,
20-
set telemetry.Settings,
19+
set telemetry.MeterSettings,
2120
componentConfig component.Config,
22-
logger *zap.Logger,
2321
) (telemetry.MeterProvider, error) {
2422
cfg := componentConfig.(*Config)
2523
if cfg.Metrics.Level == configtelemetry.LevelNone {
26-
logger.Info("Internal metrics telemetry disabled")
24+
set.Logger.Info("Internal metrics telemetry disabled")
2725
return noopMeterProvider{MeterProvider: noopmetric.NewMeterProvider()}, nil
2826
} else if cfg.Metrics.Views == nil && set.DefaultViews != nil {
2927
cfg.Metrics.Views = set.DefaultViews(cfg.Metrics.Level)
3028
}
3129

32-
res := newResource(set, cfg)
30+
res := newResource(set.Settings, cfg)
3331
mpConfig := cfg.Metrics.MeterProvider
3432
sdk, err := newSDK(ctx, res, config.OpenTelemetryConfiguration{
3533
MeterProvider: &mpConfig,

0 commit comments

Comments
 (0)