Merge new Telemetry logging with existing NetEventSources

[alnikola]

It appears that new Telemetry logging/tracing does generally the same thing as existing NetEventSources. We need to consider merge them to improve Telemetry's user experience.

See dicsussions:

• discussion_r447776811
• issuecomment-639164431

[ghost]

Tagging subscribers to this area: @dotnet/ncl Notify danmosemsft if you want to be subscribed.

[noahfalk]

A little background on this area. To the best of my knowledge Vance kicked off the pattern of creating new EventSources when he proposed creating "System.Runtime" EventSource in addition to the "Microsoft-Windows-DotNetRuntime" EventSource we've had for a long time. I assume he put some thought into it but I never heard what the rationale was. Personally I have mixed feelings about these renames. The biggest advantages I see are:

1. Branding - we are trying to push .Net Core as crossplatform so tutorials/command-lines that refer to event sources with "Windows" in the name conflict with that message.
2. Discoverability - I expect it is easier to find EventSources that reuse namespace names vs. EventSources that use "Microsoft-Windows-something"
3. Consistency? - Now that we already made new EventSources in some places we could say we are being consistent with our (recent) past decisions. (Of course we're also being inconsistent with our further past decisions so this can cut both ways)

Cons:

1. Confusion - Users who are aware of multiple event sources now need to pick between them or understand which functionality is where.
2. Maintainance - Now we have two EventSources and have to maintain both
3. Performance - There is a small startup overhead to initialize a new EventSource and check if it is enabled

Below is the guidance I suggested to @davidsh when he asked earlier. It was my best attempt to rationalize where we are:

1. If we have brand new scenarios that can stand alone (such as counters), make an EventSource named identical to the type/namespace with dots as a naming separator. The move to System.Runtime and Microsoft.AspNetCore.Hosting was deliberate.
2. If we are adding new events and those events are clearly related to existing events on an existing provider, or the scenarios that would use the new events would also need to use existing events on the existing provider, then add the new events to the existing provider.
3. Don't rename existing EventSources or existing events, that will cause compat headaches that far outweigh any benefits we'd get from the naming.

Given some time to do a diff of events on old and proposed new EventSource types I might be able to offer a more tailored suggestion but wanted to share where I am at so far. Hope it helps.

[alnikola]

@noahfalk

1. Counters is only part of the new telemetry we are implementing. It will also introduce activities which in the nutshell track the same events as the original NetEventSource.Enter/Exit, so if we leaved the original Enter/Exit logging in place, it would lead to a duplication.
2. Our event logging is supposed to work on different platforms, but as you mentioned above existing events has "Windows" in their namespaces which in my opinion will look quite confusing for people using .NET Core on Linux and other platforms.
3. It's believed that almost nobody is actually listening to current NetEventSource events, so keeping them around will lead to a confusion because there is no clear distinction in the use cases between the old ones and the new telemetry.

[noahfalk]

It's believed that almost nobody is actually listening to current NetEventSource events

Are we confident enough in that assessment that there is a plan to remove the pre-existing code? My impression from the original issue is that the question was whether or not to merge the new EventSource into the old one. If we are instead contemplating removing the old one entirely that shifts the discussion a bit : )

as you mentioned above existing events has "Windows" in their namespaces

FWIW I checked and the existing sockets EventSource doesn't have this particular problem. It is named "Microsoft-System-Net-Sockets."

Thanks!

[alnikola]

"Merge" here means - "merge old NetEventSources into the new telemetry and delete unnecessary parts".

Even if we decide to keep the old one around, we must delete some events (like Enter/Exit in the PR mentioned above) because they are duplicates to the new Start/Stop. That means the existing code must be changed anyways.

Or, could suggest any way on how to easily and unambiguously describe to the users the difference between the old logging and new telemetry in the documentation? For me the main driver to delete the old one is an absence of clear distinction between those two leading to confusion.

[noahfalk]

Can we back up to "It's believed that almost nobody is actually listening to current NetEventSource events"? I don't intend to contradict but I want to learn what evidence we have behind it. The level of confidence makes a big difference in how much design freedom we have. Thanks!

[stephentoub]

Super noisy and terribly inconsistent events that make them close to useless for any real investigation.

A total of one hit (outside of dotnet/runtime and clones) on all of GitHub for the event source names and guids, and that one being in a tool that's just logging all networking event sources.

That same one hit via grep.app.

Zero discussions with developers about the consumption of these events.

Zero docs outside of dotnet/runtime about these events.

Different source names/guids from netfx, so any use there is irrelevant.

[stephentoub]

(To be clear, I'm fine keeping the existing sources, deleting the useless stuff, and adding new stuff to them. What I want to avoid is splitting the events effectively arbitrarily across two sources. It's confusing and pointless from a consumer perspective.)

[alnikola]

@stephentoub Could you please clarify whether you suggest to keep the old types? I believed we decided to delete them and move valuable things to the new telemetry. Am I wrong?

[stephentoub]

I don't have a strong opinion on which ones to keep. I do care that:

• We only have one per assembly.
• We get rid of useless events.
• The tracing is useful and consistent, and follows best practices.
• The above happens for .NET 5.

[alnikola]

I see. Taking into account the above comment, I still believe we should delete old NetEventSources.

[noahfalk]

Thanks! Yeah that seems like pretty good evidence to me that the existing EventSource won't be missed : ) I also took a look through the events and saw that most of the usage was treating EventSource as an unstructured logging provider which would make it much harder for a customer to have created a useful automated consumption scenario.

In that case we could delete the old provider and we've got freedom to name the new provider as we please + put whatever events in it we believe are useful. Are there any events you'd plan to preserve from the old provider right away? I also think it would be fine to eliminate all the old events since they haven't proven historically valuable and consider restoring them in the new EventSource in the future if/when a clear need is identified.

[noahfalk]

@brianrob - does this plan to eliminate the pre-existing networking EventSource provider raise any concerns for you?

[alnikola]

Our plan is to firstly implement new event/activities and then go through the old ones to delete/move them. However, some pieces have been already deleted. In example @stephentoub helped us recently by deleting obviously redundant Enter/Exits.

[noahfalk]

Probably the sooner you can delete the old stuff the better so that you can flush out any issues that might be lurking. I imagine @stephentoub would be bummed if we added the new EventSource and then later discovered that we couldn't delete the old ones for some unexpected reason.

[brianrob]

No concerns on removing this one. I am not aware of any tools out there that use it, including PerfView. Thanks for checking.

[karelz]

Moving remaining work to 6.0.