What's the plan for adding more providers if we need to in the future? (e.g. something for profiling?)

[mx-psi]

What's the plan for adding more providers if we need to in the future? (e.g. something for profiling?)

Originally posted by @mx-psi in #13606 (comment)

[axw]

For context, the proposed interface looks like this:

type Factory interface {
	CreateDefaultConfig() component.Config
	CreateProviders(context.Context, Settings, component.Config) (Providers, error)

	unexportedFactoryFunc()
}

type Providers interface {
	Shutdown(context.Context) error
	Resource() pcommon.Resource
	Logger() *zap.Logger
	LoggerProvider() log.LoggerProvider
	MeterProvider() metric.MeterProvider
	TracerProvider() trace.TracerProvider
}

type Settings struct {
	BuildInfo component.BuildInfo
	ZapOptions []zap.Option
}

This means that telemetry providers are created all at once. There were two things that drove me in this direction:

1. Constructing an otelconf.SDK means creating all the providers at once
2. Some of these things are interdependent, e.g. Logger & LoggerProvider; Resource & everything else; and the logger is used when constructing the MeterProvider and TracerProvider

It's a bit awkward, but we could address (1) by creating an otelconf.SDK per signal, with only that signal enabled. It feels like misuse of the API, but it's probably OK?

We could address (2) by adding a *zap.Logger to the telemetry.Settings struct, using a nop logger when constructing the logger itself. That also feels awkward to me, but doable (e.g. have a separate Settings type for constructing the LoggerProvider, vs. the MeterProvider and TracerProvider).

The result would be along these lines, more like the old interface:

type Factory interface {
	CreateDefaultConfig() component.Config
	CreateResource(context.Context, Settings, Settings, component.Config) (pcommon.Resource, error)
	CreateLogger(context.Context, Settings, LoggerSettings, component.Config) (*zap.Logger, log.LoggerProvider, error)
	CreateMeterProvider(context.Context, MeterProviderSettings, Settings, component.Config) (metric.MeterProvider, error)
	CreateTracerProvider(context.Context, TracerProviderSettings, Settings, component.Config) (trace.TracerProvider, error)

	unexportedFactoryFunc()
}

type Settings struct {
	BuildInfo component.BuildInfo
}

type LoggerSettings struct {
	Settings
	ZapOptions []zap.Option
}

type MeterProviderSettings struct {
	Settings
	Logger *zap.Logger
}

type TracerProviderSettings struct {
	Settings
	Logger *zap.Logger
}

If this sounds sensible, I'm happy to update the interfaces.

[mx-psi]

we could address (1) by creating an otelconf.SDK per signal, with only that signal enabled. It feels like misuse of the API, but it's probably OK?

@dmathieu any thoughts about this? An alternative would be to ensure we create it only once via an internal method wrapped with e.g. a sync.Once

[dmathieu]

Having one struct per signal is a bit unconventional, but that's not a bad idea either.
What is the problem with creating all providers at once though?

[axw]

@dmathieu, @mx-psi raised the question in #13606 (comment): what do we do if we need to add a new provider? e.g. hypothetically, maybe in the future a new provider would come along for profiling.

If we create one mega interface like I've done, then to extend the interface in the future in a non-breaking way, we would need some way to upgrade the interface like described in #13606 (comment).

The alternative is that the factory can grow new methods over time. These would have sensible defaults (e.g. no-op implementations) for new factory implementations, since the Factory interface is "sealed" (has an unexported method), which means it has to be constructed by the telemetry package.

[axw]

An alternative would be to ensure we create it only once via an internal method wrapped with e.g. a sync.Once

I think that sounds OK too. We would just need to make sure that shutting down one of the providers doesn't shut down the others - but that should be doable.

[axw]

Here's what I came up with: #13739. I ended up going with the SDK-per-signal approach - it works out to be quite simple.