New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 861733 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Perf Windows bots failing to find logdog_helper and write out any logs

Project Member Reported by eyaich@chromium.org, Jul 9

Issue description

Our src side merge script, process_perf_results.py (https://cs.chromium.org/chromium/src/tools/perf/process_perf_results.py?q=logdog_helpe&sq=package:chromium&dr=C&l=27) is unable to write out logdog streams on windows.  It works on mac and linux and android.  

Is there somewhere we have to add a dependency

We get the following stack track: 
  Traceback (most recent call last):
    File "e:\b\swarm_slave\w\ir\tools\perf\process_perf_results_unittest.py", line 63, in setUp
      m2.start()
    File "e:\b\swarm_slave\w\ir\third_party\catapult\telemetry\third_party\mock\mock.py", line 1396, in start
      result = self.__enter__()
    File "e:\b\swarm_slave\w\ir\third_party\catapult\telemetry\third_party\mock\mock.py", line 1252, in __enter__
      self.target = self.getter()
    File "e:\b\swarm_slave\w\ir\third_party\catapult\telemetry\third_party\mock\mock.py", line 1414, in <lambda>
      getter = lambda: _importer(target)
    File "e:\b\swarm_slave\w\ir\third_party\catapult\telemetry\third_party\mock\mock.py", line 1102, in _importer
      thing = _dot_lookup(thing, comp, import_path)
    File "e:\b\swarm_slave\w\ir\third_party\catapult\telemetry\third_party\mock\mock.py", line 1091, in _dot_lookup
      __import__(import_path)
  ImportError: No module named logdog_helper

 
Where is process_perf_results_unittest? I don't see it in the repo... are we seeing this error outside of the tests? The import is question is guarded by an `except ImportError` that should handle this case.
oh.

Given that the import in process_perf_results is guarded by a try/except for this scenario, it likely makes sense to do something similar in the test, right?
John: the real request here we wants to have logdog_helper on Windows platform, so that we get the right logging there as well.
ah. I misunderstood.

I expect that you'll have to isolate logdog_helper and its dependencies in order to do that. Right now, it's only part of //build/android:test_runner_py; we probably don't want to add that as a data dependency of t_p_u on all platforms.
I suspect it may be easiest to either copy the implementation of logdog_helper to tools/perf or just move logdog_helper implementation into tools/swarming_client/
I don't like the idea of copying the implementation. Whether logdog_helper should be in the logdog client library is a question for the foundation folks. As-is, it's a pretty minimal wrapper around the existing client library that makes it nicer to work with.
I think the easiest way to move forward with this bug is to make sure that windows builder also include "//build/android/pylib/" and  "//tools/swarming_client/". I did that in https://chromium-review.googlesource.com/c/chromium/src/+/1127845/10/chrome/test/BUILD.gn and it does fix the issue on win_chromium_rel_ng.

John: do you know where we set which directory to include for builders? How do we know what are currently in our windows test builders?
Cc: eyaich@chromium.org
Components: Speed>Benchmarks>Waterfall
Owner: ----
Status: Available (was: Untriaged)
Cc: iannucci@chromium.org mar...@chromium.org
Marc/Robbie: is it ok to move https://cs.chromium.org/chromium/src/build/android/pylib/utils/logdog_helper.py?q=logdog_helper&dr&l=21 to src/tools/swarming_clients to make the logdog library a bit easier to work with?
Interesting fact: libs/logdog/ is literally not imported by anything inside client/.

It's used by two things:
- android specific logdog_helper
- recipes-py/recipe_engine/recipe_api.py

E.g. maybe the whole thing should be moved elsewhere?
Cc: crouleau@chromium.org perezju@chromium.org nedngu...@google.com
 Issue 892185  has been merged into this issue.
Cc: iannu...@google.com
Cc: -iannucci@chromium.org
 Issue 917094  has been merged into this issue.

Sign in to add a comment