Skip to content

Commit 79b81c0

Browse files
CopilotCopilot
authored
Add per-source telemetry for package IDs containing non-alphanumeric, dash, dot, or underscore characters (#7213)
1 parent 9a3254f commit 79b81c0

11 files changed

Lines changed: 270 additions & 4 deletions

File tree

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Collections.Generic;
99
using System.Globalization;
1010
using System.Linq;
11+
1112
using System.Threading;
1213
using System.Threading.Tasks;
1314
using NuGet.Common;
@@ -203,6 +204,25 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve
203204
{
204205
data.NupkgCount++;
205206
data.NupkgSize += ncEvent.FileSize;
207+
data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter || (ncEvent.PackageId.Length > 0 && HasNonAlphanumericDotDashOrUnderscoreCharacters(ncEvent.PackageId));
208+
}
209+
210+
bool HasNonAlphanumericDotDashOrUnderscoreCharacters(string packageId)
211+
{
212+
foreach (char c in packageId.AsSpan())
213+
{
214+
if (!IsCharacterAlphanumericDotDashOrUnderscore(c))
215+
{
216+
return true;
217+
}
218+
}
219+
220+
return false;
221+
222+
bool IsCharacterAlphanumericDotDashOrUnderscore(char c)
223+
{
224+
return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-' || c == '_';
225+
}
206226
}
207227
}
208228

@@ -261,6 +281,7 @@ internal static async Task<TelemetryEvent> ToTelemetryAsync(Data data, SourceRep
261281
telemetry[PropertyNames.Duration.Total] = data.Resources.Values.Sum(r => r.duration.TotalMilliseconds);
262282
telemetry[PropertyNames.Nupkgs.Copied] = data.NupkgCount;
263283
telemetry[PropertyNames.Nupkgs.Bytes] = data.NupkgSize;
284+
telemetry[PropertyNames.Nupkgs.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter] = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter;
264285
AddResourceProperties(telemetry, data.Resources);
265286

266287
if (data.Http.Requests > 0)
@@ -426,6 +447,7 @@ internal class Data
426447
internal HttpData Http { get; }
427448
internal int NupkgCount { get; set; }
428449
internal long NupkgSize { get; set; }
450+
internal bool IdContainsNonAlphanumericDotDashOrUnderscoreCharacter { get; set; }
429451

430452
internal Data()
431453
{
@@ -475,6 +497,7 @@ internal static class Nupkgs
475497
{
476498
internal const string Copied = "nupkgs.copied";
477499
internal const string Bytes = "nupkgs.bytes";
500+
internal const string IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = "nupkgs.idcontainsNonAlphanumericDotDashOrUnderscorecharacter";
478501
}
479502

480503
internal static class Resources

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ private readonly Dictionary<RestoreTargetGraph, Dictionary<string, LibraryInclud
5656
private const string IsCentralVersionManagementEnabled = nameof(IsCentralVersionManagementEnabled);
5757
private const string TotalUniquePackagesCount = nameof(TotalUniquePackagesCount);
5858
private const string NewPackagesInstalledCount = nameof(NewPackagesInstalledCount);
59+
private const string AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters = nameof(AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters);
5960
private const string SourcesCount = nameof(SourcesCount);
6061
private const string HttpSourcesCount = nameof(HttpSourcesCount);
6162
private const string LocalSourcesCount = nameof(LocalSourcesCount);
@@ -747,6 +748,7 @@ private async Task<EvaluateLockFileResult>
747748
}
748749

749750
telemetry.TelemetryEvent[NewPackagesInstalledCount] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Install).Distinct().Count();
751+
telemetry.TelemetryEvent[AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Flattened).Any(i => HasNonAlphanumericDotDashOrUnderscoreCharacters(i.Key.Name));
750752
telemetry.TelemetryEvent[RestoreSuccess] = success;
751753
}
752754

@@ -759,6 +761,24 @@ private async Task<EvaluateLockFileResult>
759761
packagesLockFile,
760762
packagesLockFilePath,
761763
cacheFile);
764+
765+
bool HasNonAlphanumericDotDashOrUnderscoreCharacters(string packageId)
766+
{
767+
foreach (char c in packageId.AsSpan())
768+
{
769+
if (!IsCharacterAlphanumericDotDashOrUnderscore(c))
770+
{
771+
return true;
772+
}
773+
}
774+
775+
return false;
776+
777+
bool IsCharacterAlphanumericDotDashOrUnderscore(char c)
778+
{
779+
return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-' || c == '_';
780+
}
781+
}
762782
}
763783

764784
/// <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‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,26 @@ public sealed class ProtocolDiagnosticNupkgCopiedEvent
88
public string Source { get; }
99
public long FileSize { get; }
1010

11+
/// <summary>
12+
/// Gets the package ID of the copied nupkg.
13+
/// </summary>
14+
public string PackageId { get; }
15+
1116
public ProtocolDiagnosticNupkgCopiedEvent(
1217
string source,
1318
long fileSize)
19+
: this(source, fileSize, packageId: string.Empty)
20+
{
21+
}
22+
23+
public ProtocolDiagnosticNupkgCopiedEvent(
24+
string source,
25+
long fileSize,
26+
string packageId)
1427
{
1528
Source = source;
1629
FileSize = fileSize;
30+
PackageId = packageId;
1731
}
1832
}
1933
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ public async Task<bool> CopyNupkgFileToAsync(string destinationFilePath, Cancell
189189

190190
await source.CopyToAsync(destination, bufferSize, cancellationToken);
191191

192-
ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(Source, destination.Length));
192+
ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(Source, destination.Length, _packageIdentity.Id));
193193

194194
return true;
195195
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ public override async Task<bool> CopyNupkgToStreamAsync(
176176
using (var fileStream = File.OpenRead(info.Path))
177177
{
178178
await fileStream.CopyToAsync(destination, cancellationToken);
179-
ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length));
179+
ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length, id));
180180
return true;
181181
}
182182
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ public override async Task<bool> CopyNupkgToStreamAsync(
222222
fileStream.Position = 0;
223223

224224
await fileStream.CopyToAsync(destination, cancellationToken);
225-
ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length));
225+
ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length, id));
226226
return true;
227227
}
228228
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
#nullable enable
2+
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string!
3+
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string! packageId) -> void

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
#nullable enable
2+
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string!
3+
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string! packageId) -> void

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ public async Task<bool> CopyNupkgToStreamAsync(
137137
try
138138
{
139139
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));
141141
}
142142
catch when (!token.IsCancellationRequested)
143143
{

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,78 @@ public void AddNupkgCopiedData_MultipleEvents_AccumulatesCorrectly()
137137
Assert.Equal(sizes.Sum(), result.NupkgSize);
138138
}
139139

140+
[Theory]
141+
[InlineData("Newtonsoft.Json")]
142+
[InlineData("NuGet.Protocol")]
143+
[InlineData("My-Package.1")]
144+
[InlineData("ALLCAPS")]
145+
[InlineData("alllower")]
146+
[InlineData("123Numeric")]
147+
[InlineData("a")]
148+
public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsFalse(string packageId)
149+
{
150+
// Arrange
151+
var data = CreateDataDictionary(SampleSource);
152+
var nce = new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, packageId);
153+
154+
// Act
155+
PackageSourceTelemetry.AddNupkgCopiedData(nce, data);
156+
157+
// Assert
158+
var result = Assert.Single(data).Value;
159+
Assert.False(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
160+
}
161+
162+
[Theory]
163+
[InlineData("[email protected]")]
164+
[InlineData("Ünïcödé")]
165+
[InlineData("Package Name")]
166+
[InlineData("package+extra")]
167+
public void AddNupkgCopiedData_NonstandardPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsTrue(string packageId)
168+
{
169+
// Arrange
170+
var data = CreateDataDictionary(SampleSource);
171+
var nce = new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, packageId);
172+
173+
// Act
174+
PackageSourceTelemetry.AddNupkgCopiedData(nce, data);
175+
176+
// Assert
177+
var result = Assert.Single(data).Value;
178+
Assert.True(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
179+
}
180+
181+
[Fact]
182+
public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsTrue()
183+
{
184+
// Arrange
185+
var data = CreateDataDictionary(SampleSource);
186+
187+
// Act
188+
PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Standard.Package"), data);
189+
PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Nonstandard@Package"), data);
190+
PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Another.Standard"), data);
191+
192+
// Assert
193+
var result = Assert.Single(data).Value;
194+
Assert.True(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
195+
}
196+
197+
[Fact]
198+
public void AddNupkgCopiedData_EmptyPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsFalse()
199+
{
200+
// Arrange
201+
var data = CreateDataDictionary(SampleSource);
202+
var nce = new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000);
203+
204+
// Act
205+
PackageSourceTelemetry.AddNupkgCopiedData(nce, data);
206+
207+
// Assert
208+
var result = Assert.Single(data).Value;
209+
Assert.False(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
210+
}
211+
140212
[Fact]
141213
public async Task AddData_IsThreadSafe()
142214
{
@@ -308,6 +380,7 @@ public async Task ToTelemetry_WithData_CreatesTelemetryProperties(string package
308380

309381
Assert.Equal(data.NupkgCount, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Copied]);
310382
Assert.Equal(data.NupkgSize, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Bytes]);
383+
Assert.Equal(data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter, result[PackageSourceTelemetry.PropertyNames.Nupkgs.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter]);
311384

312385
Assert.Equal(data.Resources.Sum(r => r.Value.count), result[PackageSourceTelemetry.PropertyNames.Resources.Calls]);
313386
foreach (var resource in data.Resources)

0 commit comments

Comments
 (0)