OnDisk persistent metrics can be reported with the wrong field trials |
|||||
Issue descriptionOnDisk persistent metrics can be reported with the wrong field trials. See this report: https://groups.google.com/a/google.com/forum/#!topic/uma-users/tvraijd1Ucw
,
Feb 24 2017
This could be because we have a timing whole on how we capture the previous system profile - it could come from a run that doesn't correspond to the OnDisk persistent histogram file that we're uploading. To fix this, how about we add the system profile into the memory mapped metrics file - and report them in a separate log with that profile?
,
Feb 24 2017
That should be easily done. The allocator can take any kind of record; they'll just be ignored by the histogram system. Am I correct in understanding that this would have affected all previous "stability" histograms as well?
,
Feb 24 2017
How about we take a small step back and think about the right way to do this? Right now it sounds like we're proposing that the field trials state be stored in no fewer than 3 shared memory segment: 1. The browser->renderer segment. 2. The crumbs. 3. The histogram segment. This seems - suboptimal - to me?
,
Feb 24 2017
Also, saving the records isn't too difficult. Keeping them up-to-date is more so.
,
Feb 24 2017
So if we don't save them together, then there's a chance things they can get out of sync - since each of those have different lifetimes. Maybe we can get rid of the one in local state entirely if we put things in the histograms segment. Maybe crumbs could also use the info from the histograms segment?
,
Mar 6 2017
The histograms segment is currently cleaned up very early during browser startup so that would require some changes. However, the upcoming "config" segment might be a better choice. It's what is shared with all sub-processes and will need to be kept around as a reference for the next browser startup anyway.
,
Apr 3 2017
,
Apr 12 2017
There is a Q2 OKR to embed the full system profile inside the .pma file and so always properly associate the metrics within.
,
Apr 12 2017
I'd love to discuss this with whoever takes it on. Esp. wrt crumbs duplication.
,
May 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4cde5b7b932ff3ae33107778f592fc21021a63c1 commit 4cde5b7b932ff3ae33107778f592fc21021a63c1 Author: bcwhite <bcwhite@chromium.org> Date: Tue May 30 15:26:43 2017 Support persistent system profiles. This allows the entire system profile to be saved into one or more persistent memory segments for use after the process exits. The initial version saves only the snapshot known during metrics reporting but later versions will build upon this to keep the profile up-to-date in real time. BUG=695880 Review-Url: https://codereview.chromium.org/2907543003 Cr-Commit-Position: refs/heads/master@{#475534} [modify] https://crrev.com/4cde5b7b932ff3ae33107778f592fc21021a63c1/base/metrics/persistent_memory_allocator.cc [modify] https://crrev.com/4cde5b7b932ff3ae33107778f592fc21021a63c1/base/metrics/persistent_memory_allocator.h [modify] https://crrev.com/4cde5b7b932ff3ae33107778f592fc21021a63c1/chrome/browser/chrome_browser_field_trials.cc [modify] https://crrev.com/4cde5b7b932ff3ae33107778f592fc21021a63c1/components/metrics/BUILD.gn [modify] https://crrev.com/4cde5b7b932ff3ae33107778f592fc21021a63c1/components/metrics/metrics_log.cc [add] https://crrev.com/4cde5b7b932ff3ae33107778f592fc21021a63c1/components/metrics/persistent_system_profile.cc [add] https://crrev.com/4cde5b7b932ff3ae33107778f592fc21021a63c1/components/metrics/persistent_system_profile.h [add] https://crrev.com/4cde5b7b932ff3ae33107778f592fc21021a63c1/components/metrics/persistent_system_profile_unittest.cc
,
Jun 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e72aa59dfdea6e261b06d8061e488f76801898f commit 8e72aa59dfdea6e261b06d8061e488f76801898f Author: bcwhite <bcwhite@chromium.org> Date: Fri Jun 09 18:59:18 2017 Persistent metrics files that have an embedded profile don't need to be sent during startup. Instead, send them as independent logs once the browser is already running. BUG=695880 Review-Url: https://codereview.chromium.org/2918533003 Cr-Commit-Position: refs/heads/master@{#478359} [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/chrome/browser/metrics/chrome_metrics_service_client.cc [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/file_metrics_provider.cc [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/file_metrics_provider.h [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/file_metrics_provider_unittest.cc [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/metrics_log.cc [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/metrics_log.h [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/metrics_log_store.cc [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/metrics_provider.cc [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/metrics_provider.h [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/metrics_service.cc [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/metrics_service.h [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/metrics_service_unittest.cc [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/persistent_system_profile.cc [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/persistent_system_profile.h [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/components/metrics/persistent_system_profile_unittest.cc [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/tools/metrics/histograms/enums.xml [modify] https://crrev.com/8e72aa59dfdea6e261b06d8061e488f76801898f/tools/metrics/histograms/histograms.xml
,
Jun 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8ccb6aa1a42c3e72c4348b80932679b7ac6edb21 commit 8ccb6aa1a42c3e72c4348b80932679b7ac6edb21 Author: bcwhite <bcwhite@chromium.org> Date: Wed Jun 14 21:45:03 2017 Persist core system profile during startup. Almost 40% of persistent metrics files are missing the embedded system profile which is stored about 1 minute into the process lifetime when the initial metrics upload is sent. This distinguishes between basic and complete system profiles allowing the core profile to be stored when the Log is created and be replaced by a more complete version later. Thus basic profile will thus be available even if the browser crashes almost immediately. BUG=695880 Review-Url: https://codereview.chromium.org/2938013002 Cr-Commit-Position: refs/heads/master@{#479513} [modify] https://crrev.com/8ccb6aa1a42c3e72c4348b80932679b7ac6edb21/components/metrics/file_metrics_provider_unittest.cc [modify] https://crrev.com/8ccb6aa1a42c3e72c4348b80932679b7ac6edb21/components/metrics/metrics_log.cc [modify] https://crrev.com/8ccb6aa1a42c3e72c4348b80932679b7ac6edb21/components/metrics/persistent_system_profile.cc [modify] https://crrev.com/8ccb6aa1a42c3e72c4348b80932679b7ac6edb21/components/metrics/persistent_system_profile.h [modify] https://crrev.com/8ccb6aa1a42c3e72c4348b80932679b7ac6edb21/components/metrics/persistent_system_profile_unittest.cc
,
Jun 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c1ddec34b40c28a89bea7e53eea719e62d6d3df3 commit c1ddec34b40c28a89bea7e53eea719e62d6d3df3 Author: bcwhite <bcwhite@chromium.org> Date: Thu Jun 29 14:13:04 2017 Support add-on 'field trial' records. Field trials are activated at any time so its important to record their presence immediately rather than wait for the next "complete" system profile to be provided. These are written as "extension" records and otherwise ignored until recovery of the profile at which point their contents are used to extend the stored version. BUG=695880 Review-Url: https://codereview.chromium.org/2950173003 Cr-Commit-Position: refs/heads/master@{#483355} [modify] https://crrev.com/c1ddec34b40c28a89bea7e53eea719e62d6d3df3/chrome/browser/metrics/field_trial_synchronizer.cc [modify] https://crrev.com/c1ddec34b40c28a89bea7e53eea719e62d6d3df3/components/metrics/persistent_system_profile.cc [modify] https://crrev.com/c1ddec34b40c28a89bea7e53eea719e62d6d3df3/components/metrics/persistent_system_profile.h [modify] https://crrev.com/c1ddec34b40c28a89bea7e53eea719e62d6d3df3/components/metrics/persistent_system_profile_unittest.cc [modify] https://crrev.com/c1ddec34b40c28a89bea7e53eea719e62d6d3df3/components/variations/active_field_trials.cc [modify] https://crrev.com/c1ddec34b40c28a89bea7e53eea719e62d6d3df3/components/variations/active_field_trials.h
,
Jun 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/757034c63af4b1c61eeebf070e2123f787be51f7 commit 757034c63af4b1c61eeebf070e2123f787be51f7 Author: bcwhite <bcwhite@chromium.org> Date: Fri Jun 30 17:45:56 2017 Add histograms to learn about metrics files dropped for lack of an embedded profile. BUG=695880 Review-Url: https://codereview.chromium.org/2966773002 Cr-Commit-Position: refs/heads/master@{#483746} [modify] https://crrev.com/757034c63af4b1c61eeebf070e2123f787be51f7/components/metrics/file_metrics_provider.cc [modify] https://crrev.com/757034c63af4b1c61eeebf070e2123f787be51f7/tools/metrics/histograms/histograms.xml
,
Jul 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/38d8bb9328a2d0a1b7d8373ddd70971c1ce4eeb6 commit 38d8bb9328a2d0a1b7d8373ddd70971c1ce4eeb6 Author: bcwhite <bcwhite@chromium.org> Date: Tue Jul 04 15:48:00 2017 Put BrowserMetrics with embedded profiles into subdir for auto-upload. Because the BrowserMetrics file now has an embedded profile, it isn't sent as part of the startup sequence but a minute or so after. That means the browser could exit and not send anything with the "previous run" metrics file being overwritten at the next startup. Instead, append a date-stamp to the last-run filename and move it to a subdirectory from which files will be removed only after they are uploaded. BUG=695880 Review-Url: https://codereview.chromium.org/2938263002 Cr-Commit-Position: refs/heads/master@{#484116} [modify] https://crrev.com/38d8bb9328a2d0a1b7d8373ddd70971c1ce4eeb6/base/metrics/persistent_histogram_allocator.cc [modify] https://crrev.com/38d8bb9328a2d0a1b7d8373ddd70971c1ce4eeb6/base/metrics/persistent_histogram_allocator.h [modify] https://crrev.com/38d8bb9328a2d0a1b7d8373ddd70971c1ce4eeb6/chrome/browser/chrome_browser_field_trials.cc [modify] https://crrev.com/38d8bb9328a2d0a1b7d8373ddd70971c1ce4eeb6/chrome/browser/metrics/chrome_metrics_service_client.cc [modify] https://crrev.com/38d8bb9328a2d0a1b7d8373ddd70971c1ce4eeb6/components/metrics/file_metrics_provider.cc [modify] https://crrev.com/38d8bb9328a2d0a1b7d8373ddd70971c1ce4eeb6/components/metrics/file_metrics_provider.h [modify] https://crrev.com/38d8bb9328a2d0a1b7d8373ddd70971c1ce4eeb6/components/metrics/file_metrics_provider_unittest.cc
,
Aug 8 2017
,
Aug 8 2017
Is it possible to know which version this was fixed?
,
Aug 8 2017
Would you settle for M62 branch point? It's been active since before M61 went to beta.
,
Aug 8 2017
So, M61 beta/stable would have this. That's great!! Thank you so much for fixing this.
,
Oct 19
Verify this is working for Android. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by asvitk...@chromium.org
, Feb 24 2017