chromeos: Add --external-metrics-collection-interval.

Add a switch that can be used to supply an alternate
interval (in seconds) between the ExternalMetrics class
reading UMA events and histograms reported by Chrome OS
system daemons from /var/lib/metrics/uma-events.

I intend to use this in a test that will verify this
reporting path (without needing to wait the current default
of 30 seconds).

Bug: 878641
Change-Id: I83f3ba70b7c32ddd598e1793a71900ef5b72b667
Reviewed-on: https://chromium-review.googlesource.com/1202023
Commit-Queue: Dan Erat <derat@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588991}

chrome/browser/chromeos/external_metrics.cc

@@ -12,12 +12,15 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/metrics/user_metrics.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/task/post_task.h"
 #include "chrome/browser/metrics/chromeos_metrics_provider.h"
+#include "chromeos/chromeos_switches.h"
 #include "components/metrics/serialization/metric_sample.h"
 #include "components/metrics/serialization/serialization_utils.h"
 #include "content/public/browser/browser_thread.h"
@@ -46,14 +49,30 @@ bool CheckLinearValues(const std::string& name, int maximum) {
   return CheckValues(name, 1, maximum, maximum + 1);
 }
 
-// The interval between external metrics collections.
-constexpr base::TimeDelta kExternalMetricsCollectionInterval =
-    base::TimeDelta::FromSeconds(30);
+// The file from which externally-reported metrics are read.
 constexpr char kEventsFilePath[] = "/var/lib/metrics/uma-events";
 
+// Default interval between externally-reported metrics being collected.
+constexpr base::TimeDelta kDefaultCollectionInterval =
+    base::TimeDelta::FromSeconds(30);
+
 }  // namespace
 
-ExternalMetrics::ExternalMetrics() : uma_events_file_(kEventsFilePath) {
+ExternalMetrics::ExternalMetrics()
+    : uma_events_file_(kEventsFilePath),
+      collection_interval_(kDefaultCollectionInterval) {
+  const std::string flag_value =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kExternalMetricsCollectionInterval);
+  if (!flag_value.empty()) {
+    int seconds = -1;
+    if (base::StringToInt(flag_value, &seconds) && seconds > 0) {
+      collection_interval_ = base::TimeDelta::FromSeconds(seconds);
+    } else {
+      LOG(WARNING) << "Ignoring bad value \"" << flag_value << "\" in --"
+                   << switches::kExternalMetricsCollectionInterval;
+    }
+  }
 }
 
 ExternalMetrics::~ExternalMetrics() {}
@@ -161,7 +180,7 @@ void ExternalMetrics::ScheduleCollector() {
       FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
       base::BindOnce(&chromeos::ExternalMetrics::CollectEventsAndReschedule,
                      this),
-      kExternalMetricsCollectionInterval);
+      collection_interval_);
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/external_metrics.h

@@ -11,6 +11,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/time/time.h"
 
 namespace metrics {
 class MetricSample;
@@ -42,8 +43,9 @@ class ExternalMetrics : public base::RefCountedThreadSafe<ExternalMetrics> {
   friend class base::RefCountedThreadSafe<ExternalMetrics>;
   friend class ExternalMetricsTest;
 
-  FRIEND_TEST_ALL_PREFIXES(ExternalMetricsTest, CanReceiveHistogram);
+  FRIEND_TEST_ALL_PREFIXES(ExternalMetricsTest, CustomInterval);
   FRIEND_TEST_ALL_PREFIXES(ExternalMetricsTest, HandleMissingFile);
+  FRIEND_TEST_ALL_PREFIXES(ExternalMetricsTest, CanReceiveHistogram);
   FRIEND_TEST_ALL_PREFIXES(ExternalMetricsTest,
                            IncorrectHistogramsAreDiscarded);
 
@@ -89,6 +91,9 @@ class ExternalMetrics : public base::RefCountedThreadSafe<ExternalMetrics> {
   // File used by libmetrics to send metrics to Chrome.
   std::string uma_events_file_;
 
+  // Interval between metrics being read from |uma_events_file_|.
+  base::TimeDelta collection_interval_;
+
   DISALLOW_COPY_AND_ASSIGN(ExternalMetrics);
 };
 

chrome/browser/chromeos/external_metrics_unittest.cc

@@ -6,10 +6,12 @@
 
 #include <memory>
 
+#include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "chromeos/chromeos_switches.h"
 #include "components/metrics/serialization/metric_sample.h"
 #include "components/metrics/serialization/serialization_utils.h"
 #include "content/public/test/test_browser_thread_bundle.h"
@@ -19,7 +21,10 @@ namespace chromeos {  // Need this because of the FRIEND_TEST
 
 class ExternalMetricsTest : public testing::Test {
  public:
-  void SetUp() override {
+  ExternalMetricsTest() = default;
+  ~ExternalMetricsTest() override = default;
+
+  void Init() {
     ASSERT_TRUE(dir_.CreateUniqueTempDir());
     external_metrics_ = ExternalMetrics::CreateForTesting(
         dir_.GetPath().Append("testfile").value());
@@ -30,14 +35,25 @@ class ExternalMetricsTest : public testing::Test {
   content::TestBrowserThreadBundle thread_bundle_;
 };
 
+TEST_F(ExternalMetricsTest, CustomInterval) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kExternalMetricsCollectionInterval, "5");
+  Init();
+
+  EXPECT_EQ(base::TimeDelta::FromSeconds(5),
+            external_metrics_->collection_interval_);
+}
+
 TEST_F(ExternalMetricsTest, HandleMissingFile) {
+  Init();
   ASSERT_TRUE(base::DeleteFile(
       base::FilePath(external_metrics_->uma_events_file_), false));
 
   EXPECT_EQ(0, external_metrics_->CollectEvents());
 }
 
 TEST_F(ExternalMetricsTest, CanReceiveHistogram) {
+  Init();
   base::HistogramTester histogram_tester;
 
   std::unique_ptr<metrics::MetricSample> hist =
@@ -52,6 +68,7 @@ TEST_F(ExternalMetricsTest, CanReceiveHistogram) {
 }
 
 TEST_F(ExternalMetricsTest, IncorrectHistogramsAreDiscarded) {
+  Init();
   base::HistogramTester histogram_tester;
 
   // Malformed histogram (min > max).

chromeos/chromeos_switches.cc

@@ -408,6 +408,11 @@ const char kEnterpriseEnrollmentInitialModulus[] =
 const char kEnterpriseEnrollmentModulusLimit[] =
     "enterprise-enrollment-modulus-limit";
 
+// Interval in seconds between Chrome reading external metrics from
+// /var/lib/metrics/uma-events.
+const char kExternalMetricsCollectionInterval[] =
+    "external-metrics-collection-interval";
+
 // An absolute path to the chroot hosting the DriveFS to use. This is only used
 // when running on Linux, i.e. when IsRunningOnChromeOS() returns false.
 const char kFakeDriveFsLauncherChrootPath[] =

chromeos/chromeos_switches.h

@@ -117,6 +117,7 @@ CHROMEOS_EXPORT extern const char kEnterpriseEnableInitialEnrollment[];
 CHROMEOS_EXPORT extern const char kEnterpriseEnableZeroTouchEnrollment[];
 CHROMEOS_EXPORT extern const char kEnterpriseEnrollmentInitialModulus[];
 CHROMEOS_EXPORT extern const char kEnterpriseEnrollmentModulusLimit[];
+CHROMEOS_EXPORT extern const char kExternalMetricsCollectionInterval[];
 CHROMEOS_EXPORT extern const char kFirstExecAfterBoot[];
 CHROMEOS_EXPORT extern const char kFakeDriveFsLauncherChrootPath[];
 CHROMEOS_EXPORT extern const char kFakeDriveFsLauncherSocketPath[];