Replace VS OpenTelemetry with Microsoft.VisualStudio.Telemetry #12843
Conversation
|
@JanProvaznik please take a look if i didn't remove something extra |
JanProvaznik
left a comment
JanProvaznik
left a comment
There was a problem hiding this comment.
it removes some reusability of behavior between framework and core
| </PropertyGroup> | ||
| <PropertyGroup> | ||
| <MicrosoftVisualStudioOpenTelemetryVersion>0.2.104-beta</MicrosoftVisualStudioOpenTelemetryVersion> | ||
| <MicrosoftVisualStudioTelemetryVersion>17.14.18</MicrosoftVisualStudioTelemetryVersion> |
There was a problem hiding this comment.
does this match version in VS? how do we coordinate changes?
There was a problem hiding this comment.
I think it's not a big question since we exclude all assets and rely on the version that comes with VS.
There was a problem hiding this comment.
I would put this into eng/dependabot/Packages.props so that it at least gets updated when there's a new version on nuget.org / our feeds.
There was a problem hiding this comment.
If we're excluding assets and taking whatever is from VS then we don't want to bump dependencies, right? We want to compile against deliberately -older versions so that we run no matter what version is loaded by the host.
There was a problem hiding this comment.
If that assembly revs its assembly version then yes. Not keeping dependencies up-to-date just creates work down the line in servicing.
YuliiaKovalova
force-pushed
the
dev/ykovalova/telemetry_spike
branch
from
af884cf to
79f1712
Compare
YuliiaKovalova
changed the title
Added PackageReference for System.Diagnostics.DiagnosticSource for consistency.
There was a problem hiding this comment.
Pull request overview
This PR replaces the VS OpenTelemetry infrastructure with the simpler Microsoft.VisualStudio.Telemetry package to improve Visual Studio performance and reduce dependency complexity. The change removes ~20 package dependencies related to OpenTelemetry and replaces the System.Diagnostics.Activity-based approach with VS Telemetry's OperationEvent model.
Key Changes
- Simplified telemetry stack: Replaced OpenTelemetry SDK packages with single Microsoft.VisualStudio.Telemetry package
- New abstraction layer: Introduced
IActivityinterface to abstract telemetry collection across .NET Framework (VS Telemetry) and .NET Core (System.Diagnostics.Activity) - Removed complexity: Eliminated TracerProvider, Collector, and extensive middleware configuration
Reviewed changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Framework/Telemetry/TelemetryManager.cs | New telemetry manager replacing OpenTelemetryManager; handles initialization and session management |
| src/Framework/Telemetry/VSTelemetryActivity.cs | New wrapper for VS Telemetry operations on .NET Framework |
| src/Framework/Telemetry/DiagnosticActivity.cs | New wrapper for System.Diagnostics.Activity on .NET Core |
| src/Framework/Telemetry/IActivity.cs | New abstraction interface for telemetry activities |
| src/Framework/Telemetry/MSBuildActivitySource.cs | Updated to use IActivity abstraction instead of raw Activity |
| src/Framework/Telemetry/TelemetryDataUtils.cs | Moved from Build project to Framework; converts node telemetry to structured data |
| src/Framework/Telemetry/BuildInsights.cs | New container for build telemetry insights |
| src/Framework/Telemetry/BuildTelemetry.cs | Updated to support both GetActivityProperties() and GetProperties() |
| src/MSBuild/XMake.cs | Simplified telemetry initialization; now only initializes when opt-in enabled |
| src/Build/BackEnd/BuildManager/BuildManager.cs | Moved telemetry initialization to CreateLoggingService; removed error logging for assembly load failures |
| src/Framework/Microsoft.Build.Framework.csproj | Package reference updated to Microsoft.VisualStudio.Telemetry |
| eng/Versions.props | Version updated from MicrosoftVisualStudioOpenTelemetryVersion to MicrosoftVisualStudioTelemetryVersion |
| eng/Packages.props | Package references updated |
| src/Package/MSBuild.VSSetup/files.swr | Removed 18 OpenTelemetry and Microsoft.Extensions DLL entries |
| src/Package/Microsoft.Build.UnGAC/Program.cs | Removed OpenTelemetry assembly references from ungac list |
| src/MSBuild/app*.config | Removed assembly binding redirects for OpenTelemetry dependencies |
| THIRDPARTYNOTICES.txt | Removed OpenTelemetry license notices |
| NuGet.config | Removed vs-impl package source mapping for OpenTelemetry packages |
| documentation/specs/* | Removed telemetry design documents (now obsolete) |
| src/Build/Resources/Strings.resx | Removed "OpenTelemetryLoadFailed" error message |
| src/Build.UnitTests/Telemetry/* | Removed all OpenTelemetry-specific tests (325+ lines) |
JanProvaznik
left a comment
JanProvaznik
left a comment
There was a problem hiding this comment.
looks almost ready, I think adjusting instead of removing the test for telemetry forwarding would be valuable
YuliiaKovalova
force-pushed
the
dev/ykovalova/telemetry_spike
branch
from
7b133a5 to
7ca2245
Compare
Summary
This PR replaces the VS OpenTelemetry-based telemetry infrastructure with the simpler Microsoft.VisualStudio.Telemetry package, simplifying the telemetry implementation while maintaining the ability to collect build telemetry in Visual Studio scenarios.
Motivation
The VS OpenTelemetry integration introduced dependencies on many packages and made VS perf sad.
Notes
Telemetry collection continues to be opt-in via the MSBUILD_TELEMETRY_OPTIN environment variable
The change removes the System.Diagnostics.Activity-based telemetry approach in favor of VS Telemetry's simpler model