opentelemetry-collector icon indicating copy to clipboard operation
opentelemetry-collector copied to clipboard

Define dependency hierarchy between core modules

Open bogdandrutu opened this issue 1 year ago • 1 comments

We are about to release more packages as stable, and we need to have a clear structure of dependencies between packages. We need to document (maybe enforce) restrictions on which packages can depend on which packages (e.g. component and confmap cannot depend on config modules). Here is how I see it so far (a larger number can depend on a smaller number):

  1. featuregate
  2. pdata
  3. confmap
  4. consumer
  5. component
  6. config modules
  7. receiver, processor, exporter, connector, extension
  8. receiver, processor, exporter, connector, extension implementations like otlpexporter.
  9. service (cannot depend on 8)
  10. otelcol (cannot depend on 8)

bogdandrutu avatar Feb 06 '24 17:02 bogdandrutu

One unsure relationship is extension with configs since some configs wants the list of extensions to find a storage or an auth extension. Need to maybe dig dipper here.

bogdandrutu avatar Feb 06 '24 17:02 bogdandrutu

Some config modules seem to do more than expected. For example, does func (hss *ServerConfig) ToServer(component.Host, component.TelemetrySettings, http.Handler, opts ...ToServerOption) (*http.Server, error) need to be part of confighttp? It seems like it lives there because there is no other good place for a server functionality. Maybe we can introduce more modules under pkg and clean up the config module to keep only the user config interface?

dmitryax avatar Feb 13 '24 22:02 dmitryax

@dmitryax it feels pretty natural from an ergonomic perspective within our components to have a single config struct and and then be able to call ToServer on it to get back a server. This requires libraries users to need to know about less packages and results in less dependencies imports.

It seems like the alternative is to create a new package with a ToServer(config confighttp.ServerConfig, component.Host, component.TelemetrySettings, http.Handler, opts ...ToServerOption) *http.server). This is also ergonomic, but requires users to know that this other package exists to help make a server from confighttp.ServerConfig. My fear would be that ToServer make get replicated independently because users wouldnt know to use the separate package.

TylerHelmuth avatar Feb 21 '24 21:02 TylerHelmuth

I am still not sure why 4 & 5 need to go before 6. It may be the case for specific config modules, but certainly not for all of them, right?

mx-psi avatar Feb 29 '24 15:02 mx-psi

I've updated the list in https://github.com/open-telemetry/opentelemetry-collector/issues/9375 to reflect the dependency order mentioned here. Closing as completed

TylerHelmuth avatar Apr 17 '24 18:04 TylerHelmuth