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

Issue 810616 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Telemetry should only use story name for indexing wpr archives

Project Member Reported by nednguyen@chromium.org, Feb 9 2018

Issue description

From sunnpys@: 
"the espn page is named "ESPN" but in the json file it's only referenced by URL. Some other pages which are named, e.g. wikipedia, are referenced by their name in the json"

When I look at the implementation, this is because we have the fallback code in https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py?rcl=e653c4b823f98c26bd29e4cf4e3bdacdffb4b041&l=118

This seems to be very error-prone. We should make sure that Telemetry only use story.name for indexing archive
 
I tweaked fetch_benchmark_deps.py to scan for name not in archives cases, results:

story.name "Wikipedia" not found in archive: top_10.json
story.url "http://en.wikipedia.org/wiki/Wikipedia" is in archive
story.name "abcnews" not found in archive: desktop_power_stories.json
story.url "http://abcnews.go.com/" is in archive
story.name "slideshare" not found in archive: desktop_power_stories.json
story.url "http://www.slideshare.net/patrickmeenan" is in archive
story.name "instagram" not found in archive: desktop_power_stories.json
story.url "https://instagram.com/cnn/" is in archive
story.name "sina" not found in archive: desktop_power_stories.json
story.url "http://www.sina.com.cn" is in archive
story.name "uol" not found in archive: desktop_power_stories.json
story.url "http://www.uol.com.br" is in archive
story.name "indiatimes" not found in archive: desktop_power_stories.json
story.url "http://www.indiatimes.com" is in archive
story.name "microsoft" not found in archive: desktop_power_stories.json
story.url "http://www.microsoft.com" is in archive

Is it OK to modify top_10.json and desktop_power_stories.json directly to fix these name not in archive issues?
The "ESPN" case is fixed by https://crrev.com/c/942319
Cc: nednguyen@chromium.org
I think it should be fine to just fix those json files, and that seems the most simple thing to do. Right?
Scanned all data/*.json that includes http:// or https://

dromaeo.json, jetstream.json, kraken.json, octane.json, oortonline.json, speedometer.json, startup_pages.json: name equals to url.

intl_ar_fa_he.json, intl_es_fr_pt-BR.json, intl_hi_ru.json, intl_ja_zh.json, intl_ko_th_vi.json, typical_10_mobile.json, typical_25.json, v8_top_25.json: name equals to url with suffix

desktop_power_stories.json: will fix the indices
dromaeo.*.json: will be removed
top_10.json: the index wikipedia should be fixed. name equals to url in other indices.
top_25.json: not used by any benchmark, shall be removed.

All active benchmark would use name for archive indexing after fixing the indices listed in #1.
long_running_idle_gmail_page.json: name equals to url
Cc: vovoy@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e4516e1c336f2647d92073f75c6552325fb15eac

commit e4516e1c336f2647d92073f75c6552325fb15eac
Author: Kuo-Hsin Yang <vovoy@chromium.org>
Date: Thu Aug 02 15:29:56 2018

Use story name instead of story url for archive index

Also remove unused indices in desktop_power_stories.json.

Bug:  chromium:810616 
Change-Id: Id12e4ed6bf8433de5a7c1a08e5e86b3ad1b56bb0
Reviewed-on: https://chromium-review.googlesource.com/1160122
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Vovo Yang <vovoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580199}
[modify] https://crrev.com/e4516e1c336f2647d92073f75c6552325fb15eac/tools/perf/page_sets/data/desktop_power_stories.json
[modify] https://crrev.com/e4516e1c336f2647d92073f75c6552325fb15eac/tools/perf/page_sets/data/top_10.json

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 3

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/e16a2e80be666155a5be3ec6a0fc957b4697ba13

commit e16a2e80be666155a5be3ec6a0fc957b4697ba13
Author: Kuo-Hsin Yang <vovoy@chromium.org>
Date: Fri Aug 03 01:10:20 2018

Look up wpr archives by story name only

Archive entries indexed by story url have been cleaned up. Remove the
fallback code to look up archive by story url.

Bug:  chromium:810616 
Change-Id: I40fb40354e1e3a8049dd2a5413fb1a2364cf7b14
Reviewed-on: https://chromium-review.googlesource.com/1160826
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Vovo Yang <vovoy@chromium.org>

[modify] https://crrev.com/e16a2e80be666155a5be3ec6a0fc957b4697ba13/telemetry/telemetry/wpr/archive_info.py

Owner: vovoy@chromium.org
Status: Fixed (was: Untriaged)
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c942eaaa913713e49506b3f961e0bc754648143

commit 2c942eaaa913713e49506b3f961e0bc754648143
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Aug 03 03:18:03 2018

Roll src/third_party/catapult eae13a4b34cc..e16a2e80be66 (2 commits)

https://chromium.googlesource.com/catapult.git/+log/eae13a4b34cc..e16a2e80be66


git log eae13a4b34cc..e16a2e80be66 --date=short --no-merges --format='%ad %ae %s'
2018-08-03 vovoy@chromium.org Look up wpr archives by story name only
2018-08-02 zmo@chromium.org Hookup finder_options.browser_options.wpr_mode for gpu integration tests.


Created with:
  gclient setdep -r src/third_party/catapult@e16a2e80be66

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:810616 ,chromium:867155
TBR=sullivan@chromium.org

Change-Id: I18bcbd9dffcaadf0d66ca2ccc92ee7be45238e82
Reviewed-on: https://chromium-review.googlesource.com/1161146
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#580433}
[modify] https://crrev.com/2c942eaaa913713e49506b3f961e0bc754648143/DEPS

Comment 11 by benhenry@google.com, Jan 16 (6 days ago)

Components: Test>Telemetry

Comment 12 by benhenry@google.com, Jan 16 (6 days ago)

Components: -Speed>Telemetry

Sign in to add a comment