-
Notifications
You must be signed in to change notification settings - Fork 384
[dotnet-trace] Add collect-linux verb #5570
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?
Changes from 1 commit
536cd50
49a875b
b4787fc
c921869
e1c15c7
b4112b5
e5f3975
677e54e
31186a8
44593bb
b190d73
9f1ef6d
c8e53ea
590a203
8e2453f
573d769
1b3b583
e6d9de9
676efa9
d90d1be
096f325
1f75125
55bc84a
4c1a361
208086f
40c5905
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,10 +18,10 @@ namespace Microsoft.Diagnostics.Tools.Trace | |
{ | ||
internal partial class CollectLinuxCommandHandler | ||
{ | ||
private static bool s_stopTracing; | ||
private static Stopwatch s_stopwatch = new(); | ||
private static LineRewriter s_rewriter; | ||
private static bool s_printingStatus; | ||
private bool s_stopTracing; | ||
private Stopwatch s_stopwatch = new(); | ||
private LineRewriter s_rewriter; | ||
private bool s_printingStatus; | ||
|
||
internal sealed record CollectLinuxArgs( | ||
CancellationToken Ct, | ||
|
@@ -48,7 +48,7 @@ internal int CollectLinux(CollectLinuxArgs args) | |
if (!OperatingSystem.IsLinux()) | ||
{ | ||
Console.Error.WriteLine("The collect-linux command is only supported on Linux."); | ||
return (int)ReturnCode.ArgumentError; | ||
return (int)ReturnCode.PlatformNotSupportedError; | ||
} | ||
|
||
args.Ct.Register(() => s_stopTracing = true); | ||
|
@@ -72,6 +72,11 @@ internal int CollectLinux(CollectLinuxArgs args) | |
s_stopwatch.Start(); | ||
ret = RecordTraceInvoker(command, (UIntPtr)command.Length, OutputHandler); | ||
} | ||
catch (Exception ex) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In dotnet-trace collect we have the CommandLineErrorException catch handler that I think we should replicate here. The intent is to separate Exceptions into two categories:
All of the issues handling command-line arguments should fall in the first group. |
||
{ | ||
Console.Error.WriteLine($"[ERROR] {ex}"); | ||
ret = (int)ReturnCode.TracingError; | ||
} | ||
finally | ||
{ | ||
if (!string.IsNullOrEmpty(scriptPath)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
using System.Threading.Tasks; | ||
using Microsoft.Diagnostics.Tests.Common; | ||
using Microsoft.Diagnostics.Tools.Trace; | ||
using Microsoft.Internal.Common.Utils; | ||
using Xunit; | ||
|
||
namespace Microsoft.Diagnostics.Tools.Trace | ||
|
@@ -51,22 +52,33 @@ public sealed record CollectArgs( | |
public async Task CollectCommandProviderConfigurationConsolidation(CollectArgs args, string[] expectedSubset) | ||
{ | ||
MockConsole console = new(200, 30); | ||
string[] rawLines = await RunAsync(args, console).ConfigureAwait(true); | ||
console.AssertSanitizedLinesEqual(CollectSanitizer, expectedSubset); | ||
int exitCode = await RunAsync(args, console).ConfigureAwait(true); | ||
Assert.Equal((int)ReturnCode.Ok, exitCode); | ||
console.AssertSanitizedLinesEqual(CollectSanitizer, expectedLines: expectedSubset); | ||
|
||
byte[] expected = Encoding.UTF8.GetBytes(ExpectedPayload); | ||
Assert.Equal(expected, args.EventStream.ToArray()); | ||
} | ||
|
||
private static async Task<string[]> RunAsync(CollectArgs config, MockConsole console) | ||
[Theory] | ||
[MemberData(nameof(InvalidProviders))] | ||
public async Task CollectCommandInvalidProviderConfiguration_Throws(CollectArgs args, string[] expectedException) | ||
{ | ||
MockConsole console = new(200, 30); | ||
int exitCode = await RunAsync(args, console).ConfigureAwait(true); | ||
Assert.Equal((int)ReturnCode.TracingError, exitCode); | ||
console.AssertSanitizedLinesEqual(CollectSanitizer, true, expectedException); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm guessing the reason this test is ignoring past expected is because there is an unpredictable StackTrace there? Hopefully my suggestion around the catch handlers and removing the stack trace text above also means we no longer need to ignore extra output. |
||
} | ||
|
||
private static async Task<int> RunAsync(CollectArgs config, MockConsole console) | ||
{ | ||
var handler = new CollectCommandHandler(); | ||
handler.StartTraceSessionAsync = (client, cfg, ct) => Task.FromResult<CollectCommandHandler.ICollectSession>(new TestCollectSession()); | ||
handler.ResumeRuntimeAsync = (client, ct) => Task.CompletedTask; | ||
handler.CollectSessionEventStream = (name) => config.EventStream; | ||
handler.Console = console; | ||
|
||
int exit = await handler.Collect( | ||
return await handler.Collect( | ||
config.ct, | ||
config.cliConfig, | ||
config.ProcessId, | ||
|
@@ -87,12 +99,7 @@ private static async Task<string[]> RunAsync(CollectArgs config, MockConsole con | |
config.stoppingEventPayloadFilter, | ||
config.rundown, | ||
config.dsrouter | ||
).ConfigureAwait(true); | ||
if (exit != 0) | ||
{ | ||
throw new InvalidOperationException($"Collect exited with return code {exit}."); | ||
} | ||
return console.Lines; | ||
).ConfigureAwait(false); | ||
} | ||
|
||
private static string[] CollectSanitizer(string[] lines) | ||
|
@@ -123,7 +130,6 @@ public void Stop() {} | |
|
||
public static IEnumerable<object[]> BasicCases() | ||
{ | ||
FileInfo fi = new("trace.nettrace"); | ||
yield return new object[] | ||
{ | ||
new CollectArgs(), | ||
|
@@ -214,14 +220,46 @@ public static IEnumerable<object[]> BasicCases() | |
}; | ||
} | ||
|
||
public static IEnumerable<object[]> InvalidProviders() | ||
{ | ||
yield return new object[] | ||
{ | ||
new CollectArgs(profile: new[] { "cpu-sampling" }), | ||
new [] { FormatException("The specified profile 'cpu-sampling' does not apply to `dotnet-trace collect`.", "System.ArgumentException") } | ||
}; | ||
|
||
yield return new object[] | ||
{ | ||
new CollectArgs(profile: new[] { "unknown" }), | ||
new [] { FormatException("Invalid profile name: unknown", "System.ArgumentException") } | ||
}; | ||
|
||
yield return new object[] | ||
{ | ||
new CollectArgs(providers: new[] { "Foo:::Bar=0", "Foo:::Bar=1" }), | ||
new [] { FormatException($"Provider \"Foo\" is declared multiple times with filter arguments.", "System.ArgumentException") } | ||
}; | ||
|
||
yield return new object[] | ||
{ | ||
new CollectArgs(clrevents: "unknown"), | ||
new [] { FormatException("unknown is not a valid CLR event keyword", "System.ArgumentException") } | ||
}; | ||
|
||
yield return new object[] | ||
{ | ||
new CollectArgs(clrevents: "gc", clreventlevel: "unknown"), | ||
new [] { FormatException("Unknown EventLevel: unknown", "System.ArgumentException") } | ||
}; | ||
} | ||
|
||
private static string outputFile = $"Output File : {Directory.GetCurrentDirectory() + Path.DirectorySeparatorChar}trace.nettrace"; | ||
private const string ProviderHeader = "Provider Name Keywords Level Enabled By"; | ||
private static readonly string[] CommonTail = [ | ||
"Process : <PROCESS>", | ||
outputFile, | ||
"", | ||
"", | ||
"", | ||
"Trace completed." | ||
]; | ||
|
||
|
@@ -238,5 +276,7 @@ private static string FormatProvider(string name, string keywordsHex, string lev | |
string.Format("{0, -8}", $"{levelName}({levelValue})"); | ||
return string.Format("{0, -80}", display) + enabledBy; | ||
} | ||
|
||
private static string FormatException(string message, string exceptionType) => $"[ERROR] {exceptionType}: {message}"; | ||
} | ||
} |
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.
Nit: non-static variables shouldn't have s_ prefixes on them.