New issue
Advanced search Search tips

Issue 855745 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Jul 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Add histogram delta option to getHistogram(s) devtools API.

Reported by br...@amazon.com, Jun 22 2018

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/65.0.3325.181 Chrome/65.0.3325.181 Safari/537.36

Steps to reproduce the problem:
The Chromium dev tools protocol supports getting histograms with
|getHistogram(s)|:
https://chromedevtools.github.io/devtools-protocol/tot/Browser/#method-getHistograms.
This returns the cumulative counts across the lifetime of the histograms.  It
would be good to have the option to request just the _delta_ of counts since the
previous call to getHistogram(s).  This would support the use case, particularly
in headless, where one wishes to periodically report metric data back to a
metrics collection server, while avoiding double-counting observations.

It looks like Chromium already has most of the code in place to support this use
case, as periodic reporting of deltas is how Google Chrome handles histograms.
As such, I believe the code additions/modifications would be fairly
straight-forward.  For example, instead of using
HistogramBase::SnapshotSamples() as is done here:
https://cs.chromium.org/chromium/src/content/browser/devtools/protocol/browser_handler.cc?l=53&rcl=2a90c8832699db230eb17897101296527c289605.
Use HistogramBase::SnapshotDelta().

What is the expected behavior?
.

What went wrong?
.

Did this work before? N/A 

Chrome version: 65.0.3325.181  Channel: n/a
OS Version: 
Flash Version:
 
@bryct: we'll be happy to review a CL!
Labels: Needs-Milestone
@bryct: as lushnikov@ is saying, if you have cycles to work on it, we will provide help with reviewing and landing!

Comment 4 by br...@amazon.com, Jun 27 2018

Sure, I have a draft change.  I'll follow up with a CL shortly.
Labels: Needs-Feedback
bryct@,
Friendly ping to get an update on this issue as per C#4.
Thanks in advance!!
Project Member

Comment 7 by sheriffbot@chromium.org, Jul 3

Cc: jmukthavaram@chromium.org
Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 3

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

commit ce008f8a181f10ee92901d85b1f6d2e44da533e2
Author: Bryce Thomas <bryct@amazon.com>
Date: Tue Jul 03 22:24:47 2018

Add histogram delta to getHistogram(s) devtools API.

This CL adds the option to query histogram deltas since the previous call,
rather than cumulative counts since launch.

Bug:  855745 
Change-Id: I4c2965613d1e1ca39975bbeda3af2c5004c1f990
Reviewed-on: https://chromium-review.googlesource.com/1118846
Commit-Queue: Bryce Thomas <bryct@amazon.com>
Reviewed-by: Andrey Lushnikov <lushnikov@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572374}
[modify] https://crrev.com/ce008f8a181f10ee92901d85b1f6d2e44da533e2/content/browser/devtools/protocol/browser_handler.cc
[modify] https://crrev.com/ce008f8a181f10ee92901d85b1f6d2e44da533e2/content/browser/devtools/protocol/browser_handler.h
[modify] https://crrev.com/ce008f8a181f10ee92901d85b1f6d2e44da533e2/third_party/blink/renderer/core/inspector/browser_protocol.pdl

This was resolved with the landing of https://chromium.googlesource.com/chromium/src.git/+/ce008f8a181f10ee92901d85b1f6d2e44da533e2.  Feel free to resolve this issue.
Cc: kkaluri@chromium.org
Status: WontFix (was: Unconfirmed)
As per comment #9, closing this issue.

Sign in to add a comment