Add "PackageDeprecate" telemetry event (#7245)

Scott Bommarito · web-flow · commit a9171b28e05a · 2019-06-07T15:16:35.000-07:00
diff --git a/src/NuGetGallery.Services/Telemetry/ITelemetryService.cs b/src/NuGetGallery.Services/Telemetry/ITelemetryService.cs
@@ -34,6 +34,13 @@ public interface ITelemetryService
 
         void TrackPackageRevalidate(Package package);
 
+        void TrackPackageDeprecate(
+            IReadOnlyList<Package> packages,
+            PackageDeprecationStatus status,
+            PackageRegistration alternateRegistration,
+            Package alternatePackage,
+            bool hasCustomMessage);
+
         void TrackPackageReadMeChangeEvent(Package package, string readMeSourceType, PackageEditReadMeState readMeState);
 
         void TrackCreatePackageVerificationKeyEvent(string packageId, string packageVersion, User user, IIdentity identity);
diff --git a/src/NuGetGallery/Controllers/ManageDeprecationJsonApiController.cs b/src/NuGetGallery/Controllers/ManageDeprecationJsonApiController.cs
@@ -17,18 +17,15 @@ namespace NuGetGallery
     public partial class ManageDeprecationJsonApiController
         : AppController
     {
-        private readonly IAuditingService _auditingService;
         private readonly IPackageService _packageService;
         private readonly IPackageDeprecationService _deprecationService;
         private readonly IFeatureFlagService _featureFlagService;
 
         public ManageDeprecationJsonApiController(
-            IAuditingService auditingService,
             IPackageService packageService,
             IPackageDeprecationService deprecationService,
             IFeatureFlagService featureFlagService)
         {
-            _auditingService = auditingService ?? throw new ArgumentNullException(nameof(auditingService));
             _packageService = packageService ?? throw new ArgumentNullException(nameof(packageService));
             _deprecationService = deprecationService ?? throw new ArgumentNullException(nameof(deprecationService));
             _featureFlagService = featureFlagService ?? throw new ArgumentNullException(nameof(featureFlagService));
@@ -169,16 +166,6 @@ await _deprecationService.UpdateDeprecation(
                 customMessage,
                 currentUser);
 
-            foreach (var packageToUpdate in packagesToUpdate)
-            {
-                await _auditingService.SaveAuditRecordAsync(
-                    new PackageAuditRecord(
-                        packageToUpdate,
-                        status == PackageDeprecationStatus.NotDeprecated ? AuditedPackageAction.Undeprecate : AuditedPackageAction.Deprecate,
-                        status == PackageDeprecationStatus.NotDeprecated ? PackageUndeprecatedVia.Web : PackageDeprecatedVia.Web));
-
-            }
-
             return Json(HttpStatusCode.OK);
         }
 
diff --git a/src/NuGetGallery/Services/PackageDeprecationService.cs b/src/NuGetGallery/Services/PackageDeprecationService.cs
@@ -7,23 +7,27 @@
 using System.Linq;
 using System.Threading.Tasks;
 using NuGet.Services.Entities;
+using NuGetGallery.Auditing;
 
 namespace NuGetGallery
 {
     public class PackageDeprecationService : IPackageDeprecationService
     {
         private readonly IEntitiesContext _entitiesContext;
         private readonly IPackageUpdateService _packageUpdateService;
-        private readonly IIndexingService _indexingService;
+        private readonly ITelemetryService _telemetryService;
+        private readonly IAuditingService _auditingService;
 
         public PackageDeprecationService(
            IEntitiesContext entitiesContext,
            IPackageUpdateService packageUpdateService,
-           IIndexingService indexingService)
+           ITelemetryService telemetryService,
+           IAuditingService auditingService)
         {
             _entitiesContext = entitiesContext ?? throw new ArgumentNullException(nameof(entitiesContext));
             _packageUpdateService = packageUpdateService ?? throw new ArgumentNullException(nameof(packageUpdateService));
-            _indexingService = indexingService ?? throw new ArgumentNullException(nameof(indexingService));
+            _telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService));
+            _auditingService = auditingService ?? throw new ArgumentNullException(nameof(auditingService));
         }
 
         public async Task UpdateDeprecation(
@@ -103,6 +107,22 @@ public async Task UpdateDeprecation(
                 await _packageUpdateService.UpdatePackagesAsync(packages);
 
                 transaction.Commit();
+
+                _telemetryService.TrackPackageDeprecate(
+                    packages,
+                    status,
+                    alternatePackageRegistration,
+                    alternatePackage,
+                    !string.IsNullOrWhiteSpace(customMessage));
+
+                foreach (var package in packages)
+                {
+                    await _auditingService.SaveAuditRecordAsync(
+                        new PackageAuditRecord(
+                            package,
+                            status == PackageDeprecationStatus.NotDeprecated ? AuditedPackageAction.Undeprecate : AuditedPackageAction.Deprecate,
+                            status == PackageDeprecationStatus.NotDeprecated ? PackageUndeprecatedVia.Web : PackageDeprecatedVia.Web));
+                }
             }
         }
 
diff --git a/src/NuGetGallery/Services/TelemetryService.cs b/src/NuGetGallery/Services/TelemetryService.cs
@@ -7,6 +7,7 @@
 using System.Net.Http;
 using System.Security.Principal;
 using System.Web;
+using System.Web.Helpers;
 using Newtonsoft.Json;
 using NuGet.Services.Entities;
 using NuGet.Services.FeatureFlags;
@@ -39,6 +40,7 @@ internal class Events
             public const string PackageUnlisted = "PackageUnlisted";
             public const string PackageListed = "PackageListed";
             public const string PackageDelete = "PackageDelete";
+            public const string PackageDeprecate = "PackageDeprecate";
             public const string PackageReupload = "PackageReupload";
             public const string PackageHardDeleteReflow = "PackageHardDeleteReflow";
             public const string PackageRevalidate = "PackageRevalidate";
@@ -96,7 +98,7 @@ internal class Events
         // ODataCustomQuery properties
         public const string IsCustomQuery = "IsCustomQuery";
 
-        // Package push properties
+        // Package event properties
         public const string AuthenticationMethod = "AuthenticationMethod";
         public const string ClientVersion = "ClientVersion";
         public const string ProtocolVersion = "ProtocolVersion";
@@ -106,6 +108,13 @@ internal class Events
         public const string KeyCreationDate = "KeyCreationDate";
         public const string PackageId = "PackageId";
         public const string PackageVersion = "PackageVersion";
+        public const string PackageVersions = "PackageVersions";
+
+        // Package deprecate properties
+        public const string DeprecationReason = "PackageDeprecationReason";
+        public const string DeprecationAlternatePackageId = "PackageDeprecationAlternatePackageId";
+        public const string DeprecationAlternatePackageVersion = "PackageDeprecationAlternatePackageVersion";
+        public const string DeprecationCustomMessage = "PackageDeprecationCustomMessage";
 
         // User properties
         public const string RegistrationMethod = "RegistrationMethod";
@@ -392,6 +401,25 @@ public void TrackPackageRevalidate(Package package)
             TrackMetricForPackage(Events.PackageRevalidate, package);
         }
 
+        public void TrackPackageDeprecate(
+            IReadOnlyList<Package> packages,
+            PackageDeprecationStatus status,
+            PackageRegistration alternateRegistration,
+            Package alternatePackage,
+            bool hasCustomMessage)
+        {
+            TrackMetricForPackageVersions(
+                Events.PackageDeprecate, 
+                packages,
+                properties =>
+                {
+                    properties.Add(DeprecationReason, ((int)status).ToString());
+                    properties.Add(DeprecationAlternatePackageId, alternateRegistration?.Id ?? alternatePackage?.Id);
+                    properties.Add(DeprecationAlternatePackageVersion, alternatePackage?.NormalizedVersion);
+                    properties.Add(DeprecationCustomMessage, hasCustomMessage.ToString());
+                });
+        }
+
         public void TrackPackageMetadataComplianceError(string packageId, string packageVersion, IEnumerable<string> complianceFailures)
         {
             TrackMetricForPackage(
@@ -648,6 +676,39 @@ private void TrackMetricForPackage(
             });
         }
 
+        private void TrackMetricForPackageVersions(
+            string metricName,
+            IReadOnlyList<Package> packages,
+            Action<Dictionary<string, string>> addProperties = null)
+        {
+            if (packages == null || !packages.Any() || packages.Select(p => p.PackageRegistrationKey).Distinct().Count() > 1)
+            {
+                throw new ArgumentException(nameof(packages));
+            }
+
+            TrackMetricForPackageVersions(
+                metricName,
+                packages.First().PackageRegistration.Id,
+                packages.Select(p => p.NormalizedVersion).ToList(),
+                addProperties);
+        }
+
+        private void TrackMetricForPackageVersions(
+            string metricName,
+            string packageId,
+            IReadOnlyList<string> packageVersions,
+            Action<Dictionary<string, string>> addProperties = null)
+        {
+            TrackMetric(metricName, packageVersions.Count(), properties => {
+                properties.Add(ClientVersion, GetClientVersion());
+                properties.Add(ProtocolVersion, GetProtocolVersion());
+                properties.Add(ClientInformation, GetClientInformation());
+                properties.Add(PackageId, packageId);
+                properties.Add(PackageVersion, BuildArrayProperty(packageVersions));
+                addProperties?.Invoke(properties);
+            });
+        }
+
         public void TrackUserPackageDeleteChecked(UserPackageDeleteEvent details, UserPackageDeleteOutcome outcome)
         {
             if (details == null)
@@ -727,7 +788,7 @@ public void TrackAccountDeletionCompleted(User deletedUser, User deletedBy, bool
             }
 
             TrackMetric(Events.AccountDeleteCompleted, 1, properties => {
-                properties.Add(AccountDeletedByRole, string.Join(",", deletedBy.Roles?.Select( role => role.Name) ?? new List<string>()));
+                properties.Add(AccountDeletedByRole, BuildArrayProperty(deletedBy.Roles?.Select(role => role.Name) ?? new string[0]));
                 properties.Add(AccountIsSelfDeleted, $"{deletedUser.Key == deletedBy.Key}");
                 properties.Add(AccountDeletedIsOrganization, $"{deletedUser is Organization}");
                 properties.Add(AccountDeleteSucceeded, $"{success}");
@@ -766,7 +827,7 @@ public void TrackMetricForTyposquattingCheckResultAndTotalTime(
             TrackMetric(Events.TyposquattingCheckResultAndTotalTimeInMs, totalTime.TotalMilliseconds, properties => {
                 properties.Add(PackageId, packageId);
                 properties.Add(WasUploadBlocked, wasUploadBlocked.ToString());
-                properties.Add(CollisionPackageIds, string.Join(",", collisionPackageIds.Take(TyposquattingCollisionIdsMaxPropertyValue)));
+                properties.Add(CollisionPackageIds, BuildArrayProperty(collisionPackageIds.Take(TyposquattingCollisionIdsMaxPropertyValue)));
                 properties.Add(CollisionPackageIdsCount, collisionPackageIds.Count.ToString());
                 properties.Add(CheckListLength, checkListLength.ToString());
                 properties.Add(HasExtraCollisionPackageIds, (collisionPackageIds.Count > TyposquattingCollisionIdsMaxPropertyValue).ToString());
@@ -900,5 +961,10 @@ protected virtual void TrackMetric(string metricName, double value, Action<Dicti
 
             _telemetryClient.TrackMetric(metricName, value, telemetryProperties);
         }
+
+        private string BuildArrayProperty(IEnumerable<string> list)
+        {
+            return JsonConvert.SerializeObject(list);
+        }
     }
 }
diff --git a/tests/NuGetGallery.Facts/Controllers/ManageDeprecationJsonApiControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/ManageDeprecationJsonApiControllerFacts.cs
@@ -831,8 +831,6 @@ private async Task AssertSuccessful(
                     .Completes()
                     .Verifiable();
 
-                var auditingService = GetService<IAuditingService>();
-
                 var controller = GetController<ManageDeprecationJsonApiController>();
                 controller.SetCurrentUser(currentUser);
 
@@ -852,31 +850,6 @@ private async Task AssertSuccessful(
                 // Assert
                 AssertSuccessResponse(controller);
 
-                if (expectedStatus == PackageDeprecationStatus.NotDeprecated)
-                {
-                    foreach (var normalizedVersion in packageNormalizedVersions)
-                    {
-                        auditingService.WroteRecord<PackageAuditRecord>(
-                            r => r.Action == AuditedPackageAction.Undeprecate
-                            && r.Reason == PackageUndeprecatedVia.Web
-                            && r.DeprecationRecord == null
-                            && r.Id == id
-                            && r.PackageRecord.NormalizedVersion == normalizedVersion);
-                    }
-                }
-                else
-                {
-                    foreach (var normalizedVersion in packageNormalizedVersions)
-                    {
-                        auditingService.WroteRecord<PackageAuditRecord>(
-                            r => r.Action == AuditedPackageAction.Deprecate
-                            && r.Reason == PackageDeprecatedVia.Web
-                            && r.DeprecationRecord.Status == (int)expectedStatus
-                            && r.Id == id
-                            && r.PackageRecord.NormalizedVersion == normalizedVersion);
-                    }
-                }
-
                 featureFlagService.Verify();
                 packageService.Verify();
                 deprecationService.Verify();
diff --git a/tests/NuGetGallery.Facts/Services/PackageDeprecationServiceFacts.cs b/tests/NuGetGallery.Facts/Services/PackageDeprecationServiceFacts.cs
diff --git a/tests/NuGetGallery.Facts/Services/TelemetryServiceFacts.cs b/tests/NuGetGallery.Facts/Services/TelemetryServiceFacts.cs
