Add headless MCP server for AI-driven trace analysis#37
Add headless MCP server for AI-driven trace analysis#37jeffbromberger wants to merge 7 commits intomainfrom
Conversation
Add ProfileExplorer.McpServer - a standalone headless MCP server that calls ProfileExplorerCore directly without WPF dependencies. Provides 8 tools: process enumeration, async trace loading with multi-process selection, function/binary listing, instruction-level assembly with Capstone disassembly, source line mapping, inline function info, and caller/callee analysis. Add public methods to ETWProfileDataProvider for MCP consumption: GetDebugInfoForModule, GetDebugInfoForFunction, DisassembleFunctionAsync. Adjust for CPU IP skid in GetFunctionAssembly: sampled instruction pointers often land on the instruction after the one that was executing. Shift each sample to the preceding valid instruction offset using the disassembly map, matching the UI's TryFindElementForOffset behavior. Gate the existing embedded MCP server (ProfileExplorer.Mcp) behind a --mcp command-line flag so it no longer starts unconditionally with the UI. Update ARCHITECTURE.md to document both MCP servers and their differences. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a standalone headless MCP (Model Context Protocol) server that exposes Profile Explorer's ETL trace analysis capabilities to AI assistants, enabling programmatic access to profiling data without requiring the WPF UI. The PR also gates the existing embedded MCP server behind a --mcp command-line flag to prevent it from starting unconditionally.
Changes:
- Added new
ProfileExplorer.McpServerconsole application (.NET 8) with 8 MCP tools for trace analysis, including process enumeration, function profiling, disassembly with CPU IP skid correction, and caller/callee relationships - Extended
ETWProfileDataProviderwith three public methods (GetDebugInfoForModule,GetDebugInfoForFunction,DisassembleFunctionAsync) to support headless disassembly and source line resolution - Modified
App.xaml.csto only start embedded MCP server when--mcpflag is passed, avoiding unconditional startup - Added comprehensive
ARCHITECTURE.mddocumentation covering module structure, data flow, and dual-MCP architecture
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ProfileExplorerUI/App.xaml.cs | Gates embedded MCP server startup behind --mcp command-line flag |
| src/ProfileExplorerCore/Profile/ETW/ETWProfileDataProvider.cs | Adds three public methods to expose debug info and disassembly capabilities for headless MCP |
| src/ProfileExplorer.McpServer/Program.cs | New headless MCP server with 8 tools for trace analysis, including async trace loading and IP skid correction |
| src/ProfileExplorer.McpServer/ProfileExplorer.McpServer.csproj | New .NET 8 console project definition with MCP and hosting dependencies |
| ARCHITECTURE.md | Comprehensive project documentation covering architecture, data flow, MCP comparison, and repository structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var cancelTask3 = new CancelableTask(); | ||
| var profile = await provider.LoadTraceAsync( | ||
| profileFilePath, | ||
| processIds, | ||
| options, | ||
| symbolSettings, | ||
| report, | ||
| progress => { Trace.WriteLine($"Load progress: {progress}"); }, | ||
| cancelTask3); |
There was a problem hiding this comment.
The cancelTask3 instance is not disposed, which causes a resource leak. Since CancelableTask implements IDisposable and manages a ManualResetEvent and CancellationTokenSource, it must be disposed to prevent resource leaks. Wrap it in a using statement.
| public static ProfileData? LoadedProfile { get; set; } | ||
| public static ETWProfileDataProvider? Provider { get; set; } | ||
| public static SymbolFileSourceSettings? SymbolSettings { get; set; } | ||
| public static string? LoadedFilePath { get; set; } | ||
| public static string? LoadedProcessName { get; set; } | ||
| public static List<int> LoadedProcessIds { get; set; } = new(); | ||
| public static TimeSpan TotalWeight { get; set; } | ||
|
|
||
| // Async loading state | ||
| public static Task<ProfileData?>? PendingLoad { get; set; } | ||
| public static string? PendingFilePath { get; set; } | ||
| public static string? PendingProcessId { get; set; } | ||
| public static Exception? LoadException { get; set; } |
There was a problem hiding this comment.
The ProfileSession static class is accessed concurrently by multiple tool methods without any synchronization mechanism. If MCP tool calls can be executed in parallel, this could lead to race conditions when reading and writing shared state like LoadedProfile, PendingLoad, and LoadException. Consider adding lock-based synchronization or ensure that the MCP framework serializes tool calls.
| public static ProfileData? LoadedProfile { get; set; } | |
| public static ETWProfileDataProvider? Provider { get; set; } | |
| public static SymbolFileSourceSettings? SymbolSettings { get; set; } | |
| public static string? LoadedFilePath { get; set; } | |
| public static string? LoadedProcessName { get; set; } | |
| public static List<int> LoadedProcessIds { get; set; } = new(); | |
| public static TimeSpan TotalWeight { get; set; } | |
| // Async loading state | |
| public static Task<ProfileData?>? PendingLoad { get; set; } | |
| public static string? PendingFilePath { get; set; } | |
| public static string? PendingProcessId { get; set; } | |
| public static Exception? LoadException { get; set; } | |
| // Synchronization root for all shared session state. | |
| private static readonly object _syncRoot = new(); | |
| // Exposed in case external callers need to coordinate with the same lock. | |
| public static object SyncRoot => _syncRoot; | |
| private static ProfileData? _loadedProfile; | |
| public static ProfileData? LoadedProfile | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _loadedProfile; | |
| } | |
| } | |
| set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _loadedProfile = value; | |
| } | |
| } | |
| } | |
| private static ETWProfileDataProvider? _provider; | |
| public static ETWProfileDataProvider? Provider | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _provider; | |
| } | |
| } | |
| set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _provider = value; | |
| } | |
| } | |
| } | |
| private static SymbolFileSourceSettings? _symbolSettings; | |
| public static SymbolFileSourceSettings? SymbolSettings | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _symbolSettings; | |
| } | |
| } | |
| set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _symbolSettings = value; | |
| } | |
| } | |
| } | |
| private static string? _loadedFilePath; | |
| public static string? LoadedFilePath | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _loadedFilePath; | |
| } | |
| } | |
| set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _loadedFilePath = value; | |
| } | |
| } | |
| } | |
| private static string? _loadedProcessName; | |
| public static string? LoadedProcessName | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _loadedProcessName; | |
| } | |
| } | |
| set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _loadedProcessName = value; | |
| } | |
| } | |
| } | |
| private static readonly List<int> _loadedProcessIds = new(); | |
| public static List<int> LoadedProcessIds | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _loadedProcessIds; | |
| } | |
| } | |
| } | |
| private static TimeSpan _totalWeight; | |
| public static TimeSpan TotalWeight | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _totalWeight; | |
| } | |
| } | |
| set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _totalWeight = value; | |
| } | |
| } | |
| } | |
| // Async loading state | |
| private static Task<ProfileData?>? _pendingLoad; | |
| public static Task<ProfileData?>? PendingLoad | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _pendingLoad; | |
| } | |
| } | |
| set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _pendingLoad = value; | |
| } | |
| } | |
| } | |
| private static string? _pendingFilePath; | |
| public static string? PendingFilePath | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _pendingFilePath; | |
| } | |
| } | |
| set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _pendingFilePath = value; | |
| } | |
| } | |
| } | |
| private static string? _pendingProcessId; | |
| public static string? PendingProcessId | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _pendingProcessId; | |
| } | |
| } | |
| set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _pendingProcessId = value; | |
| } | |
| } | |
| } | |
| private static Exception? _loadException; | |
| public static Exception? LoadException | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _loadException; | |
| } | |
| } | |
| set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _loadException = value; | |
| } | |
| } | |
| } |
| long[]? sortedOffsets = null; | ||
| if (disasmMap != null && data.InstructionWeight != null) | ||
| { | ||
| sortedOffsets = disasmMap.Keys.OrderBy(k => k).ToArray(); |
There was a problem hiding this comment.
The variable sortedOffsets is assigned on line 465 but never used. This appears to be dead code that should be removed to improve maintainability.
| long[]? sortedOffsets = null; | |
| if (disasmMap != null && data.InstructionWeight != null) | |
| { | |
| sortedOffsets = disasmMap.Keys.OrderBy(k => k).ToArray(); | |
| if (disasmMap != null && data.InstructionWeight != null) | |
| { |
|
|
||
| ProfileSession.TotalWeight = totalWeight; | ||
| ProfileSession.LoadedProfile = profile; | ||
| ProfileSession.LoadedFilePath = profileFilePath; |
There was a problem hiding this comment.
When a new trace is loaded via OpenTrace, a new ETWProfileDataProvider instance is created and stored in ProfileSession.Provider. However, if a previous provider exists, it is not disposed before being replaced. Since ETWProfileDataProvider implements IDisposable, this creates a resource leak. The previous provider should be disposed before assigning the new one.
| ProfileSession.LoadedFilePath = profileFilePath; | |
| ProfileSession.LoadedFilePath = profileFilePath; | |
| var oldProvider = ProfileSession.Provider as IDisposable; | |
| if (!ReferenceEquals(oldProvider, provider)) | |
| { | |
| oldProvider?.Dispose(); | |
| } |
| public static ETWProfileDataProvider? Provider { get; set; } | ||
| public static SymbolFileSourceSettings? SymbolSettings { get; set; } | ||
| public static string? LoadedFilePath { get; set; } | ||
| public static string? LoadedProcessName { get; set; } |
There was a problem hiding this comment.
The LoadedProcessName field is declared but never assigned or read anywhere in the code. This appears to be dead code that should be removed to improve maintainability, or if it was intended to be used, it should be populated during trace loading.
| public static string? LoadedProcessName { get; set; } |
- Wrap CancelableTask in using statement to prevent resource leak - Dispose previous ETWProfileDataProvider when loading a new trace - Remove unused sortedOffsets variable (dead code) - Remove unused LoadedProcessName property (dead code) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add ClearResolvedCache() to PDBDebugInfoProvider and PEBinaryInfoProvider to clear static symbol/binary caches between trace loads - Add ProfileSession.Reset() that clears all session state and caches - Add SemaphoreSlim concurrency guard to prevent overlapping OpenTrace calls - Add CloseTrace tool to abandon stuck loads and reset state - Force-enable DiagnosticLogger (always-on for headless MCP server) - Add [MCP] tagged DiagnosticLogger calls for tool invocations, params, process matching, symbol path setup, task lifecycle, and errors - Surface per-module symbol resolution report in GetTraceLoadStatus (SymbolsLoaded, BinaryState, PdbPath, BinaryPath per module) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GetFunctionCallerCallee now returns both InclusivePct (percentage of total trace time) and FunctionPct (percentage of this function's time). FunctionPct matches the GUI's drill-down behavior where percentages are relative to the selected function, not the entire trace. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ResolveFunctionName helpers look up FunctionDebugInfo.Name from profile data instead of using IRTextFunction.Name (which is frozen as a hex placeholder during ETL parsing). FindFunction now searches by resolved PDB name first. All display points updated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…logging Document new CloseTrace tool, expanded ProfileSession fields, ResolveFunctionName helpers, FunctionPct vs WeightPct distinction, diagnostic logging, custom symbol path support, and updated tool count from 8 to 9. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| | **ProfileExplorerUI** | WPF app (.NET 8) | Main desktop application. All UI panels, session management, profiling views, scripting, VS extension integration. | | ||
| | **ProfileExplorerCore** | Class library | UI-independent engine: ETW trace loading, profile data model, call tree construction, IR parsing, binary analysis, symbol resolution, Capstone disassembly, Tree-sitter parsing, graph algorithms. | | ||
| | **ProfileExplorer.McpServer** | Console app (.NET 8) | Headless MCP server exposing profiling tools over stdio. Uses `ProfileExplorerCore` directly — no UI dependency. **This is the active MCP server.** | | ||
| | **ProfileExplorer.Mcp** | Class library + mock exe | Original MCP server designed to be embedded inside the WPF app via `IMcpActionExecutor`. Not used by the headless server. See [MCP: Embedded vs Headless](#mcp-embedded-vs-headless) below. | |
There was a problem hiding this comment.
This is super confusing. It was confusing enough already with 2 MCP servers, but now we have 3. We should either rename "ProfileExplorer.McpServer" to like a test server OR just have unit tests or special mode. I think when we are done I would like just 2 MCP servers (GUI + headless console) + MCP tests (in some form)
There was a problem hiding this comment.
I was unaware of a 3rd? I see the original ProfileExplorer.Mcp and the new ProfileExplorer.McpServer. What are you considering to be the 3rd one? This arch was generated by copilot so it may not be accurately describing something as well.
There was a problem hiding this comment.
The UI is the 3rd one (that is the only existing one that works). I recommend just 2 MCP servers - your headless one and UI
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="ModelContextProtocol" Version="0.3.0-preview.4" /> |
There was a problem hiding this comment.
I'm wondering if we want to bump to the latest version?
There was a problem hiding this comment.
I bumped it on this latest commit
- Revert --mcp flag gate: embedded MCP server starts unconditionally again (per ivberg) - Clarify ARCHITECTURE.md: 2 MCP servers (embedded UI + headless console), not 3 - Bump ModelContextProtocol NuGet package from 0.3.0-preview.4 to 1.1.0 (stable) in both ProfileExplorer.McpServer and ProfileExplorer.Mcp projects (per trgibeau) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Adds a standalone headless MCP (Model Context Protocol) server that exposes Profile Explorer's ETL analysis capabilities to AI assistants like GitHub Copilot. Also gates the existing embedded MCP behind a
--mcpflag so it doesn't start unconditionally.What's included
New:
ProfileExplorer.McpServer(headless console app)A
net8.0console application that loads ETL traces viaProfileExplorerCoredirectly — no WPF dependency, no UI. Exposes 8 MCP tools:OpenTraceCloseTraceGetAvailableProcessesGetAvailableBinariesGetAvailableFunctionsGetFunctionAssemblyGetFunctionCallerCalleeGetTraceLoadStatusGetHelpKey features
TryFindElementForOffsetbehavior exactlyFindSourceLineByRVAper instruction instead of function-level lookupsymbolPathparameter onOpenTracefor private/custom PDB pathsInclusivePct/WeightPct) and function-relative (FunctionPct) percentages, matching the GUI's drill-down behaviorResolveFunctionNamehelpers look upFunctionDebugInfo.Namefrom profile data, so functions display their PDB names (e.g.,ExpWaitForSpinLockSharedAndAcquire) instead of hex placeholders (28BC63)CloseTracefully resets session state including symbol caches; concurrency guard prevents overlapping loadsModified: Embedded MCP gated behind
--mcpApp.xaml.csnow only starts the embeddedProfileExplorer.Mcpserver when--mcpis passed on the command line.New:
ARCHITECTURE.mdComprehensive project documentation covering module structure, data flow, and the dual-MCP architecture (embedded vs headless).
Modified:
ETWProfileDataProvider.csAdded public methods to expose debug info and disassembly capabilities needed by the headless MCP:
GetDebugInfoForModule()GetDebugInfoForFunction()DisassembleFunctionAsync()Modified:
PDBDebugInfoProvider.csAdded
ClearResolvedCache()to allow resetting the static symbol resolution cache between trace loads.Commits
CloseTracetool, session reset, diagnostic logging, concurrency guardResolveFunctionNamehelpers,FindFunctionsearches by PDB name firstTesting
Validated against multi-process diskspd ETL traces with custom kernel symbols (
ntoskrnl_landy.exe):