Skip to content

[Design PR] Spec for dotnet-counters and dotnet-trace run #1522

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

[Design PR] Spec for dotnet-counters and dotnet-trace run #1522

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e58639f
spec for dotnet-counters and dotnet-trace run
sywhang Sep 3, 2020
cb27452
counters mode
sywhang Sep 3, 2020
c02484b
update spec
sywhang Sep 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 99 additions & 2 deletions documentation/design-docs/dotnet-tools.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,12 @@ COLLECT

1. Collect the runtime performance counters at a refresh interval of 10 seconds and export it as a JSON file named "test.json".
```
dotnet run collect --process-id 863148 --refresh-interval 10 --output test --format json
dotnet-counters collect --process-id 863148 --refresh-interval 10 --output test --format json
```

2. Collect the runtime performance counters as well as the ASP.NET hosting performance counters at the default refresh interval (1 second) and export it as a CSV file named "mycounter.csv".
```
dotnet run collect --process-id 863148 --output mycounter --format csv System.Runtime Microsoft.AspNetCore.Hosting
dotnet-counters collect --process-id 863148 --output mycounter --format csv System.Runtime Microsoft.AspNetCore.Hosting
```


Expand Down Expand Up @@ -280,6 +280,55 @@ COLLECT

```

MONITOR / COLLECT on Startup (on .NET 5 or later)
Copy link
Member

@noahfalk noahfalk Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error case is also pretty interesting. What happens if I run this command on an executable that doesn't wind up starting a runtime version >= 5?


Examples:

1. Launch my-server-app.exe and start monitoring it from its startup with default list of counters and interval.
```
dotnet-counters monitor -- C:\path\to\my-server-app.exe
Copy link
Member

@noahfalk noahfalk Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal either way, but will it also work if I write?

dotnet-counters monitor C:\path\to\my-server-app.exe

Typically the "--" means everything after that point should be treated as an argument, not an option, regardless if it starts with a -. However in this case there are no tokens that start with "-" after the "--" so it isn't ambiguous. As far as I know most tools that utilize the "--" syntax don't require it in the non-ambiguous case.

```

2. Launch my-server-app.exe and start collecting metrics into a CSV file from its startup with the runtime and Kestrel counters and update interval of once per minute.
```
dotnet-counters collect System.Runtime Microsoft-AspNetCore-Server-Kestrel --format csv -- C:\path\to\my-server-app.exe --refresh-interval 60
Copy link
Member

@noahfalk noahfalk Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my understanding of how "--" is usually interpretted in other apps (and probably how System.CommandLine would interpret it?) I would expect what this actually should do is attempt to run the process:

System.Runtime Microsoft-AspNetCore-Server-Kestrel C:\path\to\my-server-app.exe --refresh-interval 60

I don't think that is what you meant to have it do. My understanding of the parse rules are something like this pseudo-code:

bool parsingOptions = true;
foreach(string token in commandLine.Split(whitespace))
{
    if(token == "--" && parsingOptions)
    {
        parsingOptions = false;
        continue;
    }

    if(parsingOptions && token.StartsWith("-"))
    {
        // this is an option, skip it an potentially the next token too if the option consumes a value (not shown)
        continue;
    }

    arguments.Add(token);
}

System.Runtime and Microsoft-AspNetCore-Server-Kestrel are arguments because they don't start with a "-". "--format csv" is an option because it does start with a dash and it consumes the following token. "--" disables all further options. "C:\path\to\my-server-app.exe" "--refresh-interval" and "60" are all arguments because regardless what they start with option parsing is disabled. Then the command-line is inferred to be all the arguments joined together with single spaces.

This is why counters using using a counter_list as its current argument list is problematic, it collides with interpreting arguments as being parts of the command-line. My suggestion is to change the syntax to something like this:

dotnet-counters collect --counters System.Runtime,Microsoft-AspNetCore-Server-Kestrel --format csv --refresh-interval 60 -- C:\path\to\my-server-app.exe

Back-compat with old syntax can still work by observing if the -p {PID} option is present and interpreting any arguments as elements in the counter list in that case only.

Copy link
Contributor

@josalem josalem Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect that everything after -- will be used with the subprocess. Did you mean to have --refresh-interval 60 before --?

```

Syntax:

```
dotnet-counters collect [-h||--help]
Copy link
Contributor

@josalem josalem Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dotnet-counters collect [-h||--help]
dotnet-counters run [-h||--help]

Copy link
Contributor

@shirhatti shirhatti Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the verb run to be confusing. Without reading the proposal, I'd assumed dotnet counters run was similar to dotnet run where I could point it to a directory containing a project file as opposed to needing to specify an executable

[--exec|--executable <path>]
Copy link
Member

@noahfalk noahfalk Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[--exec|--executable <path>]

[-o|--output <name>]
[--mode <monitor|collect>]
Copy link
Member

@noahfalk noahfalk Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[--mode <monitor|collect>]

[--format <csv|json>]
[--refreshInterval <sec>]
counter_list
Copy link
Member

@noahfalk noahfalk Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
counter_list
--counters counter_list

[ -- <command-to-execute>]
Copy link
Contributor

@josalem josalem Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[ -- <command-to-execute>]
[ -- <command-to-execute> [command options]]


-h, --help
Show command line help

--exec,--executable
Copy link
Contributor

@josalem josalem Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need --exec if we use the -- <command> [command args] model?

The full path to the executable to be monitored from startup

--mode
The UI mode - either collect the values into an exported file, or monitor the metrics in real time.

-o, --output
The name of the output file. Only valid if --mode is specified as collect.

--format
The format to be exported. Only valid if --mode is specified to be collect. Currently available: csv, json

--refresh-interval
The number of seconds to delay between updating the displayed counters

counter_list
A space separated list of counters. Counters can be specified provider_name[:counter_name]. If the
provider_name is used without a qualifying counter_name then all counters will be shown. To discover
provider and counter names, use the list command.


### dotnet-trace

Expand Down Expand Up @@ -396,6 +445,54 @@ CONVERT
Writing: ./trace.speedscope.json
Conversion complete


COLLECT on startup (on .NET 5 or later)

dotnet-trace collect
[-h|--help]
[-o|--output <trace-file-path>]
[--profile <profile_name>]
[--providers <list-of-comma-separated-providers>]
[--format <trace-file-format>]
-- <command-to-execute>
Copy link
Contributor

@josalem josalem Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
-- <command-to-execute>
[-- <command-to-execute> [command args]]

Launches the given executable and collect a trace of it from its startup.

-h, --help
Show command line help

-o, --output
The output path for the collected trace data. If not specified it defaults to ./trace.nettrace

--profile
A named pre-defined set of provider configurations that allows common tracing scenarios to be specified
succinctly. The options are:
cpu-sampling Useful for tracking CPU usage and general .NET runtime information. This is the default
option if no profile or providers are specified.
gc-verbose Tracks GC collection and sampled object allocations
gc-collect Tracks GC collection only at very low overhead

--providers
A list of comma separated EventPipe providers to be enabled.
These providers are in addition to any providers implied by the --profile argument. If there is any
discrepancy for a particular provider, the configuration here takes precedence over the implicit
configuration from the profile.
A provider consists of the name and optionally the keywords, verbosity level, and custom key/value pairs.

The string is written 'Provider[,Provider]'
Provider format: KnownProviderName[:Keywords[:Level][:KeyValueArgs]]
KnownProviderName - The provider's name
Keywords - 8 character hex number bit mask
Level - A number in the range [0, 5], or their corresponding text values (refer to https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.tracing.eventlevel?view=netframework-4.8).
KeyValueArgs - A semicolon separated list of key=value
KeyValueArgs format: '[key1=value1][;key2=value2]'

--buffersize <Size>
Sets the size of the in-memory circular buffer in megabytes. Default 256 MB.

--format
The format of the output trace file. The default value is nettrace.


### dotnet-dump

SYNOPSIS
Expand Down