Skip to content

Commit eb4da67

Browse files
cvpoienarucvpoienaru
authored
Added telemetry data point for extensions loaded during test discovery/run (#3511)
1 parent 8f67269 commit eb4da67

File tree

21 files changed

+413
-71
lines changed

21 files changed

+413
-71
lines changed

‎src/Microsoft.TestPlatform.Client/Discovery/DiscoveryRequest.cs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
using System.Linq;
77
using System.Threading;
88

9+
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework;
10+
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework.Utilities;
911
using Microsoft.VisualStudio.TestPlatform.Common.Telemetry;
1012
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
1113
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces;
@@ -248,6 +250,22 @@ public void HandleDiscoveryComplete(DiscoveryCompleteEventArgs discoveryComplete
248250
OnDiscoveredTests.SafeInvoke(this, discoveredTestsEvent, "DiscoveryRequest.DiscoveryComplete");
249251
}
250252

253+
// Add extensions discovered by vstest.console.
254+
//
255+
// TODO(copoiena): Writing telemetry twice is less than ideal.
256+
// We first write telemetry data in the _requestData variable in the ParallelRunEventsHandler
257+
// and then we write again here. We should refactor this code and write only once.
258+
discoveryCompleteEventArgs.DiscoveredExtensions = TestExtensions.CreateMergedDictionary(
259+
discoveryCompleteEventArgs.DiscoveredExtensions,
260+
TestPluginCache.Instance.TestExtensions.GetCachedExtensions());
261+
262+
if (RequestData.IsTelemetryOptedIn)
263+
{
264+
TestExtensions.AddExtensionTelemetry(
265+
discoveryCompleteEventArgs.Metrics,
266+
discoveryCompleteEventArgs.DiscoveredExtensions);
267+
}
268+
251269
LoggerManager.HandleDiscoveryComplete(discoveryCompleteEventArgs);
252270
OnDiscoveryComplete.SafeInvoke(this, discoveryCompleteEventArgs, "DiscoveryRequest.DiscoveryComplete");
253271
}
@@ -406,6 +424,25 @@ private string UpdateRawMessageWithTelemetryInfo(DiscoveryCompletePayload discov
406424

407425
// Collecting Total Time Taken
408426
discoveryCompletePayload.Metrics[TelemetryDataConstants.TimeTakenInSecForDiscovery] = discoveryFinalTimeTakenForDesignMode.TotalSeconds;
427+
428+
// Add extensions discovered by vstest.console.
429+
//
430+
// TODO(copoiena):
431+
// Doing extension merging here is incorrect because we can end up not merging the
432+
// cached extensions for the current process (i.e. vstest.console) and hence have
433+
// an incomplete list of discovered extensions. This can happen because this method
434+
// is called only if telemetry is opted in (see: HandleRawMessage). We should handle
435+
// this merge a level above in order to be consistent, but that means we'd have to
436+
// deserialize all raw messages no matter if telemetry is opted in or not and that
437+
// would probably mean a performance hit.
438+
discoveryCompletePayload.DiscoveredExtensions = TestExtensions.CreateMergedDictionary(
439+
discoveryCompletePayload.DiscoveredExtensions,
440+
TestPluginCache.Instance.TestExtensions.GetCachedExtensions());
441+
442+
// Write extensions to telemetry data.
443+
TestExtensions.AddExtensionTelemetry(
444+
discoveryCompletePayload.Metrics,
445+
discoveryCompletePayload.DiscoveredExtensions);
409446
}
410447

411448
if (message is VersionedMessage message1)

‎src/Microsoft.TestPlatform.Client/Execution/TestRunRequest.cs

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
using System.Linq;
99
using System.Threading;
1010

11+
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework;
12+
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework.Utilities;
1113
using Microsoft.VisualStudio.TestPlatform.Common.Telemetry;
1214
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
1315
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces;
@@ -400,6 +402,22 @@ public void HandleTestRunComplete(TestRunCompleteEventArgs runCompleteArgs!!, Te
400402
runCompleteArgs.InvokedDataCollectors,
401403
_runRequestTimeTracker.Elapsed);
402404

405+
// Add extensions discovered by vstest.console.
406+
//
407+
// TODO(copoiena): Writing telemetry twice is less than ideal.
408+
// We first write telemetry data in the _requestData variable in the ParallelRunEventsHandler
409+
// and then we write again here. We should refactor this code and write only once.
410+
runCompleteArgs.DiscoveredExtensions = TestExtensions.CreateMergedDictionary(
411+
runCompleteArgs.DiscoveredExtensions,
412+
TestPluginCache.Instance.TestExtensions.GetCachedExtensions());
413+
414+
if (_requestData.IsTelemetryOptedIn)
415+
{
416+
TestExtensions.AddExtensionTelemetry(
417+
runCompleteArgs.Metrics,
418+
runCompleteArgs.DiscoveredExtensions);
419+
}
420+
403421
// Ignore the time sent (runCompleteArgs.ElapsedTimeInRunningTests)
404422
// by either engines - as both calculate at different points
405423
// If we use them, it would be an incorrect comparison between TAEF and Rocksteady
@@ -420,7 +438,6 @@ public void HandleTestRunComplete(TestRunCompleteEventArgs runCompleteArgs!!, Te
420438
// Notify the waiting handle that run is complete
421439
_runCompletionEvent.Set();
422440

423-
424441
var executionTotalTimeTaken = DateTime.UtcNow - _executionStartTime;
425442

426443
// Fill in the time taken to complete the run
@@ -583,6 +600,25 @@ private string UpdateRawMessageWithTelemetryInfo(TestRunCompletePayload testRunC
583600
// Fill in the time taken to complete the run
584601
var executionTotalTimeTakenForDesignMode = DateTime.UtcNow - _executionStartTime;
585602
testRunCompletePayload.TestRunCompleteArgs.Metrics[TelemetryDataConstants.TimeTakenInSecForRun] = executionTotalTimeTakenForDesignMode.TotalSeconds;
603+
604+
// Add extensions discovered by vstest.console.
605+
//
606+
// TODO(copoiena):
607+
// Doing extension merging here is incorrect because we can end up not merging the
608+
// cached extensions for the current process (i.e. vstest.console) and hence have
609+
// an incomplete list of discovered extensions. This can happen because this method
610+
// is called only if telemetry is opted in (see: HandleRawMessage). We should handle
611+
// this merge a level above in order to be consistent, but that means we'd have to
612+
// deserialize all raw messages no matter if telemetry is opted in or not and that
613+
// would probably mean a performance hit.
614+
testRunCompletePayload.TestRunCompleteArgs.DiscoveredExtensions = TestExtensions.CreateMergedDictionary(
615+
testRunCompletePayload.TestRunCompleteArgs.DiscoveredExtensions,
616+
TestPluginCache.Instance.TestExtensions.GetCachedExtensions());
617+
618+
// Write extensions to telemetry data.
619+
TestExtensions.AddExtensionTelemetry(
620+
testRunCompletePayload.TestRunCompleteArgs.Metrics,
621+
testRunCompletePayload.TestRunCompleteArgs.DiscoveredExtensions);
586622
}
587623

588624
if (message is VersionedMessage message1)

‎src/Microsoft.TestPlatform.Common/ExtensionFramework/TestPluginCache.cs

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ public List<string> GetExtensionPaths(string endsWithPattern, bool skipDefaultEx
117117
/// <returns>
118118
/// The <see cref="Dictionary"/>. of test plugin info.
119119
/// </returns>
120-
public Dictionary<string, TPluginInfo> DiscoverTestExtensions<TPluginInfo, TExtension>(string endsWithPattern)
120+
public Dictionary<string, TPluginInfo> DiscoverTestExtensions<TPluginInfo, TExtension>(
121+
string endsWithPattern)
121122
where TPluginInfo : TestPluginInformation
122123
{
123124
EqtTrace.Verbose("TestPluginCache.DiscoverTestExtensions: finding test extensions in assemblies ends with: {0} TPluginInfo: {1} TExtension: {2}", endsWithPattern, typeof(TPluginInfo), typeof(TExtension));
@@ -309,35 +310,37 @@ internal IEnumerable<string> DefaultExtensionPaths
309310
/// <returns>
310311
/// The <see cref="Dictionary"/>.
311312
/// </returns>
312-
internal Dictionary<string, TPluginInfo> GetTestExtensions<TPluginInfo, TExtension>(string extensionAssembly, bool skipCache = false) where TPluginInfo : TestPluginInformation
313+
internal Dictionary<string, TPluginInfo> GetTestExtensions<TPluginInfo, TExtension>(
314+
string extensionAssembly,
315+
bool skipCache = false)
316+
where TPluginInfo : TestPluginInformation
313317
{
314318
if (skipCache)
315319
{
316320
return GetTestExtensions<TPluginInfo, TExtension>(new List<string>() { extensionAssembly });
317321
}
318-
else
319-
{
320-
// Check if extensions from this assembly have already been discovered.
321-
var extensions = TestExtensions?.GetExtensionsDiscoveredFromAssembly(
322-
TestExtensions.GetTestExtensionCache<TPluginInfo>(),
323-
extensionAssembly);
324322

325-
if (extensions != null && extensions.Count > 0)
326-
{
327-
return extensions;
328-
}
323+
// Check if extensions from this assembly have already been discovered.
324+
var extensions = TestExtensions?.GetExtensionsDiscoveredFromAssembly(
325+
TestExtensions.GetTestExtensionCache<TPluginInfo>(),
326+
extensionAssembly);
329327

330-
var pluginInfos = GetTestExtensions<TPluginInfo, TExtension>(new List<string>() { extensionAssembly });
328+
if (extensions?.Count > 0)
329+
{
330+
return extensions;
331+
}
331332

332-
// Add extensions discovered to the cache.
333-
if (TestExtensions == null)
334-
{
335-
TestExtensions = new TestExtensions();
336-
}
333+
var pluginInfos = GetTestExtensions<TPluginInfo, TExtension>(new List<string>() { extensionAssembly });
337334

338-
TestExtensions.AddExtension(pluginInfos);
339-
return pluginInfos;
335+
// Add extensions discovered to the cache.
336+
if (TestExtensions == null)
337+
{
338+
TestExtensions = new TestExtensions();
340339
}
340+
341+
TestExtensions.AddExtension(pluginInfos);
342+
343+
return pluginInfos;
341344
}
342345

343346
/// <summary>

‎src/Microsoft.TestPlatform.Common/ExtensionFramework/Utilities/TestExtensions.cs

Lines changed: 138 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
using System.Collections.Generic;
66
using System.Linq;
77
using System.Reflection;
8+
using System.Text;
89

910
using Microsoft.VisualStudio.TestPlatform.Common.DataCollector;
10-
11+
using Microsoft.VisualStudio.TestPlatform.Common.Telemetry;
1112
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
12-
1313
using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions;
1414

1515
#nullable disable
@@ -91,6 +91,80 @@ public class TestExtensions
9191
/// </summary>
9292
internal bool AreDataCollectorsCached { get; set; }
9393

94+
/// <summary>
95+
/// Merge two extension dictionaries.
96+
/// </summary>
97+
///
98+
/// <param name="first">First extension dictionary.</param>
99+
/// <param name="second">Second extension dictionary.</param>
100+
///
101+
/// <returns>
102+
/// A dictionary representing the merger between the two input dictionaries.
103+
/// </returns>
104+
internal static Dictionary<string, HashSet<string>> CreateMergedDictionary(
105+
Dictionary<string, HashSet<string>> first,
106+
Dictionary<string, HashSet<string>> second)
107+
{
108+
var isFirstNullOrEmpty = first == null || first.Count == 0;
109+
var isSecondNullOrEmpty = second == null || second.Count == 0;
110+
111+
// Sanity checks.
112+
if (isFirstNullOrEmpty && isSecondNullOrEmpty)
113+
{
114+
return new Dictionary<string, HashSet<string>>();
115+
}
116+
if (isFirstNullOrEmpty)
117+
{
118+
return new Dictionary<string, HashSet<string>>(second);
119+
}
120+
if (isSecondNullOrEmpty)
121+
{
122+
return new Dictionary<string, HashSet<string>>(first);
123+
}
124+
125+
// Copy all the keys in the first dictionary into the resulting dictionary.
126+
var result = new Dictionary<string, HashSet<string>>(first);
127+
128+
foreach (var kvp in second)
129+
{
130+
// If the "source" set is empty there's no reason to continue merging for this key.
131+
if (kvp.Value == null || kvp.Value.Count == 0)
132+
{
133+
continue;
134+
}
135+
136+
// If there's no key-value pair entry in the "destination" dictionary for the current
137+
// key in the "source" dictionary, we copy the "source" set wholesale.
138+
if (!result.ContainsKey(kvp.Key))
139+
{
140+
result.Add(kvp.Key, kvp.Value);
141+
continue;
142+
}
143+
144+
// Getting here means there's already an entry for the "source" key in the "destination"
145+
// dictionary which means we need to copy individual set elements from the "source" set
146+
// to the "destination" set.
147+
result[kvp.Key] = MergeSets(result[kvp.Key], kvp.Value);
148+
}
149+
150+
return result;
151+
}
152+
153+
/// <summary>
154+
/// Add extension-related telemetry.
155+
/// </summary>
156+
///
157+
/// <param name="metrics">A collection representing the telemetry data.</param>
158+
/// <param name="extensions">The input extension collection.</param>
159+
internal static void AddExtensionTelemetry(
160+
IDictionary<string, object> metrics,
161+
Dictionary<string, HashSet<string>> extensions)
162+
{
163+
metrics.Add(
164+
TelemetryDataConstants.DiscoveredExtensions,
165+
SerializeExtensionDictionary(extensions));
166+
}
167+
94168
/// <summary>
95169
/// Adds the extensions specified to the current set of extensions.
96170
/// </summary>
@@ -307,6 +381,27 @@ internal void SetTestExtensionsCacheStatusToTrue<TPluginInfo>() where TPluginInf
307381
}
308382
}
309383

384+
/// <summary>
385+
/// Gets the cached extensions for the current process.
386+
/// </summary>
387+
///
388+
/// <returns>A dictionary representing the cached extensions for the current process.</returns>
389+
internal Dictionary<string, HashSet<string>> GetCachedExtensions()
390+
{
391+
var extensions = new Dictionary<string, HashSet<string>>();
392+
393+
// Write all "known" cached extension.
394+
AddCachedExtensionToDictionary(extensions, "TestDiscoverers", TestDiscoverers?.Values);
395+
AddCachedExtensionToDictionary(extensions, "TestExecutors", TestExecutors?.Values);
396+
AddCachedExtensionToDictionary(extensions, "TestExecutors2", TestExecutors2?.Values);
397+
AddCachedExtensionToDictionary(extensions, "TestSettingsProviders", TestSettingsProviders?.Values);
398+
AddCachedExtensionToDictionary(extensions, "TestLoggers", TestLoggers?.Values);
399+
AddCachedExtensionToDictionary(extensions, "TestHosts", TestHosts?.Values);
400+
AddCachedExtensionToDictionary(extensions, "DataCollectors", DataCollectors?.Values);
401+
402+
return extensions;
403+
}
404+
310405
/// <summary>
311406
/// The invalidate cache of plugin infos.
312407
/// </summary>
@@ -390,4 +485,45 @@ private void SetTestExtensionCache<TPluginInfo>(Dictionary<string, TPluginInfo>
390485
}
391486
}
392487

488+
private void AddCachedExtensionToDictionary<T>(
489+
Dictionary<string, HashSet<string>> extensionDict,
490+
string extensionType,
491+
IEnumerable<T> extensions)
492+
where T : TestPluginInformation
493+
{
494+
if (extensions == null)
495+
{
496+
return;
497+
}
498+
499+
extensionDict.Add(extensionType, new HashSet<string>(extensions.Select(e => e.IdentifierData)));
500+
}
501+
502+
private static string SerializeExtensionDictionary(IDictionary<string, HashSet<string>> extensions)
503+
{
504+
StringBuilder sb = new();
505+
506+
foreach (var kvp in extensions)
507+
{
508+
if (kvp.Value?.Count > 0)
509+
{
510+
sb.AppendFormat("{0}=[{1}];", kvp.Key, string.Join(",", kvp.Value));
511+
}
512+
}
513+
514+
return sb.ToString();
515+
}
516+
517+
private static HashSet<string> MergeSets(HashSet<string> firstSet, HashSet<string> secondSet)
518+
{
519+
var mergedSet = new HashSet<string>(firstSet);
520+
521+
// No need to worry about duplicates as the set implementation handles this already.
522+
foreach (var key in secondSet)
523+
{
524+
mergedSet.Add(key);
525+
}
526+
527+
return mergedSet;
528+
}
393529
}

‎src/Microsoft.TestPlatform.Common/Telemetry/TelemetryDataConstants.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ namespace Microsoft.VisualStudio.TestPlatform.Common.Telemetry;
1010
/// </summary>
1111
internal static class TelemetryDataConstants
1212
{
13+
// ******************** General ***********************
14+
public static readonly string DiscoveredExtensions = "VS.TestPlatform.DiscoveredExtensions";
15+
1316
// ******************** Execution ***********************
1417
public static readonly string ParallelEnabledDuringExecution = "VS.TestRun.ParallelEnabled";
1518

‎src/Microsoft.TestPlatform.CommunicationUtilities/Messages/DiscoveryCompletePayload.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,9 @@ public class DiscoveryCompletePayload
4848
/// Gets or sets list of sources which were not discovered at all.
4949
/// </summary>
5050
public IList<string> NotDiscoveredSources { get; set; } = new List<string>();
51+
52+
/// <summary>
53+
/// Gets or sets the collection of discovered extensions.
54+
/// </summary>
55+
public Dictionary<string, HashSet<string>> DiscoveredExtensions { get; set; } = new();
5156
}

‎src/Microsoft.TestPlatform.CommunicationUtilities/PublicAPI/PublicAPI.Unshipped.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,5 @@ Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.Discovery
66
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.PartiallyDiscoveredSources.get -> System.Collections.Generic.IList<string>
77
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.PartiallyDiscoveredSources.set -> void
88
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.TestRequestSender.SendDiscoveryAbort() -> void
9+
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.DiscoveredExtensions.get -> System.Collections.Generic.Dictionary<string, System.Collections.Generic.HashSet<string>>
10+
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.DiscoveredExtensions.set -> void

‎src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,9 @@ private void OnDiscoveryMessageReceived(ITestDiscoveryEventsHandler2 discoveryEv
579579
discoveryCompletePayload.IsAborted,
580580
discoveryCompletePayload.FullyDiscoveredSources,
581581
discoveryCompletePayload.PartiallyDiscoveredSources,
582-
discoveryCompletePayload.NotDiscoveredSources);
582+
discoveryCompletePayload.NotDiscoveredSources,
583+
discoveryCompletePayload.DiscoveredExtensions);
584+
583585
discoveryCompleteEventArgs.Metrics = discoveryCompletePayload.Metrics;
584586
discoveryEventsHandler.HandleDiscoveryComplete(
585587
discoveryCompleteEventArgs,

0 commit comments

Comments
 (0)