[dotnet-trace] Small changes (#173)

- Replace hardcoded numbers with existing/equivalent enum.
- Move non-common options to Collect, and started plumbing pack options.
- Remove comment.
- Remove pack changes

diff --git a/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs b/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs
index 392260e7dbb98bce165a8737841f6e570f69305c..507eee49a4d5602c107cb59b5c4d51c2e2827ed8 100644 (file)
--- a/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs
+++ b/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs
@@ -28,7 +28,7 @@ namespace Microsoft.Diagnostics.Tools.Trace
         /// <param name="profile">A named pre-defined set of provider configurations that allows common tracing scenarios to be specified succinctly.</param>
         /// <param name="format">The desired format of the created trace file.</param>
         /// <returns></returns>
-        public static async Task<int> Collect(IConsole console, int processId, string output, uint buffersize, string providers, string profile, TraceFileFormat format)
+        private static async Task<int> Collect(IConsole console, int processId, FileInfo output, uint buffersize, string providers, string profile, TraceFileFormat format)
         {
             try
             {
@@ -47,6 +47,8 @@ namespace Microsoft.Diagnostics.Tools.Trace
                 var providerCollection = Extensions.ToProviders(providers);
                 if (selectedProfile.Providers != null)
                     providerCollection.AddRange(selectedProfile.Providers);
+                if (providerCollection.Count <= 0)
+                    throw new ArgumentException("No providers were specified to start a trace.");
 
                 PrintProviders(providerCollection);
 
@@ -69,7 +71,7 @@ namespace Microsoft.Diagnostics.Tools.Trace
                     }
 
                     var collectingTask = new Task(() => {
-                        using (var fs = new FileStream(output, FileMode.Create, FileAccess.Write))
+                        using (var fs = new FileStream(output.FullName, FileMode.Create, FileAccess.Write))
                         {
                             Console.Out.WriteLine($"Process     : {process.MainModule.FileName}");
                             Console.Out.WriteLine($"Output File : {fs.Name}");
@@ -94,7 +96,7 @@ namespace Microsoft.Diagnostics.Tools.Trace
                     collectingTask.Start();
 
                     Console.Out.WriteLine("Press <Enter> or <Ctrl+C> to exit...");
-                    System.Console.CancelKeyPress += (sender, args) => {
+                    Console.CancelKeyPress += (sender, args) => {
                         args.Cancel = true;
                         shouldExit.Set();
                     };
@@ -110,7 +112,7 @@ namespace Microsoft.Diagnostics.Tools.Trace
                 Console.Out.WriteLine();
                 Console.Out.WriteLine("Trace completed.");
 
-                TraceFileFormatConverter.ConvertToFormat(format, output);
+                TraceFileFormatConverter.ConvertToFormat(format, output.FullName);
 
                 await Task.FromResult(0);
                 return sessionId != 0 ? 0 : 1;
@@ -133,6 +135,7 @@ namespace Microsoft.Diagnostics.Tools.Trace
         private static int prevBufferWidth = 0;
         private static string clearLineString = "";
         private static int lineToClear = 0;
+
         private static void ResetCurrentConsoleLine(bool isVTerm)
         {
             if (isVTerm)
@@ -173,19 +176,44 @@ namespace Microsoft.Diagnostics.Tools.Trace
                 description: "Collects a diagnostic trace from a currently running process",
                 symbols: new Option[] {
                     CommonOptions.ProcessIdOption(),
-                    CommonOptions.CircularBufferOption(),
-                    CommonOptions.OutputPathOption(),
-                    CommonOptions.ProvidersOption(),
+                    CircularBufferOption(),
+                    OutputPathOption(),
+                    ProvidersOption(),
                     ProfileOption(),
                     CommonOptions.FormatOption(),
                 },
-                handler: System.CommandLine.Invocation.CommandHandler.Create<IConsole, int, string, uint, string, string, TraceFileFormat>(Collect));
+                handler: System.CommandLine.Invocation.CommandHandler.Create<IConsole, int, FileInfo, uint, string, string, TraceFileFormat>(Collect));
+
+        private static uint DefaultCircularBufferSizeInMB => 256;
+
+        private static Option CircularBufferOption() =>
+            new Option(
+                alias: "--buffersize",
+                description: $"Sets the size of the in-memory circular buffer in megabytes. Default {DefaultCircularBufferSizeInMB} MB",
+                argument: new Argument<uint>(defaultValue: DefaultCircularBufferSizeInMB) { Name = "size" },
+                isHidden: false);
+
+        private static string DefaultTraceName => "trace.netperf";
+
+        private static Option OutputPathOption() =>
+            new Option(
+                aliases: new[] { "-o", "--output" },
+                description: $"The output path for the collected trace data. If not specified it defaults to '{DefaultTraceName}'",
+                argument: new Argument<FileInfo>(defaultValue: new FileInfo(DefaultTraceName)) { Name = "trace-file-path" },
+                isHidden: false);
+
+        private static Option ProvidersOption() =>
+            new Option(
+                alias: "--providers",
+                description: @"A list of EventPipe providers to be enabled. This is in the form 'Provider[,Provider]', where Provider is in the form: '(GUID|KnownProviderName)[:Flags[:Level][:KeyValueArgs]]', and KeyValueArgs is in the form: '[key1=value1][;key2=value2]'",
+                argument: new Argument<string>(defaultValue: "") { Name = "list-of-comma-separated-providers" }, // TODO: Can we specify an actual type?
+                isHidden: false);
 
-        public static Option ProfileOption() =>
+        private static Option ProfileOption() =>
             new Option(
                 alias: "--profile",
                 description: @"A named pre-defined set of provider configurations that allows common tracing scenarios to be specified succinctly.",
-                argument: new Argument<string>(defaultValue: "runtime-basic") { Name = "profile_name" }, // TODO: Can we specify an actual type?
+                argument: new Argument<string>(defaultValue: "runtime-basic") { Name = "profile_name" },
                 isHidden: false);
     }
 }

diff --git a/src/Tools/dotnet-trace/CommandLine/Commands/ProfilesCommandHandler.cs b/src/Tools/dotnet-trace/CommandLine/Commands/ProfilesCommandHandler.cs
index a6a9b0446aa54fcb6643a3fb58089e2d70ddf3e0..d0ea7cb0dafa22ebc1fa7acf3c5ffd5052308a11 100644 (file)
--- a/src/Tools/dotnet-trace/CommandLine/Commands/ProfilesCommandHandler.cs
+++ b/src/Tools/dotnet-trace/CommandLine/Commands/ProfilesCommandHandler.cs
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using Microsoft.Diagnostics.Tools.RuntimeClient;
+using Microsoft.Diagnostics.Tracing.Parsers;
 using System;
 using System.Collections.Generic;
 using System.CommandLine;
@@ -44,21 +45,21 @@ namespace Microsoft.Diagnostics.Tools.Trace
                 "runtime-basic",
                 new Provider[] {
                     new Provider("Microsoft-DotNETCore-SampleProfiler"),
-                    new Provider("Microsoft-Windows-DotNETRuntime", 0x00000004C14FCCBD, EventLevel.Informational),
+                    new Provider("Microsoft-Windows-DotNETRuntime", (ulong)ClrTraceEventParser.Keywords.Default, EventLevel.Informational),
                 },
                 "Useful for tracking CPU usage and general runtime information. This the default option if no profile is specified."),
             new Profile(
                 "gc",
                 new Provider[] {
                     new Provider("Microsoft-DotNETCore-SampleProfiler"),
-                    new Provider("Microsoft-Windows-DotNETRuntime", 0x0000000000000001, EventLevel.Verbose),
+                    new Provider("Microsoft-Windows-DotNETRuntime", (ulong)ClrTraceEventParser.Keywords.GC, EventLevel.Verbose),
                 },
                 "Tracks allocation and collection performance."),
             new Profile(
                 "gc-collect",
                 new Provider[] {
                     new Provider("Microsoft-DotNETCore-SampleProfiler"),
-                    new Provider("Microsoft-Windows-DotNETRuntime", 0x0000000000000001, EventLevel.Informational),
+                    new Provider("Microsoft-Windows-DotNETRuntime", (ulong)ClrTraceEventParser.Keywords.GC, EventLevel.Informational),
                 },
                 "Tracks GC collection only at very low overhead."),
 

diff --git a/src/Tools/dotnet-trace/CommandLine/Options/CommonOptions.cs b/src/Tools/dotnet-trace/CommandLine/Options/CommonOptions.cs
index a12a6d9e62ed512c17cc294d48c0fcb3662f995b..c3fde6988a65849905466ad149151db09b09cdfe 100644 (file)
--- a/src/Tools/dotnet-trace/CommandLine/Options/CommonOptions.cs
+++ b/src/Tools/dotnet-trace/CommandLine/Options/CommonOptions.cs
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System.CommandLine;
+using System.IO;
 using System.Runtime.InteropServices;
 
 namespace Microsoft.Diagnostics.Tools.Trace
@@ -16,29 +17,6 @@ namespace Microsoft.Diagnostics.Tools.Trace
                 argument: new Argument<int> { Name = "pid" },
                 isHidden: false);
 
-        public static Option OutputPathOption() =>
-            new Option(
-                aliases: new[] { "-o", "--output" },
-                description: "The output path for the collected trace data. If not specified it defaults to 'trace.netperf'",
-                argument: new Argument<string>(defaultValue: $"trace.netperf") { Name = "trace-file-path" },
-                isHidden: false);
-
-        public static Option ProvidersOption() =>
-            new Option(
-                alias: "--providers",
-                description: @"A list of EventPipe providers to be enabled. This is in the form 'Provider[,Provider]', where Provider is in the form: '(GUID|KnownProviderName)[:Flags[:Level][:KeyValueArgs]]', and KeyValueArgs is in the form: '[key1=value1][;key2=value2]'",
-                argument: new Argument<string> { Name = "list-of-comma-separated-providers" }, // TODO: Can we specify an actual type?
-                isHidden: false);
-
-        // This is a hidden option, currently not in the design-doc spec.
-        private static uint DefaultCircularBufferSizeInMB => 256;
-        public static Option CircularBufferOption() =>
-            new Option(
-                alias: "--buffersize",
-                description: $"Sets the size of the in-memory circular buffer in megabytes. Default {DefaultCircularBufferSizeInMB} MB",
-                argument: new Argument<uint>(defaultValue: DefaultCircularBufferSizeInMB) { Name = "size" },
-                isHidden: false);
-
         public static TraceFileFormat DefaultTraceFileFormat => 
             RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? TraceFileFormat.netperf : TraceFileFormat.speedscope;
 

diff --git a/src/Tools/dotnet-trace/Extensions.cs b/src/Tools/dotnet-trace/Extensions.cs
index 8c60cc20fff3f72420f77b84675a53d4dddf0f01..0967d6da9adbd99cbcb9f096ba8846772b73ea9c 100644 (file)
--- a/src/Tools/dotnet-trace/Extensions.cs
+++ b/src/Tools/dotnet-trace/Extensions.cs
@@ -14,11 +14,10 @@ namespace Microsoft.Diagnostics.Tools.Trace
     {
         public static List<Provider> ToProviders(string providers)
         {
-            if (string.IsNullOrWhiteSpace(providers))
+            if (providers == null)
                 throw new ArgumentNullException(nameof(providers));
-            return providers.Split(',')
-                .Select(ToProvider)
-                .ToList();
+            return string.IsNullOrWhiteSpace(providers) ?
+                new List<Provider>() : providers.Split(',').Select(ToProvider).ToList();
         }
 
         private static Provider ToProvider(string provider)