Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e4fa61e0 authored by Bertrand SIMONNET's avatar Bertrand SIMONNET Committed by ChromeOS Commit Bot
Browse files

metrics: don't upload metrics when metrics are disabled 

The uploader should only send metrics samples when the metrics are
enabled.
The uploader daemon is still started when the metrics are disabled so
that:
* When we enable the metrics, we don't require a restart of
  metrics_daemon to start uploading metrics.
* The metrics file is truncated periodically and avoid taking too much
  space on long running system with metrics disabled.

BUG=chromium:459636
TEST=unittests
TEST=`test_that -b gizmo gizmo platform_MetricsUploader` works
TEST=manual: uploader does not upload metrics if metrics are disabled.

CQ-DEPEND=CL:250980

Change-Id: I9f5da3457066a183c5791b5488e985b7ab13b6e1
Reviewed-on: https://chromium-review.googlesource.com/250822


Trybot-Ready: Bertrand Simonnet <bsimonnet@chromium.org>
Tested-by: default avatarBertrand Simonnet <bsimonnet@chromium.org>
Reviewed-by: default avatarNathan Bullock <nathanbullock@google.com>
Commit-Queue: Bertrand Simonnet <bsimonnet@chromium.org>
parent 067ec8ba

metrics/metrics_daemon.cc

+2 −1
Original line number Original line Diff line number Diff line
@@ -191,6 +191,7 @@ int MetricsDaemon::Run() { 
void MetricsDaemon::RunUploaderTest() {

void MetricsDaemon::RunUploaderTest() {

  upload_service_.reset(new UploadService(new SystemProfileCache(true,

  upload_service_.reset(new UploadService(new SystemProfileCache(true,

                                                                 config_root_),

                                                                 config_root_),

                                          metrics_lib_,

                                          server_));

                                          server_));

  upload_service_->Init(upload_interval_, metrics_file_);

  upload_service_->Init(upload_interval_, metrics_file_);

  upload_service_->UploadEvent();

  upload_service_->UploadEvent();
@@ -341,7 +342,7 @@ int MetricsDaemon::OnInit() { 
    if (IsOnOfficialBuild()) {

    if (IsOnOfficialBuild()) {

      LOG(INFO) << "uploader enabled";

      LOG(INFO) << "uploader enabled";

      upload_service_.reset(

      upload_service_.reset(

          new UploadService(new SystemProfileCache(), server_));

          new UploadService(new SystemProfileCache(), metrics_lib_, server_));

      upload_service_->Init(upload_interval_, metrics_file_);

      upload_service_->Init(upload_interval_, metrics_file_);

    } else {

    } else {

      LOG(INFO) << "uploader disabled on non-official build";

      LOG(INFO) << "uploader disabled on non-official build";

metrics/metrics_library.h

+2 −1
Original line number Original line Diff line number Diff line
@@ -19,6 +19,7 @@ 
class MetricsLibraryInterface {

class MetricsLibraryInterface {

 public:

 public:

  virtual void Init() = 0;

  virtual void Init() = 0;

  virtual bool AreMetricsEnabled() = 0;

  virtual bool SendToUMA(const std::string& name, int sample,

  virtual bool SendToUMA(const std::string& name, int sample,

                         int min, int max, int nbuckets) = 0;

                         int min, int max, int nbuckets) = 0;

  virtual bool SendEnumToUMA(const std::string& name, int sample, int max) = 0;

  virtual bool SendEnumToUMA(const std::string& name, int sample, int max) = 0;
@@ -40,7 +41,7 @@ class MetricsLibrary : public MetricsLibraryInterface { 
  bool IsGuestMode();

  bool IsGuestMode();





  // Returns whether or not metrics collection is enabled.

  // Returns whether or not metrics collection is enabled.

  bool AreMetricsEnabled();

  bool AreMetricsEnabled() override;





  // Sends histogram data to Chrome for transport to UMA and returns

  // Sends histogram data to Chrome for transport to UMA and returns

  // true on success. This method results in the equivalent of an

  // true on success. This method results in the equivalent of an

metrics/metrics_library_mock.h

+4 −0
Original line number Original line Diff line number Diff line
@@ -13,6 +13,8 @@ 




class MetricsLibraryMock : public MetricsLibraryInterface {

class MetricsLibraryMock : public MetricsLibraryInterface {

 public:

 public:

  bool metrics_enabled_ = true;



  MOCK_METHOD0(Init, void());

  MOCK_METHOD0(Init, void());

  MOCK_METHOD5(SendToUMA, bool(const std::string& name, int sample,

  MOCK_METHOD5(SendToUMA, bool(const std::string& name, int sample,

                               int min, int max, int nbuckets));

                               int min, int max, int nbuckets));
@@ -20,6 +22,8 @@ class MetricsLibraryMock : public MetricsLibraryInterface { 
                                   int max));

                                   int max));

  MOCK_METHOD2(SendSparseToUMA, bool(const std::string& name, int sample));

  MOCK_METHOD2(SendSparseToUMA, bool(const std::string& name, int sample));

  MOCK_METHOD1(SendUserActionToUMA, bool(const std::string& action));

  MOCK_METHOD1(SendUserActionToUMA, bool(const std::string& action));



  bool AreMetricsEnabled() override {return metrics_enabled_;};

};

};





#endif  // METRICS_METRICS_LIBRARY_MOCK_H_
 
#endif  // METRICS_METRICS_LIBRARY_MOCK_H_

metrics/uploader/upload_service.cc

+11 −1
Original line number Original line Diff line number Diff line
@@ -26,17 +26,20 @@ 
const int UploadService::kMaxFailedUpload = 10;

const int UploadService::kMaxFailedUpload = 10;





UploadService::UploadService(SystemProfileSetter* setter,

UploadService::UploadService(SystemProfileSetter* setter,

                             MetricsLibraryInterface* metrics_lib,

                             const std::string& server)

                             const std::string& server)

    : system_profile_setter_(setter),

    : system_profile_setter_(setter),

      metrics_lib_(metrics_lib),

      histogram_snapshot_manager_(this),

      histogram_snapshot_manager_(this),

      sender_(new HttpSender(server)),

      sender_(new HttpSender(server)),

      testing_(false) {

      testing_(false) {

}

}





UploadService::UploadService(SystemProfileSetter* setter,

UploadService::UploadService(SystemProfileSetter* setter,

                             MetricsLibraryInterface* metrics_lib,

                             const std::string& server,

                             const std::string& server,

                             bool testing)

                             bool testing)

    : UploadService(setter, server) {

    : UploadService(setter, metrics_lib, server) {

  testing_ = testing;

  testing_ = testing;

}

}
@@ -94,6 +97,13 @@ void UploadService::UploadEvent() { 
void UploadService::SendStagedLog() {

void UploadService::SendStagedLog() {

  CHECK(staged_log_) << "staged_log_ must exist to be sent";

  CHECK(staged_log_) << "staged_log_ must exist to be sent";





  // If metrics are not enabled, discard the log and exit.

  if (!metrics_lib_->AreMetricsEnabled()) {

    LOG(INFO) << "Metrics disabled. Don't upload metrics samples.";

    staged_log_.reset();

    return;

  }



  std::string log_text;

  std::string log_text;

  staged_log_->GetEncodedLog(&log_text);

  staged_log_->GetEncodedLog(&log_text);

  if (!sender_->Send(log_text, base::SHA1HashString(log_text))) {

  if (!sender_->Send(log_text, base::SHA1HashString(log_text))) {

metrics/uploader/upload_service.h

+5 −0
Original line number Original line Diff line number Diff line
@@ -10,6 +10,8 @@ 
#include "base/metrics/histogram_base.h"

#include "base/metrics/histogram_base.h"

#include "base/metrics/histogram_flattener.h"

#include "base/metrics/histogram_flattener.h"

#include "base/metrics/histogram_snapshot_manager.h"

#include "base/metrics/histogram_snapshot_manager.h"



#include "metrics/metrics_library.h"

#include "metrics/uploader/metrics_log.h"

#include "metrics/uploader/metrics_log.h"

#include "metrics/uploader/sender.h"

#include "metrics/uploader/sender.h"

#include "metrics/uploader/system_profile_cache.h"

#include "metrics/uploader/system_profile_cache.h"
@@ -55,6 +57,7 @@ class SystemProfileSetter; 
class UploadService : public base::HistogramFlattener {

class UploadService : public base::HistogramFlattener {

 public:

 public:

  explicit UploadService(SystemProfileSetter* setter,

  explicit UploadService(SystemProfileSetter* setter,

                         MetricsLibraryInterface* metrics_lib,

                         const std::string& server);

                         const std::string& server);





  void Init(const base::TimeDelta& upload_interval,

  void Init(const base::TimeDelta& upload_interval,
@@ -99,6 +102,7 @@ class UploadService : public base::HistogramFlattener { 




  // Private constructor for use in unit testing.

  // Private constructor for use in unit testing.

  UploadService(SystemProfileSetter* setter,

  UploadService(SystemProfileSetter* setter,

                MetricsLibraryInterface* metrics_lib,

                const std::string& server,

                const std::string& server,

                bool testing);

                bool testing);
@@ -134,6 +138,7 @@ class UploadService : public base::HistogramFlattener { 
  MetricsLog* GetOrCreateCurrentLog();

  MetricsLog* GetOrCreateCurrentLog();





  scoped_ptr<SystemProfileSetter> system_profile_setter_;

  scoped_ptr<SystemProfileSetter> system_profile_setter_;

  MetricsLibraryInterface* metrics_lib_;

  base::HistogramSnapshotManager histogram_snapshot_manager_;

  base::HistogramSnapshotManager histogram_snapshot_manager_;

  scoped_ptr<Sender> sender_;

  scoped_ptr<Sender> sender_;

  int failed_upload_count_;

  int failed_upload_count_;