Skip to content

Conversation

@guerrerotook
Copy link

Added the modifier --non-interactive-console that will disable the update process while collecting diagnostic information. This flag will prevent the tool to stop collecting traces when the process is being launched in a process that doesn't have a console attached to it.

That may happen when we're redirecting the standard output or when running the process inside a side car container.

[ERROR] System.ArgumentOutOfRangeException: The value must be greater than or equal to zero and less than the console's buffer size in that dimension. (Parameter 'top') Actual value was -1. at System.Console.SetCursorPosition(Int32 left, Int32 top) at Microsoft.Internal.Common.Utils.LineRewriter.SystemConsoleLineRewriter() in /_/src/Tools/Common/Commands/Utils.cs:line 162 at Microsoft.Internal.Common.Utils.LineRewriter.RewriteConsoleLine() in /_/src/Tools/Common/Commands/Utils.cs:line 158 at Microsoft.Diagnostics.Tools.Trace.CollectCommandHandler.<>c__DisplayClass1_2.<Collect>b__4() in /_/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs:line 264 at Microsoft.Diagnostics.Tools.Trace.CollectCommandHandler.Collect(CancellationToken ct, IConsole console, Int32 processId, FileInfo output, UInt32 buffersize, String providers, String profile, TraceFileFormat format, TimeSpan duration, String clrevents, String clreventlevel, String name, String diagnosticPort, Boolean showchildio) in /_/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs:line 275

…ak the collect process when no console is available.
@guerrerotook guerrerotook requested a review from a team as a code owner June 28, 2021 13:10
@dnfadmin
Copy link

dnfadmin commented Jun 28, 2021

CLA assistant check
All CLA requirements met.

@noahfalk
Copy link
Member

Thanks @guerrerotook!
Do you know if we could we auto-detect that there is no interactive console available to automatically apply this behavior?

@guerrerotook
Copy link
Author

Thanks @noahfalk, there is a property in the Environment class, UserInteractive, that may help us, but the Unix implementation always used true.

My other approach is looking at the Console.IsInputRedirected property and if this is true do not try to move the cursor to make the update of the status.

@noahfalk
Copy link
Member

My other approach is looking at the Console.IsInputRedirected

Thanks! Did you mean IsOutputRedirected or is it really is the input stream that matters on this one? Naively I would have guessed that moving the cursor still worked if only the input stream was redirected.
@jonsequitur @josalem @davmason - any other suggestions you are aware of on best way to detect if we've got an interactive console?

@mikem8361
Copy link
Contributor

mikem8361 commented Jun 29, 2021 via email

Copy link
Contributor

@josalem josalem left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution and welcome to the dotnet repos! I was a little confused when I saw this change since I recalled that dotnet-trace handled this case already. Looks like this is a regression introduced with #1968. We used to check Console.IsOutputRedirected, but #1968 switched to only checking Console.IsInputRedirected. I think we should be able to add this functionality back in by using both of those APIs without adding a new commandline flag. Would that work for the scenario you're addressing with this PR?

@guerrerotook
Copy link
Author

Thanks @josalem in our internal code if we only redirect the Output but not the Input its fails. Now we are testing redirecting Input and Output and we have the expect behavior.

Given said that, should I close my PR or investigate why by only redirecting the Output doesn't work?

@josalem
Copy link
Contributor

josalem commented Jul 1, 2021

Given said that, should I close my PR or investigate why by only redirecting the Output doesn't work?

Only redirecting output breaks because PrintStatusOverTime is being set to false only if input has been redirected.

printStatusOverTime = !Console.IsInputRedirected;

This results in RewriteConsoleLine() getting called which uses properties and methods that don't work under output redirection.

CancelOnEnter is set to false if input is redirected which guards against calling Console.ReadKey() when redirected:

while (!shouldExit.WaitOne(100) && !(cancelOnEnter && Console.KeyAvailable && Console.ReadKey(true).Key == ConsoleKey.Enter))
printStatus();

@guerrerotook, I think changing the patch to make status printing only happen when output isn't redirected, e.g.,

printStatusOverTime = !Console.IsOutputRedirected;

should solve the issue.

Here:

printStatusOverTime = !Console.IsInputRedirected;

@davidwrighton please correct me if I'm wrong, but this seems like it may have been the original intent in #1968.

Ideally, the result would be that the tool transparently handles redirection of input or output or both.

@davidwrighton
Copy link
Member

Ah, I don't think I meant to break this. I think that change would be OK.

@josalem
Copy link
Contributor

josalem commented Sep 20, 2021

This was fixed in #2448

@josalem josalem closed this Sep 20, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants