-
Notifications
You must be signed in to change notification settings - Fork 384
Add default iOS/Android commands to dotnet-dsrouter. #4090
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
Add default iOS/Android commands to dotnet-dsrouter. #4090
Conversation
|
|
||
| private static Command AndroidEmulatorRouterCommand() => | ||
| new( | ||
| name: "android-emu", |
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.
I've always used identical commands for Android emulator vs device. Do we need android-emu as an option?
Or is the idea to just support it and do the same thing?
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.
In https://github.com/dotnet/runtime/blob/main/docs/design/mono/diagnostics-tracing.md I actually documented the emulator scenario to use the 10.0.2.2 IP address so you don't need to depend on having a reverse port adb command running that you would need in the device scenario. In order to have that option available I split android-emu and android into two different things, but you could of course use android for both scenarios, just that the tooling will depend on more things running when not really needed, so using the 10.0.2.2 in the emulator case is simplifying the infrastructure needed.
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.
android-emu option will log out a preconfigured DOTNET_DiagnosticPorts= that uses 10.0.2.2 instead of 127.0.0.1 that would need adb reverse ports running.
Similar to running:
dotnet-dsrouter server-server -tcps 127.0.0.1:9000
and using DOTNET_DiagnosticPorts=10.0.2.2:9000 for the launched app.
android option will log out a preconfigured DOTNET_DiagnosticPorts= that uses 127.0.0.1 and it will use the forward-port argument automatically setting up needed adb reverse ports needed for that configuration to work.
Similar to running:
dotnet-dsrouter server-server -tcps 127.0.0.1:9000 --forward-port android
and using DOTNET_DiagnosticPorts=127.0.0.1:9000 for the launched app.
In the end, you can use android command to get things to work on both emulator and device, just that the emulator scenario could be simplified and not depend on running adb reverse port command (since it uses 10.0.2.2 by default) and I thought that having less moving parts would reduce complexity or the emulator scenario.
b5d5e5e to
75699fe
Compare
| message.AppendLine($"DOTNET_DiagnosticPorts={deviceTcpIpAddress},nosuspend{listenMode}"); | ||
| message.AppendLine($"DOTNET_DiagnosticPorts={deviceTcpIpAddress},suspend{listenMode}"); |
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.
Does the 'nosuspend' option make sense if deviceListenMode=true?
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.
Yes, the default in EventPipe is nosuspend, suspending at startup is normally considered a "special case" and only used when doing startup tracing. The default IPC listeners used on EventPipe desktop builds (Windows, Linux, MacOS, MacCatalyst) is running in listener mode using nosuspend.
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.
Second question: are all 4 variations ("suspend,listen", "suspend", "nosuspend,listen", "nosuspend") valid on all platforms? I was under the impression that "listen" was required on iOS device, although I don't remember the technical details.
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.
Second question: are all 4 variations ("suspend,listen", "suspend", "nosuspend,listen", "nosuspend") valid on all platforms? I was under the impression that "listen" was required on iOS device, although I don't remember the technical details.
You can run all modes, but by default we used listen mode on iOS device since it works with usbmux, but you can set it up in connect mode as well, but you need to make sure the iOS device can connect to the tcp/ip address,port bound to dsrouter. We don't recommend it since you need to use a none loopback interface on the development machine, so that's why we choose listen mode + usbmux as default.
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.
@rolfbjarne Is there any variant when running app on iOS device where you can set it up to connect using 127.0.0.1:port and be automatically routed to a dev pc that it has been paired with (https://help.apple.com/xcode/mac/9.0/index.html?localePath=en.lproj#/devbc48d1bad)? If not, then I assume the only alternative when running on device using connect mode, is to use an IP address that can be routed to the development machine, run dsrouter bound to a IP address that can be accessed from iOS device wifi interface and make sure the traffic finds it way through the firewall.
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.
Right now it will display the mode used to communicate with the running instance of dotnet-dsrouter from the app, if you use ios-sim, it will default to use connect mode and only list that and if you use device mode it will use listen mode and only list that. If we only want to default to listen mode for both ios-sim and ios then we would need to make sure you can bind and connect to loopback interface on ios-simulator using for example 127.0.0.1:9000 and then connect to that from dotnet-dsrouter using 127.0.0.1:9000, I havn't tested that on ios-sim, but if that works as expected we could switch to listen mode for both simulator and device., but it will be a change compare to what we used so far in most documentation.
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.
If the default doesn't match your scenario, you can always spell out all the arguments needed like currently done, so no change there, the new defaults will just simplify the process to configure dotnet-dsrouter for the most common used scenarios.
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.
if you use ios-sim, it will default to use connect mode and only list that and if you use device mode it will use listen mode and only list that
The code seems to print both, or am I missing something?
message.AppendLine($"DOTNET_DiagnosticPorts={deviceTcpIpAddress},nosuspend{listenMode}");
message.AppendLine($"DOTNET_DiagnosticPorts={deviceTcpIpAddress},suspend{listenMode}");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.
Ah, I see, I'm getting confused between nosuspend/suspend and listen/nolisten.
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.
Yes, it will print two options, since you can run in nosuspend and suspend mode (used for startup tracing), so for example if you run with ios-sim it will print:
DOTNET_DiagnosticPorts=127.0.0.1,nosuspend,connect
DOTNET_DiagnosticPorts=127.0.0.1,suspend,connect
and with ios it will print:
DOTNET_DiagnosticPorts=127.0.0.1,nosuspend,listen
DOTNET_DiagnosticPorts=127.0.0.1,suspend,listen
the app can be launched with any version of the env variable for each scenario, but if you use the nosuspend mode the app won't wait for tools to connect during startup.
hoyosjs
left a comment
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.
From a code perspective, I only have that comment. The scenarios and UX is something I leave for Rolf and Jonathan to give you feedback on. I am not familiar on the workflows and nits of the platforms.
|
Hello @hoyosjs! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Context: dotnet/diagnostics#4081 Context: dotnet/diagnostics#4090 Context: dotnet/runtime#88634 Context: dotnet/diagnostics#4337 Using the latest `dotnet-trace` tooling: > dotnet tool list -g Package Id Version Commands -------------------------------------------------------------------------------------- dotnet-dsrouter 7.0.447801 dotnet-dsrouter dotnet-gcdump 7.0.447801 dotnet-gcdump dotnet-trace 7.0.447801 dotnet-trace There is a simplified way to profile Android apps. For an Android emulator: > dotnet-dsrouter android-emu Start an application on android emulator with one of the following environment variables set: DOTNET_DiagnosticPorts=10.0.2.2:9000,nosuspend,connect DOTNET_DiagnosticPorts=10.0.2.2:9000,suspend,connect > adb shell setprop debug.mono.profile '10.0.2.2:9000,suspend,connect' > dotnet-trace ps 3248 dotnet-dsrouter > dotnet-trace collect -p 3248 ... [00:00:00:09] Recording trace 3.2522 (MB) Press <Enter> or <Ctrl+C> to exit... For an Android device you will eventually be able to do `dotnet-dsrouter android`, except we found one issue. Until dotnet/diagnostics#4337 is resolved, we can instead do: > adb reverse tcp:9000 tcp:9001 > dotnet-dsrouter server-server -tcps 127.0.0.1:9001 > adb shell setprop debug.mono.profile '127.0.0.1:9000,suspend,connect' > dotnet-trace ps > dotnet-trace collect -p 3248 We can also now use `dotnet-gcdump`! Instead of `dotnet-trace collect -p`, you can simply do: > dotnet-gcdump collect -p 3248 In both cases these tools *know* that the process ID is a `dotnet-dsrouter` process and to do the right thing.
Context: dotnet/diagnostics#4081 Context: dotnet/diagnostics#4090 Context: dotnet/runtime#88634 Context: dotnet/diagnostics#4337 Using the latest `dotnet-trace` tooling: > dotnet tool list -g Package Id Version Commands -------------------------------------------------------------------------------------- dotnet-dsrouter 7.0.447801 dotnet-dsrouter dotnet-gcdump 7.0.447801 dotnet-gcdump dotnet-trace 7.0.447801 dotnet-trace There is a simplified way to profile Android apps. For an Android emulator: > dotnet-dsrouter android-emu Start an application on android emulator with one of the following environment variables set: DOTNET_DiagnosticPorts=10.0.2.2:9000,nosuspend,connect DOTNET_DiagnosticPorts=10.0.2.2:9000,suspend,connect > adb shell setprop debug.mono.profile '10.0.2.2:9000,suspend,connect' > dotnet-trace ps 3248 dotnet-dsrouter > dotnet-trace collect -p 3248 ... [00:00:00:09] Recording trace 3.2522 (MB) Press <Enter> or <Ctrl+C> to exit... For an Android device you will eventually be able to do `dotnet-dsrouter android`, except we found one issue. Until dotnet/diagnostics#4337 is resolved, we can instead do: > adb reverse tcp:9000 tcp:9001 > dotnet-dsrouter server-server -tcps 127.0.0.1:9001 > adb shell setprop debug.mono.profile '127.0.0.1:9000,suspend,connect' > dotnet-trace ps > dotnet-trace collect -p 3248 We can also now use `dotnet-gcdump`! Instead of `dotnet-trace collect -p`, you can simply do: > dotnet-gcdump collect -p 3248 In both cases these tools *know* that the process ID is a `dotnet-dsrouter` process and to do the right thing.
dotnet-dsrouter can be a little hard to configure for mobile use cases since it needs a number of arguments, both to setup its local IPC client|server and the corresponding TCP client|server and what arguments to use depends on what mobile platform and scenario the user runs.
There are currently a number of different scenarios described in different sources of documentation:
Runtime docs:
https://github.com/dotnet/runtime/blob/main/docs/design/mono/diagnostics-tracing.md
iOS SDK docs:
https://github.com/xamarin/xamarin-macios/wiki/Profiling
Android SDK docs:
https://github.com/xamarin/xamarin-android/blob/main/Documentation/guides/tracing.md
They all fall into a number of predefined scenarios and a number of "default" parameters that should be used.
This PR creates 4 new commands in dotnet-dsrouter to explicitly use the defaults described in the documentation, ios-sim, ios, android-emu, android. They all fallback to default IPC server listener for dsrouter and in order to make that simpler in use with diagnostic tooling, changes done in #4081 is also needed to simplify the process.
So lets take an example form the docs, running an app on iOS simulator. Before this PR the following dsrouter command needs to be run:
With this PR (and #4081) the above will look like:
dontet-dsrouter will output both its pid as well as what DOTNET_DiagnosticPorts that can be used to connect to it on startup.
Using a different mobile platform/scenario, like android emulator is pretty much identical, just switch ios-sim to android-emu. dotnet-dsrouter will output the exact DOTNET_DiagnosticPorts that should be used with any of the configured scenarios.