Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2762563003:
Track data use of WebSockets (Closed)
DescriptionTrack data use of WebSockets
BUG=684756
Review-Url: https://codereview.chromium.org/2762563003
Cr-Commit-Position: refs/heads/master@{#460021}
Committed: https://chromium.googlesource.com/chromium/src/+/75fe34fbf030a27e7be3b1b1365ba395a894e0e0
Patch Set 1 #Patch Set 2 : Track data use of WebSockets #
Total comments: 8
Patch Set 3 : Addressed tyoshino@ comments #
Messages
Total messages: 22 (14 generated)
The CQ bit was checked by rajendrant@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: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by rajendrant@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...
rajendrant@google.com changed reviewers: + rajendrant@google.com, tyoshino@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
is it fine to be rooted on Net.WebSocket. not DataUse.? I see some histograms defined in data_use_measurement.cc the root of that is DataUse. lgtm https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:37836: + Records the downstream data use of web sockets. Logged on every read please use WebSockets for consistency with other entries. https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:37837: + operation in the socket. the WebSocket? https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:37845: + Records the upstream data use of web sockets. Logged on every write ditto https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:37846: + operation in the socket. ditto
On 2017/03/24 11:58:29, tyoshino wrote: > is it fine to be rooted on Net.WebSocket. not DataUse.? I see some histograms > defined in data_use_measurement.cc the root of that is DataUse. > > lgtm > > https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... > File tools/metrics/histograms/histograms.xml (right): > > https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... > tools/metrics/histograms/histograms.xml:37836: + Records the downstream data > use of web sockets. Logged on every read > please use WebSockets for consistency with other entries. > > https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... > tools/metrics/histograms/histograms.xml:37837: + operation in the socket. > the WebSocket? > > https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... > tools/metrics/histograms/histograms.xml:37845: + Records the upstream data > use of web sockets. Logged on every write > ditto > > https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... > tools/metrics/histograms/histograms.xml:37846: + operation in the socket. > ditto Yes. Net.WebSocket.DataUse.* is fine.
rajendrant@chromium.org changed reviewers: + holte@chromium.org - rajendrant@google.com
holte: ptal histograms.xml https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:37836: + Records the downstream data use of web sockets. Logged on every read On 2017/03/24 11:58:29, tyoshino wrote: > please use WebSockets for consistency with other entries. Done. https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:37837: + operation in the socket. On 2017/03/24 11:58:29, tyoshino wrote: > the WebSocket? Done. https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:37845: + Records the upstream data use of web sockets. Logged on every write On 2017/03/24 11:58:28, tyoshino wrote: > ditto Done. https://codereview.chromium.org/2762563003/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:37846: + operation in the socket. On 2017/03/24 11:58:28, tyoshino wrote: > ditto Done.
lgtm
The CQ bit was checked by rajendrant@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tyoshino@chromium.org Link to the patchset: https://codereview.chromium.org/2762563003/#ps40001 (title: "Addressed tyoshino@ comments")
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": 1490675307471330,
"parent_rev": "379b1ec8fc03311f140ac8ca7f54314416ddaadb", "commit_rev":
"75fe34fbf030a27e7be3b1b1365ba395a894e0e0"}
Message was sent while issue was closed.
Description was changed from ========== Track data use of WebSockets BUG=684756 ========== to ========== Track data use of WebSockets BUG=684756 Review-Url: https://codereview.chromium.org/2762563003 Cr-Commit-Position: refs/heads/master@{#460021} Committed: https://chromium.googlesource.com/chromium/src/+/75fe34fbf030a27e7be3b1b1365b... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/75fe34fbf030a27e7be3b1b1365b...
Message was sent while issue was closed.
lgtm |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
