[component] Move internal/sharedcomponent to component/sharedcomponent #9321
Conversation
atoulme
force-pushed
the
move_sharedcomponent
branch
from
6a795ba to
6d69f67
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9321 +/- ##
==========================================
+ Coverage 90.34% 90.41% +0.06%
==========================================
Files 340 340
Lines 17989 17989
==========================================
+ Hits 16253 16265 +12
+ Misses 1412 1401 -11
+ Partials 324 323 -1 ☔ View full report in Codecov by Sentry. |
| @@ -4,7 +4,7 @@ | |||
| // Package sharedcomponent exposes functionality for components | |||
| // to register against a shared key, such as a configuration object, in order to be reused across signal types. | |||
| // This is particularly useful when the component relies on a shared resource such as os.File or http.Server. | |||
| package sharedcomponent // import "go.opentelemetry.io/collector/internal/sharedcomponent" | |||
| package sharedcomponent // import "go.opentelemetry.io/collector/component/sharedcomponent" | |||
There was a problem hiding this comment.
As mentioned in my previous comment, I am not sure this is right now:
- The fact that we need to pass TelemetrySettings feels a hack and I want to better look into why we need that. I do understand some use-case but maybe we have a different "SharedTelemetrySettings" for that. Also not sure why start/stop calls the TelemetrySettings that way vs we return the right status all the time for multiple calls.
- Right now we stop first time, which may not be ideal. Ideally we should call start first time and stop last time, but this is not possible for grpc servers. But for other "shared" components that probably should be the case and an external mechanism block receiving/emitting data to match the Receiver/Processor behaviors.
There was a problem hiding this comment.
I can file issues for both items.
I'm not clear on 1, but I'll give it my best try.
I assumed for 2. we abandoned because that wasn't easily possible, but we can look again with your suggestion.
There was a problem hiding this comment.
I have opened #9527 to try and tackle 1. Thank you for your comment, took me a bit but I think this change is in line with it. Let' s see how it does.
There was a problem hiding this comment.
Regarding 2. maybe we can have the sharedcomponents.Map take a configuration to shut down components early or late, so folks can opt in.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closing - I think we can release 1.0 without this package. |
Description:
Move internal/sharedcomponent to component/sharedcomponent.
Link to tracking Issue:
Fixes #4266