-
Notifications
You must be signed in to change notification settings - Fork 742
Properly rewrite log capturing (#17333) #19612
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
Conversation
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
🟢 |
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 replaces the old TStdErrCapture approach with a new TLogBackendWithCapture for log collection, removes the deprecated stderr capture code, and integrates the new backend into TUI workflows. It also adds comprehensive unit tests for the new backend.
- Remove
stderr_capture.*and addlog_backend.* - Update build files and code paths in
runner.cppandimport.cppto useTLogBackendWithCapture - Add unit tests covering the new log capturing behavior
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/library/workload/tpcc/ya.make | Added log_backend.cpp, removed stderr_capture.cpp |
| ydb/library/workload/tpcc/ut/ya.make | Added log_capture_ut.cpp to unit test build |
| ydb/library/workload/tpcc/ut/log_capture_ut.cpp | New tests for TLogBackendWithCapture |
| ydb/library/workload/tpcc/stderr_capture.h/.cpp | Removed obsolete stderr capture implementation |
| ydb/library/workload/tpcc/log_backend.h/.cpp | New TLogBackendWithCapture implementation |
| ydb/library/workload/tpcc/runner.cpp | Switched TUI log capture to use TLogBackendWithCapture |
| ydb/library/workload/tpcc/import.cpp | Switched import logic to use TLogBackendWithCapture |
Comments suppressed due to low confidence (1)
ydb/library/workload/tpcc/log_backend.h:16
- [nitpick] The public interface of
TLogBackendWithCapturelacks comments describing thread-safety guarantees and expected usage patterns. Adding brief doc comments forStartCapture,StopCapture, andGetLogLineswould improve maintainability.
class TLogBackendWithCapture : public TLogBackend {
Changelog entry
...
Changelog category
Description for reviewers
...