Extend PredefinedDynamicTraceEventParser to Support Dynamic Events from EventPipeEventSource

[Copilot]

This PR extends PredefinedDynamicTraceEventParser to support dynamic events from EventPipeEventSource in addition to ETWTraceEventSource. It also updates UniversalEventsTraceEventParser to use this functionality because it follows the self-describing pattern of not using fixed event IDs.

Fixes #2231.

[brianrob]

@copilot, this code looks correct. I'd now like for you to update UniversalEventsTraceEventParser to inherit from PredefinedDynamicTraceEventParser. Be sure to update each of the events as well. The event payloads are the same and don't need to be changed, but the events need to be converted to the pattern that PredefinedDynamicTraceEventParser follows.

[Copilot]

@copilot, this code looks correct. I'd now like for you to update UniversalEventsTraceEventParser to inherit from PredefinedDynamicTraceEventParser. Be sure to update each of the events as well....

I've updated UniversalEventsTraceEventParser to inherit from PredefinedDynamicTraceEventParser and converted the events to follow the PredefinedDynamicTraceEventParser pattern. The changes include:

1. Changed inheritance from TraceEventParser to PredefinedDynamicTraceEventParser
2. Created CpuSampleEvent and CswitchSampleEvent classes that inherit from PredefinedDynamicEvent
3. Registered the event templates in the constructor using RegisterTemplate()
4. Removed the custom EnumerateTemplates method to use the base class implementation
5. Maintained backward compatibility by keeping the existing event signatures with SampleTraceData

The implementation provides both dynamic event support (for EventPipe sources) and backward compatibility for existing code. Commit: 87a570a

[brianrob]

@copilot please update the cpu and cswitch events in UniversalEventsTraceEventParser to properly use the registered dynamic events.

[Copilot]

@copilot please update the cpu and cswitch events in UniversalEventsTraceEventParser to properly use the registered dynamic events.

I've updated the cpu and cswitch events in UniversalEventsTraceEventParser to properly use the registered dynamic events. The changes include:

1. Changed event signatures from Action<SampleTraceData> to Action<CpuSampleEvent> and Action<CswitchSampleEvent>
2. Updated event implementation to use AddCallbackForEvent<T>() and RemoveCallback<T>() instead of manual template registration
3. Updated consuming code in PerfViewData.cs and NettraceUniversalConverter.cs to use the new event types

The events now properly leverage the PredefinedDynamicTraceEventParser infrastructure and work with the registered dynamic event templates. Commit: 48c23bd

[brianrob]

There is a more structural problem here that I hadn't appreciated. PredefinedDynamicTraceEventParser has an implementation that handles unknown events, and so does ExternalTraceEventParser. I think we should make PredefinedDynamicTraceEventParser inherit from ExternalTraceEventParser and that way it can use the implementation for unknown events in ExternalTraceEventParser. This will also require changing the implementation of ExternalTraceEventParser - specifically the implementation of delegate passed to RegisterUnhandledEvent so that it can find both events in the ExternalTraceEventParserState and those in PredefinedDynamicTraceEventParser. I think that we should be able to merge their implementations into ExternalTraceEventParser instead of having a separate implementation in PredefinedDynamicTraceEventParser.

[Copilot]

I've refactored the architecture as requested. Changes made:

1. Changed inheritance: PredefinedDynamicTraceEventParser now inherits from ExternalTraceEventParser instead of TraceEventParser
2. Implemented TryLookup: Replaced the custom OnUnhandledEvent handler with the standard TryLookup method
3. Eliminated duplication: Removed the separate unhandled event registration logic
4. Unified implementation: Both ETW and EventPipe event metadata parsing now use the same ExternalTraceEventParser framework

The implementations are now merged - ExternalTraceEventParser handles the unhandled event registration, and PredefinedDynamicTraceEventParser.TryLookup provides the predefined dynamic event lookup. This eliminates the structural problem while maintaining all existing functionality.

Commit: 618992c

[brianrob]

@copilot, this file contains a standalone repro test case that can be built and run, passing in an argument with the path to repro.nettrace.

Current Behavior: If an instance of DynamicParser is used to parse an event, then the event is not properly dispatched to the UniversalEventsTraceEventParser. For this repro case, the CPU events are written to the console with the prefix "Dynamic Parser:".

Expected Behavior: I expect the events to be dispatched by both the DynamicTraceEventParser and the UniversalEventsTraceEventParser if they both register to receive the event. Thus for cpu events, I expect to see console output that is prefixed by Dynamic Parser: and then on a separate line CPU:. Which of these prefixes shows up first does not matter.

Given that this is not working, can you please build and run the test case, confirm that is not working, and see if you can develop a fix.

[Copilot]

I've confirmed the issue and identified the root cause. The problem occurs when both DynamicTraceEventParser and UniversalEventsTraceEventParser register handlers for the same event - only one parser receives the events due to template registration conflicts.

Key Findings:

• ✅ UniversalEventsTraceEventParser works correctly when used alone
• ✅ Both parsers work when handling different events
• ❌ When both handle the same event, only the first-registered parser gets events

Current Status:

I've made partial fixes to the template registration mechanism, but the core issue with multiple parsers handling identical events still needs to be resolved. The template chaining logic exists but appears to have dispatch issues.

Working on a complete solution to ensure both parsers can receive the same events simultaneously. Commit: 37104ad

[brianrob]

Copilot Post-Mortem:

• Copilot did a good job getting started initially, but ultimately was not able to produce code that worked.
• I believe a big issue here is that I was not able to adequately coach it. Copilot was able to produce code that worked with a source type of EventPipeEventSource, but it did not adequately understand how things would work when the input source type is TraceLog, and I could not adequately coach it because I didn't realize exactly how DynamicTraceEventParser works with a TraceLog as input.
• It will be interesting to keep track of the types of issues that copilot can solve without the user being able to adequately coach it towards a solution - this is certainly one that it could not, and I had to fallback to writing and testing the code myself.
• I tried a new way to coach copilot in this PR, which is to check-in a repro test case that doesn't work and ask it to solve it. It didn't work this time, but it did acknowledge that it hadn't fully fixed it. I intend to try this again with future work.

[marklio]

Looks good. I have a preference for new C# features that reduce the boilerplate on alot of this, but this seems like it should work.

[brianrob]

Looks good. I have a preference for new C# features that reduce the boilerplate on alot of this, but this seems like it should work.

Thanks much! I was thinking I might do an experiment to bump the language version and see what kinds of readability improvements we can make.