Skip to content
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.

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

Add PackageDownload vulnerability telemetry #6180

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

Conversation

Nigusu-Allehu
Copy link
Contributor

@Nigusu-Allehu Nigusu-Allehu commented Dec 4, 2024

Bug

Fixes: https://github.com/NuGet/Client.Engineering/issues/3112

Description

Telemetry design https://github.com/NuGet/Client.Engineering/blob/main/designs/telemetry/telemetry-design-PackageDownload-vulnerabilities.md
This PR introduces telemetry collection for vulnerabilities in <PackageDownload> packages:

Summary of Changes

  • Collects the list of vulnerable package IDs at the end of the restore operation.

Goals

This telemetry will enable us to:

  • Determine how many restore operations generate warnings due to vulnerable <PackageDownload> packages.
  • Use package names to estimate the proportion of warnings attributable to SDK-managed packages.

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.

@Nigusu-Allehu Nigusu-Allehu self-assigned this Dec 4, 2024
@Nigusu-Allehu Nigusu-Allehu marked this pull request as ready for review December 6, 2024 00:00
@Nigusu-Allehu Nigusu-Allehu requested a review from a team as a code owner December 6, 2024 00:00
Copy link
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

Consider using a telemetry template issue: https://github.com/NuGet/Client.Engineering/issues/new?assignees=&labels=Type%3AEngineering%2CPriority%3A2&projects=&template=telemetry.md.

I think we've forgotten to use it reliable (includes all of us :) )

@jeffkl jeffkl self-requested a review December 10, 2024 23:09
@Nigusu-Allehu Nigusu-Allehu marked this pull request as draft December 17, 2024 22:15
@Nigusu-Allehu Nigusu-Allehu marked this pull request as ready for review December 19, 2024 18:15
@Nigusu-Allehu Nigusu-Allehu requested a review from zivkan December 30, 2024 13:47
zivkan
zivkan previously approved these changes Dec 30, 2024
Copy link
Member

@zivkan zivkan left a comment

Choose a reason for hiding this comment

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

I'm confident that this PR now achieves its stated goal, without introducing regressions.

I still think we can make improvements to reduce the risk of future regressions though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants