Repository navigation
Report perfcollect initialization failures in controller output - #901
Open
LoopedBard3 wants to merge 4 commits into
Open
LoopedBard3 wants to merge 4 commits into
LoopedBard3 wants to merge 4 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
LoopedBard3
marked this pull request as draft
October 5, 2026 21:36
LoopedBard3
marked this pull request as draft
October 5, 2026 21:36
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
sebastienros
approved these changes
Oct 6, 2026
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Collector state is not job-scoped, and several failure paths can still report or download invalid traces incorrectly.
Review effort: Balanced
Findings: 2
Open (4)
What changed in this PR
Improves legacy Linux trace failure reporting and documents modern tracing alternatives.
Changes:
- Captures bounded perfcollect diagnostics and validates trace output.
- Refreshes agent errors after failed trace downloads.
- Adds regression tests and Linux tracing guidance.
| File | Description |
|---|---|
Startup.cs |
Tracks perfcollect completion, errors, and trace validity. |
JobConnection.cs |
Retrieves collector errors after trace 404s. |
PerfcollectTests.cs |
Tests collector failures and trace validation. |
TraceDownloadTests.cs |
Tests controller error reporting and fallback. |
setup_linux.md |
Documents LTTng compatibility and collect-linux. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
Legacy Linux trace collection can exit during initialization (for example,
LTTng not installed) while the benchmark continues. Crank currently reportsTrace collected, then shows only an HTTP 404 when downloading the missing trace.Job.Errorfield.JobContext, including native/Docker startup, finalization, abort, and deletion cleanup.collect-linuxfor supported .NET 10+ Linux hosts, including kernel, root, user-events, and tracefs prerequisites; do not recommend disabling LTTng.No new collection options or serialized job states. The benchmark is not aborted when the collector fails; existing controller handling of
Job.Errorcan return a nonzero exit code even when the benchmark itself succeeds.Documentation references: dotnet/runtime#57784, perfcollect installer, and collect-linux prerequisites.
Validation
Passed 104 related tracing unit tests on Windows, including all four Copilot review regressions:
Coverage includes launch/exit failures, bounded diagnostics, successful collection, partial/missing/empty traces, stopping one job while another collector remains running, endpoint rejection of empty traces, and stale-error/refresh-timeout fallback. The timeout regression uses an infinite HttpClient timeout and a handler that waits until the request's cancellation token is canceled.
A quick local review by GPT-5.6 Sol Fast identified a refresh-cancellation fallback issue, which was fixed; the subsequent Copilot PR review findings are addressed in 5a9fdc9.
The process tests use fake collectors, and the controller tests use an in-memory HTTP handler. A real Linux perfcollect capture was not run.