Skip to content

Commit

Permalink
Telemetry to Count Top Level & Transitive mappings automatically crea…
Browse files Browse the repository at this point in the history
…ted (#5377)
  • Loading branch information
donnie-msft authored Aug 29, 2023
1 parent fd095ec commit 545b3ce
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,9 @@ private async Task PerformActionImplAsync(

await _lockService.ExecuteNuGetOperationAsync(async () =>
{
int? countCreatedTopLevelSourceMappings = null;
int? countCreatedTransitiveSourceMappings = null;

try
{
uiService.BeginOperation();
Expand Down Expand Up @@ -466,7 +469,7 @@ await _lockService.ExecuteNuGetOperationAsync(async () =>
if (!cancellationToken.IsCancellationRequested)
{
List<string>? addedPackageIds = addedPackages != null ? addedPackages.Select(pair => pair.Item1).Distinct().ToList() : null;
PackageSourceMappingUtility.ConfigureNewPackageSourceMapping(userAction, addedPackageIds, sourceMappingProvider, existingPackageSourceMappingSourceItems);
PackageSourceMappingUtility.ConfigureNewPackageSourceMapping(userAction, addedPackageIds, sourceMappingProvider, existingPackageSourceMappingSourceItems, out countCreatedTopLevelSourceMappings, out countCreatedTransitiveSourceMappings);

await projectManagerService.ExecuteActionsAsync(
actions,
Expand Down Expand Up @@ -559,7 +562,9 @@ await projectManagerService.ExecuteActionsAsync(
removedPackages,
updatedPackagesOld,
updatedPackagesNew,
frameworks);
frameworks,
countCreatedTopLevelSourceMappings,
countCreatedTransitiveSourceMappings);

if (packageToInstallWasTransitive.HasValue)
{
Expand Down Expand Up @@ -609,7 +614,9 @@ internal static void AddUiActionEngineTelemetryProperties(
List<string>? removedPackages,
List<Tuple<string, string>>? updatedPackagesOld,
List<Tuple<string, string>>? updatedPackagesNew,
IReadOnlyCollection<string> targetFrameworks)
IReadOnlyCollection<string> targetFrameworks,
int? countCreatedTopLevelSourceMappings,
int? countCreatedTransitiveSourceMappings)
{
// log possible cancel reasons
if (!continueAfterPreview)
Expand Down Expand Up @@ -715,6 +722,16 @@ internal static void AddUiActionEngineTelemetryProperties(
{
actionTelemetryEvent["TargetFrameworks"] = string.Join(";", targetFrameworks);
}

if (countCreatedTopLevelSourceMappings.HasValue)
{
actionTelemetryEvent["CreatedTopLevelSourceMappingsCount"] = countCreatedTopLevelSourceMappings.Value;
}

if (countCreatedTransitiveSourceMappings.HasValue)
{
actionTelemetryEvent["CreatedTransitiveSourceMappingsCount"] = countCreatedTransitiveSourceMappings.Value;
}
}

private async Task<bool> CheckPackageManagementFormatAsync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,53 +12,80 @@ namespace NuGet.PackageManagement.UI
{
internal static class PackageSourceMappingUtility
{
/// <summary>
/// Determines if new package source mappings should be written to the <paramref name="sourceMappingProvider"/> which is returned as a count.
/// </summary>
/// <param name="userAction"></param>
/// <param name="addedPackageIds"></param>
/// <param name="sourceMappingProvider"></param>
/// <param name="existingPackageSourceMappingSourceItems"></param>
/// <param name="countCreatedTopLevelSourceMappings">For Top-Level packages: <c>null</c> if not applicable; 0 if none needed to be added; > 0 is the count of new package source mappings added.</param>
/// <param name="countCreatedTransitiveSourceMappings">For Transitive packages: <c>null</c> if not applicable; 0 if none needed to be added; > 0 is the count of new package source mappings added.</param>
internal static void ConfigureNewPackageSourceMapping(
UserAction? userAction,
IReadOnlyList<string>? addedPackageIds,
PackageSourceMappingProvider sourceMappingProvider,
IReadOnlyList<PackageSourceMappingSourceItem> existingPackageSourceMappingSourceItems)
IReadOnlyList<PackageSourceMappingSourceItem> existingPackageSourceMappingSourceItems,
out int? countCreatedTopLevelSourceMappings,
out int? countCreatedTransitiveSourceMappings)
{
countCreatedTopLevelSourceMappings = null;
countCreatedTransitiveSourceMappings = null;

if (userAction?.SelectedSourceName is null || addedPackageIds is null)
{
return;
}

countCreatedTopLevelSourceMappings = 0;
countCreatedTransitiveSourceMappings = 0;

string topLevelPackageId = userAction.PackageId;
Dictionary<string, IReadOnlyList<string>> patternsReadOnly = existingPackageSourceMappingSourceItems
.ToDictionary(pair => pair.Key, pair => (IReadOnlyList<string>)(pair.Patterns.Select(p => p.Pattern).ToList()));

PackageSourceMapping packageSourceMapping = new(patternsReadOnly);

// Expand all patterns/globs so we can later check if this package ID was already mapped.
List<string> addedPackageIdsWithoutExistingMappings = new();
List<string> addedPackageIdsWithoutExistingMappings = new(capacity: addedPackageIds.Count + 1);

foreach (string addedPackageId in addedPackageIds)
{
IReadOnlyList<string> configuredSource = packageSourceMapping.GetConfiguredPackageSources(addedPackageId);
if (configuredSource.Count == 0)

// Top-level package was looked up.
if (addedPackageId == topLevelPackageId)
{
// The top-level package is not already mapped to the selected source.
if (configuredSource.Count == 0 || !configuredSource.Contains(userAction.SelectedSourceName))
{
countCreatedTopLevelSourceMappings = 1;
addedPackageIdsWithoutExistingMappings.Add(topLevelPackageId);
}
}
// Transitive package was looked up.
else if (configuredSource.Count == 0)
{
addedPackageIdsWithoutExistingMappings.Add(addedPackageId);
}
}

// Get all newly added package IDs that were not previously Source Mapped.
// Always include the Package ID being installed since it takes precedence over any globbing.
string[] packageIdsNeedingNewSourceMappings = addedPackageIdsWithoutExistingMappings
.Append(userAction.PackageId)
.ToArray();
countCreatedTransitiveSourceMappings = addedPackageIdsWithoutExistingMappings.Count - countCreatedTopLevelSourceMappings;

CreateAndSavePackageSourceMappings(
userAction.SelectedSourceName,
packageIdsNeedingNewSourceMappings,
addedPackageIdsWithoutExistingMappings,
sourceMappingProvider,
existingPackageSourceMappingSourceItems);
}

private static void CreateAndSavePackageSourceMappings(
string sourceName,
string[] newPackageIdsToSourceMap,
List<string> newPackageIdsToSourceMap,
PackageSourceMappingProvider mappingProvider,
IReadOnlyList<PackageSourceMappingSourceItem> existingPackageSourceMappingSourceItems)
{
if (string.IsNullOrWhiteSpace(sourceName) || newPackageIdsToSourceMap is null || newPackageIdsToSourceMap.Length == 0)
if (string.IsNullOrWhiteSpace(sourceName) || newPackageIdsToSourceMap is null || newPackageIdsToSourceMap.Count == 0)
{
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,9 @@ public void AddUiActionEngineTelemetryProperties_AddsVulnerabilityInfo_Succeeds(
removedPackages: null,
updatedPackagesOld: null,
updatedPackagesNew: null,
targetFrameworks: null);
targetFrameworks: null,
countCreatedTopLevelSourceMappings: null,
countCreatedTransitiveSourceMappings: null);

// Act
var service = new NuGetVSTelemetryService(telemetrySession.Object);
Expand All @@ -290,6 +292,63 @@ public void AddUiActionEngineTelemetryProperties_AddsVulnerabilityInfo_Succeeds(
item => Assert.Equal(1, item),
item => Assert.Equal(3, item));
Assert.Equal(3, pkgSeverities.Count());
Assert.Null(lastTelemetryEvent["CreatedTopLevelSourceMappingsCount"]);
Assert.Null(lastTelemetryEvent["CreatedTransitiveSourceMappingsCount"]);
}

[Fact]
public void ActionCreatingSourceMappings_TopLevelCountAndTransitiveCount_AddsValue()
{
// Arrange
int expectedCountCreatedTopLevelSourceMappings = 42;
int expectedCountCreatedTransitiveSourceMappings = 24;
var telemetrySession = new Mock<ITelemetrySession>();
TelemetryEvent lastTelemetryEvent = null;
telemetrySession
.Setup(x => x.PostEvent(It.IsAny<TelemetryEvent>()))
.Callback<TelemetryEvent>(x => lastTelemetryEvent = x);

var operationId = Guid.NewGuid().ToString();

var actionTelemetryData = new VSActionsTelemetryEvent(
operationId,
projectIds: new[] { Guid.NewGuid().ToString() },
operationType: NuGetOperationType.Install,
source: OperationSource.PMC,
startTime: DateTimeOffset.Now.AddSeconds(-1),
status: NuGetOperationStatus.NoOp,
packageCount: 1,
endTime: DateTimeOffset.Now,
duration: .40,
isPackageSourceMappingEnabled: false);

UIActionEngine.AddUiActionEngineTelemetryProperties(
actionTelemetryEvent: actionTelemetryData,
continueAfterPreview: true,
acceptedLicense: true,
userAction: null,
selectedIndex: 0,
recommendedCount: 0,
recommendPackages: false,
recommenderVersion: null,
topLevelVulnerablePackagesCount: 3,
topLevelVulnerablePackagesMaxSeverities: new List<int> { 1, 1, 3 }, // each package has its own max severity
existingPackages: null,
addedPackages: null,
removedPackages: null,
updatedPackagesOld: null,
updatedPackagesNew: null,
targetFrameworks: null,
countCreatedTopLevelSourceMappings: expectedCountCreatedTopLevelSourceMappings,
countCreatedTransitiveSourceMappings: expectedCountCreatedTransitiveSourceMappings);

// Act
var service = new NuGetVSTelemetryService(telemetrySession.Object);
service.EmitTelemetryEvent(actionTelemetryData);

// Assert
Assert.Equal(expectedCountCreatedTopLevelSourceMappings, (int)lastTelemetryEvent["CreatedTopLevelSourceMappingsCount"]);
Assert.Equal(expectedCountCreatedTransitiveSourceMappings, (int)lastTelemetryEvent["CreatedTransitiveSourceMappingsCount"]);
}

[Theory]
Expand Down

0 comments on commit 545b3ce

Please sign in to comment.