Change Waves Refactoring

[benvillalobos]

Issue Description

Change Waves shipped with a few quirks. In rough order of importance:

To Do

• Referencing ChangeWaves from traits produces does not exist errors, and when including it in the projects where it was previously unavailable, it produces the error The type 'ChangeWaves' exists in both 'Microsoft.Build' .... and 'Microsoft.Build.Tasks.Core'.
• Now tracking here: Cannot Reference ChangeWaves from Traits.cs #5903
• The warnings that are thrown are currently thrown multiple times, rather than once per build.
  • Warnings thrown from BuildManager.BeginBuild are not caught by VS at all.
  • Related? Mux logger does not preserve events fired by BuildManager.BeginBuild #5056
• Now being tracked here: Change Waves Warnings Don't Report to Error/Warning Window in VS and Log Many Times #5902

Done

• Make ChangeWaves internal
• Update change wave docs (Tracking here: Update Change Wave Docs #5851)
• Assign a code to the warnings thrown by change waves.
• Throwing warnings is not correctly caught in build logs
  • We believe this is an issue with the structured log viewer. Tracking here: Binlog Viewer Not Displaying Warnings KirillOsenkov/MSBuildStructuredLog#399
• When testing change waves on existing functionality, there's a chance that a new test will set the MSBuildDisableFeaturesFromVersion env var, and THEN an existing test runs (that is now conditioned on change waves), failing the existing test. This is worked around by calling SetChangeWave(string.Empty) on TestEnvironment's Dispose (or Cleanup) functions. We should call BuildEnvironmentHelper.ResetInstance_ForUnitTestsOnly(); here as well.
  • Fixed via Follow up fix #5755
• Convert change waves to use versions exclusively.
• Refer to Enable Change Waves #5710

[benvillalobos]

Warnings thrown from BuildManager.BeginBuild do not show in Output window.

These warnings are not caught at all through VS. I tested this by capturing a binlog and replaying it with -flp:v=diag and MSBuildDisableFeaturesFromVersion=16.5. cmd line shows the proper warning, VS does not see this at all.

[benvillalobos]

Referencing ChangeWaves from traits produces does not exist errors, and when including it in the projects where it was previously unavailable, it produces the error The type 'ChangeWaves' exists in both 'Microsoft.Build' .... and 'Microsoft.Build.Tasks.Core'.

This is an issue for developers that want set escape hatches and feature flags based on a change wave, rather than placing the change wave check at the site of their feature, which I'm not entirely convinced is better. Unfortunately, devs don't have the choice and enabling that would be great.

[japj]

Could you perhaps also add some information on why “risky features are developed under the same opt-out flag”.
That confused me a bit, since if they are risky why not have it be opt-in until it is “safe”?

[benvillalobos]

@japj Unfortunately, some of what we work on are improvements that would never be considered safe because it may break some very specific scenario. If there's something we think is worth the risk, whether it be for performance or functionality improvement, we found that Change Waves were a good middle ground between making necessary changes and warning customers of these changes.

Opt-out is a better approach for us because we'd likely get limited feedback when a feature impacts customer builds. When a feature does impact a customer negatively, it's a quick switch to disable and allows time to adapt. The key aspect to Change Waves is that it smooths the transition for customers adapting to risky changes that the MSBuild team feels strongly enough to implement.

[benvillalobos]

This will be closed out when #5864 merges in, as the last two items have separate issues tracking them.

[benvillalobos]

Closed as of #5864.