-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add telemetry tracking for task factory names and runtime usage #12989
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: main
Are you sure you want to change the base?
Conversation
|
Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo. |
Co-authored-by: YuliiaKovalova <[email protected]>
…sanitization Co-authored-by: YuliiaKovalova <[email protected]>
Co-authored-by: YuliiaKovalova <[email protected]>
Co-authored-by: YuliiaKovalova <[email protected]>
…ests Co-authored-by: YuliiaKovalova <[email protected]>
Co-authored-by: YuliiaKovalova <[email protected]>
|
@copilot update description according to the pushed changes |
Updated the PR description to reflect the refactored implementation that tracks factory name and runtime in TaskExecutionStats rather than using a separate dictionary in ProjectTelemetry. The new approach captures this information for all tasks and includes it in the worker node telemetry data. |
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 pull request adds telemetry tracking for task factory usage patterns to inform deprecation decisions for custom TaskFactory implementations and CodeTaskFactory. The implementation extends the existing telemetry infrastructure to capture and report task factory names alongside task execution statistics.
Key Changes
- Extended
TaskExecutionStatsto includeTaskFactoryNameandTaskHostRuntimefields - Updated telemetry serialization to transmit factory information across worker nodes
- Added hashing logic for custom factory names while keeping built-in factory names readable
- Modified telemetry collection points to capture and forward factory information
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Framework/Telemetry/TaskExecutionStats.cs | Added TaskFactoryName and TaskHostRuntime properties to track factory metadata |
| src/Framework/Telemetry/WorkerNodeTelemetryData.cs | Updated AddTask method signature to accept factory parameters; modernized constructor with collection expressions |
| src/Framework/Telemetry/WorkerNodeTelemetryEventArgs.cs | Extended serialization to include factory name and runtime fields |
| src/Framework/Telemetry/TelemetryDataUtils.cs | Added KnownTaskFactoryNames list and hashing logic for custom factory names |
| src/Build/TelemetryInfra/ITelemetryForwarder.cs | Extended AddTask interface with factory parameters |
| src/Build/TelemetryInfra/TelemetryForwarderProvider.cs | Updated implementations to pass factory information through telemetry pipeline |
| src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs | Modified task statistics collection to extract and forward factory metadata |
| src/Build/BackEnd/Components/Logging/ProjectTelemetry.cs | Refactored property building with AddIfNotEmpty helper; improved TryGetValue pattern usage |
| src/Build.UnitTests/Telemetry/Telemetry_Tests.cs | Added tests for factory name tracking and hashing behavior |
| src/Framework.UnitTests/WorkerNodeTelemetryEventArgs_Tests.cs | Updated serialization test with factory parameters |
| src/Build.UnitTests/BackEnd/ProjectTelemetry_Tests.cs | Formatting cleanup |
| src/Tasks.UnitTests/TelemetryTaskTests.cs | Removed empty comment lines |
| src/Framework/TelemetryEventArgs.cs | Minor formatting (blank line) |
| src/Build/TelemetryInfra/InternalTelemetryConsumingLogger.cs | Minor formatting (blank lines) |
| private static void AddIfNotEmpty(Dictionary<string, string> properties, string propertyName, int count) | ||
| { | ||
| if (count > 0) | ||
| { | ||
| properties[propertyName] = count.ToString(CultureInfo.InvariantCulture); | ||
| } | ||
| } |
Copilot
AI
Jan 9, 2026
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.
The AddIfNotEmpty helper method is only used for converting integer counts to strings for telemetry. Consider using a more descriptive name like AddCountIfNonZero to better communicate its specific purpose and constraints.
| } | ||
|
|
||
| public void AddTask(TaskOrTargetTelemetryKey task, TimeSpan cumulativeExectionTime, int executionsCount, long totalMemoryConsumption) | ||
| public void AddTask(TaskOrTargetTelemetryKey task, TimeSpan cumulativeExectionTime, int executionsCount, long totalMemoryConsumption, string? factoryName, string? taskHostRuntime) |
Copilot
AI
Jan 9, 2026
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.
The parameter name has a typo: cumulativeExectionTime should be cumulativeExecutionTime (missing 'u' in 'Execution').
| "TaskHostFactory", | ||
| "CodeTaskFactory", | ||
| "RoslynCodeTaskFactory", | ||
| "XamlTaskFactory", |
Copilot
AI
Jan 9, 2026
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.
The KnownTaskFactoryNames HashSet is missing "IntrinsicTaskFactory" which is listed as a built-in factory in ProjectTelemetry.cs. If IntrinsicTaskFactory is used, it will be incorrectly hashed as a custom factory name instead of being reported as-is. Consider adding "IntrinsicTaskFactory" to this HashSet to match the other built-in factories.
| "XamlTaskFactory", | |
| "XamlTaskFactory", | |
| "IntrinsicTaskFactory", |
| public WorkerNodeTelemetryData() | ||
| : this(new Dictionary<TaskOrTargetTelemetryKey, TaskExecutionStats>(), new Dictionary<TaskOrTargetTelemetryKey, bool>()) | ||
| { } | ||
| public WorkerNodeTelemetryData() : this([], new Dictionary<TaskOrTargetTelemetryKey, bool>()) { } |
Copilot
AI
Jan 9, 2026
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.
Inconsistent usage of collection expressions. The first parameter uses [] (collection expression) while the second uses new Dictionary<TaskOrTargetTelemetryKey, bool>(). For consistency with the coding guidelines that prefer collection expressions, consider using [] for both dictionaries.
| public WorkerNodeTelemetryData() : this([], new Dictionary<TaskOrTargetTelemetryKey, bool>()) { } | |
| public WorkerNodeTelemetryData() : this([], []) { } |
Context
MSBuild needs visibility into TaskFactory usage patterns to inform deprecation decisions. This PR implements telemetry to track which task factories are being used for task execution, including custom factories and the deprecated CodeTaskFactory.
Changes Made
TaskExecutionStats.cs
TaskFactoryNameproperty to track which factory created each task (AssemblyTaskFactory, CodeTaskFactory, RoslynCodeTaskFactory, XamlTaskFactory, or custom factory names)TaskHostRuntimeproperty to track out-of-process execution runtime ("CLR2", "CLR4", "NET", or null)Accumulate,Equals, andGetHashCodemethods to include new propertiesTelemetryDataUtils.cs
WorkerNodeTelemetryData.cs, WorkerNodeTelemetryEventArgs.cs
ProjectTelemetry.cs
AddIfNotEmptyhelper methodTelemetry_Tests.cs
WorkerNodeTelemetryCollection_TaskFactoryNametest to verify factory names are captured for both built-in and inline tasksTelemetryDataUtils_HashesCustomFactoryNametest to verify custom factory names are hashed while known factories are notProjectTelemetry_Tests.cs
Testing
Notes
This telemetry captures factory information for ALL tasks (not just custom ones) and associates it with individual task execution statistics. This approach provides comprehensive visibility into:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.