-
Notifications
You must be signed in to change notification settings - Fork 9
Improve background job stream handling #2235
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
Improve background job stream handling #2235
Conversation
Remove job control handling from subprocess observation function.
ae4aeab to
16ff446
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the handling of background job streams to prevent process hangs when child processes inherit stdout/stderr and don't terminate with their parent. The changes introduce timeout-based waiting mechanisms and restructure the GitHub Actions logging pipeline to use named pipes with explicit cleanup.
Key changes:
- Added
_otel_wait_for_process_with_timeout()utility function to wait for processes with a configurable timeout - Refactored GitHub Actions step logging to use a dual-pipe architecture with timeout-based cleanup
- Reduced various timeout values to minimize potential hang durations
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/usr/share/opentelemetry_shell/api.sh | Adds new timeout-based process waiting utility function |
| src/usr/share/opentelemetry_shell/api.observe.subprocesses.sh | Removes job control logic and adds output suppression to background process |
| src/usr/share/opentelemetry_shell/api.observe.pipes.sh | Implements timeout-based waiting for pipe observer processes and consolidates cleanup |
| src/usr/share/opentelemetry_shell/api.observe.logs.sh | Removes job control logic, adds output suppression, and removes stderr_pid wait |
| actions/instrument/job/inject_and_init.sh | Reduces timeout from 60s to 5s for Python process cleanup |
| actions/instrument/job/decorate_action.sh | Restructures logging using dual named pipes with timeout-based cleanup |
| .github/workflows/test_github.yml | Adds test case for background job handling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Pull request was converted to draft
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
No description provided.