Add per-source telemetry for package IDs containing non-alphanumeric, dash, dot, or underscore characters

src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs

@@ -8,6 +8,7 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
+
 using System.Threading;
 using System.Threading.Tasks;
 using NuGet.Common;
@@ -203,6 +204,25 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve
             {
                 data.NupkgCount++;
                 data.NupkgSize += ncEvent.FileSize;
+                data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter || (ncEvent.PackageId.Length > 0 && HasNonAlphanumericDotDashOrUnderscoreCharacters(ncEvent.PackageId));
+            }
+
+            bool HasNonAlphanumericDotDashOrUnderscoreCharacters(string packageId)
+            {
+                foreach (char c in packageId.AsSpan())
+                {
+                    if (!IsCharacterAlphanumericDotDashOrUnderscore(c))
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
+
+                bool IsCharacterAlphanumericDotDashOrUnderscore(char c)
+                {
+                    return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-' || c == '_';
+                }
             }
         }
 
@@ -261,6 +281,7 @@ internal static async Task<TelemetryEvent> ToTelemetryAsync(Data data, SourceRep
                 telemetry[PropertyNames.Duration.Total] = data.Resources.Values.Sum(r => r.duration.TotalMilliseconds);
                 telemetry[PropertyNames.Nupkgs.Copied] = data.NupkgCount;
                 telemetry[PropertyNames.Nupkgs.Bytes] = data.NupkgSize;
+                telemetry[PropertyNames.Nupkgs.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter] = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter;
                 AddResourceProperties(telemetry, data.Resources);
 
                 if (data.Http.Requests > 0)
@@ -426,6 +447,7 @@ internal class Data
             internal HttpData Http { get; }
             internal int NupkgCount { get; set; }
             internal long NupkgSize { get; set; }
+            internal bool IdContainsNonAlphanumericDotDashOrUnderscoreCharacter { get; set; }
 
             internal Data()
             {
@@ -475,6 +497,7 @@ internal static class Nupkgs
             {
                 internal const string Copied = "nupkgs.copied";
                 internal const string Bytes = "nupkgs.bytes";
+                internal const string IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = "nupkgs.idcontainsNonAlphanumericDotDashOrUnderscorecharacter";
             }
 
             internal static class Resources

src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs

@@ -54,6 +54,7 @@ private readonly Dictionary<RestoreTargetGraph, Dictionary<string, LibraryInclud
         private const string IsCentralVersionManagementEnabled = nameof(IsCentralVersionManagementEnabled);
         private const string TotalUniquePackagesCount = nameof(TotalUniquePackagesCount);
         private const string NewPackagesInstalledCount = nameof(NewPackagesInstalledCount);
+        private const string AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters = nameof(AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters);
         private const string SourcesCount = nameof(SourcesCount);
         private const string HttpSourcesCount = nameof(HttpSourcesCount);
         private const string LocalSourcesCount = nameof(LocalSourcesCount);
@@ -743,6 +744,7 @@ private async Task<EvaluateLockFileResult>
                 }
 
                 telemetry.TelemetryEvent[NewPackagesInstalledCount] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Install).Distinct().Count();
+                telemetry.TelemetryEvent[AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Flattened).Any(i => HasNonAlphanumericDotDashOrUnderscoreCharacters(i.Key.Name));
                 telemetry.TelemetryEvent[RestoreSuccess] = success;
             }
 
@@ -755,6 +757,24 @@ private async Task<EvaluateLockFileResult>
                 packagesLockFile,
                 packagesLockFilePath,
                 cacheFile);
+
+            bool HasNonAlphanumericDotDashOrUnderscoreCharacters(string packageId)
+            {
+                foreach (char c in packageId.AsSpan())
+                {
+                    if (!IsCharacterAlphanumericDotDashOrUnderscore(c))
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
+
+                bool IsCharacterAlphanumericDotDashOrUnderscore(char c)
+                {
+                    return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-' || c == '_';
+                }
+            }
         }
 
         /// <summary>Run NuGetAudit on the project's resolved restore graphs, and log messages and telemetry with the results.</summary>

src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs

@@ -8,12 +8,26 @@ public sealed class ProtocolDiagnosticNupkgCopiedEvent
         public string Source { get; }
         public long FileSize { get; }
 
+        /// <summary>
+        /// Gets the package ID of the copied nupkg.
+        /// </summary>
+        public string PackageId { get; }
+
         public ProtocolDiagnosticNupkgCopiedEvent(
             string source,
             long fileSize)
+            : this(source, fileSize, packageId: string.Empty)
+        {
+        }
+
+        public ProtocolDiagnosticNupkgCopiedEvent(
+            string source,
+            long fileSize,
+            string packageId)
         {
             Source = source;
             FileSize = fileSize;
+            PackageId = packageId;
         }
     }
 }

src/NuGet.Core/NuGet.Protocol/LocalPackageArchiveDownloader.cs

@@ -189,7 +189,7 @@ public async Task<bool> CopyNupkgFileToAsync(string destinationFilePath, Cancell
 
                     await source.CopyToAsync(destination, bufferSize, cancellationToken);
 
-                    ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(Source, destination.Length));
+                    ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(Source, destination.Length, _packageIdentity.Id));
 
                     return true;
                 }

src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV2FindPackageByIdResource.cs

@@ -176,7 +176,7 @@ public override async Task<bool> CopyNupkgToStreamAsync(
                     using (var fileStream = File.OpenRead(info.Path))
                     {
                         await fileStream.CopyToAsync(destination, cancellationToken);
-                        ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length));
+                        ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length, id));
                         return true;
                     }
                 }

src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV3FindPackageByIdResource.cs

@@ -222,7 +222,7 @@ public override async Task<bool> CopyNupkgToStreamAsync(
                         fileStream.Position = 0;
 
                         await fileStream.CopyToAsync(destination, cancellationToken);
-                        ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length));
+                        ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length, id));
                         return true;
                     }
                 }

src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt

@@ -1 +1,3 @@
 #nullable enable
+NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string!
+NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string! packageId) -> void

src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt

@@ -1 +1,3 @@
 #nullable enable
+NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string!
+NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string! packageId) -> void

src/NuGet.Core/NuGet.Protocol/Utility/FindPackagesByIdNupkgDownloader.cs

@@ -137,7 +137,7 @@ public async Task<bool> CopyNupkgToStreamAsync(
                         try
                         {
                             await stream.CopyToAsync(destination, token);
-                            ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_httpSource.PackageSource, destination.Length));
+                            ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_httpSource.PackageSource, destination.Length, identity.Id));
                         }
                         catch when (!token.IsCancellationRequested)
                         {

test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs

@@ -137,6 +137,78 @@ public void AddNupkgCopiedData_MultipleEvents_AccumulatesCorrectly()
             Assert.Equal(sizes.Sum(), result.NupkgSize);
         }
 
+        [Theory]
+        [InlineData("Newtonsoft.Json")]
+        [InlineData("NuGet.Protocol")]
+        [InlineData("My-Package.1")]
+        [InlineData("ALLCAPS")]
+        [InlineData("alllower")]
+        [InlineData("123Numeric")]
+        [InlineData("a")]
+        public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsFalse(string packageId)
+        {
+            // Arrange
+            var data = CreateDataDictionary(SampleSource);
+            var nce = new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, packageId);
+
+            // Act
+            PackageSourceTelemetry.AddNupkgCopiedData(nce, data);
+
+            // Assert
+            var result = Assert.Single(data).Value;
+            Assert.False(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
+        }
+
+        [Theory]
+        [InlineData("Package@1.0")]
+        [InlineData("Ünïcödé")]
+        [InlineData("Package Name")]
+        [InlineData("package+extra")]
+        public void AddNupkgCopiedData_NonstandardPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsTrue(string packageId)
+        {
+            // Arrange
+            var data = CreateDataDictionary(SampleSource);
+            var nce = new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, packageId);
+
+            // Act
+            PackageSourceTelemetry.AddNupkgCopiedData(nce, data);
+
+            // Assert
+            var result = Assert.Single(data).Value;
+            Assert.True(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
+        }
+
+        [Fact]
+        public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsTrue()
+        {
+            // Arrange
+            var data = CreateDataDictionary(SampleSource);
+
+            // Act
+            PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Standard.Package"), data);
+            PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Nonstandard@Package"), data);
+            PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Another.Standard"), data);
+
+            // Assert
+            var result = Assert.Single(data).Value;
+            Assert.True(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
+        }
+
+        [Fact]
+        public void AddNupkgCopiedData_EmptyPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsFalse()
+        {
+            // Arrange
+            var data = CreateDataDictionary(SampleSource);
+            var nce = new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000);
+
+            // Act
+            PackageSourceTelemetry.AddNupkgCopiedData(nce, data);
+
+            // Assert
+            var result = Assert.Single(data).Value;
+            Assert.False(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
+        }
+
         [Fact]
         public async Task AddData_IsThreadSafe()
         {
@@ -308,6 +380,7 @@ public async Task ToTelemetry_WithData_CreatesTelemetryProperties(string package
 
             Assert.Equal(data.NupkgCount, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Copied]);
             Assert.Equal(data.NupkgSize, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Bytes]);
+            Assert.Equal(data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter, result[PackageSourceTelemetry.PropertyNames.Nupkgs.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter]);
 
             Assert.Equal(data.Resources.Sum(r => r.Value.count), result[PackageSourceTelemetry.PropertyNames.Resources.Calls]);
             foreach (var resource in data.Resources)