opentelemetry-collector
opentelemetry-collector copied to clipboard
Declare how to share a receiver across multiple signals with factory options
Description
Declare how to share a receiver across multiple signals with factory options.
Right now, we have each component looking to reuse across signals use a library under internal/sharedcomponent, and declare their own map of components. We move this map to the factory and use a factory option to set a consumer of a different signal on the existing receiver.
This removes the need to declare and manage internal/sharedcomponent
.
This PR lacks test coverage and is meant to collect feedback on the feasibility.
Testing
WIP
Documentation
API docs added.
Codecov Report
Attention: Patch coverage is 29.72973%
with 52 lines
in your changes missing coverage. Please review.
Project coverage is 91.59%. Comparing base (
cad2734
) to head (5ae1cc8
). Report is 463 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
receiver/receiver.go | 0.00% | 51 Missing and 1 partial :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #10059 +/- ##
==========================================
- Coverage 91.87% 91.59% -0.28%
==========================================
Files 360 359 -1
Lines 16725 16722 -3
==========================================
- Hits 15366 15317 -49
- Misses 1021 1069 +48
+ Partials 338 336 -2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32809 is open to fix contrib.
There are two behaviors related to status reporting that were implemented in sharedcomponent
that need to be preserved in this PR. A shared component represents multiple, logical component instances as a single component. For the purposes of status reporting, the shared component must report status for each of the logical components it represents. To complicate matters sightly, there are two ways to report status for a component. It can be reported from outside the component (via the service status.Reporter) or from within the component (via
component.TelemetrySettings). In both cases, a single call to ReportStatus
needs to report status for each component instance. For example, if you have an OTLP receiver that is part of three pipelines, one for traces, logs, and metrics, three status events would be emitted for each call to ReportStatus
. With that as background, I'll explain the two special cases that sharedcomponent currently handles.
-
sharedcomponent
currently chains together thetelemetrysettings.ReportStatus
functions for each instance of the component. this is how it emits n status events for n logical instances. This function is called from within the component. -
graph
does automatic status reporting for components during startup. It callsReportStatus
on the service'sstatus.Reporter
. This reports status from outside the component. The graph starts components one by one, however, this is a problem for thesharedcomponent
, as it should transition all logical instances toStarting
beforeStart
is called. This is handled in the shared component by reporting its status from within, preemptively, via its chainedtelemetrysettings.ReportStatus
method. Shutdown works similarly.
The solutions currently part of sharedcomponent
are not ideal, IMO, and it'd be great if we can improve upon them in the redesign. Let me know if there is anything I can do to help.
This PR was marked stale due to lack of activity. It will be closed in 14 days.
@mwear can we create an integration test that faithfully checks both scenarios you describe? This way we can make sure this continues to work.
@mwear can we create an integration test that faithfully checks both scenarios you describe? This way we can make sure this continues to work.
Sure thing. That's an oversight from the original implementation.
This PR was marked stale due to lack of activity. It will be closed in 14 days.
@atoulme, sorry for the delay. I looked into this today and the two cases I describe are in fact tested by an existing test case: https://github.com/open-telemetry/opentelemetry-collector/blob/65d59d14011f1b4cfa100aa70d42bb0ea8415c54/internal/sharedcomponent/sharedcomponent_test.go#L182-L273 Let me know if any further explanations are needed.
This PR was marked stale due to lack of activity. It will be closed in 14 days.
@atoulme Does this relate to #10534?
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.
@atoulme Does this relate to #10534?
No, this was a spike to look at how to move away from internal/sharedcomponent.
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.