Manually report status for core exporters #8788
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8788 +/- ##
=======================================
Coverage 90.27% 90.27%
=======================================
Files 343 343
Lines 18003 18069 +66
=======================================
+ Hits 16252 16312 +60
- Misses 1424 1429 +5
- Partials 327 328 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Looking at this in comparison with #8684, I like the approach in the other PR better, it feels like the opportunities for most of the cases is covered automatically for components, while still giving enough flexibility for edge cases.
With the approach in this PR, it feels like most of the code will end up being quite repetitive across components. I'm thinking of how much code this would mean for the contrib repo especially.
|
If I were only reporting status for exporters, I would choose #8684 because it introduces a flexible opt-in approach that reduces the overall amount of code that needs to be written. My only reservation is that that approach wouldn't apply fully to other component types. We could add similar |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
This is part of the continued component status reporting effort. Currently we have automated status reporting for the following component lifecycle events: `Starting`, `Stopping`, `Stopped` as well as definitive errors that occur in the starting or stopping process (e.g. as determined by an error return value). This leaves the responsibility to the component to report runtime status after start and before stop. We'd like to be able to extend the automatic status reporting to report `StatusOK` if `Start` completes without an error. One complication with this approach is that some components spawn async work (via goroutines) that, depending on the Go scheduler, can report status before `Start` returns. As such, we cannot assume a nil return value from `Start` means the component has started properly. The solution is to detect if the component has already reported status when start returns, if it has, we will use the component-reported status and will not automatically report status. If it hasn't, and `Start` returns without an error, we can report `StatusOK`. Any subsequent reports from the component (async or otherwise) will transition the component status accordingly. The tl;dr is that we cannot control the execution of async code, that's up to the Go scheduler, but we can handle the race, report the status based on the execution, and not clobber status reported from within the component during the startup process. That said, for components with async starts, you may see a `StatusOK` before the component-reported status, or just the component-reported status depending on the actual execution of the code. In both cases, the end status will be same. The work in this PR will allow us to simplify #8684 and #8788 and ultimately choose which direction we want to go for runtime status reporting. **Link to tracking Issue:** #7682 **Testing:** units / manual --------- Co-authored-by: Alex Boten <aboten@lightstep.com>
mwear
force-pushed
the
core-exporters-manual-status
branch
3 times, most recently
from
c465fe3 to
8f96171
Compare
This is part of the continued component status reporting effort. Currently we have automated status reporting for the following component lifecycle events: `Starting`, `Stopping`, `Stopped` as well as definitive errors that occur in the starting or stopping process (e.g. as determined by an error return value). This leaves the responsibility to the component to report runtime status after start and before stop. We'd like to be able to extend the automatic status reporting to report `StatusOK` if `Start` completes without an error. One complication with this approach is that some components spawn async work (via goroutines) that, depending on the Go scheduler, can report status before `Start` returns. As such, we cannot assume a nil return value from `Start` means the component has started properly. The solution is to detect if the component has already reported status when start returns, if it has, we will use the component-reported status and will not automatically report status. If it hasn't, and `Start` returns without an error, we can report `StatusOK`. Any subsequent reports from the component (async or otherwise) will transition the component status accordingly. The tl;dr is that we cannot control the execution of async code, that's up to the Go scheduler, but we can handle the race, report the status based on the execution, and not clobber status reported from within the component during the startup process. That said, for components with async starts, you may see a `StatusOK` before the component-reported status, or just the component-reported status depending on the actual execution of the code. In both cases, the end status will be same. The work in this PR will allow us to simplify open-telemetry#8684 and open-telemetry#8788 and ultimately choose which direction we want to go for runtime status reporting. **Link to tracking Issue:** open-telemetry#7682 **Testing:** units / manual --------- Co-authored-by: Alex Boten <aboten@lightstep.com>
|
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. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
It implies the batch size is misconfigured.
mwear
force-pushed
the
core-exporters-manual-status
branch
from
8f96171 to
9772f45
Compare
|
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. |
|
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. |
Description:
This is a PR is an alternative to #8684. It adds manual status reporting to the otlp, otlphttp, and debug exporters. Reviewers, please compare with #8684 and weigh in on what approach you prefer, and/or additional suggestions.
Link to tracking Issue: #7682
Testing: Units, manual.