-
Notifications
You must be signed in to change notification settings - Fork 386
[dotnet-trace][CollectLinux] Add capability to probe .NET processes for UserEvents IPC Command Support #5657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
I'm going through some refactoring to make things cleaner, and I'm planning on utilizing the |
3451aa3 to
6dd3210
Compare
af59bda to
8312e6c
Compare
| bool supports = ProcessSupportsUserEventsIpcCommand(args.ProcessId, args.Name, out int resolvedPid, out string resolvedName, out string detectedRuntimeVersion); | ||
| BuildProcessSupportCsv(resolvedPid, resolvedName, supports, supportedCsv, unsupportedCsv); | ||
|
|
||
| Console.WriteLine($"Does .NET process '{resolvedName} ({resolvedPid})' support the EventPipe UserEvents IPC command used by collect-linux?"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we output to console, maybe this should be a little more user oriented than true/false, Similar to what you output when looking at all processes, something like:
".NET process '{resolvedName} ({resolvedPid})' supports the EventPipe UserEvents IPC command used by collect-linux"
and
".NET process '{resolvedName} ({resolvedPid})' do NOT support the EventPipe UserEvents IPC command used by collect-linux"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had considered that, but I figured that if a user chose to automate dotnet-trace collect-linux --probe using console output instead of the output file, the true/false would be simpler to identify. And from a quick readability standpoint it would be easier to look for a beginning true/false than looking in the middle of the output message for support.
And it seemed awkward to have
.NET process '{resolvedName} ({resolvedPid})' supports the EventPipe UserEvents IPC command used by collect-linux
true.NET process '{resolvedName} ({resolvedPid})' does NOT support the EventPipe UserEvents IPC command used by collect-linux
false^ might be misunderstood as a double negative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should have just csv output to console for the automation scenario and then format the regular output as if a human would read it, so in that case we shouldn't do the true/false output and if -o stdout for example, it will just dump the csv content to console, better suited for tooling support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, I previously misread the suggestion as just emitting csv output as normal console output.
instead, its more like this?
collect-linux --probe -> human readable console output
collect-linux --probe -o MyFile -> human readable console output + csv file
collect-linux --probe -o stdout -> csv console output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jupp, just made #5657 (comment) that is inline with that with one exception, maybe the -o MyFile should only have the logging that we created the csv file since its mainly for automation scenarios as well.
collect-linux --probe -> human readable console output
collect-linux --probe -o MyFile -> only message that csv file has been written, success/fail
collect-linux --probe -o stdout -> csv console output
| Console.WriteLine(unsupportedProcesses.ToString()); | ||
| } | ||
|
|
||
| if (generateCsv) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have an option to generate the csv output directly into the console/stdout as well, could be an effective way to do get structured output without the need to go over an additional file. In that case we should only output the csv data and no other console output from the command making it straight forward to parse from a tool running dotnet-trace and capture stdout output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. In the recent update, I added diagnostics like Required runtime: 10.0.0. Detected runtime: 9.0.11 for the single process probe, and in the multi process probe, I added Detected runtime: <version> to the ones that do not support. That diagnostic seems more useful from a manual usage standpoint compared to automation, where I'm guessing all that matters is PID - true/false?
Should we remove that hint about why the tool determined a particular .NET process didn't support the IPC command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should output more diagnostics in the case there is a user looking at the data, but if we run using -o stdout or similar, then we should only dump the raw content of the csv into the console without any additional information. Maybe we should have the -o as the switch indicating that we should have minimal console output since its probably intended for automation instead of user facing output. If we then add -o stdout as well, it would just dump the csv as is directly to the console without any additional logging.
So lets say just running --probe, we could have full nice output intended for customer facing output. If we have -o file.csv, then we would just log that we written the file and if we use -o stdout we would only dump the csv content directly to the console.
This PR gives
dotnet-trace collect-linuxthe option--probeto display which .NET processes are capable of parsing an EventPipe UserEvents IPC Command without collecting a trace. The option is compatible with-p|--process-idand-n|--nameto probe a single .NET process. It is also compatible with-o|--outputto generate a csv file ordered with supported processes first followed by unsupported in the formatpid,processName,supportsCollectLinux(e.g.1234,MyApp,true).This also changes the behavior of
collect-linuxwhen tracing a single process from a silent failure to erroring with a message indicating that the single .NET process' runtime is not > 10.0.0.Probe option
Example outputs
One MyApp on .NET 8 that doesn't support userevents and one MyApp on .NET 10 that does support userevents
Generated csv
Targeting a single process
Targeting a single process that doesn't support the EventPipe UserEvents IPC command used by collect-linux