From 61942faae8e79dc07a1250373d96f29e2d83b750 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 17:23:55 +0000 Subject: [PATCH 1/5] Initial plan From 60ab977df9eed646247076270fe2e54d4be90ada Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 17:31:48 +0000 Subject: [PATCH 2/5] Add HasNonstandardId telemetry for non-standard package ID characters per feed source Co-authored-by: jeffkl <17556515+jeffkl@users.noreply.github.com> --- .../Telemetry/PackageSourceTelemetry.cs | 16 ++++ .../ProtocolDiagnosticNupkgCopiedEvent.cs | 14 ++++ .../LocalPackageArchiveDownloader.cs | 2 +- .../LocalV2FindPackageByIdResource.cs | 2 +- .../LocalV3FindPackageByIdResource.cs | 2 +- .../PublicAPI/net472/PublicAPI.Unshipped.txt | 2 + .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 2 + .../FindPackagesByIdNupkgDownloader.cs | 2 +- .../Telemetry/PackageSourceTelemetryTests.cs | 74 +++++++++++++++++++ 9 files changed, 112 insertions(+), 4 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs index bdc107c6b6e..57d87a521a9 100644 --- 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 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 index d42ea8a49d5..570ae205aa7 100644 --- 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; } + /// + /// Gets the package ID of the copied nupkg, or if not available. + /// + 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 index 0ca015f1efe..ca249de6c6c 100644 --- a/src/NuGet.Core/NuGet.Protocol/LocalPackageArchiveDownloader.cs +++ b/src/NuGet.Core/NuGet.Protocol/LocalPackageArchiveDownloader.cs @@ -189,7 +189,7 @@ public async Task 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 index 33b1af863c1..e2cb9bc145a 100644 --- 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 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 index 7fbf0e3f90b..d477af8a3a3 100644 --- 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 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 index 7dc5c58110b..938c0d98a7d 100644 --- 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 index 7dc5c58110b..938c0d98a7d 100644 --- 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 index f87e02f7fbd..f80def2671a 100644 --- 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 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 index 2a0ffc920dc..d30df622792 100644 --- 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) From a9de1db715092cad924fc7e0be3919b99db3fbbd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 20:03:10 +0000 Subject: [PATCH 3/5] Rename HasNonstandardId to IdContainsNonAsciiCharacter throughout Co-authored-by: jeffkl <17556515+jeffkl@users.noreply.github.com> --- .../Telemetry/PackageSourceTelemetry.cs | 10 +++++----- .../Telemetry/PackageSourceTelemetryTests.cs | 18 +++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs index 57d87a521a9..e83b88396d5 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs @@ -207,9 +207,9 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve data.NupkgCount++; data.NupkgSize += ncEvent.FileSize; - if (!data.HasNonstandardId && ncEvent.PackageId != null && HasNonstandardCharacters(ncEvent.PackageId)) + if (!data.IdContainsNonAsciiCharacter && ncEvent.PackageId != null && HasNonstandardCharacters(ncEvent.PackageId)) { - data.HasNonstandardId = true; + data.IdContainsNonAsciiCharacter = true; } } } @@ -274,7 +274,7 @@ internal static async Task 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; + telemetry[PropertyNames.Nupkgs.IdContainsNonAsciiCharacter] = data.IdContainsNonAsciiCharacter; AddResourceProperties(telemetry, data.Resources); if (data.Http.Requests > 0) @@ -440,7 +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 bool IdContainsNonAsciiCharacter { get; set; } internal Data() { @@ -490,7 +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 const string IdContainsNonAsciiCharacter = "nupkgs.idcontainsnonasciicharacter"; } internal static class Resources 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 index d30df622792..7360bfe4ee9 100644 --- a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs @@ -145,7 +145,7 @@ public void AddNupkgCopiedData_MultipleEvents_AccumulatesCorrectly() [InlineData("alllower")] [InlineData("123Numeric")] [InlineData("a")] - public void AddNupkgCopiedData_StandardPackageId_HasNonstandardIdIsFalse(string packageId) + public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAsciiCharacterIsFalse(string packageId) { // Arrange var data = CreateDataDictionary(SampleSource); @@ -156,7 +156,7 @@ public void AddNupkgCopiedData_StandardPackageId_HasNonstandardIdIsFalse(string // Assert var result = Assert.Single(data).Value; - Assert.False(result.HasNonstandardId); + Assert.False(result.IdContainsNonAsciiCharacter); } [Theory] @@ -165,7 +165,7 @@ public void AddNupkgCopiedData_StandardPackageId_HasNonstandardIdIsFalse(string [InlineData("Ünïcödé")] [InlineData("Package Name")] [InlineData("package+extra")] - public void AddNupkgCopiedData_NonstandardPackageId_HasNonstandardIdIsTrue(string packageId) + public void AddNupkgCopiedData_NonstandardPackageId_IdContainsNonAsciiCharacterIsTrue(string packageId) { // Arrange var data = CreateDataDictionary(SampleSource); @@ -176,11 +176,11 @@ public void AddNupkgCopiedData_NonstandardPackageId_HasNonstandardIdIsTrue(strin // Assert var result = Assert.Single(data).Value; - Assert.True(result.HasNonstandardId); + Assert.True(result.IdContainsNonAsciiCharacter); } [Fact] - public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_HasNonstandardIdIsTrue() + public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAsciiCharacterIsTrue() { // Arrange var data = CreateDataDictionary(SampleSource); @@ -192,11 +192,11 @@ public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_HasNonstandardIdIs // Assert var result = Assert.Single(data).Value; - Assert.True(result.HasNonstandardId); + Assert.True(result.IdContainsNonAsciiCharacter); } [Fact] - public void AddNupkgCopiedData_NullPackageId_HasNonstandardIdIsFalse() + public void AddNupkgCopiedData_NullPackageId_IdContainsNonAsciiCharacterIsFalse() { // Arrange var data = CreateDataDictionary(SampleSource); @@ -207,7 +207,7 @@ public void AddNupkgCopiedData_NullPackageId_HasNonstandardIdIsFalse() // Assert var result = Assert.Single(data).Value; - Assert.False(result.HasNonstandardId); + Assert.False(result.IdContainsNonAsciiCharacter); } [Fact] @@ -381,7 +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.IdContainsNonAsciiCharacter, result[PackageSourceTelemetry.PropertyNames.Nupkgs.IdContainsNonAsciiCharacter]); Assert.Equal(data.Resources.Sum(r => r.Value.count), result[PackageSourceTelemetry.PropertyNames.Resources.Calls]); foreach (var resource in data.Resources) From e1b070550caf5b7ec6126c8ec78198774b1f91ac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 20:10:29 +0000 Subject: [PATCH 4/5] Replace Regex with ReadOnlySpan char check in HasNonstandardCharacters Co-authored-by: jeffkl <17556515+jeffkl@users.noreply.github.com> --- .../Telemetry/PackageSourceTelemetry.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs index e83b88396d5..d412c216121 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs @@ -8,7 +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; @@ -30,8 +30,6 @@ 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, @@ -216,7 +214,14 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve private static bool HasNonstandardCharacters(string packageId) { - return s_nonstandardIdCharRegex.IsMatch(packageId); + foreach (char c in packageId.AsSpan()) + { + if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-')) + { + return true; + } + } + return false; } public void Dispose() From 79664c2ae0123c5d5891a48c8fad9fa0870cc5f3 Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Mon, 30 Mar 2026 13:11:15 -0700 Subject: [PATCH 5/5] Add telemetry to RestoreCommand as well --- .../Telemetry/PackageSourceTelemetry.cs | 20 ++- .../RestoreCommand/RestoreCommand.cs | 19 +++ .../RestoreCommandTests.cs | 136 +++++++++++++++++- 3 files changed, 162 insertions(+), 13 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs index d412c216121..2c5c672ce21 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs @@ -204,24 +204,20 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve { data.NupkgCount++; data.NupkgSize += ncEvent.FileSize; - - if (!data.IdContainsNonAsciiCharacter && ncEvent.PackageId != null && HasNonstandardCharacters(ncEvent.PackageId)) - { - data.IdContainsNonAsciiCharacter = true; - } + data.IdContainsNonAsciiCharacter = data.IdContainsNonAsciiCharacter || (ncEvent.PackageId != null && HasNonASCIICharacters(ncEvent.PackageId)); } - } - private static bool HasNonstandardCharacters(string packageId) - { - foreach (char c in packageId.AsSpan()) + bool HasNonASCIICharacters(string packageId) { - if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-')) + foreach (char c in packageId.AsSpan()) { - return true; + if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-')) + { + return true; + } } + return false; } - return false; } public void Dispose() diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 13df4d863f0..dae776327a1 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -54,6 +54,7 @@ private readonly Dictionary } telemetry.TelemetryEvent[NewPackagesInstalledCount] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Install).Distinct().Count(); + telemetry.TelemetryEvent[AnyPackageIdContainsNonASCIICharacters] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Flattened).Any(i => HasNonASCIICharacters(i.Key.Name)); telemetry.TelemetryEvent[RestoreSuccess] = success; } @@ -753,6 +755,23 @@ private async Task packagesLockFile, packagesLockFilePath, cacheFile); + + bool HasNonASCIICharacters(string packageId) + { + if (string.IsNullOrWhiteSpace(packageId)) + { + return false; + } + + foreach (char c in packageId.AsSpan()) + { + if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-')) + { + return true; + } + } + return false; + } } /// Run NuGetAudit on the project's resolved restore graphs, and log messages and telemetry with the results. diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs index d3d7c627af7..29688ab1b25 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs @@ -2957,6 +2957,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( ["NoOpDuration"] = value => value.Should().NotBeNull(), ["TotalUniquePackagesCount"] = value => value.Should().Be(1), ["NewPackagesInstalledCount"] = value => value.Should().Be(1), + ["AnyPackageIdContainsNonASCIICharacters"] = value => value.Should().Be(false), ["EvaluateLockFileDuration"] = value => value.Should().NotBeNull(), ["CreateRestoreTargetGraphDuration"] = value => value.Should().NotBeNull(), ["GenerateRestoreGraphDuration"] = value => value.Should().NotBeNull(), @@ -3235,7 +3236,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); - projectInformationEvent.Count.Should().Be(49); + projectInformationEvent.Count.Should().Be(50); projectInformationEvent["RestoreSuccess"].Should().Be(true); projectInformationEvent["NoOpResult"].Should().Be(false); projectInformationEvent["TotalUniquePackagesCount"].Should().Be(2); @@ -3644,6 +3645,139 @@ private static PackageSpec CreatePackageSpec(List tf return packageSpec; } + [Fact] + public async Task ExecuteAsync_WithASCIIPackageId_AnyPackageIdContainsNonASCIICharactersIsFalse() + { + // Arrange + using var pathContext = new SimpleTestPathContext(); + var projectName = "TestProject"; + var projectPath = Path.Combine(pathContext.SolutionRoot, projectName); + PackageSpec packageSpec = ProjectTestHelpers.GetPackageSpec(projectName, pathContext.SolutionRoot, "net472", "My.Package1"); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + new SimpleTestPackageContext("My.Package1", "1.0.0")); + var logger = new TestLogger(); + + var request = new TestRestoreRequest(packageSpec, new PackageSource[] { new PackageSource(pathContext.PackageSource) }, pathContext.UserPackagesFolder, logger) + { + LockFilePath = Path.Combine(projectPath, "project.assets.json"), + ProjectStyle = ProjectStyle.PackageReference, + }; + + // Set-up telemetry service - Important to set-up the service *after* the package source creation call as that emits telemetry! + var telemetryEvents = new ConcurrentQueue(); + var _telemetryService = new Mock(MockBehavior.Loose); + _telemetryService + .Setup(x => x.EmitTelemetryEvent(It.IsAny())) + .Callback(x => telemetryEvents.Enqueue(x)); + + TelemetryActivity.NuGetTelemetryService = _telemetryService.Object; + + // Act + var restoreCommand = new RestoreCommand(request); + RestoreResult result = await restoreCommand.ExecuteAsync(); + + // Assert + result.Success.Should().BeTrue(because: logger.ShowMessages()); + var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); + projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(false); + } + + [Theory] + [InlineData("My_Package")] // underscore + [InlineData("Pac\u212Bage")] // Kelvin sign K (U+212A) + [InlineData("Package\u03B1")] // Greek lowercase alpha (U+03B1) + [InlineData("Package\u00E9")] // Latin small letter e with acute (U+00E9) + [InlineData("\u0410.Package")] // Cyrillic capital A (U+0410) + public async Task ExecuteAsync_WithNonASCIIPackageId_AnyPackageIdContainsNonASCIICharactersIsTrue(string packageId) + { + // Arrange + using var pathContext = new SimpleTestPathContext(); + var projectName = "TestProject"; + var projectPath = Path.Combine(pathContext.SolutionRoot, projectName); + PackageSpec packageSpec = ProjectTestHelpers.GetPackageSpec(projectName, pathContext.SolutionRoot, "net472", packageId); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + new SimpleTestPackageContext(packageId, "1.0.0")); + var logger = new TestLogger(); + + var request = new TestRestoreRequest(packageSpec, new PackageSource[] { new PackageSource(pathContext.PackageSource) }, pathContext.UserPackagesFolder, logger) + { + LockFilePath = Path.Combine(projectPath, "project.assets.json"), + ProjectStyle = ProjectStyle.PackageReference, + }; + + // Set-up telemetry service - Important to set-up the service *after* the package source creation call as that emits telemetry! + var telemetryEvents = new ConcurrentQueue(); + var _telemetryService = new Mock(MockBehavior.Loose); + _telemetryService + .Setup(x => x.EmitTelemetryEvent(It.IsAny())) + .Callback(x => telemetryEvents.Enqueue(x)); + + TelemetryActivity.NuGetTelemetryService = _telemetryService.Object; + + // Act + var restoreCommand = new RestoreCommand(request); + RestoreResult result = await restoreCommand.ExecuteAsync(); + + // Assert + result.Success.Should().BeTrue(because: logger.ShowMessages()); + var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); + projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(true); + } + + [Theory] + [InlineData("My_Package")] // underscore + [InlineData("Pac\u212Bage")] // Kelvin sign K (U+212A) + [InlineData("Package\u03B1")] // Greek lowercase alpha (U+03B1) + public async Task ExecuteAsync_WithMixedPackageIds_AnyPackageIdContainsNonASCIICharactersIsTrue(string packageId) + { + // Arrange + using var pathContext = new SimpleTestPathContext(); + var projectName = "TestProject"; + var projectPath = Path.Combine(pathContext.SolutionRoot, projectName); + var asciiOnlyPkg = new SimpleTestPackageContext("Some.Package", "1.0.0"); + var nonASCIIPkg = new SimpleTestPackageContext(packageId, "1.0.0"); + asciiOnlyPkg.Dependencies.Add(nonASCIIPkg); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + asciiOnlyPkg, + nonASCIIPkg); + + PackageSpec packageSpec = ProjectTestHelpers.GetPackageSpec(projectName, pathContext.SolutionRoot, "net472", "Some.Package"); + var logger = new TestLogger(); + + var request = new TestRestoreRequest(packageSpec, new PackageSource[] { new PackageSource(pathContext.PackageSource) }, pathContext.UserPackagesFolder, logger) + { + LockFilePath = Path.Combine(projectPath, "project.assets.json"), + ProjectStyle = ProjectStyle.PackageReference, + }; + + // Set-up telemetry service - Important to set-up the service *after* the package source creation call as that emits telemetry! + var telemetryEvents = new ConcurrentQueue(); + var _telemetryService = new Mock(MockBehavior.Loose); + _telemetryService + .Setup(x => x.EmitTelemetryEvent(It.IsAny())) + .Callback(x => telemetryEvents.Enqueue(x)); + + TelemetryActivity.NuGetTelemetryService = _telemetryService.Object; + + // Act + var restoreCommand = new RestoreCommand(request); + RestoreResult result = await restoreCommand.ExecuteAsync(); + + // Assert + result.Success.Should().BeTrue(because: logger.ShowMessages()); + var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); + projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(true); + } + private Task> DoWalkAsync(RemoteDependencyWalker walker, string name, NuGetFramework framework) { var range = new LibraryRange