New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 682680 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Sign in to add a comment

Investigate Negative Counts in Histograms

Project Member Reported by bcwh...@chromium.org, Jan 19 2017

Issue description

UMA is reporting some counts as negative.  These seem to be largely correlated to having persistent metrics held on disk to be reported by the next run in the case of an unclean shutdown.

See also: b/34354653
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 20 2017

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

commit 48897934e17d9043535dd1213738cb5fce8bda89
Author: bcwhite <bcwhite@chromium.org>
Date: Fri Jan 20 21:43:35 2017

Report reasons for a negative histogram sample count.

Samples reported this way will also be prevented from going negative during the upload.

BUG=682680

Review-Url: https://codereview.chromium.org/2644143006
Cr-Commit-Position: refs/heads/master@{#445177}

[modify] https://crrev.com/48897934e17d9043535dd1213738cb5fce8bda89/base/metrics/persistent_sample_map.cc
[modify] https://crrev.com/48897934e17d9043535dd1213738cb5fce8bda89/tools/metrics/histograms/histograms.xml

Labels: Merge-Request-57
Project Member

Comment 3 by sheriffbot@chromium.org, Feb 2 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

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

Comment 4 by bugdroid1@chromium.org, Feb 2 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4b4ad0aea38aa1440b90908d436131dbdf3b395

commit a4b4ad0aea38aa1440b90908d436131dbdf3b395
Author: Brian White <bcwhite@chromium.org>
Date: Thu Feb 02 20:15:08 2017

Report reasons for a negative histogram sample count.

Samples reported this way will also be prevented from going negative during the upload.

BUG=682680

Review-Url: https://codereview.chromium.org/2644143006
Cr-Commit-Position: refs/heads/master@{#445177}
(cherry picked from commit 48897934e17d9043535dd1213738cb5fce8bda89)

Review-Url: https://codereview.chromium.org/2668383005 .
Cr-Commit-Position: refs/branch-heads/2987@{#279}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/a4b4ad0aea38aa1440b90908d436131dbdf3b395/base/metrics/persistent_sample_map.cc
[modify] https://crrev.com/a4b4ad0aea38aa1440b90908d436131dbdf3b395/tools/metrics/histograms/histograms.xml

This change has been in Beta since 57.0.2987.37 and Dev since 58.0.3004.3.

So far, no histogram reports have surfaced in UMA.
Still all good?

Whatever the cause, it's fairly localized.  The top 20 offenders over the past week (that my Windows desktop build also created) are, in order of frequency:

DataUse.MessageSize.AllServices.Upstream.Foreground.NotCellular  (330, #261)
DataUse.MessageSize.AllServices.Downstream.Foreground.NotCellular  (263, #263)
9602191951683816207  (152, #908)
6498830640825499827  (147, #893)
Net.SSL_CipherSuite  (142, #16)

Net.HttpRequestCompletionErrorCodes  (133, #240)
Net.ErrorCodesForSubresources2  (126, #80)
1814897965385695883  (107, #???)
1949333067601840416  (101, #365)
Net.SSLSignatureAlgorithm  (101, #123)

WebCore.Animation.CSSProperties  (97, #733)
17857241101111754619  (95, #???)
1373634115225205851  (85, #194)
Net.ErrorCodesForImages  (69, #126)
Net.SpdySession.ClosedOnError  (59, #155)

Net.SSL_KeyExchange.ECDHE  (59, #41)
9661179978895180426  (58, #577)
9739559947754382936  (54, #385)
7078458288699255198  (42, #84)
17589461206290228926 (39, #256)

Numbers are given when my WindowsDesktop did not (immediately) create a histogram of the same hash; they either appear later or don't apply to the build, such as "Cellular" histograms.  #??? means unknown (not in the top 1000).

The numbers in parenthesis after are the number of negative counts and its rank in the number of collected counts over the week.  If it's purely random, I'd expect these to roughly match with the top reported records.

Most importantly, these are all sparse histograms!

But the CL submitted before was supposed to report problems found in sparse histograms yet there are no instances of it in the last 28 days.
https://uma.googleplex.com/p/chrome/histograms/?endDate=20170422&dayCount=28&histograms=UMA.NegativeSamples.Reason&fixupData=true&showMax=true&filters=isofficial%2Ceq%2CTrue&implicitFilters=isofficial

Need to investigate that...

Comment 8 Deleted

Comment 9 Deleted

Comment 10 Deleted

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 27 2017

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

commit 9c2f79bed5d1eb92360a6c2f88d4970e63438f8f
Author: bcwhite <bcwhite@chromium.org>
Date: Thu Apr 27 00:40:27 2017

Monitor Add for negative results.

The incoming "count" should not be negative.  If so, record that.  Take
special note if it causes the result to become negative (and clamp it).

BUG=682680

Review-Url: https://codereview.chromium.org/2841143002
Cr-Commit-Position: refs/heads/master@{#467531}

[modify] https://crrev.com/9c2f79bed5d1eb92360a6c2f88d4970e63438f8f/base/metrics/persistent_sample_map.cc
[modify] https://crrev.com/9c2f79bed5d1eb92360a6c2f88d4970e63438f8f/tools/metrics/histograms/histograms.xml

Project Member

Comment 12 by bugdroid1@chromium.org, May 24 2017

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

commit ba6281e4333f2faeedff5a9d10e96a24e6b91fa4
Author: bcwhite <bcwhite@chromium.org>
Date: Wed May 24 18:29:49 2017

Updated negative-counts histogram for changes in logged/unlogged accounting.

Histograms now keep an "logged" and "unlogged" instead of "total" and
"logged" so the calls to AddSubtractImpl no longer mean the same things
and can no longer be used to detect negative uploads to the server.

Move the counter to the Accumulate method to see if there are bad
calls being made.

BUG=682680

Review-Url: https://codereview.chromium.org/2902173002
Cr-Commit-Position: refs/heads/master@{#474362}

[modify] https://crrev.com/ba6281e4333f2faeedff5a9d10e96a24e6b91fa4/base/metrics/persistent_sample_map.cc
[modify] https://crrev.com/ba6281e4333f2faeedff5a9d10e96a24e6b91fa4/tools/metrics/histograms/enums.xml

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 16 2017

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

commit be9206e0f9f62cb4b9a4159a63b88ff4d0f7fc09
Author: bcwhite <bcwhite@chromium.org>
Date: Fri Jun 16 16:37:17 2017

Add histograms to track which histograms are going negative and why.

BUG=682680

Review-Url: https://codereview.chromium.org/2940053004
Cr-Commit-Position: refs/heads/master@{#480069}

[modify] https://crrev.com/be9206e0f9f62cb4b9a4159a63b88ff4d0f7fc09/base/metrics/persistent_sample_map.cc
[modify] https://crrev.com/be9206e0f9f62cb4b9a4159a63b88ff4d0f7fc09/tools/metrics/histograms/histograms.xml

DataUse.MessageSize.* is most of them:
https://bugs.chromium.org/p/chromium/issues/detail?id=739925

-742109866 = Stability.BrowserExitCodes
These appear to be incremented one at a time.  Don't see how it would overflow.

-355731177 = UMA.SamplingRatePerMille
This is a stability profile that should only have a single increment at startup.
Project Member

Comment 16 by bugdroid1@chromium.org, Jul 13 2017

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

commit 18723f5dbb8ba5a8197f4c4e07a776b1e0b2d178
Author: Brian White <bcwhite@chromium.org>
Date: Thu Jul 13 22:25:08 2017

Define DataUse/AllServices histogram hashes.

These are known offenders of the UMA.NegativeSamples.Histogram
metric.

https://bugs.chromium.org/p/chromium/issues/detail?id=739925

Bug: 682680, 739925 
Change-Id: Ie3e394baf5e1dfb2cf8f1028c5273c0a3d19ab4a
Reviewed-on: https://chromium-review.googlesource.com/570262
Reviewed-by: Alexei Svitkine (slow) <asvitkine@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486490}
[modify] https://crrev.com/18723f5dbb8ba5a8197f4c4e07a776b1e0b2d178/tools/metrics/histograms/enums.xml

Cc: sdoyon@chromium.org
Blockedon: 774506
Blockedon: 774509
Blockedon: 774511
Project Member

Comment 21 by bugdroid1@chromium.org, Oct 13 2017

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

commit d7b4dac43ac4d547587ea577cf5fdd2a3cd86855
Author: Brian White <bcwhite@chromium.org>
Date: Fri Oct 13 20:27:18 2017

Fix incorrect accumulation count.

The previous count was increment every time the result was negative
which meant it was counted many times since a negative count is likely
to remain negative for some time.

Also add some new histograms that are overflowing to the enum.

Bug: 682680
Change-Id: Ib7c7c5040c6a507e3b125280e975db4bff37a56b
Reviewed-on: https://chromium-review.googlesource.com/718977
Commit-Queue: Brian White <bcwhite@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508798}
[modify] https://crrev.com/d7b4dac43ac4d547587ea577cf5fdd2a3cd86855/base/metrics/persistent_sample_map.cc
[modify] https://crrev.com/d7b4dac43ac4d547587ea577cf5fdd2a3cd86855/tools/metrics/histograms/enums.xml

Blockedon: 779670
Blockedon: 779674
Project Member

Comment 24 by bugdroid1@chromium.org, Nov 1 2017

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

commit 537e41ab58c47623f2f59d7335d04e3792f52b2c
Author: Brian White <bcwhite@chromium.org>
Date: Wed Nov 01 03:02:13 2017

Record sample-count overflow in sample-vector.

This has been running only with sparse histograms for some time now and
has been helpful in finding metrics that overflow.  Now extend this to
check the remaining bulk of histograms.

While the original CL looked for many different reasons of acquiring a
negative sample count, it's now known that there is only a single
reason and so that is the only reason checked for here.

Bug: 682680
Change-Id: I96785436468fdbe34f4221caf7a79da299edeca2
Reviewed-on: https://chromium-review.googlesource.com/741004
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513059}
[modify] https://crrev.com/537e41ab58c47623f2f59d7335d04e3792f52b2c/base/metrics/histogram_samples.cc
[modify] https://crrev.com/537e41ab58c47623f2f59d7335d04e3792f52b2c/base/metrics/histogram_samples.h
[modify] https://crrev.com/537e41ab58c47623f2f59d7335d04e3792f52b2c/base/metrics/persistent_sample_map.cc
[modify] https://crrev.com/537e41ab58c47623f2f59d7335d04e3792f52b2c/base/metrics/sample_vector.cc
[modify] https://crrev.com/537e41ab58c47623f2f59d7335d04e3792f52b2c/tools/metrics/histograms/enums.xml

Blockedon: 782231
Blockedon: 782232
Blockedon: 782233
Project Member

Comment 28 by bugdroid1@chromium.org, Nov 7 2017

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

commit 6e63494b4e379a4f22219996a68a11de52fdf30b
Author: Brian White <bcwhite@chromium.org>
Date: Tue Nov 07 20:55:40 2017

Add enums for overflowing vector histograms.

Bug: 682680
Change-Id: Iaa23d4571f6c008ffb83f77e556e1cda771235d1
Reviewed-on: https://chromium-review.googlesource.com/757036
Commit-Queue: Brian White <bcwhite@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514568}
[modify] https://crrev.com/6e63494b4e379a4f22219996a68a11de52fdf30b/tools/metrics/histograms/enums.xml

Labels: -Pri-1 Pri-3
Blockedon: 809658
Blockedon: 809661
Blockedon: 809668
Blockedon: 809672
Project Member

Comment 34 by bugdroid1@chromium.org, Feb 7 2018

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

commit 694c7171c043619b0c9374e579218e3b366e4f8d
Author: Brian White <bcwhite@chromium.org>
Date: Wed Feb 07 13:02:25 2018

Added more histograms that have negative sample counts.

Bug: 682680
Change-Id: I1062fccb8b19d3bd20812c52cbc2df2fcf84528a
Reviewed-on: https://chromium-review.googlesource.com/905267
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534989}
[modify] https://crrev.com/694c7171c043619b0c9374e579218e3b366e4f8d/tools/metrics/histograms/enums.xml

Project Member

Comment 35 by bugdroid1@chromium.org, Feb 7 2018

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

commit 2224547f3d96beb34ddd60481b9d260d29c8ed67
Author: Brian White <bcwhite@chromium.org>
Date: Wed Feb 07 18:35:03 2018

Make histogram CountKiB reusable.  Add CountKilo, too.

There are several histograms that overflow because they are counting
by large amounts with each increment.  These helper methods will reduce
the precision but remain reasonably accurate.

Bug: 682680
Change-Id: I06af28a66fb5252b6e7263d0d1cd9b8fbb0f7fea

// just extracted the code into a common location
TBR=sclittle@chromium.org

Change-Id: I06af28a66fb5252b6e7263d0d1cd9b8fbb0f7fea
Reviewed-on: https://chromium-review.googlesource.com/905148
Commit-Queue: Brian White <bcwhite@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535070}
[modify] https://crrev.com/2224547f3d96beb34ddd60481b9d260d29c8ed67/base/metrics/histogram_base.cc
[modify] https://crrev.com/2224547f3d96beb34ddd60481b9d260d29c8ed67/base/metrics/histogram_base.h
[modify] https://crrev.com/2224547f3d96beb34ddd60481b9d260d29c8ed67/base/metrics/histogram_base_unittest.cc
[modify] https://crrev.com/2224547f3d96beb34ddd60481b9d260d29c8ed67/components/data_use_measurement/core/data_use_measurement.cc

Project Member

Comment 36 by bugdroid1@chromium.org, May 24 2018

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

commit fffb3583cc95b3d9b8e3b09297205239e69ce7c9
Author: Brian White <bcwhite@chromium.org>
Date: Thu May 24 21:17:01 2018

Add ScaledLinearHistogram and associated histogram macros.

Some metrics record accumulated counts such as bytes or milliseconds
that can rapidly overflow the 31-bit counters for them.  This new
histogram type auto-scales the counts down by some factor while
keeping the "remainder" to add to future additions, thus keeping
accuracy until such time as the program exits.

This is similar to the AddScaled() method but is faster because it
doesn't required generation of random numbers and also rounds the
reported value to the nearest number rather than truncating it.

Bug: 682680
Change-Id: I109229970924627a584bcb1b0984fc8455e6294d
Reviewed-on: https://chromium-review.googlesource.com/1055623
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561631}
[modify] https://crrev.com/fffb3583cc95b3d9b8e3b09297205239e69ce7c9/base/metrics/histogram.cc
[modify] https://crrev.com/fffb3583cc95b3d9b8e3b09297205239e69ce7c9/base/metrics/histogram.h
[modify] https://crrev.com/fffb3583cc95b3d9b8e3b09297205239e69ce7c9/base/metrics/histogram_base.h
[modify] https://crrev.com/fffb3583cc95b3d9b8e3b09297205239e69ce7c9/base/metrics/histogram_macros.h
[modify] https://crrev.com/fffb3583cc95b3d9b8e3b09297205239e69ce7c9/base/metrics/histogram_macros_internal.h
[modify] https://crrev.com/fffb3583cc95b3d9b8e3b09297205239e69ce7c9/base/metrics/histogram_unittest.cc

Blockedon: 859167
Project Member

Comment 38 by bugdroid1@chromium.org, Jun 29 2018

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

commit 36c9bc714d4a664b0bd11a140457c7aea90d4946
Author: Brian White <bcwhite@chromium.org>
Date: Fri Jun 29 21:50:42 2018

Add new histogram hash.

Bug: 682680
Change-Id: I913cdcced074d92ad60452a02a3781e8fb1b00cf
Reviewed-on: https://chromium-review.googlesource.com/1120693
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571651}
[modify] https://crrev.com/36c9bc714d4a664b0bd11a140457c7aea90d4946/tools/metrics/histograms/enums.xml

Sign in to add a comment