Skip to content
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 a target to run application with logging #8324

Merged
merged 6 commits into from
Sep 21, 2023

Conversation

grendello
Copy link
Contributor

We are frequently faced with the need to look at some detailed
information optionally logged by our runtime (as well as by the MonoVM
runtime in dotnet) and in order to obtain the necessary data, we ask our
users to perform a series of steps on command line.

This is a task we can easily wrap in a nice target, so that instead of
having to type 5 lines of commands the user invokes a single command
which will do the rest for them:

dotnet build -t:RunWithLogging

The above command will set the debug.mono.log property to the desired
value, increase logcat buffer, clear the buffer, start the application
waiting for it to be fully started, then pause for 1s and dump the
logcat buffer to a file.

Optionally, the $(_RunLogVerbose) property can be set to true in
order to log verbose MonoVM data and the $(_RunLogFilePath) can be set
to a logcat output file path (defaults to
$(IntermediateOutputPath)/logcat.txt)

We are frequently faced with the need to look at some detailed
information optionally logged by our runtime (as well as by the MonoVM
runtime in dotnet) and in order to obtain the necessary data, we ask our
users to perform a series of steps on command line.

This is a task we can easily wrap in a nice target, so that instead of
having to type 5 lines of commands the user invokes a single command
which will do the rest for them:

    dotnet build -t:RunWithLogging

The above command will set the `debug.mono.log` property to the desired
value, increase logcat buffer, clear the buffer, start the application
waiting for it to be fully started, then pause for 1s and dump the
logcat buffer to a file.

Optionally, the `$(_RunLogVerbose)` property can be set to `true` in
order to log verbose MonoVM data and the `$(_RunLogFilePath)` can be set
to a logcat output file path (defaults to
`$(IntermediateOutputPath)/logcat.txt`)
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Overall the code changes look good to me if CI is green. 👍

For a new public target, I think we usually document in here how to use it:

https://github.com/xamarin/xamarin-android/blob/main/Documentation/guides/building-apps/build-targets.md

<PropertyGroup>
<_MonoLog>default,assembly,timing=bare</_MonoLog>
<_MonoLog Condition=" '$(_RunLogVerbose)' != 'false' ">$(_MonoLog),mono_log_level=debug,mono_log_mask=all</_MonoLog>
<_SleepTime>1000</_SleepTime> <!-- milliseconds -->
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this a public-facing MSBuild property? So, customers could pass -p:Duration=10000? I could see that an app might take a few seconds before it crashes, and this would be helpful to grab the log.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, however I think Duration is too generic a name. Will think of something.

@grendello
Copy link
Contributor Author

Overall the code changes look good to me if CI is green. 👍

For a new public target, I think we usually document in here how to use it:

https://github.com/xamarin/xamarin-android/blob/main/Documentation/guides/building-apps/build-targets.md

I added the docs to https://github.com/xamarin/xamarin-android/blob/main/Documentation/workflow/DevelopmentTips.md as it seemed to be more contextually relevant there.

@jonathanpeppers
Copy link
Member

You might put it in both docs, but build-targets.md occasionally is synced to:

https://learn.microsoft.com/en-us/xamarin/android/deploy-test/building-apps/build-targets

Properties which affect how the target works:

* `/p:RunLogVerbose=true` enables even more verbose logging from MonoVM
* `/p:RunLogDelay=X` where `X` should be replaced with time in milliseconds to wait before writing the
Copy link
Contributor

Choose a reason for hiding this comment

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

RunLogDelayMS or RunLogDelayInMS might be a nicer UX.

Copy link
Contributor

@dellis1972 dellis1972 left a comment

Choose a reason for hiding this comment

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

Looks great.

I do wonder if we should add a unit test to the On Device Tests to check that this target runs and produces a log file... just in case something breaks it in the future.
It might not be needed though.

@grendello
Copy link
Contributor Author

Looks great.

I do wonder if we should add a unit test to the On Device Tests to check that this target runs and produces a log file... just in case something breaks it in the future. It might not be needed though.

The commands it executes are pretty basic, if they break then we're probably going to notice similar issues in all the on-device tests, since they also execute a set of similar adb shell commands on app deployment.

@grendello
Copy link
Contributor Author

I think the MAUI integration test failures can be ignored, they are workload installation issues, not related to this PR.

@jonpryor jonpryor merged commit 791fedb into dotnet:main Sep 21, 2023
45 of 47 checks passed
@grendello grendello deleted the android-logging branch September 21, 2023 22:16
grendello added a commit to grendello/xamarin-android that referenced this pull request Sep 22, 2023
* main:
  [Xamarin.Android.Build.Tasks] Add `RunWithLogging` target (dotnet#8324)
  [Xamarin.Android.Build.Tasks] AndroidJavaSource refs dependent jars (dotnet#8194)
  Bump to xamarin/Java.Interop/main@75d8221 (dotnet#8336)
  Bump NDK to r26 (dotnet#8213)
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 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.

5 participants