Dump logs on failing devicelab test to recipe artifact location #74378

jmagman · 2021-01-21T04:53:35Z

Upload test logs for LUCI tests.

HostAgent.dumpDirectory defaults to env FLUTTER_LOGS_DIR which is a path set up in the LUCI recipe which will be uploaded to GCS and viewable in the test run UI (https://flutter-review.googlesource.com/c/recipes/+/9944):

Fallback to a tmp directory when FLUTTER_LOGS_DIR env does not exist (like when run locally).

As a proof-of-concept, use the new HostAgent.dumpDirectory in a test. Persist module_test_ios XCUITest results (This would have helped with test failures like #70630).

Future work

Add dump method to the Device class to collect additional logs, to be called on test failures for connected devices. For example, AndroidDevice.dump() could upload abd dumpsys meminfo, abd dumpsys procstats, etc to the uploaded directory.

#74344

jmagman · 2021-01-22T19:04:31Z

https://flutter-review.googlesource.com/c/recipes/+/9941 merged.

Trying at https://ci.chromium.org/p/flutter/builders/try/Mac%20module_test_ios/2701

jmagman · 2021-01-22T20:00:51Z

Logs show it writing files to the recipe_cleanup directory
-resultBundlePath /opt/s/w/ir/k/recipe_cleanup/flutter_logs_dir/module_test_ios-objc-2021-01-22T11:12:20.902358
but I don't see any files in run module_test_ios https://ci.chromium.org/ui/p/flutter/builders/try/Mac%20module_test_ios/2701/steps?succeeded=true&debug=false

@godofredoc Is there more needed to get this working than the recipe change?

[Edit: I know the recipe change was reverted, we can look once it re-lands]

jmagman · 2021-01-22T22:12:27Z

I think this is working (it found the recipe_cleanup/flutter_logs_dir log directory from FLUTTER_LOGS_DIR env and wrote test results there). We can merge this whenever and investigate why the logs weren't actually uploaded later.

godofredoc · 2021-01-23T03:34:54Z

I think this is working (it found the recipe_cleanup/flutter_logs_dir log directory from FLUTTER_LOGS_DIR env and wrote test results there). We can merge this whenever and investigate why the logs weren't actually uploaded later.

Here is a task execution using this PR and my updates to the recipes: https://chromium-swarm.appspot.com/task?id=5145de6679dca410 please take a look.

jmagman · 2021-01-25T17:54:02Z

It sure looks like it's working @godofredoc!

I'm going to get rid of the simulator system log collection, seems like it'll be a lot of noise, though it was a good test.

jmagman · 2021-01-25T18:24:14Z

@godofredoc I can't figure out that UI, how can I see this PR, your updates to the recipe, and the module_test_ios test?

jonahwilliams

I don't really have the context to review this, but happy to see more logs being made available :)

jonahwilliams · 2021-01-25T19:52:46Z

(that said, if you wanted to GVC real quick and walk me through it, that might help)

jmagman · 2021-01-25T20:17:12Z

I don't really have the context to review this, but happy to see more logs being made available :)

I expanded on the description a bit for context. Essentially, it's finding the FLUTTER_LOGS_DIR environment variable and saving any interesting logs to that path (with fallback to a tmp directory if not found). The contents of that directory will be uploaded and be available on the test run summary page for download (the upload is handled in the recipe, so not this PR). As a proof-of-concept I added uploading the module_test_ios Xcode test results bundle to this location so we can see if it works.

jonahwilliams · 2021-01-25T20:45:42Z

dev/devicelab/lib/framework/host_agent.dart

Is there a risk of us dumping old logs on a particular test with this sort of host based structure?

i.e. an agent runs a simulator task, then crashes running a physical ios test. The dumped logs include both combined.

or I guess this could happen from the same agent - do these logs get cleaned up?

I asked this same question. 😄

Me:

Does this directory need to be removed after upload so the logs don't bleed between tests?

@godofredoc:

The good news is that directory is the in the api.path['cleanup'] which luci cleans up automatically after the task execution.

As for these particular simulator logs, they actually aren't cleaned up between tests, so we'd need to correlate timestamps. For example, the example task execution at started at Jan 22 18:26 but shows simulator logs from Jan 17 18:08:47 (I've since removed the system.log files from upload, but it does upload app crashes from the simulator on task failure). I can file a recipe issue to remove this directory before the Xcode select step, but I don't think this test runner code should worry about that.

However if we say uploaded a adb logcat with a timestamp of the beginning of the test and saved it to the directory, it would be cleaned up after task execution and wouldn't be present in any other tasks.

something, something great minds.

I'd be concerned about leaving all of the responsibility of deleting old logs (outside of the upload dir) to the recipes. If someone less familiar with our CI system (most of us) wants to add new logs, then they would also need to know the change the recipe in the right way.

Checking timestamps here seems like a reasonable approach. And yeah, if logs come through adb with a timestamp start they should be fairly up to date.

If someone less familiar with our CI system (most of us) wants to add new logs, then they would also need to know the change the recipe in the right way.

I hope that would be rare and these artifacts would be more transient, like screenshots, or device log dumps, or IDE test logs, or size analysis files. We could push more of that stuff into this upload directory instead of dumping it all into the task stdout as we do now. I just happened to think of simulator app crashes first, and I had hoped they would be cleaned up between Xcode installs, but I guess not.

what I would like is for this to fail in a way that doesn't upload logs/files instead of uploading old logs/files, because the former is much easier to notice immediately

Are those logs generated in deriveddata? if yes, deriveddata is cleaned up before running every task.

Are those logs generated in deriveddata? if yes, deriveddata is cleaned up before running every task.

Unfortunately not.

This has been removed.

godofredoc · 2021-01-26T02:35:30Z

see this PR, your updates to the recipe, and the module_test_ios test?

I relanded the change a few hours ago, let me re-trigger the all the build in this PR to see the output.

jmagman · 2021-01-27T23:49:34Z

I narrowed the scope of this change. I removed the platform implementations of HostAgent and just exposed the dumpDirectory. All this does now is upload the XCUITest results for module_test_ios.

I can add more things we find useful in future PRs.

jmagman · 2021-01-29T22:36:44Z

Ready for re-review, @jonahwilliams

jonahwilliams

LGTM

jmagman added c: contributor-productivity Team-specific productivity, code health, technical debt. team: flakes labels Jan 21, 2021

jmagman self-assigned this Jan 21, 2021

google-cla bot added the cla: yes label Jan 21, 2021

jmagman mentioned this pull request Jan 21, 2021

Add functionality to copy logs, screenshots, and other artifacts to LUCI recipe location in engine repo #74344

Closed

jmagman force-pushed the xcuitest-save branch from 9db8864 to faeb23f Compare January 21, 2021 19:09

jmagman requested review from christopherfujino and jonahwilliams January 22, 2021 22:10

jmagman changed the title ~~Dump state on failing devicelab test to recipe artifact location~~ Dump logs on failing devicelab test to recipe artifact location Jan 25, 2021

jmagman force-pushed the xcuitest-save branch from 17cc4b4 to 2b1f961 Compare January 25, 2021 19:08

jonahwilliams reviewed Jan 25, 2021

View reviewed changes

jmagman added 4 commits January 27, 2021 15:43

Dump state on failing devicelab test to recipe artifact location

41df408

Add log to see where the dump directory lives

48f4fdb

Remove sim system log collection

ddcb13b

Remove platform-specific implementation, simulator logs

ae9fff7

jmagman force-pushed the xcuitest-save branch from 2b1f961 to ae9fff7 Compare January 27, 2021 23:47

jonahwilliams approved these changes Feb 1, 2021

View reviewed changes

jmagman merged commit f4f33fd into flutter:master Feb 1, 2021

jmagman deleted the xcuitest-save branch February 1, 2021 23:00

jmagman mentioned this pull request Feb 9, 2021

Compress module_test_ios xcresult in LUCI logs #75730

Closed

jmagman mentioned this pull request Feb 17, 2021

Upload test result files to recipe artifact location #76245

Closed

Dump logs on failing devicelab test to recipe artifact location #74378

Dump logs on failing devicelab test to recipe artifact location #74378

Uh oh!

Conversation

jmagman commented Jan 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Future work

Uh oh!

jmagman commented Jan 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmagman commented Jan 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmagman commented Jan 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

godofredoc commented Jan 23, 2021

Uh oh!

jmagman commented Jan 25, 2021

Uh oh!

jmagman commented Jan 25, 2021

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Jan 25, 2021

Uh oh!

jmagman commented Jan 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

godofredoc commented Jan 26, 2021

Uh oh!

jmagman commented Jan 27, 2021

Uh oh!

jmagman commented Jan 29, 2021

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jmagman commented Jan 21, 2021 •

edited

Loading

jmagman commented Jan 22, 2021 •

edited

Loading

jmagman commented Jan 22, 2021 •

edited

Loading

jmagman commented Jan 22, 2021 •

edited

Loading

jmagman commented Jan 25, 2021 •

edited

Loading