Update DynamicallyAccessedMembers annotation on EventSource

[MichalStrehovsky]

This takes advantage of #109814. This is in theory a breaking change in case someone took advantage of our annotation and is doing their own reflection on EventSource descendants.

Someone else reflecting on EventSource is problematic however. We placed around various suppressions due to our own annotations like this:

runtime/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs

Lines 414 to 424 in 0d62887

	[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2114:ReflectionToDynamicallyAccessedMembers",
	Justification = "EnsureDescriptorsInitialized's use of GetType preserves this method which " +
	"has dynamically accessed members requirements, but EnsureDescriptorsInitialized does not " +
	"access this member and is safe to call.")]
	public static string? GenerateManifest(
	[DynamicallyAccessedMembers(ManifestMemberTypes)]
	Type eventSourceType,
	string? assemblyPathToIncludeInManifest)
	{
	return GenerateManifest(eventSourceType, assemblyPathToIncludeInManifest, EventManifestOptions.None);
	}

It means that if someone else is reflection-accessing various members on EventSource, they would not get a trimming warning. Hopefully, nobody does that. This PR also assumes that nobody does that (and the PR should therefore not be breaking in practice).

Cc @eerhardt @dotnet/illink

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[dotnet-policy-service]

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

[eerhardt]

It means that if someone else is reflection-accessing various members on EventSource, they would not get a trimming warning.

Why wouldn't they get a trimming warning?

[MichalStrehovsky]

It means that if someone else is reflection-accessing various members on EventSource, they would not get a trimming warning.

Why wouldn't they get a trimming warning?

Because we suppressed them with the suppression I quoted. We annotate EventSource as "this is going to reflect on all methods" and we have a bunch of methods that should never be reflection-invoked. We suppressed it but the suppression will apply to anyone else taking advantage of the annotation because annotation doesn't discriminate.

[eerhardt]

It means that if someone else is reflection-accessing various members on EventSource, they would not get a trimming warning.

Why wouldn't they get a trimming warning?

Because we suppressed them with the suppression I quoted. We annotate EventSource as "this is going to reflect on all methods" and we have a bunch of methods that should never be reflection-invoked. We suppressed it but the suppression will apply to anyone else taking advantage of the annotation because annotation doesn't discriminate.

If someone else is reflection-accessing these specific methods (GenerateManifest, WriteEventCore, WriteEventWithRelatedActivityIdCore, WriteEvent(int, object[]), CreateManifestAndDescriptors) they won't get warnings. But that is the same as it is today with using .All, so this PR isn't making it worse.

The part that could be considered breaking is that someone could have been calling GetType() on some EventSource object and getting the properties/fields/etc from it. Before they wouldn't get a warning since we used .All on the whole class. Now we are only doing methods and nested types. But in this case they WILL get a warning.

[MichalStrehovsky]

The part that could be considered breaking is that someone could have been calling GetType() on some EventSource object and getting the properties/fields/etc from it. Before they wouldn't get a warning since we used .All on the whole class. Now we are only doing methods and nested types. But in this case they WILL get a warning.

Yep, what I meant is that we annotate this as reflected-on "for us", not for someone else to use. The suppressions are one of the indicators that this is very specifically meant for us. Unfortunately there's no annotation that could make this contractually private so someone could be still taking advantage of it.