bug 1438335 - Record when we have to clamp large Telemetry accumulations. r=Dexter,francois

We need to clamp accumulations to fit in our data representation (int). This
patch records the number of times, and for which probes, we had to do so.

MozReview-Commit-ID: GSs3oFvLKlL

--HG--
extra : rebase_source : 03aff1b8b645a7934ecbf51a1c2a66cc63dd29f4

toolkit/components/telemetry/Scalars.yaml

@@ -1002,6 +1002,24 @@ telemetry:
     record_in_processes:
       - all
 
+  accumulate_clamped_values:
+    bug_numbers:
+      - 1438335
+    description: >
+      The count of accumulations that had to be clamped down to not overflow,
+      keyed to the histogram name of the overflowing accumulation.
+      This is doubled for non-keyed desktop Firefox Histograms because of
+      saved-session pings.
+    expires: never
+    kind: uint
+    keyed: true
+    notification_emails:
+      - telemetry-client-dev@mozilla.com
+      - chutten@mozilla.com
+    release_channel_collection: opt-in
+    record_in_processes:
+      - 'main'
+
 telemetry.discarded:
   accumulations:
     bug_numbers:

toolkit/components/telemetry/TelemetryHistogram.cpp

@@ -613,6 +613,9 @@ internal_HistogramAdd(Histogram& histogram,
   // Clamp large values of `value` to be INT_MAX so they continue to be treated
   // as large values (instead of negative ones).
   if (value > INT_MAX) {
+    TelemetryScalar::Add(
+      mozilla::Telemetry::ScalarID::TELEMETRY_ACCUMULATE_CLAMPED_VALUES,
+      NS_ConvertASCIItoUTF16(gHistogramInfos[id].name()), 1);
     value = INT_MAX;
   }
 
@@ -823,6 +826,16 @@ KeyedHistogram::Add(const nsCString& key, uint32_t sample,
   }
 #endif
 
+  // The internal representation of a base::Histogram's buckets uses `int`.
+  // Clamp large values of `sample` to be INT_MAX so they continue to be treated
+  // as large values (instead of negative ones).
+  if (sample > INT_MAX) {
+    TelemetryScalar::Add(
+      mozilla::Telemetry::ScalarID::TELEMETRY_ACCUMULATE_CLAMPED_VALUES,
+      NS_ConvertASCIItoUTF16(mHistogramInfo.name()), 1);
+    sample = INT_MAX;
+  }
+
   histogram->Add(sample);
 #if !defined(MOZ_WIDGET_ANDROID)
   subsession->Add(sample);

toolkit/components/telemetry/tests/gtest/TestHistograms.cpp

@@ -312,6 +312,7 @@ TEST_F(TelemetryTestFixture, AccumulateLinearHistogram_DifferentSamples)
 
   AutoJSContextWithGlobal cx(mCleanGlobal);
 
+  mTelemetry->ClearScalars();
   GetAndClearHistogram(cx.GetJSContext(), mTelemetry, NS_LITERAL_CSTRING("TELEMETRY_TEST_LINEAR"),
                         false);
 
@@ -350,6 +351,15 @@ TEST_F(TelemetryTestFixture, AccumulateLinearHistogram_DifferentSamples)
   const uint32_t kExpectedCountLast = 3;
   ASSERT_EQ(uCountFirst, kExpectedCountFirst) << "The first bucket did not accumulate the correct number of values";
   ASSERT_EQ(uCountLast, kExpectedCountLast) << "The last bucket did not accumulate the correct number of values";
+
+  // We accumulated two values that had to be clamped. We expect the count in
+  // 'telemetry.accumulate_clamped_values' to be 4
+  const uint32_t expectedAccumulateClampedCount = 4;
+  JS::RootedValue scalarsSnapshot(cx.GetJSContext());
+  GetScalarsSnapshot(true, cx.GetJSContext(),&scalarsSnapshot);
+  CheckKeyedUintScalar("telemetry.accumulate_clamped_values",
+                       "TELEMETRY_TEST_LINEAR", cx.GetJSContext(),
+                       scalarsSnapshot, expectedAccumulateClampedCount);
 }
 
 TEST_F(TelemetryTestFixture, AccumulateKeyedCountHistogram_MultipleSamples)
@@ -392,10 +402,11 @@ TEST_F(TelemetryTestFixture, TestKeyedLinearHistogram_MultipleSamples)
 {
   AutoJSContextWithGlobal cx(mCleanGlobal);
 
+  mTelemetry->ClearScalars();
   GetAndClearHistogram(cx.GetJSContext(), mTelemetry,
                        NS_LITERAL_CSTRING("TELEMETRY_TEST_KEYED_LINEAR"), true);
 
-  const nsTArray<uint32_t> samples({1,5,250000});
+  const nsTArray<uint32_t> samples({1, 5, 250000, UINT_MAX});
   // Test the accumulation on the key 'testkey', using
   // the API that accepts histogram IDs.
   Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_LINEAR,
@@ -434,11 +445,20 @@ TEST_F(TelemetryTestFixture, TestKeyedLinearHistogram_MultipleSamples)
   JS::ToUint32(cx.GetJSContext(), countLast, &uCountLast);
 
   const uint32_t kExpectedCountFirst = 2;
-  const uint32_t kExpectedCountLast = 1;
+  const uint32_t kExpectedCountLast = 2;
   ASSERT_EQ(uCountFirst, kExpectedCountFirst)
     << "The first bucket did not accumulate the correct number of values for key 'testkey'";
   ASSERT_EQ(uCountLast, kExpectedCountLast)
     << "The last bucket did not accumulate the correct number of values for key 'testkey'";
+
+  // We accumulated one keyed values that had to be clamped. We expect the
+  // count in 'telemetry.accumulate_clamped_values' to be 1
+  const uint32_t expectedAccumulateClampedCount = 1;
+  JS::RootedValue scalarsSnapshot(cx.GetJSContext());
+  GetScalarsSnapshot(true, cx.GetJSContext(),&scalarsSnapshot);
+  CheckKeyedUintScalar("telemetry.accumulate_clamped_values",
+                       "TELEMETRY_TEST_KEYED_LINEAR", cx.GetJSContext(),
+                       scalarsSnapshot, expectedAccumulateClampedCount);
 }
 
 TEST_F(TelemetryTestFixture, TestKeyedKeysHistogram_MultipleSamples)