opentelemetry-collector
opentelemetry-collector copied to clipboard
Remotetap extension concept
Description
This is the core part of the remote tap concept described here: https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/34096
This will be used by the new version of the remotetap extension in contrib.
This new version uses the processorhelper to tap into the processor pipeline. ALL processors using the processorhelper to be created will automatically support this feature.
To see what a full version could look like you can check this POC: https://github.com/open-telemetry/opentelemetry-collector/pull/10962
When no remotetap stream is open, the additional work on the processor pipeline is negligible (1 map read / proc consumption if the extension is not enabled, 2 map read / proc consumption if the extension is enabled but no streams are open)
Idealy, in the future, the feature would be available to all kinds of components. Taping into the processors that use the processorhelper pkg is the first easy step in this direction.
Please create this PR in contrib? We will not have new components in this repository right now.
Ah okay, then I will move the component back to contrib and will only update the processorhelper logic in core
@atoulme I moved the component from this PR to contrib: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/34872. Only the changes related to the framework changes are left in this PR
@djaglowski I made the changes, let me know what you think :)
Can we move them elsewhere, maybe to processorhelper?
I could not move it to processorhelper because it causes some cyclic dep problems + ideally in the future I would like this interface by other types of components. I moved it to the component pkg, what do you think? IMO it fits great there, next to the telemetry.go that provides components the ability to report telemetry
Unfortunately I think we have the same stability concern with component. Basically, anything mentioned here as part of the roadmap is not going to work for something so new. Maybe @open-telemetry/collector-approvers have an idea.
Moved the publisher interface to the processor/internal pkg. It's not ideal because in the future it would be nice to expand the functionality to other components but that should be ok for a first step
@wildum I spiked out an implementation for a related need. The idea is to wrap components and capture metrics about the telemetry flowing in and out of components. I believe the same mechanism could be used for other things such as remote tapping, and it would be much more robust and useful. There's a lot that needs to be done including seeing what the maintainers say but I wanted to call this out as it may be a better path forward.
@djaglowski I like your approach :) Helpers should be used to provide optional features for components that require special behaviors (for example "WithStart", "WithCapabilities" and "WithShutdown"). Forcing all components to use the helpers by putting features such as the telemetry or the remote tap in the helpers defeats the purpose of the helpers. Such functionalities should be rooted at a deeper level and make use of the graph structure.
I'm happy to close my PR and support yours. The bad news is that it's not great timing with the 1.0 work as you mentioned above but the good news is that afaik the modifications don't introduce breaking changes (as long as we keep the helpers)?
The bad news is that it's not great timing with the 1.0 work
I believe the changes are isolated to the service package, which is not part of the 1.0 effort, if I understand correctly. :)
This PR was marked stale due to lack of activity. It will be closed in 14 days.
This PR was marked stale due to lack of activity. It will be closed in 14 days.
Closed as inactive. Feel free to reopen if this PR is still being worked on.