Use provider type name for CodeAction telemetry for code actions...

[mavasani]

...created through CodeAction.Create factory methods

It only took 7 years, but...
Fixes #4919

Currently, we always use the CodeAction's full type name to generate the telemetry ID to log for applied code actions. This is reasonable for special CodeActions that sub-type the CodeAction type and have custom logic. However, majority of fixers and refactorings do not need special CodeAction sub-type and can/should use the CodeAction.Create factory methods to create the code actions to register. Until now, these providers were forced to create a dummy nested type (generally named MyCodeAction) and use it to allow telemetry to capture the outer type's full type name.

With this change, we now keep track of whether a CodeAction was created with a factory method or not. If it was created with a factory method, then we use the registering fixer/refactoring provider's full type name for telemetry. Otherwise, we use the CodeAction's full type name. Additionally, we also append the EquivalenceKey in the telemetry ID in both the cases, which should allow multiple code actions registered by the same fixer/refactoring to be differentiated.

NOTE: I will create a follow-up PRs in both Roslyn and Roslyn-Analyzers repos to delete all the stub MyCodeAction types once this goes in.

[Youssef1313]

NOTE: I will create a follow-up PRs in both Roslyn and Roslyn-Analyzers repos to delete all the stub MyCodeAction types once this goes in.

Should this really be done for roslyn-analyzers if Microsoft.CodeAnalysis packages isn't updated to latest?

[mavasani]

Should this really be done for roslyn-analyzers if Microsoft.CodeAnalysis packages isn't updated to latest?

I don’t believe anyone is actively looking at CodeFixer telemetry to be affected if we have a small intermediate period without useful telemetry. So I think we should be fine. Also note that the analyzer package itself doesn’t need to move to reference newer MS.CA packages as there are no public API changes here.

[CyrusNajmabadi]

Will this mean we can get rid of all our 'MyCideAction' subclasses?

[Youssef1313]

Will this mean we can get rid of all our 'MyCideAction' subclasses?

For 99.99% of the cases yes (per my understanding). The 0.01% are cases like this where the subclass does something useful:

roslyn/src/Features/CSharp/Portable/CodeRefactorings/EnableNullable/EnableNullableCodeRefactoringProvider.cs

Lines 255 to 276 in 1f2d474

	private sealed class MyCodeAction : CodeAction.SolutionChangeAction
	{
	private readonly Func<CodeActionPurpose, CancellationToken, Task<Solution>> _createChangedSolution;
	public MyCodeAction(Func<CodeActionPurpose, CancellationToken, Task<Solution>> createChangedSolution)
	: base(
	CSharpFeaturesResources.Enable_nullable_reference_types_in_project,
	cancellationToken => createChangedSolution(CodeActionPurpose.Apply, cancellationToken),
	nameof(CSharpFeaturesResources.Enable_nullable_reference_types_in_project))
	{
	_createChangedSolution = createChangedSolution;
	}
	protected override async Task<IEnumerable<CodeActionOperation>> ComputePreviewOperationsAsync(CancellationToken cancellationToken)
	{
	var changedSolution = await _createChangedSolution(CodeActionPurpose.Preview, cancellationToken).ConfigureAwait(false);
	if (changedSolution is null)
	return Array.Empty<CodeActionOperation>();
	return new CodeActionOperation[] { new ApplyChangesOperation(changedSolution) };
	}
	}

[mavasani]

Yes, agree with @Youssef1313

[CyrusNajmabadi]

Great. That makes me super happy.

[JoeRobich]

@mavasani Would you also be able to update the tool that generates a mapping table of CodeAction Provider to TelemetryId? https://github.com/mavasani/roslyn/tree/CodeActionTelemetry/src/Tools/BuildActionTelemetryTable

Nevermind, not sure how I missed it.

[jmarolf]

Thanks @mavasani! No issues with the PR, will just need to remember to keep both kinds of maps in our queries so we don't drop stuff from before this change goes in.

[mavasani]

Would you also be able to update the tool that generates a mapping table of CodeAction Provider to TelemetryId?

Actually, good observation. I have done partial fixes to that tool, but can get it fully working again only after I am able to cleanup and remove the MyCodeAction nested types. More changes to the tool are coming in the next PR.

[Youssef1313]

@mavasani There is an unrelated failure here. Can you restart it so the PR can be merged?