Brief doc on feedback/logging/data systems#13554
Conversation
We discussed this with some new team members yesterday; writing it down.
There was a problem hiding this comment.
Pull request overview
Adds a short internal-facing document describing MSBuild’s primary data collection surfaces (loggers, tracing/events, telemetry) and how they differ in purpose and audience.
Changes:
- Introduces a new overview doc for MSBuild logging/feedback/data collection systems.
- Summarizes key built-in loggers and common post-build analysis workflows.
- Documents tracing (EventSource/ETW) and telemetry at a high level with links to existing docs.
There was a problem hiding this comment.
Expert Review — Documentation PR
Clean documentation-only PR providing a useful overview of MSBuild's three data-collection systems. All internal cross-references (Logging-behavior.md, specs/event-source.md, VS-Telemetry-Data.md) resolve to existing files, and the external link to (msbuildlog.com/redacted) is correct. Content is technically accurate.
Findings
One inline NIT on curly/smart quotes vs ASCII quotes inconsistency.
Minor suggestion (not blocking): Consider adding a link to this new file from documentation/README.md — e.g. in the "Logging" section — so it's discoverable from the docs index.
Dimension Summary
| # | Dimension | Verdict |
|---|---|---|
| 1 | Backwards Compatibility | ✅ LGTM |
| 2 | ChangeWave Discipline | ✅ LGTM |
| 3 | Performance & Allocation | ✅ LGTM |
| 4 | Test Coverage | ✅ LGTM |
| 5 | Error Message Quality | ✅ LGTM |
| 6 | Logging & Diagnostics | ✅ LGTM |
| 7 | String Comparison | ✅ LGTM |
| 8 | API Surface Discipline | ✅ LGTM |
| 9 | Target Authoring | ✅ LGTM |
| 10 | Design Before Implementation | ✅ LGTM |
| 11 | Cross-Platform Correctness | ✅ LGTM |
| 12 | Code Simplification | ✅ LGTM |
| 13 | Concurrency & Thread Safety | ✅ LGTM |
| 14 | Naming Precision | ✅ LGTM |
| 15 | SDK Integration Boundaries | ✅ LGTM |
| 16 | Idiomatic C# Patterns | ✅ LGTM |
| 17 | File I/O & Path Handling | ✅ LGTM |
| 18 | Documentation Accuracy | |
| 19 | Build Infrastructure Care | ✅ LGTM |
| 20 | Scope & PR Discipline | ✅ LGTM |
| 21 | Evaluation Model Integrity | ✅ LGTM |
| 22 | Correctness & Edge Cases | ✅ LGTM |
| 23 | Dependency Management | ✅ LGTM |
| 24 | Security Awareness | ✅ LGTM |
Note
🔒 Integrity filter blocked 2 items
The following items were blocked because they don't meet the GitHub integrity level.
- #13554
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved". - #13554
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by Expert Code Review (on open) for issue #13554 · ● 1.8M
|
Thanks Rainer, this helps! |
Updated [Microsoft.Build](https://github.com/dotnet/msbuild) from 18.6.3 to 18.7.1. <details> <summary>Release notes</summary> _Sourced from [Microsoft.Build's releases](https://github.com/dotnet/msbuild/releases)._ ## 18.7.1 ## What's Changed * Fix TraceEngine file contention deadlock in multithreaded mode by @JanProvaznik in dotnet/msbuild#13446 * Remove duplicate test cases in MultithreadableTaskAnalyzer by @Youssef1313 in dotnet/msbuild#13483 * Ensure ThreadSafeTaskAnalyzer.Tests is considered as a unit test project by @Youssef1313 in dotnet/msbuild#13481 * Fix MSBuildTask0002 analyzer warnings in already-migrated tasks by @JanProvaznik in dotnet/msbuild#13466 * Fix race conditions in task host path resolution by @AR-May in dotnet/msbuild#13485 * Migrate ToolTask and Al task to TaskEnvironment API by @OvesN in dotnet/msbuild#13423 * Bump main to 18.7, add vs18.6 to merge flow by @MichalPavlik in dotnet/msbuild#13472 * Avoid allocations in GetHashCode implementations by @DustinCampbell in dotnet/msbuild#13475 * Add PATs rotation to agentic workflow(s) by @JanKrivanek in dotnet/msbuild#13496 * Fix ASP.NET WebSite projects to copy netstandard.dll facade when required by @JanProvaznik in dotnet/msbuild#13058 * Migrate AspNetCompiler to TaskEnvironment API by @OvesN in dotnet/msbuild#13424 * Add review workflow by @JanKrivanek in dotnet/msbuild#13503 * Strengthen reviewer skill: add step-back analysis dimensions by @JanProvaznik in dotnet/msbuild#13504 * Add 'Request Speedometer Perf Run' to VS experimental insertion build policies by @Copilot in dotnet/msbuild#13505 * Remove duplicate @ prefix from issueAuthor in GitOps by @akoeplinger in dotnet/msbuild#13492 * Improve review aw by @JanKrivanek in dotnet/msbuild#13510 * Migrates unit tests to use RoslynCodeTaskFactory to enable running tests under .NET Core by @jankratochvilcz in dotnet/msbuild#13500 * Fix cross-AppDomain TaskItem modifier cache regression by @DustinCampbell in dotnet/msbuild#13493 * Discourage review agent from approving PRs by @JanKrivanek in dotnet/msbuild#13512 * Stop trying to deploy ValueTuple by @rainersigwald in dotnet/msbuild#13507 * Ad-hoc re-sign bootstrap dotnet on macOS to prevent SIGKILL by @jankratochvilcz in dotnet/msbuild#13513 * RoslynCodeTaskFactory: Log MSB3753 when task class does not implement ITask by @jankratochvilcz in dotnet/msbuild#13517 * Update gh-aw (upon mcp policy changes) by @JanKrivanek in dotnet/msbuild#13526 * Eliminate XmlChildNodes allocations in GetXmlNodeInnerContents by @nareshjo in dotnet/msbuild#13509 * Fix telemetry allocation regression: per-engine collector ownership by @JanProvaznik in dotnet/msbuild#13516 * Migrate to xunit.v3 by @Youssef1313 in dotnet/msbuild#13482 * Fix stray brace in HandleBuildCancel trace string causing MSB1025 by @Copilot in dotnet/msbuild#13535 * Bumping to 10.0.4 runtime packages by @MichalPavlik in dotnet/msbuild#13533 * Remove early return in GetCanonicalForm, always call System.IO.Path by @OvesN in dotnet/msbuild#13532 * Do not overwrite GetCopyToOutputDirectoryItemsDependsOn, just add new… by @snechaev in dotnet/msbuild#13474 * Migrate GetReferenceAssemblyPaths task to TaskEnvironment API by @OvesN in dotnet/msbuild#13495 * Stabilize ToolTaskThatTimeoutAndRetry test by @rainersigwald in dotnet/msbuild#13489 * [automated] Merge branch 'vs18.6' => 'main' by @github-actions[bot] in dotnet/msbuild#13506 * Add extra test assertions around tests by @Youssef1313 in dotnet/msbuild#13536 * Add static eval for repo skills/agents via skill-validator by @JanKrivanek in dotnet/msbuild#13537 * Migrate SGen task to Task environment API by @OvesN in dotnet/msbuild#13457 * Fix TerminalLogger assert failure for metaproj files and cached project eval ID by @OvesN in dotnet/msbuild#13480 * Filter out approving review from pr-reviewer agent by @JanKrivanek in dotnet/msbuild#13553 * Use a unique task name per invocation to tabilize RoslynCodeTaskFactory_ReuseCompilation test by @huulinhnguyen-dev in dotnet/msbuild#13551 * Brief doc on feedback/logging/data systems by @rainersigwald in dotnet/msbuild#13554 * Localized file check-in by OneLocBuild Task: Build definition ID 9434: Build ID 13881982 by @dotnet-bot in dotnet/msbuild#13437 * Stage 3: Forward BuildProjectFile* callbacks from OOP TaskHost to worker node by @JanProvaznik in dotnet/msbuild#13350 * Enable TaskHost Callbacks by default by @JanProvaznik in dotnet/msbuild#13579 * Remove unactionable info from reviewer agent by @JanKrivanek in dotnet/msbuild#13578 * Enlighten RequiresFramework35SP1Assembly task for multithreaded mode by @jankratochvilcz in dotnet/msbuild#13575 * Make SdkResolver-provided environment variables take precedence over ambient environment by @Copilot in dotnet/msbuild#12655 * Add dotnet/skills marketplace and enable plugins by @Evangelink in dotnet/msbuild#13582 * The skills/agents check filters-in only touched files by @JanKrivanek in dotnet/msbuild#13586 * Fix skill-validation workflow failing when agents directory is deleted by @JeremyKuhne in dotnet/msbuild#13592 ... (truncated) Commits viewable in [compare view](dotnet/msbuild@v18.6.3...v18.7.1). </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
We discussed this with some new team members yesterday; writing it down.