豆豆友情提示：这是一个非官方 GitHub 代理镜像，主要用于网络测试或访问加速。请勿在此进行登录、注册或处理任何敏感信息。进行这些操作请务必访问官方网站 github.com。 Raw 内容也通过此代理提供。
Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 7 commits into
base: dev
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
61942fa
Initial plan
Copilot Mar 13, 2026
60ab977
Add HasNonstandardId telemetry for non-standard package ID characters…
Copilot Mar 13, 2026
a9de1db
Rename HasNonstandardId to IdContainsNonAsciiCharacter throughout
Copilot Mar 16, 2026
e1b0705
Replace Regex with ReadOnlySpan char check in HasNonstandardCharacters
Copilot Mar 16, 2026
79664c2
Add telemetry to RestoreCommand as well
jeffkl Mar 30, 2026
b0d3c49
Feedback
jeffkl Apr 15, 2026
5a894ff
Fix nullability annotations in ProtocolDiagnosticNupkgCopiedEvent and…
Copilot Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Collections.Generic;
using System.Globalization;
using System.Linq;

using System.Threading;
using System.Threading.Tasks;
using NuGet.Common;
Expand Down Expand Up @@ -203,6 +204,25 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve
{
data.NupkgCount++;
data.NupkgSize += ncEvent.FileSize;
data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter || (ncEvent.PackageId != null && 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 == '_';
}
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -741,6 +742,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;
}

Expand All @@ -753,6 +755,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>
Expand Down
16 changes: 14 additions & 2 deletions src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,33 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

#nullable disable

namespace NuGet.Protocol.Events
{
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; }
Copy link
Copy Markdown
Member

@nkolev92 nkolev92 Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeffkl This can be non null if we force copilot to just "break" the API potentially.

We can also just force is to annotate as non-null on purpose and annotate the old constructor as null and default the package ID to string.Empty?

I tend to prefer non-null whenever possible so I'm fine with the breaking change.

cc @zivkan

Copy link
Copy Markdown
Member

@zivkan zivkan Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus "ProtocolDiagnostics" were always intended to be used by our telemetry. Since we refuse to use InternalsVisibleTo for anything other than tests, we have no technical means to make it internal to NuGet across product assembly boundaries. But for this reason, I'm much less concerned about breaking changes in ProtocolDiagnostics than I am about any other API.

Copy link
Copy Markdown
Member

@nkolev92 nkolev92 Apr 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we refuse to use InternalsVisibleTo for anything other than tests, we have no technical means to make it internal to NuGet across product assembly boundaries

This has been a sanity policy that tends to work super well as a general rule.
That being said, exposing InternalsVisibileTo to projects that never ship as packages, probably won't break anything.

Copy link
Copy Markdown
Member

@zivkan zivkan Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that InternalsVisibleTo doesn't allow you to limit it to just ProtocolDiagnostics or a limited list of types. It makes every internal modifier available. And when using Intellisense or similar from that target assembly, giving you a list of available types and autocomplete, you won't know if the NuGet.Protocol type is internal or public, making it too easy to use types that we really don't want to be used outside NuGet.Protocol. So, I don't like the idea of using InternalsVisibleTo some of our VS assembles.


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;
}
}
}
2 changes: 1 addition & 1 deletion src/NuGet.Core/NuGet.Protocol/LocalPackageArchiveDownloader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV2FindPackageByIdResource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV3FindPackageByIdResource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Shipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,8 +378,8 @@ NuGet.Protocol.Events.ProtocolDiagnosticHttpEventBase.IsRetry.get -> bool
~NuGet.Protocol.Events.ProtocolDiagnosticHttpEventBase.Url.get -> System.Uri
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.FileSize.get -> long
~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string source, long fileSize) -> void
~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.Source.get -> string
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize) -> void
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.Source.get -> string!
NuGet.Protocol.Events.ProtocolDiagnosticResourceEvent
NuGet.Protocol.Events.ProtocolDiagnosticResourceEvent.Duration.get -> System.TimeSpan
~NuGet.Protocol.Events.ProtocolDiagnosticResourceEvent.Method.get -> string
Expand Down
2 changes: 2 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
#nullable enable
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string?
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string? packageId) -> void
4 changes: 2 additions & 2 deletions src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Shipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,8 +378,8 @@ NuGet.Protocol.Events.ProtocolDiagnosticHttpEventBase.IsRetry.get -> bool
~NuGet.Protocol.Events.ProtocolDiagnosticHttpEventBase.Url.get -> System.Uri
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.FileSize.get -> long
~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string source, long fileSize) -> void
~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.Source.get -> string
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize) -> void
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.Source.get -> string!
NuGet.Protocol.Events.ProtocolDiagnosticResourceEvent
NuGet.Protocol.Events.ProtocolDiagnosticResourceEvent.Duration.get -> System.TimeSpan
~NuGet.Protocol.Events.ProtocolDiagnosticResourceEvent.Method.get -> string
Expand Down
2 changes: 2 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
#nullable enable
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string?
NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string? packageId) -> void
2 changes: 1 addition & 1 deletion src/NuGet.Core/NuGet.Protocol/Utility/FindPackagesByIdNupkgDownloader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
74 changes: 74 additions & 0 deletions ...Get.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_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("My_Package")]
[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_NullPackageId_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()
{
Expand Down Expand Up @@ -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.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)
Expand Down
Loading
Loading