Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2666793002:
[Sync] Record histogram of ModelTypeStoreBackend initialization result (Closed)
Description[Sync] Record histogram of ModelTypeStoreBackend initialization result
This change records reasons of ModelTypeStoreBackend failures in UMA histogram.
BUG=684774
R=maxbogue@chromium.org,asvitkine@chromium.org
Review-Url: https://codereview.chromium.org/2666793002
Cr-Commit-Position: refs/heads/master@{#447610}
Committed: https://chromium.googlesource.com/chromium/src/+/8db816e5533f4447c4c4fabc6ad9be956f354c71
Patch Set 1 #
Total comments: 4
Patch Set 2 : Update histogram enum labels #
Total comments: 2
Patch Set 3 : Move histogram recording into helper function #
Messages
Total messages: 22 (13 generated)
The CQ bit was checked by pavely@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2666793002/diff/1/tools/metrics/histograms/hi... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2666793002/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:107612: + <int value="6" label="Reading'parsing schema descriptor failed"/> Maybe just have this be "Schema descriptor issue" like the enum value? Not sure what that ' is doing there. https://codereview.chromium.org/2666793002/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:107613: + <int value="7" label="Migration failed"/> Here and below you switch to having the second words not capitalized... I don't know which way these enums typically go but please be self-consistent.
https://codereview.chromium.org/2666793002/diff/1/tools/metrics/histograms/hi... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2666793002/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:107612: + <int value="6" label="Reading'parsing schema descriptor failed"/> On 2017/01/31 01:22:14, maxbogue wrote: > Maybe just have this be "Schema descriptor issue" like the enum value? Not sure > what that ' is doing there. Done. https://codereview.chromium.org/2666793002/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:107613: + <int value="7" label="Migration failed"/> On 2017/01/31 01:22:14, maxbogue wrote: > Here and below you switch to having the second words not capitalized... I don't > know which way these enums typically go but please be self-consistent. Done.
Description was changed from ========== [Sync] Record histogram of ModelTypeStoreBackend initialization result This change records reasons of ModelTypeStoreBackend failures in UMA histogram. BUG=684774 R=maxbogue@chromium.org,asvitkine@chromium.org ========== to ========== [Sync] Record histogram of ModelTypeStoreBackend initialization result This change records reasons of ModelTypeStoreBackend failures in UMA histogram. BUG=684774 R=maxbogue@chromium.org,asvitkine@chromium.org ==========
pavely@chromium.org changed reviewers: + isherman@chromium.org
+isherman@: Could you review changes in histograms.xml
https://codereview.chromium.org/2666793002/diff/20001/components/sync/model_i... File components/sync/model_impl/model_type_store_backend.cc (right): https://codereview.chromium.org/2666793002/diff/20001/components/sync/model_i... components/sync/model_impl/model_type_store_backend.cc:159: STORE_INIT_RESULT_SUCCESS, STORE_INIT_RESULT_COUNT); Please make a helper function that logs this histogram and call it from the different places. Otherwise, each macro expands to a lot of code and this causes extra bloat for the binary.
The CQ bit was checked by pavely@chromium.org to run a CQ dry run
https://codereview.chromium.org/2666793002/diff/20001/components/sync/model_i... File components/sync/model_impl/model_type_store_backend.cc (right): https://codereview.chromium.org/2666793002/diff/20001/components/sync/model_i... components/sync/model_impl/model_type_store_backend.cc:159: STORE_INIT_RESULT_SUCCESS, STORE_INIT_RESULT_COUNT); On 2017/02/01 19:24:40, Alexei Svitkine (very slow) wrote: > Please make a helper function that logs this histogram and call it from the > different places. > > Otherwise, each macro expands to a lot of code and this causes extra bloat for > the binary. Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by pavely@chromium.org
The CQ bit was checked by pavely@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from maxbogue@chromium.org Link to the patchset: https://codereview.chromium.org/2666793002/#ps40001 (title: "Move histogram recording into helper function")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485980311224790,
"parent_rev": "2dfbf027e6a3d4aaee892c9897c28bd18a39dabb", "commit_rev":
"8db816e5533f4447c4c4fabc6ad9be956f354c71"}
Message was sent while issue was closed.
Description was changed from ========== [Sync] Record histogram of ModelTypeStoreBackend initialization result This change records reasons of ModelTypeStoreBackend failures in UMA histogram. BUG=684774 R=maxbogue@chromium.org,asvitkine@chromium.org ========== to ========== [Sync] Record histogram of ModelTypeStoreBackend initialization result This change records reasons of ModelTypeStoreBackend failures in UMA histogram. BUG=684774 R=maxbogue@chromium.org,asvitkine@chromium.org Review-Url: https://codereview.chromium.org/2666793002 Cr-Commit-Position: refs/heads/master@{#447610} Committed: https://chromium.googlesource.com/chromium/src/+/8db816e5533f4447c4c4fabc6ad9... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/8db816e5533f4447c4c4fabc6ad9... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
