Diagnostics: dotnet-trace spec alignment

[noahfalk]

Discrepancies between the implementation of dotnet-trace and the spec need to be resolved. Before we ship our demo I think we should discuss and address each item by either updating the spec, changing the tool, or filing an issue to track making a planned change before we ship 3.0. I've listed discrepancies I noticed below and a strawman suggestion for how to handle each one.

• [x] endpoints verb - exists in implementation, does not exist in the spec
  Suggestion: Remove for preview5 and suggest users use existing OS process listing tools. Create an issue to design and implement a consistent way of handing this use-case across all of our dotnet-diagnostic tools to be enabled later.

• [x] profiles verb - exists in implementation, does not exist in the spec
  Suggestion: Remove this and use help text for the --profile option to document the choices.

• [x] convert verb - exists in spec, not in the implementation
  Suggestion: File an issue to implement later

• [x] pack verb - exists in the spec, not in the implementation
  Suggestion: Move it into the future consideration portion of the spec

• [x] collect interactive command to hit 'g' - exists in spec, not in the implementation
  Suggestion: Move it into the future consideration portion of the spec

• [x] collect --format help text lists netperf as default format always, spec lists speedscope as default format on Linux
  Suggestion: Make netperf the default format always (requires both implementation and spec change). Since I know this will appear to make the Linux scenario less pleasant, a little justification... A major marketing bullet for why a customer would use EventPipe technology vs LTTNG or ETW is that it operates consistently across platforms. Selecting platform specific defaults undercuts that message. Although it may not be as convenient in the short term, assume that over time more tooling for netperf will be made xplat so that we do have a consistent xplat ecosystem.

[Edit] Made bullet points to track progress of individual items :)

[noahfalk]

cc @josalem

[noahfalk]

Also if you guys know of any other discrepancies please add them, that was just what I spotted.

[jorive]

I will suggest keeping points 1. and 2., and update the spec doc.
The features are in place, and make the experience friendlier. The tools is a preview version anyways, and we can modify/remove features based on suggestions.
For example,

• Running dotnet-trace endpoints to list the available ports is more convenient than the two different instructions for two different platforms:
  
  
  
  • On Windows, right click on the task bar, find the "dotnet" process to connect, and get its pid.
  
  
  • On Linux, users can run pidof dotnet.
  
  
• The verb "profile" feels like a better options to expand detailed information about the pre-baked profile options provided by the tool, and dotnet-trace collect --help can show the options to pick.

[josalem]

1. I think that the dotnet-trace enpoints experience is a lot nicer than having more manual instructions for every platform. @noahfalk, is the concern that we _may_ add a more global solution to this scenario that would make this feature redundant but still require support? If that's the case, is there a way we can simply pull that code out into a shared library so that every tool can simply implement the same verb? That way dotnet-collect or dotnet-futureDiagTool could very easily have the exact same verb with the same semantics. It would make consistency in experience easy while maintaining separate tools.

2. I think the profiles verb is better suited for a flag on either the raw tool invocation or the collect verb. I think having a --list-profiles would be a fair compromise here. No need to hunt through the help text, with an obvious listing mechanism. Similarly, dotnet-collect could have a --list-counters. If we decide on a consistent experience across the tools, I think that would solve the problem.

3. Agreed, an issue should track its inclusion. Can be assigned to me 😄

4. Agreed.

5. Agreed, though I'm not sure what the purpose of this feature will be until there are IPC protocol commands other than "start" and "stop" streaming.

6. The help text actually changes based on what platform you run it on. On non-windows platofrms, the help text says speedscope (or at least it's supposed to). Right now the netperf file is still created and is left on disk next to the speedscope file on non-windows platforms. Once the convert verb is implemented, I think it would be fair to make netperf the default if you want consistency. I, however, think we should make sure that dotnet-trace is immediately usable on non-windows platforms rather than setting up an implicit promise that netperf is getting xplat support. I do see your point, though, so I can modify the code when we get convert added.

[jorive]

Once we sync and get to an agreement I'll send a PR to make the tool __Preview5 Ready__ :)

[noahfalk]

1. If we want to keep something with 'endpoints'-like functionality then I propose we do this asap:
   a - Rename it to "list-processes" - The convention for dotnet tools is that after the context (trace in this case) they have a verb. 'List' would be that verb. My other objection to 'endpoints' is that the concept collect uses is 'process-id', not 'endpoint'.
   b - Add the behavior to the spec
   c - Ideally add this same command to dotnet-counters.

2. I'm fine with a --list-profiles option although I'm still not sure why everyone wouldn't just use the help? If the list of profiles was long or dynamic it makes more sense, but for a short fixed list I'd imagine user doesn't understand, they hit "help collect" to learn more, and then they are good to go.

3. All its saying is that Linux users who want to use speedscope add --format speedscope to the command. That doesn't feel particularly onerous to me? I could imagine plenty of users even on Linux want to bring the file back to a Windows machine for analysis with PerfView/VS. If we think this is just for Preview5 I won't poke at it too much and we'll get convert implemented soon I trust.

Thanks!

[noahfalk]

Looking at the help a little more, was the issue there that the text is layed out in an awkward way that is hard to read? I was hoping we could get a better layout, but I agree if that is what we are stuck with it would be much less usable.

[josalem]

Offline Sync Notes:

For Preview 5

• [x] profiles verb -> list-profiles verb
  
  
  
  • add to spec and re-evaluate
  
  
• [x] endpoints verb -> list-processes verb
  
  
  
  • add to spec and re-evaluate
  
  
• [x] Change default format on all platforms to netperf, and add convert verb (-f|--format flag is already there for collect verb)
  
  
  
  • Make convert spec match impl or _vice versa_
  
  

[noahfalk]

We've completed everything, closing.