OfflinePages.ClearStoragePreRunUsage.* are being reported in bytes instead of MiB |
||||||||||||
Issue descriptionThe reported values of the OfflinePages.ClearStoragePreRunUsage.* series of metrics are not being converted from the raw bytes values to MiB.
,
Oct 19 2017
dimich@ as you didn't participate in the CL review, would you please do a review pass of your own to comply with "Have the bug verified and assured that it's considered safe by other developers working in the same area". This is a step needed to allow merging to the beta branch.
,
Oct 20 2017
Android Canary 64.0.3245.0 was released today and includes the CL from #c1. Starting the 24h wait time now.
,
Oct 21 2017
I did review the CL in question as required by the process outlined in "M63 branch is here!" email sent to the team. The change LGTM and seems to be small, local and free of issues. I think it is relatively safe to merge, if otherwise approved.
,
Oct 23 2017
I'd like to merge the change from #1 into the M63 branch. It has tests, has been doubly verified and has been deployed with Canary for more than 24h. The histograms fixed by that change were found to be reported using the wrong unit what rendered their results practically useless. We intend to rely on them for monitoring disk space usage of two Offline Pages related features: Prefetching of Offline Pages (in experimentation, target launch of M63) and Offline Page Cache (launched; will be reviewed for landing). Without this fix there will be no direct measurement of that metric.
,
Oct 24 2017
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/179b754f77b7007ce76f173cc43ed0b36e802524 commit 179b754f77b7007ce76f173cc43ed0b36e802524 Author: Carlos Knippschild <carlosk@chromium.org> Date: Tue Oct 24 19:53:50 2017 Fix reporting of OfflinePages.ClearStoragePreRunUsage.* metrics (M63 merge) Fixes the OfflinePages.ClearStoragePreRunUsage.* series of metrics -- renaming them to ClearStoragePreRunUsage2 -- so that they report their values in KiB. They were previously expected to be reported in MiB but were incorrectly reported in bytes. Bug: 775321 Change-Id: Ia12f0c039172292770ee066efc128b0fd9c8204e Reviewed-on: https://chromium-review.googlesource.com/722033 Commit-Queue: Carlos Knippschild <carlosk@chromium.org> Reviewed-by: Justin DeWitt <dewittj@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Reviewed-by: Peter Williamson <petewil@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#510191}(cherry picked from commit de426dcb636e12f2a90ea783cd8f42476473488e) Reviewed-on: https://chromium-review.googlesource.com/736119 Reviewed-by: Carlos Knippschild <carlosk@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#189} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/179b754f77b7007ce76f173cc43ed0b36e802524/components/offline_pages/core/offline_page_storage_manager.cc [modify] https://crrev.com/179b754f77b7007ce76f173cc43ed0b36e802524/components/offline_pages/core/offline_page_storage_manager.h [modify] https://crrev.com/179b754f77b7007ce76f173cc43ed0b36e802524/components/offline_pages/core/offline_page_storage_manager_unittest.cc [modify] https://crrev.com/179b754f77b7007ce76f173cc43ed0b36e802524/tools/metrics/histograms/histograms.xml
,
Oct 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7169f118b1c26911a504e1746d33559c345e8045 commit 7169f118b1c26911a504e1746d33559c345e8045 Author: Carlos Knippschild <carlosk@chromium.org> Date: Wed Oct 25 23:41:52 2017 Fix ClearStoragePreRunUsage histogram name This histogram was renamed but the code reporting it was not updated accordingly. This CL fixes that. Bug: 775321 Change-Id: I155239b3ccf187754c0c2536f6f44535ac3fe089 Reviewed-on: https://chromium-review.googlesource.com/737957 Reviewed-by: Justin DeWitt <dewittj@chromium.org> Reviewed-by: Peter Williamson <petewil@chromium.org> Commit-Queue: Carlos Knippschild <carlosk@chromium.org> Cr-Commit-Position: refs/heads/master@{#511647} [modify] https://crrev.com/7169f118b1c26911a504e1746d33559c345e8045/components/offline_pages/core/offline_page_storage_manager.cc [modify] https://crrev.com/7169f118b1c26911a504e1746d33559c345e8045/components/offline_pages/core/offline_page_storage_manager_unittest.cc
,
Oct 26 2017
Today we found a histogram naming bug in the initial CL that landed on master with as described in #8. A new beta merge will be needed. Waiting for it to land on Canary.
,
Oct 26 2017
Change from #8 was released on Canary 64.0.3250.0 today. Waiting 24h.
,
Oct 27 2017
Requesting merge of the change from #8 to the M63 branch.
,
Oct 27 2017
I guess I should remove the "merged" labels from before...
,
Oct 27 2017
The bug is marked as P3 or Feature. It should not be merged as M63 is in beta. Please contact the approriate milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 31 2017
,
Oct 31 2017
Re-adding Merge-Request.
,
Oct 31 2017
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 31 2017
Approved the merge into M63 branch 3239. Please make sure to verify the fix after merging it.
,
Nov 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f2433fadbad542d2b06873faa1639b0e1c2656b8 commit f2433fadbad542d2b06873faa1639b0e1c2656b8 Author: Carlos Knippschild <carlosk@chromium.org> Date: Wed Nov 01 00:29:23 2017 Fix ClearStoragePreRunUsage histogram name (M63 merge) This histogram was renamed but the code reporting it was not updated accordingly. This CL fixes that. Bug: 775321 Change-Id: I155239b3ccf187754c0c2536f6f44535ac3fe089 Reviewed-on: https://chromium-review.googlesource.com/737957 Reviewed-by: Justin DeWitt <dewittj@chromium.org> Reviewed-by: Peter Williamson <petewil@chromium.org> Commit-Queue: Carlos Knippschild <carlosk@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#511647}(cherry picked from commit 7169f118b1c26911a504e1746d33559c345e8045) Reviewed-on: https://chromium-review.googlesource.com/748281 Reviewed-by: Carlos Knippschild <carlosk@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#330} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/f2433fadbad542d2b06873faa1639b0e1c2656b8/components/offline_pages/core/offline_page_storage_manager.cc [modify] https://crrev.com/f2433fadbad542d2b06873faa1639b0e1c2656b8/components/offline_pages/core/offline_page_storage_manager_unittest.cc
,
Nov 7 2017
The fix has landed and has been merged. I also confirmed the histograms is working by looking at the dashboards. |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by bugdroid1@chromium.org
, Oct 19 2017