Issue 609742 in chromium: [Telemetry] IsChromeTracingSupported() is called before browser spinning up

[deanliao@chromium.org via Monorail]

Status: Untriaged
Owner: ach...@chromium.org
CC: nedn...@chromium.org, tele...@chromium.org
Labels: Hotlist-Google Test-Performance
Components: Tests>Telemetry

New issue 609742 by dean...@chromium.org: [Telemetry] IsChromeTracingSupported() is called before browser spinning up
https://bugs.chromium.org/p/chromium/issues/detail?id=609742

I found that Telemetry's story_runner's _RunStoryAndProcessErrorIfNeeded()'s code block
try:
if isinstance(test, story_test.StoryTest):
test.WillRunStory(state.platform)
state.WillRunStory(story)

may cause IsChromeTracingSupported() is called before browser spinning up, which makes it returns false, which may raises exception. We should make sure that browser is started before calling IsChromeTracingSupported(). I'm not sure if it is a good idea to change the code to run state.WillRunStory(story) before test.WillRunStory().

Here is the story:

I run ToT Telemetry benchmark smoothness.maps on ChromeOS. It raises exception "Not supported":
./run_benchmark --remote=192.168.0.13 --browser=cros-chrome smoothness.maps -d
…
[ RUN ] Maps.maps_002
...
Traceback (most recent call last):
File ".../telemetry/internal/story_runner.py", line 77, in _RunStoryAndProcessErrorIfNeeded
test.WillRunStory(state.platform)
File ".../telemetry/web_perf/timeline_based_measurement.py", line 274, in WillRunStory
raise Exception('Not supported')
Exception: Not supported

The exception is raised because platform.tracing_controller.IsChromeTracingSupported() returns False in WillRunStory() in timeline_based_measurement.py().

However, smoothness.top_25_smooth can run on CrOS. Why? I compare two benchmarks’ running trace and here is the key:

In _RunStoryAndProcessErrorIfNeeded() in story_runner.py:
try:
if isinstance(test, story_test.StoryTest):
test.WillRunStory(state.platform)
state.WillRunStory(story)

And smoothness.maps’ self.test object (measurement) is TimelineBasedMeasurement (derived from base Benchmark class), which is derived from StoryTest class, so it invokes test.WillRunStory(state.platform). And TBM.WillRunStory() checks platform.tracing_controller.IsChromeTracingSupport() without spinning up CrOS browser first so it doesn’t have tracing backends at that time. Thus TBM.WillRunStory() raises exception.

For smoothness.top_25_smooth, its self.test object is Smoothness, which is derived from LegacyPageTest, which is not derived from StoryTest, so it just calls state.WillRunStory(story). In SharedPageState.WillRunStory(), it doesn’t check IsChromeTracingSupport(), but it spins up browser (self._StartBrowser). Then after that, state.RunStory(results) is called (in _RunStoryAndProcessErrorIfNeeded ), and then eventually called IsSupported in chrome_tracing_agent and gets True result.

--
You received this message because:
1. You were specifically CC'd on the issue

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Reply to this email to add a comment.

[nednguyen@google.com via Monorail]

Comment #1 on issue 609742 by nedngu...@google.com: [Telemetry] IsChromeTracingSupported() is called before browser spinning up
https://bugs.chromium.org/p/chromium/issues/detail?id=609742#c1

That was intentional for startup tracing against chrome on desktop & android. Does Cros support startup tracing?

[deanliao@chromium.org via Monorail]

Comment #2 on issue 609742 by dean...@chromium.org: [Telemetry] IsChromeTracingSupported() is called before browser spinning up
https://bugs.chromium.org/p/chromium/issues/detail?id=609742#c2

So the flow of chrome which supports startup tracing, it calls test.WillRunStory() to start tracing then calls state.WillRunStory() to restart browser if needed, right?

Now the question is: for CrOS chrome, platform.tracing_controller.IsChromeTracingSupported() returns False because it is marked as not support startup tracing and a devtool client instance is not found (because there's no browser instance at that time).

For such case, is it okay to let state.WillRunStory() runs first to spin up browser then calls test.WillRunStory() start start tracing?

[nednguyen@google.com via Monorail]

Updates:
Cc: zh...@chromium.org

Comment #3 on issue 609742 by nedngu...@google.com: [Telemetry] IsChromeTracingSupported() is called before browser spinning up
https://bugs.chromium.org/p/chromium/issues/detail?id=609742#c3

Zhenw: can help untangle this. We don't have enough buffer to handle this now, you can wait until Zhen is back.

[deanliao@chromium.org via Monorail]

Updates:
Labels: -Pri-1 Pri-2

Comment #4 on issue 609742 by dean...@chromium.org: [Telemetry] IsChromeTracingSupported() is called before browser spinning up
https://bugs.chromium.org/p/chromium/issues/detail?id=609742#c4

As long as it affects benchmark test cases with self.test derived from story_test.StoryTest (e.g. TimelineBasedMeasurement) and it seems only a few tests use that directly, it is not that urgent. Change to P2.

[achuith via monorail]

Updates:
Cc: -nedn...@chromium.org
Status: Archived

Comment #8 on issue 609742 by ach...@chromium.org: [Telemetry] IsChromeTracingSupported() is called before browser spinning up
https://bugs.chromium.org/p/chromium/issues/detail?id=609742#c8

(No comment was entered for this change.)

--
You received this message because:
1. You were specifically CC'd on the issue

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Reply to this email to add a comment or make updates.