-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Review component
public API
#6506
Comments
Some thoughts:
|
I think cannot be moved to the "service" since you need to pass this to components, so will create a circular dependency.
Same issue about dependency. |
@codeboten we also need to make sure that "consumer" package is 1.0 before :) |
IMO For |
I am wondering if we should split |
#### Description Adds an RFC for component status reporting. The main goal is to define what component status reporting is, our current, implementation, and how such a system interacts with a 1.0 component. When merged, the following issues will be unblocked: - #9823 - #10058 - #9957 - #9324 - #6506 --------- Co-authored-by: Matthew Wear <matthew.wear@gmail.com> Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
Adds an RFC for component status reporting. The main goal is to define what component status reporting is, our current, implementation, and how such a system interacts with a 1.0 component. When merged, the following issues will be unblocked: - open-telemetry#9823 - open-telemetry#10058 - open-telemetry#9957 - open-telemetry#9324 - open-telemetry#6506 --------- Co-authored-by: Matthew Wear <matthew.wear@gmail.com> Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
Did a review of Component's public API and here are my thoughts:
|
#### Description Removes an incorrect comment from the `component.Factory interface`. This comment was added via #4338 when the extension/receiver/processor/exporter factory types all still [lived in](https://github.com/mx-psi/opentelemetry-collector/blob/b4be13e74923ebefb521f69a97288ce8d6f51ea9/component/exporter.go#L61) `component`. Since the factories have been moved to other modules, `component.Factory` needs to be implementable. The specific extension/receiver/processor/exporter factory types are unimplementable since they are defined in internal packages. <!-- Issue number if applicable --> #### Link to tracking issue Related to #6506
For (2) and (3) I feel like, unless not doing it would delay component 1.0, we should keep them where they are. I said before
If memory serves this was because I though different telemetry implementations could have different telemetry settings, but I don't think that should be the case: the interfaces would stay the same. |
@TylerHelmuth Should we close this as completed since we have a different issue for (1)? |
I agree, there is not appetite for 2. and 3. |
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/componentThe text was updated successfully, but these errors were encountered: