Validate format options in dotnet-trace convert (#4514)

Fixes https://github.com/dotnet/docs/issues/39557

- Base enum at 1 since it's backed by an int and the default has
semantic value (NetTrace).
- Make the conversion format required.
- Validate the Enum parameter and make help more explicit.

diff --git a/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs b/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs
index b71930a7f4ca41d8f1a4743222d8a307e72193f1..96a13631e2ae67d32d8d3e050855415f10852bf5 100644 (file)
--- a/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs
+++ b/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs
@@ -21,8 +21,7 @@ namespace Microsoft.Diagnostics.Tools.Trace
 {
     internal static class CollectCommandHandler
     {
-        internal static bool IsQuiet
-        { get; set; }
+        internal static bool IsQuiet { get; set; }
 
         private static void ConsoleWriteLine(string str)
         {
@@ -424,7 +423,8 @@ namespace Microsoft.Diagnostics.Tools.Trace
 
                         if (format != TraceFileFormat.NetTrace)
                         {
-                            TraceFileFormatConverter.ConvertToFormat(format, output.FullName);
+                            string outputFilename = TraceFileFormatConverter.GetConvertedFilename(output.FullName, outputfile: null, format);
+                            TraceFileFormatConverter.ConvertToFormat(console, format, fileToConvert: output.FullName, outputFilename);
                         }
                     }
 

diff --git a/src/Tools/dotnet-trace/CommandLine/Commands/ConvertCommand.cs b/src/Tools/dotnet-trace/CommandLine/Commands/ConvertCommand.cs
index 9506c2f735363b83630211c761ae87b4dcae8102..3e94fc67379f6c2cfd0318e9366734cdab8b5015 100644 (file)
--- a/src/Tools/dotnet-trace/CommandLine/Commands/ConvertCommand.cs
+++ b/src/Tools/dotnet-trace/CommandLine/Commands/ConvertCommand.cs
@@ -2,38 +2,92 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Collections.Generic;
 using System.CommandLine;
+using System.CommandLine.IO;
 using System.IO;
+using System.Linq;
 using Microsoft.Tools.Common;
 
 namespace Microsoft.Diagnostics.Tools.Trace
 {
     internal static class ConvertCommandHandler
     {
+        // The first 8 bytes of a nettrace file are the ASCII string "Nettrace"
+        private static readonly byte[] NetTraceHeader = [0x4E, 0x65, 0x74, 0x74, 0x72, 0x61, 0x63, 0x65];
+
         public static int ConvertFile(IConsole console, FileInfo inputFilename, TraceFileFormat format, FileInfo output)
         {
-            if ((int)format <= 0)
+            if (!Enum.IsDefined(format))
             {
-                Console.Error.WriteLine("--format is required.");
+                console.Error.WriteLine($"Please specify a valid option for the --format. Valid options are: {string.Join(", ", Enum.GetNames<TraceFileFormat>())}.");
                 return ErrorCodes.ArgumentError;
             }
 
-            if (format == TraceFileFormat.NetTrace)
+            if (!ValidateNetTraceHeader(console, inputFilename.FullName))
             {
-                Console.Error.WriteLine("Cannot convert a nettrace file to nettrace format.");
                 return ErrorCodes.ArgumentError;
             }
 
-            if (!inputFilename.Exists)
+            string outputFilename = TraceFileFormatConverter.GetConvertedFilename(inputFilename.FullName, output?.FullName, format);
+
+            if (format != TraceFileFormat.NetTrace)
             {
-                Console.Error.WriteLine($"File '{inputFilename}' does not exist.");
-                return ErrorCodes.ArgumentError;
+                TraceFileFormatConverter.ConvertToFormat(console, format, inputFilename.FullName, outputFilename);
+                return 0;
+            }
+
+            return CopyNetTrace(console, inputFilename.FullName, outputFilename);
+
+            static bool ValidateNetTraceHeader(IConsole console, string filename)
+            {
+                try
+                {
+                    using FileStream fs = new(filename, FileMode.Open, FileAccess.Read);
+                    Span<byte> header = stackalloc byte[NetTraceHeader.Length];
+                    Span<byte> readBuffer = header;
+                    int bytesRead = 0;
+                    while (readBuffer.Length > 0 && (bytesRead = fs.Read(readBuffer)) > 0)
+                    {
+                        readBuffer = readBuffer.Slice(bytesRead);
+                    }
+
+                    if (readBuffer.Length != 0 || !header.SequenceEqual(NetTraceHeader))
+                    {
+                        console.Error.WriteLine($"'{filename}' is not a valid nettrace file.");
+                        return false;
+                    }
+                }
+                catch (Exception ex)
+                {
+                    console.Error.WriteLine($"Error reading '{filename}': {ex.Message}");
+                    return false;
+                }
+
+                return true;
             }
 
-            output ??= inputFilename;
+            static int CopyNetTrace(IConsole console, string inputfile, string outputfile)
+            {
+                if (inputfile == outputfile)
+                {
+                    console.Error.WriteLine("Input and output filenames are the same. Skipping copy.");
+                    return 0;
+                }
+
+                console.Out.WriteLine($"Copying nettrace to:\t{outputfile}");
+                try
+                {
+                    File.Copy(inputfile, outputfile);
+                }
+                catch (Exception ex)
+                {
+                    console.Error.WriteLine($"Error copying nettrace to {outputfile}: {ex.Message}");
+                    return ErrorCodes.UnknownError;
+                }
 
-            TraceFileFormatConverter.ConvertToFormat(format, inputFilename.FullName, output.FullName);
-            return 0;
+                return 0;
+            }
         }
 
         public static Command ConvertCommand() =>

diff --git a/src/Tools/dotnet-trace/CommandLine/Options/CommonOptions.cs b/src/Tools/dotnet-trace/CommandLine/Options/CommonOptions.cs
index d52c6e36d7d72d5d53bc141914858214e449287a..0500292d6da2464c9c0ce0298f91695ad5e23d1b 100644 (file)
--- a/src/Tools/dotnet-trace/CommandLine/Options/CommonOptions.cs
+++ b/src/Tools/dotnet-trace/CommandLine/Options/CommonOptions.cs
@@ -38,7 +38,8 @@ namespace Microsoft.Diagnostics.Tools.Trace
                 alias: "--format",
                 description: $"Sets the output format for the trace file conversion.")
             {
-                Argument = new Argument<TraceFileFormat>(name: "trace-file-format")
+                Argument = new Argument<TraceFileFormat>(name: "trace-file-format"),
+                IsRequired = true
             };
     }
 }

diff --git a/src/Tools/dotnet-trace/TraceFileFormatConverter.cs b/src/Tools/dotnet-trace/TraceFileFormatConverter.cs
index d068b99d8a9391996a2a6540c5c0b53b840f06a2..f383a7453d691f96eb8c7b386ac96f77228f31c7 100644 (file)
--- a/src/Tools/dotnet-trace/TraceFileFormatConverter.cs
+++ b/src/Tools/dotnet-trace/TraceFileFormatConverter.cs
@@ -3,6 +3,8 @@
 
 using System;
 using System.Collections.Generic;
+using System.CommandLine;
+using System.CommandLine.IO;
 using System.IO;
 using Microsoft.Diagnostics.Symbols;
 using Microsoft.Diagnostics.Tracing;
@@ -12,7 +14,7 @@ using Microsoft.Diagnostics.Tracing.Stacks.Formats;
 
 namespace Microsoft.Diagnostics.Tools.Trace
 {
-    internal enum TraceFileFormat { NetTrace, Speedscope, Chromium };
+    internal enum TraceFileFormat { NetTrace = 1, Speedscope, Chromium };
 
     internal static class TraceFileFormatConverter
     {
@@ -22,15 +24,19 @@ namespace Microsoft.Diagnostics.Tools.Trace
             { TraceFileFormat.Chromium,     "chromium.json" }
         };
 
-        public static void ConvertToFormat(TraceFileFormat format, string fileToConvert, string outputFilename = "")
+        internal static string GetConvertedFilename(string fileToConvert, string outputfile, TraceFileFormat format)
         {
-            if (string.IsNullOrWhiteSpace(outputFilename))
+            if (string.IsNullOrWhiteSpace(outputfile))
             {
-                outputFilename = fileToConvert;
+                outputfile = fileToConvert;
             }
 
-            outputFilename = Path.ChangeExtension(outputFilename, TraceFileFormatExtensions[format]);
-            Console.Out.WriteLine($"Writing:\t{outputFilename}");
+            return Path.ChangeExtension(outputfile, TraceFileFormatExtensions[format]);
+        }
+
+        internal static void ConvertToFormat(IConsole console, TraceFileFormat format, string fileToConvert, string outputFilename)
+        {
+            console.Out.WriteLine($"Writing:\t{outputFilename}");
 
             switch (format)
             {
@@ -40,7 +46,7 @@ namespace Microsoft.Diagnostics.Tools.Trace
                 case TraceFileFormat.Chromium:
                     try
                     {
-                        Convert(format, fileToConvert, outputFilename);
+                        Convert(console, format, fileToConvert, outputFilename);
                     }
                     // TODO: On a broken/truncated trace, the exception we get from TraceEvent is a plain System.Exception type because it gets caught and rethrown inside TraceEvent.
                     // We should probably modify TraceEvent to throw a better exception.
@@ -48,12 +54,12 @@ namespace Microsoft.Diagnostics.Tools.Trace
                     {
                         if (ex.ToString().Contains("Read past end of stream."))
                         {
-                            Console.WriteLine("Detected a potentially broken trace. Continuing with best-efforts to convert the trace, but resulting speedscope file may contain broken stacks as a result.");
-                            Convert(format, fileToConvert, outputFilename, true);
+                            console.Out.WriteLine("Detected a potentially broken trace. Continuing with best-efforts to convert the trace, but resulting speedscope file may contain broken stacks as a result.");
+                            Convert(console, format, fileToConvert, outputFilename, continueOnError: true);
                         }
                         else
                         {
-                            Console.WriteLine(ex);
+                            console.Error.WriteLine(ex.ToString());
                         }
                     }
                     break;
@@ -61,10 +67,10 @@ namespace Microsoft.Diagnostics.Tools.Trace
                     // Validation happened way before this, so we shoud never reach this...
                     throw new ArgumentException($"Invalid TraceFileFormat \"{format}\"");
             }
-            Console.Out.WriteLine("Conversion complete");
+            console.Out.WriteLine("Conversion complete");
         }
 
-        private static void Convert(TraceFileFormat format, string fileToConvert, string outputFilename, bool continueOnError = false)
+        private static void Convert(IConsole _, TraceFileFormat format, string fileToConvert, string outputFilename, bool continueOnError = false)
         {
             string etlxFilePath = TraceLog.CreateFromEventPipeDataFile(fileToConvert, null, new TraceLogOptions() { ContinueOnError = continueOnError });
             using (SymbolReader symbolReader = new(TextWriter.Null) { SymbolPath = SymbolPath.MicrosoftSymbolServerPath })