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

[Copilot]

Bug

Fixes: NuGet/Home#14212

Description

Adds telemetry to track whether any package ID contains characters outside [A-Za-z0-9.\-_] (i.e., anything other than ASCII letters, digits, ., or -, _). This is emitted in two places:

1. Per-source (PackageSourceDiagnostics event) — correlates non-standard ID usage to specific feed providers
2. Per-restore (ProjectRestoreInformation event) — indicates whether any resolved package in the restore graph has a non-standard ID

Protocol layer

• Added PackageId property to ProtocolDiagnosticNupkgCopiedEvent (nullable; new 3-param constructor, old 2-param delegates to it)
• Updated all four raise-sites to pass the package ID:
  • FindPackagesByIdNupkgDownloader → identity.Id
  • LocalPackageArchiveDownloader → _packageIdentity.Id
  • LocalV3FindPackageByIdResource / LocalV2FindPackageByIdResource → id parameter
• New public API declared in both PublicAPI.Unshipped.txt files

Per-source telemetry (PackageSourceTelemetry)

• Data class: new IdContainsNonAsciiCharacter bool, sticky-true once any non-standard ID is seen for that source
• Detection uses a zero-allocation ReadOnlySpan<char> character loop (local function HasNonASCIICharacters)
• AddNupkgCopiedData: skips check when PackageId is null (events raised by old constructor)
• ToTelemetryAsync: emits nupkgs.idcontainsnonasciicharacter alongside the existing nupkgs.copied / nupkgs.bytes

Per-restore telemetry (RestoreCommand)

• New AnyPackageIdContainsNonASCIICharacters property on the ProjectRestoreInformation telemetry event
• Scans all resolved packages across non-conflicting restore graphs using the same character-check logic (local function)
• Only emitted for non-NoOp restores (NoOp returns before this code runs)

Tests

PackageSourceTelemetryTests — Existing tests covering AddNupkgCopiedData for: all-standard IDs → false; non-standard IDs (underscore, Unicode, space, @, +) → true; mixed batch with one non-standard → sticky true; null PackageId → false. ToTelemetry_WithData_CreatesTelemetryProperties asserts the new property is emitted.

RestoreCommandTests — New tests:

• ExecuteAsync_WithASCIIPackageId_AnyPackageIdContainsNonASCIICharactersIsFalse — standard [A-Za-z0-9.-] package → false
• ExecuteAsync_WithNonASCIIPackageId_AnyPackageIdContainsNonASCIICharactersIsTrue — [Theory] with underscore, Kelvin sign (U+212A), Greek alpha (U+03B1), accented Latin (U+00E9), Cyrillic (U+0410) → true
• ExecuteAsync_WithMixedPackageIds_AnyPackageIdContainsNonASCIICharactersIsTrue — [Theory] where direct dep is standard but transitive dep has non-standard chars → true
• Updated existing exhaustive telemetry tests to include the new property

PR Checklist

• Meaningful title, helpful description and a linked NuGet/Home issue
• Added tests
• Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

[nkolev92]

@copilot
This needs rebased to fix the nullability warnings.

[Copilot]

@copilot
This needs rebased to fix the nullability warnings.

Fixed in 5a894ff. The changes incorporate dev's nullability Phase 1 work for ProtocolDiagnosticNupkgCopiedEvent:

• Removed #nullable disable from ProtocolDiagnosticNupkgCopiedEvent.cs
• PackageId property and packageId constructor parameter are now string?
• PublicAPI.Shipped.txt (both TFMs): replaced the old ~ entries for Source and the 2-param constructor with properly-annotated string! entries (matching what dev's nullability PR added)
• PublicAPI.Unshipped.txt (both TFMs): replaced ~ entries with fully-annotated forms (string? for PackageId, string!/string? for the 3-param constructor)

Build: 0 warnings, 0 errors.

[nkolev92]

@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

[zivkan]

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.

[nkolev92]

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.

[zivkan]

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.