Repository navigation
Remove the Arcade msbuild logger - #16814
Conversation
There was a problem hiding this comment.
Pull request overview
Removes the legacy Microsoft.DotNet.ArcadeLogging MSBuild logger that decorated messages with (NETCORE_ENGINEERING_TELEMETRY=...), along with the wiring that injected it into MSBuild invocations. This simplifies CI log output and eliminates the last .NET Framework-oriented build artifact from the Arcade build toolchain.
Changes:
- Deleted the
Microsoft.DotNet.ArcadeLoggingproject (logger + helper) and removed it fromArcade.slnx. - Updated
Microsoft.DotNet.Arcade.Sdkpackaging to stop referencing/packing the logger assembly into the toolset. - Removed logger injection logic from
eng/common/tools.{sh,ps1}and dropped the-pipelineslogoption/default fromeng/common/build.sh; removed the telemetry guidance doc page.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.DotNet.ArcadeLogging/PipelinesLogger.cs | Removed the MSBuild logger implementation. |
| src/Microsoft.DotNet.ArcadeLogging/MessageBuilder.cs | Removed VSO command message builder used by the logger. |
| src/Microsoft.DotNet.ArcadeLogging/Microsoft.DotNet.ArcadeLogging.csproj | Removed the logger project (multi-TFM including .NET Framework). |
| src/Microsoft.DotNet.Arcade.Sdk/Microsoft.DotNet.Arcade.Sdk.csproj | Removed ArcadeLogging project reference and stopped packing the logger DLLs into the SDK toolset. |
| eng/common/tools.sh | Removed -logger: injection and related selection logic from MSBuild invocation. |
| eng/common/tools.ps1 | Removed /logger: injection and related selection logic from MSBuild invocation. |
| eng/common/build.sh | Removed -pipelineslog flag and CI default behavior tied to the removed logger. |
| Documentation/Projects/DevOps/CI/Telemetry-Guidance.md | Deleted telemetry guidance page that described the logger-based categorization format. |
| Arcade.slnx | Removed the ArcadeLogging project from the solution. |
Copilot's findings
Comments suppressed due to low confidence (1)
Documentation/Projects/DevOps/CI/Telemetry-Guidance.md:1
- This document is being removed, but there are still in-repo documents and scripts linking to it (e.g.
Documentation/Projects/DevOps/CI/NET-Core-Telemetry-Reports.md,Documentation/Projects/DevOps/CI/Roadmap.md,Documentation/Projects/PKPIs/Triage-Design.md, andeng/configure-toolset.sh). Deleting it will leave broken links; please update those references (or replace this page with a short stub/redirect that points readers to the new guidance).
- Files reviewed: 9/10 changed files
- Comments generated: 2
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@copilot update the pr description based on all the changes in this pr |
Updated. |
|
@ViktorHofer This might have had some downstream consumers. I think this emits strings that cause errors to surface in the AzDO timeline. |
|
Fine to bring it back if necessary. Do you have specific examples? |
Nothing yet. I'm not sure whether the data that pops out of this is actually used. But a heads up. |
When the Arcade msbuild logger was removed in dotnet#16814, the InitializeToolset call in the MSBuild function (tools.{sh,ps1}) was removed alongside it. That call had a load-bearing side effect: InitializeToolset invokes GetNuGetPackageCachePath, which exports NUGET_PACKAGES. Without NUGET_PACKAGES exported, NuGet restore defaults to the user profile while RepoLayout.props sets MSBuild's $(NuGetPackageRoot) to $(RepoRoot)/.packages/ under ContinuousIntegrationBuild=true. Generated .nuget.g.props imports guarded by Exists($(NuGetPackageRoot)...) are silently skipped, and properties contributed by them (e.g. XunitConsoleNetCoreAppPath) end up undefined. Restore the InitializeToolset call in the MSBuild function gated on $ci, matching the pre-dotnet#16814 behavior. Placing it in the MSBuild function rather than at tools.{sh,ps1} load time ensures any configure-toolset overrides have already been imported. Fixes dotnet#16898 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…translator PR dotnet#16814 removed the Microsoft.DotNet.ArcadeLogging MSBuild logger. That logger is what translated MSBuild error/warning events into Azure DevOps `##vso[task.logissue type=error]` logging commands, which populate a build timeline record's issues[] collection. dotnet-helix-service Build Analysis relies on those timeline issues to detect build/compile failures; without the logger, command-line `build.cmd` compile errors are written only to the console summary and binlog, so Build Analysis sees a failed step with no issues, drops it, and reports the build green. This restores the logger, but pared down to just that job. Per the original issue (dotnet#16794), the `NETCORE_ENGINEERING_TELEMETRY=<Category>` markers were the main complaint -- they add noise to CI logs. So this does NOT bring the markers back: - PipelinesLogger is reduced to translating errors and warnings into `logissue` timeline commands (type/sourcepath/line/column/code/message). - All telemetry-category plumbing is left removed: the marker prefix, the TelemetryLogged/project category tracking, the `_NETCORE_ENGINEERING_TELEMETRY` global properties in the SDK toolset .proj files, the Helix SDK category telemetry, the `(NETCORE_ENGINEERING_TELEMETRY=...)` marker in Write-PipelineTelemetryError, and the telemetry docs. Logger re-wired into the merged MSBuild function in eng/common/tools.ps1 and tools.sh (functions were merged in dotnet#16916); build.sh keeps current CI node_reuse handling and re-enables pipelines_log in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The
Microsoft.DotNet.ArcadeLoggingproject provided an msbuild logger that prefixed CI build messages with(NETCORE_ENGINEERING_TELEMETRY=<Category>). These markers clutter CI logs, are no longer believed to be consumed by any downstream system, and are the last piece of .NET Framework build infrastructure in the Arcade build. Removing the logger now; it can be reintroduced if a consumer surfaces.Changes
src/Microsoft.DotNet.ArcadeLogging/(PipelinesLogger.cs,MessageBuilder.cs, csproj) and drop its entry fromArcade.slnx.Microsoft.DotNet.Arcade.Sdk.csproj, remove theProjectReferenceto ArcadeLogging and the two<None Pack="true">items that shippedMicrosoft.DotNet.ArcadeLogging.dllundertoolset/net/andtoolset/netframework/.tools.ps1andtools.sh, drop the logic that locatedMicrosoft.DotNet.ArcadeLogging.dlland appended/logger:…/-logger:…to MSBuild invocations. The surroundingpipelines_logblock (NUGET timeout env vars,Enable-Nuget-EnhancedRetry,InitializeToolset) is retained.To double check: