Fixed telemetry data sharing exception

src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoveryManager.cs

@@ -73,6 +73,9 @@ protected DiscoveryManager(IRequestData requestData, ITestPlatformEventSource te
     /// <param name="pathToAdditionalExtensions"> The path to additional extensions. </param>
     public void Initialize(IEnumerable<string> pathToAdditionalExtensions, ITestDiscoveryEventsHandler2 eventHandler)
     {
+        // Clear the request data metrics left over from a potential previous run.
+        _requestData.MetricsCollection?.Metrics?.Clear();
+
         _testPlatformEventSource.AdapterSearchStart();
         _testDiscoveryEventsHandler = eventHandler;
         if (pathToAdditionalExtensions != null && pathToAdditionalExtensions.Any())

src/Microsoft.TestPlatform.CrossPlatEngine/Execution/ExecutionManager.cs

@@ -64,6 +64,9 @@ protected ExecutionManager(ITestPlatformEventSource testPlatformEventSource, IRe
     /// <param name="pathToAdditionalExtensions"> The path to additional extensions. </param>
     public void Initialize(IEnumerable<string> pathToAdditionalExtensions, ITestMessageEventHandler testMessageEventsHandler)
     {
+        // Clear the request data metrics left over from a potential previous run.
+        _requestData.MetricsCollection.Metrics.Clear();
+
         _testMessageEventsHandler = testMessageEventsHandler;
         _testPlatformEventSource.AdapterSearchStart();
 

src/Microsoft.TestPlatform.CrossPlatEngine/PublicAPI/PublicAPI.Shipped.txt

@@ -102,7 +102,7 @@ Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestExtensionManager.UseAddi
 Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestHostManagerFactory
 Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestHostManagerFactory.GetDiscoveryManager() -> Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.TesthostProtocol.IDiscoveryManager
 Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestHostManagerFactory.GetExecutionManager() -> Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.TesthostProtocol.IExecutionManager
-Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestHostManagerFactory.TestHostManagerFactory(Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.IRequestData requestData) -> void
+Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestHostManagerFactory.TestHostManagerFactory(bool telemetryOptedIn) -> void
 Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestSessionPool
 override Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.ProxyOperationManagerWithDataCollection.Initialize(bool skipDefaultAdapters) -> void
 override Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.ProxyOperationManagerWithDataCollection.SetupChannel(System.Collections.Generic.IEnumerable<string> sources, string runSettings, Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.ITestMessageEventHandler eventHandler) -> bool
@@ -120,7 +120,7 @@ virtual Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.ProxyTestSessionMana
 virtual Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.ProxyTestSessionManager.EnqueueProxy(int proxyId) -> bool
 virtual Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestSessionPool.AddSession(Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.TestSessionInfo testSessionInfo, Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.ProxyTestSessionManager proxyManager) -> bool
 virtual Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestSessionPool.ReturnProxy(Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.TestSessionInfo testSessionInfo, int proxyId) -> bool
-virtual Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestSessionPool.TryTakeProxy(Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.TestSessionInfo testSessionInfo, string source, string runSettings) -> Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.ProxyOperationManager
+virtual Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestSessionPool.TryTakeProxy(Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.TestSessionInfo testSessionInfo, string source, string runSettings, Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.IRequestData requestData) -> Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.ProxyOperationManager
 Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection.DataCollectionResult
 Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection.DataCollectionResult.Attachments.get -> System.Collections.ObjectModel.Collection<Microsoft.VisualStudio.TestPlatform.ObjectModel.AttachmentSet>
 Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection.DataCollectionResult.DataCollectionResult(System.Collections.ObjectModel.Collection<Microsoft.VisualStudio.TestPlatform.ObjectModel.AttachmentSet> attachments, System.Collections.ObjectModel.Collection<Microsoft.VisualStudio.TestPlatform.ObjectModel.InvokedDataCollector> invokedDataCollectors) -> void

src/Microsoft.TestPlatform.CrossPlatEngine/TestEngine.cs

@@ -78,11 +78,9 @@ public IProxyDiscoveryManager GetDiscoveryManager(
 
         if (ShouldRunInNoIsolation(discoveryCriteria.RunSettings, parallelLevel > 1, false))
         {
-            var isTelemetryOptedIn = requestData.IsTelemetryOptedIn;
-            var newRequestData = GetRequestData(isTelemetryOptedIn);
             return new InProcessProxyDiscoveryManager(
                 testHostManager,
-                new TestHostManagerFactory(newRequestData));
+                new TestHostManagerFactory(requestData.IsTelemetryOptedIn));
         }
 
         // Create one data aggregator per parallel discovery and share it with all the proxy discovery managers.
@@ -108,7 +106,8 @@ public IProxyDiscoveryManager GetDiscoveryManager(
                     var proxyOperationManager = TestSessionPool.Instance.TryTakeProxy(
                         discoveryCriteria.TestSessionInfo,
                         source,
-                        discoveryCriteria.RunSettings);
+                        discoveryCriteria.RunSettings,
+                        requestData);
 
                     if (proxyOperationManager == null)
                     {
@@ -181,11 +180,9 @@ public IProxyExecutionManager GetExecutionManager(
                 parallelLevel > 1,
                 isDataCollectorEnabled || isInProcDataCollectorEnabled))
         {
-            var isTelemetryOptedIn = requestData.IsTelemetryOptedIn;
-            var newRequestData = GetRequestData(isTelemetryOptedIn);
             return new InProcessProxyExecutionManager(
                 testHostManager,
-                new TestHostManagerFactory(newRequestData));
+                new TestHostManagerFactory(requestData.IsTelemetryOptedIn));
         }
 
         // SetupChannel ProxyExecutionManager with data collection if data collectors are
@@ -216,7 +213,8 @@ public IProxyExecutionManager GetExecutionManager(
                         var proxyOperationManager = TestSessionPool.Instance.TryTakeProxy(
                             testRunCriteria.TestSessionInfo,
                             source,
-                            testRunCriteria.TestRunSettings);
+                            testRunCriteria.TestRunSettings,
+                            requestData);
 
                         if (proxyOperationManager == null)
                         {
@@ -494,24 +492,6 @@ private bool ShouldRunInNoIsolation(
         return false;
     }
 
-    /// <summary>
-    /// Get request data on basis of telemetry opted in or not.
-    /// </summary>
-    ///
-    /// <param name="isTelemetryOptedIn">A flag indicating if telemetry is opted in.</param>
-    ///
-    /// <returns>The request data.</returns>
-    private IRequestData GetRequestData(bool isTelemetryOptedIn)
-    {
-        return new RequestData
-        {
-            MetricsCollection = isTelemetryOptedIn
-                ? (IMetricsCollection)new MetricsCollection()
-                : new NoOpMetricsCollection(),
-            IsTelemetryOptedIn = isTelemetryOptedIn
-        };
-    }
-
     /// <summary>
     /// Gets test sources from test run criteria.
     /// </summary>

src/Microsoft.TestPlatform.CrossPlatEngine/TestHostManagerFactory.cs

@@ -1,12 +1,10 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using Microsoft.VisualStudio.TestPlatform.Common;
+using Microsoft.VisualStudio.TestPlatform.Common.Telemetry;
 using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Discovery;
-
 using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Execution;
-
-using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
-
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.TesthostProtocol;
 
 #nullable disable
@@ -20,30 +18,43 @@ public class TestHostManagerFactory : ITestHostManagerFactory
 {
     private IDiscoveryManager _discoveryManager;
     private IExecutionManager _executionManager;
-    private readonly IRequestData _requestData;
+    private readonly bool _telemetryOptedIn;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="TestHostManagerFactory"/> class.
     /// </summary>
-    /// <param name="requestData">
-    /// Provide common services and data for a discovery/run request.
+    /// 
+    /// <param name="telemetryOptedIn">
+    /// A value indicating if the telemetry is opted in or not.
     /// </param>
-    public TestHostManagerFactory(IRequestData requestData!!)
+    public TestHostManagerFactory(bool telemetryOptedIn)
     {
-        _requestData = requestData;
+        _telemetryOptedIn = telemetryOptedIn;
     }
 
     /// <summary>
     /// The discovery manager instance for any discovery related operations inside of the test host.
     /// </summary>
     /// <returns>The discovery manager.</returns>
     public IDiscoveryManager GetDiscoveryManager()
-        => _discoveryManager ??= new DiscoveryManager(_requestData);
+        => _discoveryManager ??= new DiscoveryManager(GetRequestData(_telemetryOptedIn));
 
     /// <summary>
     /// The execution manager instance for any discovery related operations inside of the test host.
     /// </summary>
     /// <returns>The execution manager.</returns>
     public IExecutionManager GetExecutionManager()
-        => _executionManager ??= new ExecutionManager(_requestData);
+        => _executionManager ??= new ExecutionManager(GetRequestData(_telemetryOptedIn));
+
+    private RequestData GetRequestData(bool telemetryOptedIn)
+    {
+        return new RequestData
+        {
+            MetricsCollection =
+                telemetryOptedIn
+                    ? new MetricsCollection()
+                    : new NoOpMetricsCollection(),
+            IsTelemetryOptedIn = telemetryOptedIn
+        };
+    }
 }

src/Microsoft.TestPlatform.CrossPlatEngine/TestSession/TestSessionPool.cs

@@ -125,12 +125,14 @@ public virtual bool KillSession(TestSessionInfo testSessionInfo, IRequestData re
     /// <param name="testSessionInfo">The test session info object.</param>
     /// <param name="source">The source to be associated to this proxy.</param>
     /// <param name="runSettings">The run settings.</param>
+    /// <param name="requestData">The request data.</param>
     /// 
     /// <returns>The proxy object.</returns>
     public virtual ProxyOperationManager TryTakeProxy(
         TestSessionInfo testSessionInfo,
         string source,
-        string runSettings)
+        string runSettings,
+        IRequestData requestData!!)
     {
         ProxyTestSessionManager sessionManager = null;
         lock (_lockObject)
@@ -147,7 +149,13 @@ public virtual ProxyOperationManager TryTakeProxy(
         try
         {
             // Deque an actual proxy to do work.
-            return sessionManager.DequeueProxy(source, runSettings);
+            var proxy = sessionManager.DequeueProxy(source, runSettings);
+
+            // Make sure we use the per-request request data instead of the request data used when
+            // creating the test session.
+            proxy.RequestData = requestData;
+
+            return proxy;
         }
         catch (InvalidOperationException ex)
         {

src/testhost.x86/DefaultEngineInvoker.cs

@@ -135,13 +135,13 @@ public void Invoke(IDictionary<string, string?> argsDictionary)
             ConnectToDatacollector(dcPort);
         }
 
-        var requestData = GetRequestData(argsDictionary);
+        var telemetryOptedIn = GetTelemetryStatusFromArgs(argsDictionary);
 
         // Start processing async in a different task
         EqtTrace.Info("DefaultEngineInvoker.Invoke: Start Request Processing.");
         try
         {
-            StartProcessingAsync(_requestHandler, new TestHostManagerFactory(requestData)).Wait();
+            StartProcessingAsync(_requestHandler, new TestHostManagerFactory(telemetryOptedIn)).Wait();
         }
         finally
         {
@@ -155,29 +155,15 @@ public void Invoke(IDictionary<string, string?> argsDictionary)
         }
     }
 
-    private static RequestData GetRequestData(IDictionary<string, string?> argsDictionary)
+    private static bool GetTelemetryStatusFromArgs(IDictionary<string, string?> argsDictionary)
     {
-        // Checks for Telemetry Opted in or not from Command line Arguments.
-        // By Default opting out in Test Host to handle scenario when user running old version of vstest.console
+        // Checks if telemetry is opted in in the command line arguments.
+        // By default opting out in testhost to handle the scenario when the user is running an old
+        // version of vstest.console.
         var telemetryStatus = CommandLineArgumentsHelper.GetStringArgFromDict(argsDictionary, TelemetryOptedIn);
-        var telemetryOptedIn = false;
-        if (!string.IsNullOrWhiteSpace(telemetryStatus))
-        {
-            if (telemetryStatus.Equals("true", StringComparison.Ordinal))
-            {
-                telemetryOptedIn = true;
-            }
-        }
 
-        var requestData = new RequestData
-        {
-            MetricsCollection =
-                telemetryOptedIn
-                    ? new MetricsCollection()
-                    : new NoOpMetricsCollection(),
-            IsTelemetryOptedIn = telemetryOptedIn
-        };
-        return requestData;
+        return !string.IsNullOrWhiteSpace(telemetryStatus)
+            && telemetryStatus.Equals("true", StringComparison.Ordinal);
     }
 
     private void ConnectToDatacollector(int dcPort)

test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyDiscoveryManagerTests.cs

@@ -529,7 +529,8 @@ public void StartTestRunShouldAttemptToTakeProxyFromPoolIfProxyIsNull()
                 var proxyOperationManager = TestSessionPool.Instance.TryTakeProxy(
                     testSessionInfo,
                     source,
-                    _discoveryCriteria.RunSettings);
+                    _discoveryCriteria.RunSettings,
+                    _mockRequestData.Object);
 
                 return proxyOperationManager;
             };
@@ -551,7 +552,8 @@ public void StartTestRunShouldAttemptToTakeProxyFromPoolIfProxyIsNull()
                     tsp => tsp.TryTakeProxy(
                         testSessionInfo,
                         It.IsAny<string>(),
-                        It.IsAny<string>()))
+                        It.IsAny<string>(),
+                        _mockRequestData.Object))
                 .Returns(mockProxyOperationManager.Object);
 
             testDiscoveryManager.Initialize(true);
@@ -563,7 +565,8 @@ public void StartTestRunShouldAttemptToTakeProxyFromPoolIfProxyIsNull()
                 tsp => tsp.TryTakeProxy(
                     testSessionInfo,
                     It.IsAny<string>(),
-                    It.IsAny<string>()),
+                    It.IsAny<string>(),
+                    _mockRequestData.Object),
                 Times.Once);
         }
         finally

test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyExecutionManagerTests.cs

@@ -742,7 +742,8 @@ public void StartTestRunShouldAttemptToTakeProxyFromPoolIfProxyIsNull()
                 var proxyOperationManager = TestSessionPool.Instance.TryTakeProxy(
                     testSessionInfo,
                     source,
-                    string.Empty);
+                    string.Empty,
+                    _mockRequestData.Object);
 
                 return proxyOperationManager;
             };
@@ -765,7 +766,8 @@ public void StartTestRunShouldAttemptToTakeProxyFromPoolIfProxyIsNull()
                     tsp => tsp.TryTakeProxy(
                         testSessionInfo,
                         It.IsAny<string>(),
-                        It.IsAny<string>()))
+                        It.IsAny<string>(),
+                        _mockRequestData.Object))
                 .Returns(mockProxyOperationManager.Object);
 
             testExecutionManager.Initialize(true);
@@ -777,7 +779,8 @@ public void StartTestRunShouldAttemptToTakeProxyFromPoolIfProxyIsNull()
                 tsp => tsp.TryTakeProxy(
                     testSessionInfo,
                     It.IsAny<string>(),
-                    It.IsAny<string>()),
+                    It.IsAny<string>(),
+                    _mockRequestData.Object),
                 Times.Once);
         }
         finally

test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/TestHostManagerFactoryTests.cs

@@ -23,13 +23,7 @@ public TestHostManagerFactoryTests()
         _mockMetricsCollection = new Mock<IMetricsCollection>();
         _mockRequestData = new Mock<IRequestData>();
         _mockRequestData.Setup(rd => rd.MetricsCollection).Returns(_mockMetricsCollection.Object);
-        _testHostManagerFactory = new TestHostManagerFactory(_mockRequestData.Object);
-    }
-
-    [TestMethod]
-    public void ConstructorShouldThrowIfRequestDataIsNull()
-    {
-        Assert.ThrowsException<ArgumentNullException>(() => new TestHostManagerFactory(null));
+        _testHostManagerFactory = new TestHostManagerFactory(false);
     }
 
     [TestMethod]

test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/TestSession/TestSessionPoolTests.cs

@@ -66,6 +66,7 @@ public void TakeProxyShouldSucceedIfMatchingCriteriaAreCorrect()
         TestSessionPool.Instance = null;
 
         var testSessionInfo = new TestSessionInfo();
+        var mockRequestData = new Mock<IRequestData>();
         var mockProxyTestSessionManager = new Mock<ProxyTestSessionManager>(
             new StartTestSessionCriteria(),
             1,
@@ -77,17 +78,17 @@ public void TakeProxyShouldSucceedIfMatchingCriteriaAreCorrect()
 
         Assert.IsNotNull(TestSessionPool.Instance);
         // Take proxy fails because test session is invalid.
-        Assert.IsNull(TestSessionPool.Instance.TryTakeProxy(new TestSessionInfo(), string.Empty, string.Empty));
+        Assert.IsNull(TestSessionPool.Instance.TryTakeProxy(new TestSessionInfo(), string.Empty, string.Empty, mockRequestData.Object));
         mockProxyTestSessionManager.Verify(tsm => tsm.DequeueProxy(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
 
         Assert.IsTrue(TestSessionPool.Instance.AddSession(testSessionInfo, mockProxyTestSessionManager.Object));
 
         // First TakeProxy fails because of throwing, see setup sequence.
-        Assert.IsNull(TestSessionPool.Instance.TryTakeProxy(testSessionInfo, string.Empty, string.Empty));
+        Assert.IsNull(TestSessionPool.Instance.TryTakeProxy(testSessionInfo, string.Empty, string.Empty, mockRequestData.Object));
         mockProxyTestSessionManager.Verify(tsm => tsm.DequeueProxy(It.IsAny<string>(), It.IsAny<string>()), Times.Once);
 
         // Second TakeProxy succeeds, see setup sequence.
-        Assert.IsNotNull(TestSessionPool.Instance.TryTakeProxy(testSessionInfo, string.Empty, string.Empty));
+        Assert.IsNotNull(TestSessionPool.Instance.TryTakeProxy(testSessionInfo, string.Empty, string.Empty, mockRequestData.Object));
         mockProxyTestSessionManager.Verify(tsm => tsm.DequeueProxy(It.IsAny<string>(), It.IsAny<string>()), Times.Exactly(2));
     }