Expose project evaluation metrics via System.Diagnostics.Metrics - #14458
Expose project evaluation metrics via System.Diagnostics.Metrics#14458OvesN wants to merge 26 commits into
Conversation
1e37e52 to
aed1472
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ab510081-14dc-4a61-a19a-cd64006e0fd4
Capture the evaluation end timestamp before publishing metrics so synchronous listener latency is excluded from the duration. Add regression coverage and move the proposed metric contract into the PR description. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0de319ca-43a2-41e8-852d-9750ccee6b88
Mirror the existing ETW Pass0-Pass5 boundaries with a tagged process-local histogram, cover partial and hidden evaluations, and verify metric pass records match EventSource stop events. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0de319ca-43a2-41e8-852d-9750ccee6b88
Use outside_build_submission instead of standalone so the metric dimension precisely describes evaluations not associated with a build submission. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0de319ca-43a2-41e8-852d-9750ccee6b88
Leave histogram aggregation and bucket selection to each metrics consumer instead of prescribing boundaries in MSBuild. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0de319ca-43a2-41e8-852d-9750ccee6b88
Use one shared pass-start helper for all evaluation passes and pass the succeeded Boolean directly to the tag list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0de319ca-43a2-41e8-852d-9750ccee6b88
Add an opt-in submission ID tag to evaluation count, duration, and pass metrics. Use process-wide IDs only while correlation is enabled so default BuildManager behavior remains unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06d4d874-b655-4ebe-8487-aa43ad301860
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e9798fd to
62fc99d
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds process-local System.Diagnostics.Metrics instrumentation to Microsoft.Build so hosts can observe project evaluation counts and durations (including per-pass timings) even for evaluations that occur outside of a build submission (object model loads, reevaluations, project graph).
Changes:
- Introduces
Microsoft.Buildmeter instruments for evaluation count, total duration, and per-pass duration with stage/origin/success tags, plus optional submission-id tagging viaMSBUILD_EVALUATION_METRICS_INCLUDE_SUBMISSION_ID. - Wires evaluator start/stop and Pass0–Pass5 timing to emit metric measurements alongside existing
MSBuildEventSourceboundaries. - Updates packaging/deployment artifacts to ship
System.Diagnostics.DiagnosticSource.dlland adds binding redirects for desktop MSBuild.
File summaries
| File | Description |
|---|---|
| src/Package/MSBuild.VSSetup/files.swr | Adds System.Diagnostics.DiagnosticSource.dll to VS setup payload (and NGEN metadata). |
| src/MSBuild/app.config | Adds binding redirect for System.Diagnostics.DiagnosticSource for desktop MSBuild. |
| src/MSBuild/app.amd64.config | Adds binding redirect + codeBase for System.Diagnostics.DiagnosticSource for amd64 MSBuild. |
| src/Build/TelemetryInfra/EvaluationMetrics.cs | New metrics helper encapsulating meter/instruments, tags, and opt-in submission-id behavior. |
| src/Build/Microsoft.Build.csproj | References System.Diagnostics.DiagnosticSource and compiles EvaluationMetrics.cs. |
| src/Build/Evaluation/Evaluator.cs | Emits evaluation + per-pass metric timings around evaluator execution. |
| src/Build/BackEnd/BuildManager/BuildManager.cs | Uses a process-wide submission id sequence when submission-id correlation is enabled. |
| src/Build.UnitTests/Telemetry/EvaluationMetrics_Tests.cs | Adds unit tests validating tags, pass alignment, opt-in submission id, and resilience to listener failures. |
| scripts/Deploy-MSBuild.ps1 | Ensures System.Diagnostics.DiagnosticSource.dll is deployed in desktop bootstrap scenarios. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@baronfel is the use of DiagnosticSource appropriate here? My telemetry knowledge is limited but I was under the impression that we use different mechanisms intentionally. |
|
We have a few different mechanisms that we need to reconcile:
Overall, I think we need to be creating telemetry that
For System.Diagnostics.Metrics today, I think we need to make sure VS won't yell at us about dll loads if we start including this. |
baronfel
left a comment
There was a problem hiding this comment.
Left a big long comment and forgot to actually hit 'review'. I'm not going to block this, but I do have some concerns.
Route evaluation EventSource events and Metrics through a shared lifecycle while preserving pass boundaries, desktop support, and failure isolation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 893844c9-d045-4386-8834-629b3cbc1850
|
Following the telemetry architecture discussion in this PR, I updated the implementation so the existing evaluation EventSource and the new Metrics instruments share a single evaluation lifecycle.
The broader questions around correlation for overlapping builds in long-lived processes and Crank time-series/tag handling remain separate follow-ups; no build IDs were added to metric tags. |
|
Is the newly introduced |
Yes. In .NET Framework/Visual Studio MSBuild, System.Diagnostics.DiagnosticSource.dll is loaded on the first project evaluation. |
|
OK. We should pay special attention to this feedback comment from Chet:
That isn't theoretical. We have seen such regressions when we introduced DiagnosticSource in the past IIRC (before my time). @AR-May might have more details. The last person who worked on this was @YuliiaKovalova. |
Fixes #14419
Context
Project evaluations initiated directly through the MSBuild object model are not visible to the binary logger attached to a later build. This includes hidden evaluations outside a build submission such as loading a
Project, creating aProjectInstance, explicit reevaluations, and project-graph construction.Changes Made
Microsoft.Buildmeter withmsbuild.project.evaluations,msbuild.project.evaluation.duration, andmsbuild.project.evaluation.pass.durationinstruments.build_submission; hidden/object-model and project-graph evaluations are taggedoutside_build_submission.MSBuildEventSourcestart/stop boundaries. Thepasstag identifiesinitial_properties,properties,item_definitions,items,using_tasks, ortargets.Metric Contract
MeterListenerenables the corresponding instrument; without a listener,Enabledchecks avoid timing and tag work. Non-critical instrumentation failures disable metrics for the remainder of the process without affecting evaluation.Possible Follow-up
Bridge these process-local metrics into
Microsoft.VisualStudio.Telemetryso evaluation aggregates can populate VS telemetry dashboards. This requires early collection for hidden pre-build evaluations and cross-process aggregation; it is intentionally not part of this PR.