Skip to content

Adopt standalone Helix monitoring for SDK performance CI - #5307

Merged
LoopedBard3 merged 10 commits into
dotnet:mainfrom
LoopedBard3:loopedbard3-sdk-perf-helix-monitor
Sep 17, 2026
Merged

LoopedBard3 merged 10 commits into
dotnet:mainfrom
LoopedBard3:loopedbard3-sdk-perf-helix-monitor

Conversation

@LoopedBard3

@LoopedBard3 LoopedBard3 commented Sep 11, 2026

Copy link
Copy Markdown
Member

Summary

Adopt the standalone Helix Job Monitor for the SDK performance flow in azure-pipelines.yml / sdk-perf-jobs.yml, releasing submitter agents after submission while retaining stage completion and work-item failure reporting.

  • Always enable asynchronous submission and include the pool provider, independent monitor job, and complete monitor parameter block in the same implicit stage, including public/PR and manual runs with no selected workloads. Preserve existing job-selection parameters, triggers, schedules, and default flags. Set allowNoHelixJobs: false: a run with no Helix jobs fails, including an internal manual run with all three workload selectors false. The monitor is not omitted for empty runs. Submitter and work-item failures continue to propagate.
  • Import DotNet-HelixApi-Access only for the internal project, including internal PR runs. Forward helixAccessToken: $(HelixApiAccessToken) directly, following the accepted runtime pipeline convention. Public runs, including public PRs, do not import the private group and use public Helix access. This simplification does not add token normalization or change Azure DevOps macro expansion.
  • Cap the monitor at 360 minutes (355 for the tool); leave existing 320-minute submitter and Helix work-item timeouts unchanged.
  • Pin Microsoft.DotNet.Helix.JobMonitor alongside the current Arcade/Helix SDK in the local tool manifest and dependency metadata. Restore eng/common/dotnet.sh's executable bit to match its pinned Arcade source, without changing its contents.
  • Prefix test-run identities with the matrix-expanded Agent.JobName. Independent channels sharing a phase and queue stay distinct, while retries of the same leg retain stable identities.

Shared runtime templates and local Python submission/download behavior are unchanged. Existing Logs_* artifacts remain; arbitrary Helix uploads are not newly copied into pipeline artifacts.

Related runtime monitor work: dotnet/runtime#132807 and dotnet/runtime#133633. This independently adopts the monitor for SDK performance; those runtime changes did not cause an SDK regression.

Internal-PR credential eligibility is aligned with merged dotnet/runtime#133885. Direct token forwarding follows the accepted review suggestion in dotnet/runtime#133726; its unrelated timeout change is not included here.

Validation

Current direct-token revision

Commit 75530bae17e2a1beb07dce81ba9da3bb36f9f641 was validated locally: seven contract checks pass, including 1,008 selection/auth combinations. These verify direct parameter/environment forwarding, the internal-only group guard, forwarding of a configured empty value without treating an undefined macro as empty, always-present monitor/parameters, zero-job failure, unchanged selectors and 360-minute cap, matrix naming, tool pins, and Linux executable metadata.

A bounded read-only Helix service comparison used public performance build 1597406 and job 0c2ccea1-53a5-4f9b-b8a6-77b88d9e9b42. Requests with no Authorization header and with the literal Authorization: token $(HelixApiAccessToken) returned the same public job/source/BuildId, the same source-and-BuildId-filtered job set, and identical work-item names/states/exit codes. This verifies public visibility for that case, not SDK normalization of unresolved tokens or private access. The public pipeline definition read did not independently establish an effective empty token value. No manual CI run was queued for this revision; no new pipeline result is claimed here.

The earlier zero-job revision (07aebb250a15a5b4ce8a4c87c4faa21d50ef46ce) changed allowNoHelixJobs to false; its regression failed before the change and passed afterward. That policy is retained, including failure for an internal manual run with all workload selectors false.

The pinned monitor's no-job branch and its existing source tests were reviewed: once other pipeline jobs complete, zero associated Helix jobs with AllowNoHelixJobs=false returns exit code 1. Submitter-failure and nonempty work-item-failure handling are unchanged. The earlier internal-PR credential and unconditional-monitor changes were validated locally; pinned-binary matrix/retry checks verified 88 distinct legs and selection of 88 latest attempts from 176 original-plus-retry records.

Earlier end-to-end evidence

Internal SDK sanity run 3073057 tested the earlier commit c813e894947b143f536e6b6075103a8f9b2af7c9, before the subsequent unconditional-monitor, internal-PR credential, empty-run policy, and direct-token revisions. It used onlySanityCheck=true, runScheduledPrivateJobs=true, runPrivateJobs=false, and runPublicJobs=false.

  • All 89 submitter jobs succeeded. Their 89 Helix GUIDs exactly matched 89 nonempty monitor processing records: 304 work items, no collapsed matrix legs or zero-work-item processing records.
  • The monitor alone kept the stage open for about 25 minutes after the final submitter finished, then completed result processing. All 89 Logs_* artifacts were published.
  • The overall run failed, correctly reporting 68 failed work items as 68 failed Azure test results. This is not a green benchmark run. Sampled failures include F# FS3888 errors also present in main run 3072588, and the existing Roslyn sanity filter matching no benchmarks. Unrelated workloads were not changed.

Earlier local checks also covered cold Linux bootstrap and tool startup, and actual pinned-SDK metadata plus the pinned monitor binary: 88 independent prior submissions remain 88 streams instead of 31; job/stage retry simulations select 88 latest attempts from 176 records. Public CI and live retries were not run. The earlier live run is not evidence of CI validation for the latest revision.

AI disclosure

Implemented and validated with assistance from GitHub Copilot.

Parker Bibus and others added 4 commits September 10, 2026 14:21
Pair internal asynchronous submissions with a credentialed stage monitor and pin its tool alongside the Helix SDK. Preserve public, PR, and local synchronous behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Evaluate the internal-run predicate directly while expanding root variables. Define the async flag, pool provider, and Helix credential group together so same-block variable lookup cannot omit monitor prerequisites.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Enable anonymous monitoring for selected public and PR jobs, keep internal credentials guarded, and bound the monitor to six hours without changing sender or work-item timeouts. Restore the Arcade bootstrap executable bit for Linux tool restore.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Prefix monitor test-run identities with the matrix-expanded agent job name so independent channels sharing a phase and queue are not treated as retries. Retain stable identities for job and stage retries and scope the naming change to monitored SDK flows.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 11, 2026 17:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

CI orchestration, authentication, and Helix result handling warrant final human review.

Pull request overview

This pull request adopts standalone Helix monitoring for SDK performance CI while preserving result reporting and artifacts.

Changes:

  • Adds conditional monitoring, authentication, and timeout configuration.
  • Pins the Helix monitor dependency and tool.
  • Restores dotnet.sh executable status.
  • Documents the monitored SDK performance flow.
File summaries
File Summary
eng/Version.Details.xml Records the Helix monitor dependency.
eng/common/dotnet.sh Restores the standard executable permission.
docs/sdk-scenarios.md Documents monitoring; add the new section to the manual list.
azure-pipelines.yml Configures asynchronous submission and monitoring.
.config/dotnet-tools.json Pins the Helix Job Monitor tool.
Review details

Suppressed comments (1)

docs/sdk-scenarios.md:7

  • The document maintains a manual section list at the top, but this new top-level section is not included in it. Add an SDK performance pipeline link so readers using the document navigation can reach the newly added guidance.
## SDK performance pipeline
  • Files reviewed: 4/5 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

lewing
lewing previously approved these changes Sep 11, 2026
Keep asynchronous submission, the pool provider, matrix identity prefix, and monitor parameter block unconditional. Use anonymous Helix credentials outside internal non-PR runs, and allow legitimate no-workload stages without changing workload selection or timeout budgets.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Pipeline wiring and empty-job monitor conditions remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

azure-pipelines.yml:83

  • This template is instantiated unconditionally, so an internal Manual run with all three workload-selection flags false still queues HelixJobMonitor even though sdk-perf-jobs.yml expands to no submitter jobs. That contradicts the PR contract that such runs do not start a monitor; gate this template on the same selection conditions used for the submitter jobs.
  - template: /eng/common/core-templates/job/helix-job-monitor.yml

docs/sdk-scenarios.md:21

  • This documents the inverse of the requested behavior: an internal manual run with no selected workloads should omit the monitor, while a monitored stage with no Helix jobs should fail. The current YAML always emits the monitor and enables allowNoHelixJobs, so update this text together with the pipeline fix rather than documenting a successful empty monitored stage.
job-selection parameters and schedules are unchanged: public runs always select
correctness jobs, while internal manual runs can select no workloads. Such runs still
include the monitor and may succeed without Helix jobs (`allowNoHelixJobs: true`).
This permits an empty stage; it does not suppress submitter or work-item failures.
  • Files reviewed: 4/5 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread azure-pipelines.yml
Comment thread azure-pipelines.yml Outdated
Align monitor credential eligibility with dotnet/runtime#133885: import the Helix group and select its token for the internal project regardless of build reason. Public runs, including public PRs, remain anonymous. Preserve workload selection and monitor settings.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 14, 2026 16:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The pipeline credential guard and corresponding documentation must be corrected before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

azure-pipelines.yml:86

  • This expression selects $(HelixApiAccessToken) for internal pull-request runs because it checks only the project. That makes the monitor non-anonymous for PRs and is inconsistent with the required non-PR-only credential policy; include the direct Build.Reason guard in this condition as well.
      helixAccessToken: ${{ iif(eq(variables['System.TeamProject'], 'internal'), '$(HelixApiAccessToken)', '') }}
  • Files reviewed: 4/5 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread azure-pipelines.yml
Comment thread docs/sdk-scenarios.md Outdated
Keep the monitor unconditional but reject zero-job discovery, including internal manual runs with all workload selectors false. Preserve workload selection, credential scope, timeout budgets, and matrix identities.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 14, 2026 18:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The pipeline and credential changes are broad, and the latest revision lacks live CI validation.

Review details
  • Files reviewed: 4/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Parker Bibus and others added 2 commits September 15, 2026 09:50
Match the accepted runtime pipeline convention by passing HelixApiAccessToken without a compile-time iif. Keep the credential group internal-only and document the public empty-value contract without assuming unresolved macros become empty.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Document the established forwarding convention without asserting an unverified empty-variable requirement. Read-only checks confirmed matching public job and work-item visibility with no auth and a literal unresolved-token header.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 15, 2026 16:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Public-run credential selection must avoid passing an unresolved Helix token macro.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 4/5 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread azure-pipelines.yml
Copilot AI review requested due to automatic review settings September 15, 2026 21:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The monitor flag is not threaded through to the sender template, and the latest revision lacks end-to-end CI validation.

Review details

Suppressed comments (1)

azure-pipelines.yml:18

  • EnableHelixJobMonitor is only defined as an Azure runtime variable here; it is never passed as the UseHelixMonitor template parameter. run-performance-job.yml invokes send-to-helix-step.yml without that parameter, whose default is false, and the step therefore emits /p:EnableHelixJobMonitor=false. As a result, submitters remain synchronous and the standalone monitor cannot provide the promised agent release. Thread UseHelixMonitor: true through jobParameters (or update the shared sender template) rather than relying on this variable.
  - name: EnableHelixJobMonitor
    value: true
  • Files reviewed: 4/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@LoopedBard3
LoopedBard3 enabled auto-merge (squash) September 16, 2026 20:57
Comment thread azure-pipelines.yml
@LoopedBard3
LoopedBard3 merged commit d92bd69 into dotnet:main Sep 17, 2026
79 checks passed
@LoopedBard3
LoopedBard3 deleted the loopedbard3-sdk-perf-helix-monitor branch September 17, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants