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

Review `component` public API

Open codeboten opened this issue 2 years ago • 5 comments

Before considering component 1.0, we need to review the public API to ensure what is exposed makes sense https://pkg.go.dev/go.opentelemetry.io/collector/component

codeboten avatar Nov 08 '22 22:11 codeboten

Some thoughts:

  • should BuildInfo be in this package? since its more directly tied to the service running, it might make more sense to move it to that package
  • TelemetrySettings should be moved into the telemetry package

codeboten avatar Nov 08 '22 22:11 codeboten

should BuildInfo be in this package? since its more directly tied to the service running, it might make more sense to move it to that package

I think cannot be moved to the "service" since you need to pass this to components, so will create a circular dependency.

TelemetrySettings should be moved into the telemetry package

Same issue about dependency.

bogdandrutu avatar Nov 08 '22 23:11 bogdandrutu

Before considering component 1.0, we need to review the public API to ensure what is exposed makes sense https://pkg.go.dev/go.opentelemetry.io/collector/component

@codeboten we also need to make sure that "consumer" package is 1.0 before :)

bogdandrutu avatar Nov 11 '22 03:11 bogdandrutu

IMO BuildInfo does not make sense in service: the components should not know anything about the service other than through the component.Host interface; ideally we should be able to completely replace service with a new implementation without changing anything about the components.

For TelemetrySettings, I think it makes sense to move it to telemetry, but my proposal would be to make telemetry into a separate module outside of service. I want to work on this at some point, but haven't gotten around to it lately.

mx-psi avatar Dec 21 '23 10:12 mx-psi

I am wondering if we should split component into further packages. For example, we could have componentstatus and componentfactory as separate packages. It currently feels like a mish-mash of important but unrelated things

mx-psi avatar Feb 07 '24 09:02 mx-psi