Add HasNonstandardId telemetry for non-standard package ID characters per feed source

Copilot · jeffkl · jeffkl · commit 60ab977df9ee · 2026-03-30T10:50:03.000-07:00
Co-authored-by: jeffkl &lt;17556515+jeffkl@users.noreply.github.com&gt;
diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/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.Text.RegularExpressions;
 using System.Threading;
 using System.Threading.Tasks;
 using NuGet.Common;
@@ -29,6 +30,8 @@ public sealed class PackageSourceTelemetry : IDisposable
 
         internal const string EventName = "PackageSourceDiagnostics";
 
+        private static readonly Regex s_nonstandardIdCharRegex = new Regex(@"[^A-Za-z0-9.\-]", RegexOptions.Compiled);
+
         public enum TelemetryAction
         {
             Unknown = 0,
@@ -203,9 +206,19 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve
             {
                 data.NupkgCount++;
                 data.NupkgSize += ncEvent.FileSize;
+
+                if (!data.HasNonstandardId && ncEvent.PackageId != null && HasNonstandardCharacters(ncEvent.PackageId))
+                {
+                    data.HasNonstandardId = true;
+                }
             }
         }
 
+        private static bool HasNonstandardCharacters(string packageId)
+        {
+            return s_nonstandardIdCharRegex.IsMatch(packageId);
+        }
+
         public void Dispose()
         {
             ProtocolDiagnostics.HttpEvent -= ProtocolDiagnostics_HttpEvent;
@@ -261,6 +274,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.HasNonstandardId] = data.HasNonstandardId;
                 AddResourceProperties(telemetry, data.Resources);
 
                 if (data.Http.Requests > 0)
@@ -426,6 +440,7 @@ internal class Data
             internal HttpData Http { get; }
             internal int NupkgCount { get; set; }
             internal long NupkgSize { get; set; }
+            internal bool HasNonstandardId { get; set; }
 
             internal Data()
             {
@@ -475,6 +490,7 @@ internal static class Nupkgs
             {
                 internal const string Copied = "nupkgs.copied";
                 internal const string Bytes = "nupkgs.bytes";
+                internal const string HasNonstandardId = "nupkgs.hasnonstandard_id";
             }
 
             internal static class Resources
diff --git a/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs b/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs
@@ -10,12 +10,26 @@ public sealed class ProtocolDiagnosticNupkgCopiedEvent
         public string Source { get; }
         public long FileSize { get; }
 
+        /// <summary>
+        /// Gets the package ID of the copied nupkg, or <see langword="null"/> if not available.
+        /// </summary>
+        public string PackageId { get; }
+
         public ProtocolDiagnosticNupkgCopiedEvent(
             string source,
             long fileSize)
+            : this(source, fileSize, packageId: null)
+        {
+        }
+
+        public ProtocolDiagnosticNupkgCopiedEvent(
+            string source,
+            long fileSize,
+            string packageId)
         {
             Source = source;
             FileSize = fileSize;
+            PackageId = packageId;
         }
     }
 }
diff --git a/src/NuGet.Core/NuGet.Protocol/LocalPackageArchiveDownloader.cs b/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;
                 }
diff --git a/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV2FindPackageByIdResource.cs b/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;
                     }
                 }
diff --git a/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV3FindPackageByIdResource.cs b/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV3FindPackageByIdResource.cs
@@ -210,7 +210,7 @@ public override async Task<bool> CopyNupkgToStreamAsync(
                     using (var fileStream = File.OpenRead(packagePath))
                     {
                         await fileStream.CopyToAsync(destination, cancellationToken);
-                        ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length));
+                        ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length, id));
                         return true;
                     }
                 }
diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt b/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
diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/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
diff --git a/src/NuGet.Core/NuGet.Protocol/Utility/FindPackagesByIdNupkgDownloader.cs b/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)
                         {
diff --git a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs
@@ -137,6 +137,79 @@ 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_HasNonstandardIdIsFalse(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.HasNonstandardId);
+        }
+
+        [Theory]
+        [InlineData("My_Package")]
+        [InlineData("Package@1.0")]
+        [InlineData("Ünïcödé")]
+        [InlineData("Package Name")]
+        [InlineData("package+extra")]
+        public void AddNupkgCopiedData_NonstandardPackageId_HasNonstandardIdIsTrue(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.HasNonstandardId);
+        }
+
+        [Fact]
+        public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_HasNonstandardIdIsTrue()
+        {
+            // 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.HasNonstandardId);
+        }
+
+        [Fact]
+        public void AddNupkgCopiedData_NullPackageId_HasNonstandardIdIsFalse()
+        {
+            // 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.HasNonstandardId);
+        }
+
         [Fact]
         public async Task AddData_IsThreadSafe()
         {
@@ -308,6 +381,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.HasNonstandardId, result[PackageSourceTelemetry.PropertyNames.Nupkgs.HasNonstandardId]);
 
             Assert.Equal(data.Resources.Sum(r => r.Value.count), result[PackageSourceTelemetry.PropertyNames.Resources.Calls]);
             foreach (var resource in data.Resources)

Original file line number	Diff line number	Diff line change
`@@ -189,7 +189,7 @@ public async Task<bool> CopyNupkgFileToAsync(string destinationFilePath, Cancell`
`189`	`189`
`190`	`190`	`await source.CopyToAsync(destination, bufferSize, cancellationToken);`
`191`	`191`
`192`		`- ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(Source, destination.Length));`
	`192`	`+ ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(Source, destination.Length, _packageIdentity.Id));`
`193`	`193`
`194`	`194`	`return true;`
`195`	`195`	`}`
Original file line number	Diff line number	Diff line change
`@@ -176,7 +176,7 @@ public override async Task<bool> CopyNupkgToStreamAsync(`
`176`	`176`	`using (var fileStream = File.OpenRead(info.Path))`
`177`	`177`	`{`
`178`	`178`	`await fileStream.CopyToAsync(destination, cancellationToken);`
`179`		`- ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length));`
	`179`	`+ ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length, id));`
`180`	`180`	`return true;`
`181`	`181`	`}`
`182`	`182`	`}`
Original file line number	Diff line number	Diff line change
`@@ -210,7 +210,7 @@ public override async Task<bool> CopyNupkgToStreamAsync(`
`210`	`210`	`using (var fileStream = File.OpenRead(packagePath))`
`211`	`211`	`{`
`212`	`212`	`await fileStream.CopyToAsync(destination, cancellationToken);`
`213`		`- ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length));`
	`213`	`+ ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length, id));`
`214`	`214`	`return true;`
`215`	`215`	`}`
`216`	`216`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1 +1,3 @@`
`1`	`1`	`#nullable enable`
	`2`	`+~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string`
	`3`	`+~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string source, long fileSize, string packageId) -> void`
Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,7 @@ public async Task<bool> CopyNupkgToStreamAsync(`
`137`	`137`	`try`
`138`	`138`	`{`
`139`	`139`	`await stream.CopyToAsync(destination, token);`
`140`		`- ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_httpSource.PackageSource, destination.Length));`
	`140`	`+ ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_httpSource.PackageSource, destination.Length, identity.Id));`
`141`	`141`	`}`
`142`	`142`	`catch when (!token.IsCancellationRequested)`
`143`	`143`	`{`