Renamed EventSources "Microsoft-"->"Private.InternalDiagnostics.".

[ManickaP]

Unified naming conventions in existing networking EventSources to follow the namespace to which they belong, as well as correspond to the newly added telemetries.

Added suffix ".InternalDiagnostics" for old logging events to prevent them from being accidentally turned on with the new telemetry.

Existing EventSources that haven't been changed:

• "Microsoft-System-Net-Quic": shared with ASP.NET Core. I assume it cannot be willy-nilly changed without prior aggreement.
• "Microsoft-Extensions-DependencyInjection": out of networking scope.
• "Microsoft-Extensions-Logging": out of networking scope.
• "Microsoft-Diagnostics-DiagnosticSource": out of networking scope.

Contributes to #38754

Rational: the logging is useful for diagnostics, we shouldn't just delete it without proper replacement.
In 6.0 we can go through individual events and clean it. This is compromise that's doable in such short time.

[ghost]

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

[karelz]

@geoffkizer @scalablecory are you ok with the suffix 'InternalDiagnostics' as name?

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[geoffkizer]

This changes the naming to use '.' instead of '-'. Is that the common convention? Is that used elsewhere?

[geoffkizer]

What are the non-internal EventSources called?

I'm a little worried that this is renaming stuff unnecessarily. Some folks (e.g. ASP.NET perf test scripts) undoubtedly have familiarity with the existing names and are going to get tripped up here. Is the value in renaming worth this inconvenience?

[ManickaP]

This changes the naming to use '.' instead of '-'. Is that the common convention? Is that used elsewhere?

Also stripping "Microsoft" from the beginning. And yes, it is newer convention, it's used across libraries, e.g.: https://github.com/dotnet/runtime/blob/master/src/libraries/System.Data.OleDb/src/System/Data/Common/DataCommonEventSource.cs#L9, https://github.com/dotnet/runtime/blob/master/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/ParallelETWProvider.cs#L19, also our new telemetry.
Some background info from Noah Falk: #38754 (comment)

What are the non-internal EventSources called?

The same without the ".InternalDiagnostics", e.g.: "System.Net.Http"

I'm a little worried that this is renaming stuff unnecessarily. Some folks (e.g. ASP.NET perf test scripts) undoubtedly have familiarity with the existing names and are going to get tripped up here. Is the value in renaming worth this inconvenience?

This comes as a result of Stephen's complaint, that having two completely different event sources in a single lib is confusing: #38620 (comment)
Initially, the ask was just to delete existing tracing, because customers aren't using it anyway and it's too verbose. But that would leave us completely without ability to diagnose internal issues. I personally have been heavily relying on our tracing to debug issues.

Either way, we should reach consensus before tomorrow.

[geoffkizer]

Ok, I guess I see the value of renaming.

My suggestion would be to name these something completely different than the "official" event sources, so it's extremely clear that these are not intended for public use. Right now they look too official, as if they are just more detailed versions of the standard events.

Something like "InternalOnlyDiagnostics.Net.[Http|Sockets|etc]"

@stephentoub should add his thoughts as well, if he has a chance.

[geoffkizer]

@noahfalk might have thoughts as well.

[ManickaP]

Side note: I'm aware that these changes must be propagated to our diagnostic tests. I'll do that if/when we settle on the final name.

[tommcdon]

@noahfalk fyi

[danmoseley]

Please just a single area label at a time 🐱

[noahfalk]

I'm not sure I fully understand the plan. Is the idea that we would change the name now and then in the future delete these EventSources after migrating the events we cared about? I agree with @geoffkizer that I am unclear what value these particular name changes are giving us. The original premise, as I understood it, was that these old EventSources are completely unused except by a small number of internal Microsoft engineers. If external customers aren't aware of the current names nor will they be aware of the new names then I can't tell what benefit we are getting by changing them?

EDIT: Although I don't see the value in these particular name changes I also don't have any concerns other than it appears like unnecessary labor. If you feel the new names will aid you and we aren't going to advertise it to customers then go for it : )

[stephentoub]

I'm sad we're not able to do the "right thing" for this release. But if we're really out of time to move the few valuable events to the new sources and delete the old cruft, c'est la vie. My concern with not deleting and not renaming is we're going to be advertising the event sources much more than we did in the past (which was basically not at all), which is much more likely to create adoption of any sources that exist, at which point we'll have a harder time removing them in 6.0 (which I would still like us to do). At least having "internal" in the name suggests folks shouldn't depend on it.

[geoffkizer]

I'm not sure I fully understand the plan. Is the idea that we would change the name now and then in the future delete these EventSources after migrating the events we cared about?

Yes. And so we want names that convey "please don't rely on this as it's not intended for public use".

[geoffkizer]

Change looks good, pending final naming approach here.

I'm approving now so @ManickaP is unblocked on Monday, assuming we close on naming by then.

[noahfalk]

we're going to be advertising the event sources much more than we did in the past (which was basically not at all), which is much more likely to create adoption of any sources that exist

Yeah if we think customers are going to start discovering the existing EventSources despite our goal to not have customers depend on them, then picking a name that makes our intentions clear has some benefit. This might be one of the amusing times where picking a name that is long, awkward, and you feel embarrassed writing it in your code is exactly what we want : ) I think there are many potential names that fit that criteria but one example could be: Private.Obsolete.PlannedForDeletion.System.Net.Http

[danmoseley]

We have _DONT_USE_InternalThread as prior art in Thread.cs 😸

[geoffkizer]

How about "Private.Internal.System.Net.*"? That seems like it makes things pretty clear without going overboard here.

[geoffkizer]

Or "Private.InternalDiagnostics.System.Net.*", to maintain the "InternalDiagnostics" part from the original proposal.

[karelz]

I like @geoffkizer's last proposal "Private.InternalDiagnostics.System.Net"

[ManickaP]

I like @geoffkizer's last proposal "Private.InternalDiagnostics.System.Net"

Isn't Private and Internal kind of redundant. What about PrivateDiagnostics.System.Net.* or just InternalDiagnostics.System.Net.*? And we can document that events with "InternalDiagnostic" in a name are subject to change and to not to depend on them.

[geoffkizer]

Isn't Private and Internal kind of redundant.

Yeah, it's a bit redundant, but that's sort of the point. We want to ensure that it's super clear that these are not public, supported events.

And we can document that events with "InternalDiagnostic" in a name are subject to change and to not to depend on them.

We should certainly document this, but we can't assume users will read docs. Most don't :)