-
Notifications
You must be signed in to change notification settings - Fork 475
Add parameter to control instance count for Host JIT trace generation #11534
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
base: dev
Are you sure you want to change the base?
Add parameter to control instance count for Host JIT trace generation #11534
Conversation
…or JIT trace generation
… 'Instance' keyword
… and stripping spaces
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 introduces a new jitTraceInstancesForRelease parameter to control the number of instances used during JIT trace generation for Host releases, addressing pipeline failures caused by large YAML files when using the default 20 instances. The implementation also updates job naming to comply with Azure DevOps naming conventions.
Key changes:
- Added
jitTraceInstancesForReleaseparameter allowing users to specify a smaller set of instances (e.g., "1,2,3") for JIT trace generation - Introduced
effectiveRunInstancescomputed variable that conditionally selects between the new parameter and the defaultrunInstances - Updated job naming from
{functionAppName}_{instanceId}to{functionAppName}_Instance{instanceId}with space-stripping logic
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| eng/ci/host.coldstart.yml | Added new parameter jitTraceInstancesForRelease, introduced effectiveRunInstances computed variable, and updated all stage loops to use the new variable instead of runInstances |
| eng/ci/templates/official/jobs/run-coldstart.yml | Updated job naming pattern to include "_Instance" prefix and added replace() function to strip spaces from instance IDs |
Critical Issues Found: The implementation has several critical bugs that will cause pipeline failures:
- Space handling is broken: The
replace()function only strips spaces from job names, not from theinstanceIdparameter values, breaking all conditional checks that compareinstanceIdwith '1' - Job naming change breaks dependencies: The new naming pattern breaks
stageDependenciesreferences inprocess-coldstart.yml(which is not included in this PR but needs updating) - Invalid environment variables: Spaces in instance IDs will create invalid environment variable names in
process-coldstart.yml - PR description is incorrect: Claims that spaces are automatically handled for inputs like "1, 2, 3", but this is not true in the implementation
Comments suppressed due to low confidence (1)
eng/ci/host.coldstart.yml:101
- The space stripping with
replace()only affects the job name but does not strip spaces from theinstanceIdparameter value itself. This will cause critical failures in conditional checks throughout this template that compareinstanceIdwith '1'.
For example, if the user provides "1, 2, 3" as input:
- After
split(), the first appId will be "1" and the second will be " 2" (with leading space) - This " 2" gets passed as
instanceIdparameter - Line 36, 42, 74, 80, 90, etc. use
eq(parameters.instanceId, '1')comparisons - These comparisons will fail for the first instance if it has any surrounding spaces (e.g., " 1" or "1 ")
The ${{ appId }} in host.coldstart.yml should be trimmed before being passed as instanceId. However, Azure DevOps YAML doesn't have a built-in trim function at the template expression level. Consider one of these solutions:
- Document that the parameter must be provided without spaces (e.g., "1,2,3" only)
- Add a preprocessing step to normalize the input
- Strip spaces in the variable definition itself using replace in the effectiveRunInstances variable
- ${{ each appId in split(variables.effectiveRunInstances, ',') }}:
- ${{ if eq(parameters.includeHelloHttpNet9, true) }}:
- template: /eng/ci/templates/official/jobs/run-coldstart.yml@self
parameters:
description: .NET9 Web Application
functionAppName: HelloHttpNet9
instanceId: ${{ appId }}
…d fix job dependency references
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 3 out of 3 changed files in this pull request and generated no new comments.
|
@Francisco-Gamino, do you need the run instances to be configurable when generating the JIT trace files? Or would it be sufficient to just always set to 3 in that case? If so, then this pipeline could use templating to alter |
yea, hard coded 3 instances are good, when "create jit trace" is checked. No need for user to provide it. |
Issue describing the changes in this PR
This PR introduces a new parameter
jitTraceInstancesForReleaseto the cold start pipeline that allows controlling the number of instances used during JIT trace generation for Host releases.Problem
Currently, the pipeline uses the
runInstancesvariable fromcoldstart.yml, which is set to 20 instances. When generating JIT traces for releases, this creates very large YAML files that cause pipeline failures when both Node and .NET apps are selected.Solution
Added
jitTraceInstancesForReleaseParameter1,2,3) for JIT trace generationcreateJitTraceis set totrueFixed Job Naming Validation
_Instanceprefix (e.g.,HelloHttpNet9_Instance1)Implementation Details
The implementation uses a computed variable
effectiveRunInstancesthat conditionally selects:jitTraceInstancesForReleasewhen bothcreateJitTraceis true and the parameter is providedrunInstances(default 20 instances) otherwiseSupported Input Formats
The parameter accepts three input formats with automatic space handling:
1,2,3-> Creates Instance1, Instance2, Instance31, 2, 3->Spaces automatically removed -> Instance1, Instance2, Instance31 , 2 , 3-> All spaces automatically removed -> Instance1, Instance2, Instance3Changes
host.coldstart.ymljitTraceInstancesForReleaseparameter with clear user guidanceeffectiveRunInstancescomputed variable with conditional logiceffectiveRunInstances:run-coldstart.yml_Instance{id}formatreplacefunction to strip spaces from instance IDsUsage Example
For release JIT trace generation (reduced instances):