microsoft / ApplicationInsights-Profiler-AspNetCore

Application Insights Profiler sample and documentation
MIT License
66 stars 22 forks source link

When you publish an update for Microsoft.ApplicationInsights.Profiler.AspNetCore, don't mark the previous version as deprecated immediately #219

Closed KirkMunroSagent closed 2 months ago

KirkMunroSagent commented 3 months ago

Reposting https://github.com/microsoft/ApplicationInsights-Profiler-AspNetCore/discussions/217 as an issue, since it seems that Discussions aren't really used in this repository.

This idea comes from the perspective of someone who does package management.

Whenever a new Microsoft.ApplicationInsights.Profiler.AspNetCore package is published, whoever publishes that package also marks the previous version as deprecated. This is different behavior than what is done on any other Microsoft.* nupkg. The challenge is that when you mark the previous version of Microsoft.ApplicationInsights.Profiler.AspNetCore as deprecated, it causes a little warning sign to appear in the NuGet Package Manager UI in Visual Studio, because suddenly projects are using a deprecated version of a package. That warning is not appropriate, because projects are fine to continue using the previous version, at least for a while.

The problem with this approach is that it masks nupkgs that really need attention. For example, if a package is found to have a known vulnerability, but that vulnerability is not very severe, a warning icon can show up in the Nuget Package Manager UI in Visual Studio. That is something that really should be getting the attention of a package manager, because they need to look into the warning details so that they can triage the package update appropriately. When a new version of Microsoft.ApplicationInsights.Profiler.AspNetCore is published, by marking the previous version as deprecated, you are potentially masking real warnings that need attention. Imagine if every product team that published NuGet packages marked all of their old versions as deprecated. Suddenly we'd have warnings on every package all of the time, making those warnings absolutely useless.

All to say, by marking a previous version as deprecated when you publish a new package, you are taking the wrong approach because it sets the expectation that the package needs attention right away, which is not the case. Please change your publishing strategy such that you only mark an old version as deprecated if it is truly not expected to work anymore.

xiaomi7732 commented 2 months ago

Hey @KirkMunroSagent, I am sorry that I can't get back to you earlier for the discussion or for this bug filed.

I agree with you that deprecating packages at the moment of releasing the new package is too aggressive. As you pointed out, it generates warnings, and it also masks the real need of deprecating.

The other side, in the sprit of keeping things relative up to date, it is our interest to push forward the new bits, which usually carry more secure dependencies.

With those considerations, how about to having a grace period of the previous released package, what about a minimum of 30 days, given there's no known vulnerability?

Thanks.

KirkMunroSagent commented 2 months ago

@xiaomi7732: A grace period would be a welcome improvement. 30 days feels short though. Even with agile development, packages might not be updated within 30 days, depending on how the dev cycle is laid out. I think a grace period of 45 or 60 days would be more appropriate. It would certainly do the trick for my place of work. It also encourages a regular package management cycle without being too overbearing.

xiaomi7732 commented 2 months ago

@KirkMunroSagent, 45 days sounds like a start point. Let's try it and if that still cause issues, either too long or too short, we can always tweak later, agree?

KirkMunroSagent commented 2 months ago

@xiaomi7732 Yes, I agree. It will be a good step in the right direction, and we'll see if it needs to be tweaked later. Thank you for this.

xiaomi7732 commented 2 months ago

Thank you for the contribution!