Enable Helix Job Monitor for the runtime pipeline#129690
Conversation
|
Tagging subscribers to this area: @akoeplinger, @matouskozak, @simonrozsival |
There was a problem hiding this comment.
Pull request overview
This PR wires up Helix Job Monitor support across the runtime Azure DevOps pipelines by introducing a useHelixMonitor parameter through the relevant YAML templates, enabling it for the runtime pipeline test legs, and adding the Helix Job Monitor .NET tool/dependency so the monitor job can run.
Changes:
- Add a
HelixJobMonitorjob toeng/pipelines/runtime.yml(Build stage) using the sharedeng/commontemplate. - Plumb a
useHelixMonitorparameter through Helix-related pipeline templates and pass/p:EnableHelixJobMonitor=...through the various “send to helix” paths. - Add
Microsoft.DotNet.Helix.JobMonitortool/dependency and pin it in.config/dotnet-tools.json.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| eng/Version.Details.xml | Adds the Microsoft.DotNet.Helix.JobMonitor toolset dependency entry. |
| eng/Version.Details.props | Adds version properties for the Helix Job Monitor dependency. |
| eng/pipelines/runtime.yml | Adds the Helix Job Monitor job and enables useHelixMonitor: true across runtime pipeline test legs. |
| eng/pipelines/libraries/run-test-job.yml | Introduces useHelixMonitor parameter and forwards it into the libraries Helix submission step. |
| eng/pipelines/libraries/helix.yml | Accepts useHelixMonitor and passes /p:EnableHelixJobMonitor=... into sendtohelix.proj. |
| eng/pipelines/installer/helix.yml | Adds useHelixMonitor, but currently passes UseHelixMonitor into /eng/common/templates/steps/send-to-helix.yml (template parameter mismatch; needs fix). |
| eng/pipelines/coreclr/nativeaot-post-build-steps.yml | Adds useHelixMonitor pass-through to the shared runtime test+Helix submit template. |
| eng/pipelines/common/templates/wasm-runtime-tests.yml | Adds and forwards useHelixMonitor into the Helix submission path. |
| eng/pipelines/common/templates/wasm-library-tests.yml | Adds and forwards useHelixMonitor into the Helix submission path. |
| eng/pipelines/common/templates/wasm-library-aot-tests.yml | Adds and forwards useHelixMonitor into nested wasm templates. |
| eng/pipelines/common/templates/wasm-coreclr-library-tests.yml | Adds and forwards useHelixMonitor into the Helix submission path. |
| eng/pipelines/common/templates/simple-wasm-build-tests.yml | Adds and forwards useHelixMonitor into the Helix submission path for WBT scenarios. |
| eng/pipelines/common/templates/runtimes/send-to-helix-step.yml | Adds useHelixMonitor and includes /p:EnableHelixJobMonitor=... in the computed send parameters. |
| eng/pipelines/common/templates/runtimes/run-test-job.yml | Adds useHelixMonitor parameter and forwards it to send-to-helix-step.yml. |
| eng/pipelines/common/templates/runtimes/build-runtime-tests-and-send-to-helix.yml | Adds useHelixMonitor parameter and forwards it to send-to-helix-step.yml. |
| eng/pipelines/common/templates/browser-wasm-coreclr-build-tests.yml | Hard-enables useHelixMonitor: true for the CoreCLR Wasm.Build.Tests Helix submission. |
| eng/pipelines/common/templates/browser-wasm-build-tests.yml | Adds and forwards useHelixMonitor for non-CoreCLR WBT Helix submissions. |
| .config/dotnet-tools.json | Adds the microsoft.dotnet.helix.jobmonitor tool entry and pins its version. |
- Add useHelixMonitor parameter to all helix pipeline templates - Enable Helix Monitor in runtime.yml for all test legs - Add helix-job-monitor.yml template job at the top of the Build stage - Add Microsoft.DotNet.Helix.JobMonitor dependency (same version as Arcade) - Pass EnableHelixJobMonitor MSBuild property through send-to-helix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
20899d1 to
624f97d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
eng/pipelines/common/templates/browser-wasm-coreclr-build-tests.yml:145
- This template hard-codes
useHelixMonitor: truewhen invoking the libraries Helix template. Since other Helix templates were updated to accept auseHelixMonitorparameter (defaulting to false) andruntime.ymlis now explicitly setting it, this hard-codedtruemakes the behavior inconsistent and harder to roll back/experiment with.
Consider adding a useHelixMonitor parameter to this template (default false) and threading it through instead of hard-coding true.
testRunNamePrefixSuffix: CoreCLR_$(_BuildConfig)_$(_hostedOs)
extraHelixArguments: /p:BrowserHost=$(_hostedOs)
scenarios:
- buildwasmapps
useHelixMonitor: true
| - template: /eng/common/core-templates/job/helix-job-monitor.yml | ||
| parameters: | ||
| helixAccessToken: $(HelixApiAccessToken) | ||
| timeoutInMinutes: 540 | ||
|
|
| testBuildArgs: nativeaot tree nativeaot | ||
| liveLibrariesBuildConfig: Release | ||
| testRunNamePrefixSuffix: NativeAOT_$(_BuildConfig) | ||
| useHelixMonitor: true | ||
| extraVariablesTemplates: |
| HelixPreCommands: '' # optional -- commands to run before Helix work item execution | ||
| HelixPostCommands: '' # optional -- commands to run after Helix work item execution | ||
| UseHelixMonitor: false # optional -- true will submit Helix jobs configured for the standalone Helix Job Monitor (results are reported/waited on out-of-band; this step will not wait, and WaitForWorkItemCompletion will be overridden) | ||
| WorkItemDirectory: '' # optional -- a payload directory to zip up and send to Helix; requires WorkItemCommand; incompatible with XUnitProjects | ||
| WorkItemCommand: '' # optional -- a command to execute on the payload; requires WorkItemDirectory; incompatible with XUnitProjects |
| - template: /eng/common/core-templates/job/helix-job-monitor.yml | ||
| parameters: | ||
| helixAccessToken: $(HelixApiAccessToken) | ||
| timeoutInMinutes: 540 | ||
|
|
| extraHelixArguments: /p:BrowserHost=$(_hostedOs) | ||
| scenarios: | ||
| - buildwasmapps | ||
| useHelixMonitor: true |
| - name: enableHelixJobMonitor | ||
| value: true | ||
|
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
eng/pipelines/common/templates/browser-wasm-coreclr-build-tests.yml:145
useHelixMonitoris hardcoded totruehere, which bypasses the repo-wideenableHelixJobMonitorswitch used everywhere else. This makes it harder to disable the monitor globally if needed and is inconsistent with the other templates updated in this PR.
extraHelixArguments: /p:BrowserHost=$(_hostedOs)
scenarios:
- buildwasmapps
useHelixMonitor: true
| - stage: Build | ||
| jobs: | ||
|
|
||
| - template: /eng/common/core-templates/job/helix-job-monitor.yml | ||
| parameters: | ||
| helixAccessToken: $(HelixApiAccessToken) | ||
| timeoutInMinutes: 540 | ||
|
|
|
@akoeplinger Feeling pretty good about this now. |
|
Needs change to ignore the monitor job in BA. |
|
BA change has been checked into Helix, needs rollout. |
|
Build Analysis should ignore the job monitor |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
eng/pipelines/common/templates/browser-wasm-coreclr-build-tests.yml:145
- This template hardcodes
useHelixMonitor: true, while the other Wasm test templates were updated to accept auseHelixMonitorparameter. Hardcoding makes it harder to centrally enable/disable Helix monitor behavior (e.g., via enableHelixJobMonitor) and creates an inconsistency across Wasm legs.
creator: dotnet-bot
testRunNamePrefixSuffix: CoreCLR_$(_BuildConfig)_$(_hostedOs)
extraHelixArguments: /p:BrowserHost=$(_hostedOs)
scenarios:
- buildwasmapps
useHelixMonitor: true
| <Project> | ||
| <PropertyGroup> | ||
| <!-- dotnet-arcade dependencies --> | ||
| <MicrosoftDotNetHelixJobMonitorPackageVersion>11.0.0-beta.26330.2</MicrosoftDotNetHelixJobMonitorPackageVersion> |
| "microsoft.dotnet.helix.jobmonitor": { | ||
| "version": "11.0.0-beta.26330.2", | ||
| "commands": [ | ||
| "dotnet-helix-job-monitor" | ||
| ] | ||
| } |
| <Dependency Name="Microsoft.DotNet.Helix.JobMonitor" Version="11.0.0-beta.26363.117"> | ||
| <Uri>https://github.com/dotnet/arcade</Uri> | ||
| <Sha>b25932e8d128b5242f1d89691f8e8171b82774c0</Sha> | ||
| </Dependency> |
| - template: /eng/pipelines/common/templates/wasm-library-tests.yml | ||
| parameters: | ||
| platforms: | ||
| - browser_wasm_win | ||
| alwaysRun: ${{ variables.isRollingBuild }} | ||
| extraBuildArgs: /p:AotHostArchitecture=x64 /p:AotHostOS=$(_hostedOS) | ||
| useHelixMonitor: ${{ variables.enableHelixJobMonitor }} | ||
| scenarios: |
This version has updated logging
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
eng/pipelines/common/templates/browser-wasm-coreclr-build-tests.yml:145
- This template hard-codes
useHelixMonitor: true, which bypasses the pipeline-level enableHelixJobMonitor toggle used elsewhere in runtime.yml. This makes it harder to roll back/experiment consistently across all Helix submissions.
testRunNamePrefixSuffix: CoreCLR_$(_BuildConfig)_$(_hostedOs)
extraHelixArguments: /p:BrowserHost=$(_hostedOs)
scenarios:
- buildwasmapps
useHelixMonitor: true
| <Dependency Name="Microsoft.DotNet.Helix.JobMonitor" Version="11.0.0-beta.26363.117"> | ||
| <Uri>https://github.com/dotnet/arcade</Uri> | ||
| <Sha>b25932e8d128b5242f1d89691f8e8171b82774c0</Sha> | ||
| </Dependency> |
| <!-- dotnet-arcade dependencies --> | ||
| <MicrosoftDotNetHelixJobMonitorPackageVersion>11.0.0-beta.26330.2</MicrosoftDotNetHelixJobMonitorPackageVersion> | ||
| <!-- dotnet-dotnet dependencies --> |
| "microsoft.dotnet.helix.jobmonitor": { | ||
| "version": "11.0.0-beta.26366.1", | ||
| "commands": [ | ||
| "dotnet-helix-job-monitor" | ||
| ] | ||
| } |
| - template: /eng/common/core-templates/job/helix-job-monitor.yml | ||
| parameters: | ||
| helixAccessToken: $(HelixApiAccessToken) | ||
| timeoutInMinutes: 540 | ||
|
|
| <Dependency Name="Microsoft.DotNet.GenAPI" Version="11.0.0-beta.26363.117"> | ||
| <Uri>https://github.com/dotnet/dotnet</Uri> | ||
| <Sha>cb8306a63c5cf24e9381108a3a9eb58907fd0f60</Sha> | ||
| <Sha>b25932e8d128b5242f1d89691f8e8171b82774c0</Sha> | ||
| </Dependency> |
| <!-- dotnet-arcade dependencies --> | ||
| <MicrosoftDotNetHelixJobMonitorPackageVersion>11.0.0-beta.26330.2</MicrosoftDotNetHelixJobMonitorPackageVersion> | ||
| <!-- dotnet-dotnet dependencies --> |
|
Workflow state for the Holistic Review Orchestrator. {
"version": 5,
"last_dispatched_commit": "c0584e7112beb2068ac9c4ffb90a1f398672c844",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "a0ad638fb14e9f3ad2c11588f6fb3a1610c8be08",
"last_reviewed_commit": "c0584e7112beb2068ac9c4ffb90a1f398672c844",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "a0ad638fb14e9f3ad2c11588f6fb3a1610c8be08",
"last_recorded_worker_run_id": "29684912062",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "c0584e7112beb2068ac9c4ffb90a1f398672c844",
"review_id": 4730671433
}
]
} |
There was a problem hiding this comment.
Holistic Review
Motivation: Enable the Helix Job Monitor (dotnet/arcade#16731) for the dotnet/runtime pipeline so that stuck or lost Helix jobs are detected and surfaced, improving CI reliability. This requires wiring a new Microsoft.DotNet.Helix.JobMonitor dependency plus a monitor job and a useHelixMonitor/EnableHelixJobMonitor plumbing parameter through the many Helix test templates.
Approach: The PR adds the Microsoft.DotNet.Helix.JobMonitor package/tool dependency, introduces a helix-job-monitor.yml template job at the top of the Build stage in runtime.yml, defines an enableHelixJobMonitor variable, and threads a useHelixMonitor parameter (defaulting to false) through every Helix send template so it becomes the EnableHelixJobMonitor / UseHelixMonitor MSBuild/Helix SDK property. The plumbing is mechanical and consistent across templates, and the parameter defaults are safe (opt-in per leg). The libraries/helix.yml change is largely a whitespace/indentation reflow with the single functional addition of the new property.
Summary: The pipeline plumbing is sound and follows existing conventions. However, the dependency metadata has two problems that should block merge until resolved: (1) the new Microsoft.DotNet.Helix.JobMonitor version is declared inconsistently in three files, and (2) the pre-existing Microsoft.DotNet.GenAPI dependency appears to have been accidentally modified (version downgrade + Sha repointed to the arcade commit), which looks like a bad rebase artifact. Both are flagged inline. Recommend regenerating the dependency files via darc to guarantee coherence. Verdict: Needs work before merge (dependency-metadata issues); the pipeline YAML changes themselves look good.
Detailed Findings
- Inconsistent JobMonitor version across three files (inline on
eng/Version.Details.xml):26363.117vs26330.2(Version.Details.props) vs26366.1(dotnet-tools.json). These should be a single version, normally kept in lockstep bydarc. - Likely-accidental
Microsoft.DotNet.GenAPIchange (inline oneng/Version.Details.xml): downgraded and repointed to the arcade Sha, inconsistent with its sibling dotnet/dotnet dependencies still at26365.101. Appears out of scope; please revert. - Non-blocking:
eng/pipelines/common/variables.ymlhardcodesenableHelixJobMonitor: truewhile every template parameter defaultsuseHelixMonitor: false. This is a reasonable global on-switch, but note there is now no single knob to disable the monitor for legs that reference the variable directly — acceptable if intentional.
Note
This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.
Generated by Holistic Review · 111.3 AIC · ⌖ 15.1 AIC · ⊞ 10K
| <Sha>cb8306a63c5cf24e9381108a3a9eb58907fd0f60</Sha> | ||
| </Dependency> | ||
| <Dependency Name="Microsoft.DotNet.GenAPI" Version="11.0.0-beta.26365.101"> | ||
| <Dependency Name="Microsoft.DotNet.Helix.JobMonitor" Version="11.0.0-beta.26363.117"> |
There was a problem hiding this comment.
The Microsoft.DotNet.Helix.JobMonitor version is inconsistent across the three places it is declared in this PR:
eng/Version.Details.xml(this line):11.0.0-beta.26363.117eng/Version.Details.props:11.0.0-beta.26330.2.config/dotnet-tools.json:11.0.0-beta.26366.1
These are normally kept in lockstep by darc. Version.Details.xml is the source of truth, so Version.Details.props and dotnet-tools.json should reference the same 11.0.0-beta.26363.117 build (or all three should be updated together). As written, the tool restored via dotnet-tools.json will differ from the package version flowed into the build, which can lead to hard-to-diagnose behavior mismatches. Please reconcile all three to a single version — ideally by letting darc update-dependencies regenerate them.
| <Uri>https://github.com/dotnet/arcade</Uri> | ||
| <Sha>b25932e8d128b5242f1d89691f8e8171b82774c0</Sha> | ||
| </Dependency> | ||
| <Dependency Name="Microsoft.DotNet.GenAPI" Version="11.0.0-beta.26363.117"> |
There was a problem hiding this comment.
This change to the existing Microsoft.DotNet.GenAPI dependency looks unintended and likely a bad rebase/merge artifact. The PR is only meant to add the Helix Job Monitor dependency, but it also downgrades GenAPI from 11.0.0-beta.26365.101 (dotnet/dotnet, Sha cb8306a...) to 11.0.0-beta.26363.117 and repoints its Sha to b25932e... — the arcade commit used for the newly added JobMonitor dependency. GenAPI flows from https://github.com/dotnet/dotnet, not arcade, and its sibling GenFacades/GenAPI-family dependencies remain at 26365.101 with the original Sha. This creates a mismatched/incoherent dependency graph that darc will not recognize. Please revert GenAPI back to its original Version/Sha and keep this PR limited to adding the JobMonitor dependency.
dotnet/arcade#16731