Skip to content

Add telemetry collection for MSTest usage analytics #7570

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.

Sign up for GitHub

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

Draft
Evangelink wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Add telemetry collection for MSTest usage analytics #7570

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
06ad3e1
Add telemetry collection for MSTest usage analytics
Evangelink Mar 17, 2026
b609215
Fix Convert.ToHexString for .NET Framework targets
Evangelink Mar 17, 2026
3e6f8b5
Suppress IDE0032 for Volatile.Read/Write backing field
Evangelink Mar 17, 2026
6265e4f
Move GetAwaiter().GetResult() to MSTestDiscoverer call site
Evangelink Mar 17, 2026
f1c1fa5
Gate telemetry collection on TESTINGPLATFORM_TELEMETRY_OPTOUT env vars
Evangelink Mar 17, 2026
cbf9140
Fix telemetry lifecycle and handler coverage gaps
Evangelink Mar 17, 2026
2a26a81
Fix TelemetryTests: regex pattern, VSTest global.json, and workingDir…
Evangelink Mar 18, 2026
6259d0e
Address PR review feedback
Evangelink Mar 18, 2026
07a82f9
Address human reviewer feedback
Evangelink Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestBridgedTestFramework.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.Testing.Platform.Logging;
using Microsoft.Testing.Platform.Messages;
using Microsoft.Testing.Platform.Services;
using Microsoft.Testing.Platform.Telemetry;
using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter;
using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Extensions;
using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Helpers;
Expand Down Expand Up @@ -41,7 +42,7 @@ protected override Task SynchronizedDiscoverTestsAsync(VSTestDiscoverTestExecuti
Debugger.Launch();
}

new MSTestDiscoverer().DiscoverTests(request.AssemblyPaths, request.DiscoveryContext, request.MessageLogger, request.DiscoverySink, _configuration);
new MSTestDiscoverer(new TestSourceHandler(), CreateTelemetrySender()).DiscoverTests(request.AssemblyPaths, request.DiscoveryContext, request.MessageLogger, request.DiscoverySink, _configuration);
return Task.CompletedTask;
}

Expand All @@ -55,7 +56,7 @@ protected override async Task SynchronizedRunTestsAsync(VSTestRunTestExecutionRe
Debugger.Launch();
}

MSTestExecutor testExecutor = new(cancellationToken);
MSTestExecutor testExecutor = new(cancellationToken, CreateTelemetrySender());
await testExecutor.RunTestsAsync(request.AssemblyPaths, request.RunContext, request.FrameworkHandle, _configuration).ConfigureAwait(false);
}

Expand Down Expand Up @@ -103,5 +104,19 @@ private static TestMethodIdentifierProperty GetMethodIdentifierPropertyFromManag
// Or alternatively, does VSTest object model expose the assembly full name somewhere?
return new TestMethodIdentifierProperty(assemblyFullName: string.Empty, @namespace, typeName, methodName, arity, parameterTypes, returnTypeFullName: string.Empty);
}

[SuppressMessage("ApiDesign", "RS0030:Do not use banned APIs", Justification = "We can use MTP from this folder")]
private Func<string, IDictionary<string, object>, Task>? CreateTelemetrySender()
{
ITelemetryInformation telemetryInformation = ServiceProvider.GetTelemetryInformation();
if (!telemetryInformation.IsEnabled)
{
return null;
}

ITelemetryCollector telemetryCollector = ServiceProvider.GetTelemetryCollector();

return (eventName, metrics) => telemetryCollector.LogEventAsync(eventName, metrics, CancellationToken.None);
}
}
#endif
26 changes: 22 additions & 4 deletions src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestDiscoverer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter;
internal sealed class MSTestDiscoverer : ITestDiscoverer
{
private readonly ITestSourceHandler _testSourceHandler;
private readonly Func<string, IDictionary<string, object>, Task>? _telemetrySender;

public MSTestDiscoverer()
Copy link
Member

@Youssef1313 Youssef1313 Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Is the parameterless constructor no longer called? Should we remove it?
  • Should we now remove the "for testing purposes" comment on the other constructor?

Copy link
Member Author

@Evangelink Evangelink Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[AI generated] Good questions. The parameterless constructor is still called — it's the public entry point used by the VSTest framework to instantiate the discoverer via reflection. The internal /* for testing purposes */ constructor with telemetrySender is used by MSTestBridgedTestFramework (MTP path) which passes a real telemetry sender. Both constructors are needed. The "for testing purposes" comment could be misleading now since it's also used in production (MTP path) — happy to update the comment if the team prefers.

: this(new TestSourceHandler())
{
}

internal /* for testing purposes */ MSTestDiscoverer(ITestSourceHandler testSourceHandler)
=> _testSourceHandler = testSourceHandler;
internal /* for testing purposes */ MSTestDiscoverer(ITestSourceHandler testSourceHandler, Func<string, IDictionary<string, object>, Task>? telemetrySender = null)
{
_testSourceHandler = testSourceHandler;
_telemetrySender = telemetrySender;
}

/// <summary>
/// Discovers the tests available from the provided source. Not supported for .xap source.
Expand All @@ -47,9 +51,23 @@ internal void DiscoverTests(IEnumerable<string> sources, IDiscoveryContext disco
Ensure.NotNull(logger);
Ensure.NotNull(discoverySink);

if (MSTestDiscovererHelpers.InitializeDiscovery(sources, discoveryContext, logger, configuration, _testSourceHandler))
// Initialize telemetry collection if not already set (e.g. first call in the session)
if (!MSTestTelemetryDataCollector.IsTelemetryOptedOut())
{
_ = MSTestTelemetryDataCollector.EnsureInitialized();
}

try
{
if (MSTestDiscovererHelpers.InitializeDiscovery(sources, discoveryContext, logger, configuration, _testSourceHandler))
{
new UnitTestDiscoverer(_testSourceHandler).DiscoverTests(sources, logger, discoverySink, discoveryContext);
}
}
finally
{
new UnitTestDiscoverer(_testSourceHandler).DiscoverTests(sources, logger, discoverySink, discoveryContext);
// Use Task.Run to avoid capturing any SynchronizationContext that could cause deadlocks
Copy link
Member

@Youssef1313 Youssef1313 Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes no sense to me.

Copy link
Member Author

@Evangelink Evangelink Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[AI generated] Could you elaborate on what doesn't make sense? This line Task.Run(() => MSTestTelemetryDataCollector.SendTelemetryAndResetAsync(_telemetrySender)).GetAwaiter().GetResult() sends telemetry at the end of discovery in the VSTest path. The Task.Run is there to avoid capturing a SynchronizationContext that could deadlock since DiscoverTests is synchronous. Happy to discuss alternatives.

Task.Run(() => MSTestTelemetryDataCollector.SendTelemetryAndResetAsync(_telemetrySender)).GetAwaiter().GetResult();
Copy link
Member

@Youssef1313 Youssef1313 Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a good pattern.

  • Do we really want to have this as a fire-and-forget?
  • Synchronously blocking a threadpool thread on an async task is bad.

Copy link
Member Author

@Evangelink Evangelink Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[AI generated] Fair points. The DiscoverTests method is synchronous (VSTest ITestDiscoverer interface), so we can't await directly. The Task.Run(...).GetAwaiter().GetResult() pattern avoids SynchronizationContext deadlocks.

Regarding fire-and-forget: telemetry sending is wrapped in a try/catch that swallows all exceptions (SendTelemetryAndResetAsync never throws), so the blocking call is safe. The alternative would be to make it truly fire-and-forget, but then we risk the process exiting before telemetry is flushed.

This is a known trade-off — the synchronous VSTest interface forces this pattern. The MTP path (in MSTestExecutor) handles this more cleanly since the execution path is already async. Open to suggestions if there's a better approach within the VSTest constraints.

}
}
}
37 changes: 34 additions & 3 deletions src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestExecutor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter;
internal sealed class MSTestExecutor : ITestExecutor
{
private readonly CancellationToken _cancellationToken;
private readonly Func<string, IDictionary<string, object>, Task>? _telemetrySender;

/// <summary>
/// Token for canceling the test run.
Expand All @@ -35,10 +36,11 @@ public MSTestExecutor()
_cancellationToken = CancellationToken.None;
}

internal MSTestExecutor(CancellationToken cancellationToken)
internal MSTestExecutor(CancellationToken cancellationToken, Func<string, IDictionary<string, object>, Task>? telemetrySender = null)
{
TestExecutionManager = new TestExecutionManager();
_cancellationToken = cancellationToken;
_telemetrySender = telemetrySender;
}

/// <summary>
Expand Down Expand Up @@ -105,12 +107,25 @@ internal async Task RunTestsAsync(IEnumerable<TestCase>? tests, IRunContext? run
Ensure.NotNull(frameworkHandle);
Ensure.NotNullOrEmpty(tests);

// Initialize telemetry collection if not already set
if (!MSTestTelemetryDataCollector.IsTelemetryOptedOut())
{
_ = MSTestTelemetryDataCollector.EnsureInitialized();
}

Comment on lines +116 to +123
Copy link
Member Author

@Evangelink Evangelink Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[AI generated] The HasData flag is set to true by TrackDiscoveredMethod and TrackDiscoveredClass during discovery, which happens before execution for both the TestCase and sources paths. Settings metrics are always added by BuildMetrics unconditionally. The early-return path now drains assertion counters (fixed in cbf9140), so there's no memory leak risk. The current design is: if no test classes/methods were discovered (HasData=false), we don't send the telemetry event but still clean up counters.

if (!MSTestDiscovererHelpers.InitializeDiscovery(from test in tests select test.Source, runContext, frameworkHandle, configuration, new TestSourceHandler()))
{
return;
}

await RunTestsFromRightContextAsync(frameworkHandle, async testRunToken => await TestExecutionManager.RunTestsAsync(tests, runContext, frameworkHandle, testRunToken).ConfigureAwait(false)).ConfigureAwait(false);
try
{
await RunTestsFromRightContextAsync(frameworkHandle, async testRunToken => await TestExecutionManager.RunTestsAsync(tests, runContext, frameworkHandle, testRunToken).ConfigureAwait(false)).ConfigureAwait(false);
}
finally
{
await SendTelemetryAsync().ConfigureAwait(false);
}
}

internal async Task RunTestsAsync(IEnumerable<string>? sources, IRunContext? runContext, IFrameworkHandle? frameworkHandle, IConfiguration? configuration)
Expand All @@ -123,14 +138,27 @@ internal async Task RunTestsAsync(IEnumerable<string>? sources, IRunContext? run
Ensure.NotNull(frameworkHandle);
Ensure.NotNullOrEmpty(sources);

// Initialize telemetry collection if not already set
if (!MSTestTelemetryDataCollector.IsTelemetryOptedOut())
{
_ = MSTestTelemetryDataCollector.EnsureInitialized();
}

TestSourceHandler testSourceHandler = new();
if (!MSTestDiscovererHelpers.InitializeDiscovery(sources, runContext, frameworkHandle, configuration, testSourceHandler))
{
return;
}

sources = testSourceHandler.GetTestSources(sources);
await RunTestsFromRightContextAsync(frameworkHandle, async testRunToken => await TestExecutionManager.RunTestsAsync(sources, runContext, frameworkHandle, testSourceHandler, testRunToken).ConfigureAwait(false)).ConfigureAwait(false);
try
{
await RunTestsFromRightContextAsync(frameworkHandle, async testRunToken => await TestExecutionManager.RunTestsAsync(sources, runContext, frameworkHandle, testSourceHandler, testRunToken).ConfigureAwait(false)).ConfigureAwait(false);
}
finally
{
await SendTelemetryAsync().ConfigureAwait(false);
}
}

/// <summary>
Expand All @@ -139,6 +167,9 @@ internal async Task RunTestsAsync(IEnumerable<string>? sources, IRunContext? run
public void Cancel()
=> _testRunCancellationToken?.Cancel();

private async Task SendTelemetryAsync()
=> await MSTestTelemetryDataCollector.SendTelemetryAndResetAsync(_telemetrySender).ConfigureAwait(false);

private async Task RunTestsFromRightContextAsync(IFrameworkHandle frameworkHandle, Func<TestRunCancellationToken, Task> runTestsAction)
{
ApartmentState? requestedApartmentState = MSTestSettings.RunConfigurationSettings.ExecutionApartmentState;
Expand Down
2 changes: 1 addition & 1 deletion src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ internal virtual TypeEnumerator GetTypeEnumerator(Type type, string assemblyFile
var typeValidator = new TypeValidator(ReflectHelper, discoverInternals);
var testMethodValidator = new TestMethodValidator(ReflectHelper, discoverInternals);

return new TypeEnumerator(type, assemblyFileName, ReflectHelper, typeValidator, testMethodValidator);
return new TypeEnumerator(type, assemblyFileName, ReflectHelper, typeValidator, testMethodValidator, MSTestTelemetryDataCollector.Current);
}

private List<UnitTestElement> DiscoverTestsInType(
Expand Down
13 changes: 12 additions & 1 deletion src/Adapter/MSTestAdapter.PlatformServices/Discovery/TypeEnumerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ internal class TypeEnumerator
private readonly TypeValidator _typeValidator;
private readonly TestMethodValidator _testMethodValidator;
private readonly ReflectHelper _reflectHelper;
private readonly MSTestTelemetryDataCollector? _telemetryDataCollector;

/// <summary>
/// Initializes a new instance of the <see cref="TypeEnumerator"/> class.
Expand All @@ -28,13 +29,15 @@ internal class TypeEnumerator
/// <param name="reflectHelper"> An instance to reflection helper for type information. </param>
/// <param name="typeValidator"> The validator for test classes. </param>
/// <param name="testMethodValidator"> The validator for test methods. </param>
internal TypeEnumerator(Type type, string assemblyFilePath, ReflectHelper reflectHelper, TypeValidator typeValidator, TestMethodValidator testMethodValidator)
/// <param name="telemetryDataCollector"> Optional telemetry data collector for tracking API usage. </param>
internal TypeEnumerator(Type type, string assemblyFilePath, ReflectHelper reflectHelper, TypeValidator typeValidator, TestMethodValidator testMethodValidator, MSTestTelemetryDataCollector? telemetryDataCollector = null)
{
_type = type;
_assemblyFilePath = assemblyFilePath;
_reflectHelper = reflectHelper;
_typeValidator = typeValidator;
_testMethodValidator = testMethodValidator;
_telemetryDataCollector = telemetryDataCollector;
}

/// <summary>
Expand All @@ -49,6 +52,13 @@ internal TypeEnumerator(Type type, string assemblyFilePath, ReflectHelper reflec
return null;
}

// Track class-level attributes for telemetry
if (_telemetryDataCollector is not null)
{
Attribute[] classAttributes = _reflectHelper.GetCustomAttributesCached(_type);
_telemetryDataCollector.TrackDiscoveredClass(_type, classAttributes);
}

// If test class is valid, then get the tests
return GetTests(warnings);
}
Expand Down Expand Up @@ -143,6 +153,7 @@ internal UnitTestElement GetTestFromMethod(MethodInfo method, bool classDisables
};

Attribute[] attributes = _reflectHelper.GetCustomAttributesCached(method);
_telemetryDataCollector?.TrackDiscoveredMethod(attributes);
TestMethodAttribute? testMethodAttribute = null;

// Backward looping for backcompat. This used to be calls to _reflectHelper.GetFirstAttributeOrDefault
Expand Down
12 changes: 12 additions & 0 deletions src/Adapter/MSTestAdapter.PlatformServices/MSTestSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,18 @@ internal static void PopulateSettings(IDiscoveryContext? context, IMessageLogger

CurrentSettings = settings;
RunConfigurationSettings = runConfigurationSettings;

// Track configuration source for telemetry
#if !WINDOWS_UWP
if (MSTestTelemetryDataCollector.Current is { } telemetry)
{
telemetry.ConfigurationSource = configuration?["mstest"] is not null
? "testconfig.json"
: !StringEx.IsNullOrEmpty(context?.RunSettings?.SettingsXml)
? "runsettings"
: "none";
}
#endif
}

/// <summary>
Expand Down
Loading
Loading