diff options
author | Bertrand SIMONNET <bsimonnet@google.com> | 2015-11-18 13:46:33 -0800 |
---|---|---|
committer | Bertrand SIMONNET <bsimonnet@google.com> | 2015-12-03 17:01:27 -0800 |
commit | 6b8629a6490d01196368ae1ed5bc6967c6f127eb (patch) | |
tree | f3ff88b6f1a3c39939c5f44759236e869e4f50c2 | |
parent | 65fc48557485e0d8afafb5c8c5644d93b53c3214 (diff) | |
download | core-6b8629a6490d01196368ae1ed5bc6967c6f127eb.tar.gz core-6b8629a6490d01196368ae1ed5bc6967c6f127eb.tar.bz2 core-6b8629a6490d01196368ae1ed5bc6967c6f127eb.zip |
metricsd: Log over binder.
This CL converts metricsd, libmetrics and metrics_collector to use
Binder to pass metrics samples.
Bug: 25670685
Change-Id: I657faecdf4ed1226ab30ce69e062028463437e7b
22 files changed, 487 insertions, 1155 deletions
diff --git a/metricsd/Android.mk b/metricsd/Android.mk index 97c0d2866..839ab659e 100644 --- a/metricsd/Android.mk +++ b/metricsd/Android.mk @@ -18,8 +18,6 @@ metrics_cpp_extension := .cc libmetrics_sources := \ c_metrics_library.cc \ metrics_library.cc \ - serialization/metric_sample.cc \ - serialization/serialization_utils.cc \ timer.cc metrics_client_sources := \ @@ -30,18 +28,18 @@ metrics_collector_common := \ collectors/cpu_usage_collector.cc \ collectors/disk_usage_collector.cc \ metrics_collector.cc \ - persistent_integer.cc \ + persistent_integer.cc metricsd_common := \ persistent_integer.cc \ - serialization/metric_sample.cc \ - serialization/serialization_utils.cc \ + uploader/bn_metricsd_impl.cc \ + uploader/crash_counters.cc \ uploader/metrics_hashes.cc \ uploader/metrics_log_base.cc \ uploader/metrics_log.cc \ uploader/sender_http.cc \ uploader/system_profile_cache.cc \ - uploader/upload_service.cc \ + uploader/upload_service.cc metrics_collector_tests_sources := \ collectors/averaged_statistics_collector_test.cc \ @@ -49,14 +47,13 @@ metrics_collector_tests_sources := \ metrics_collector_test.cc \ metrics_library_test.cc \ persistent_integer_test.cc \ - serialization/serialization_utils_unittest.cc \ - timer_test.cc \ + timer_test.cc metricsd_tests_sources := \ uploader/metrics_hashes_unittest.cc \ uploader/metrics_log_base_unittest.cc \ uploader/mock/sender_mock.cc \ - uploader/upload_service_test.cc \ + uploader/upload_service_test.cc metrics_CFLAGS := -Wall \ -Wno-char-subscripts \ @@ -70,7 +67,7 @@ metrics_CPPFLAGS := -Wno-non-virtual-dtor \ -fvisibility=default metrics_includes := external/gtest/include \ $(LOCAL_PATH)/include -libmetrics_shared_libraries := libchrome libbrillo +libmetrics_shared_libraries := libchrome libbinder libbrillo libutils metrics_collector_shared_libraries := $(libmetrics_shared_libraries) \ libbrillo-dbus \ libbrillo-http \ @@ -78,14 +75,24 @@ metrics_collector_shared_libraries := $(libmetrics_shared_libraries) \ libdbus \ libmetrics \ librootdev \ - libweaved \ + libweaved metricsd_shared_libraries := \ + libbinder \ libbrillo \ libbrillo-http \ libchrome \ libprotobuf-cpp-lite \ libupdate_engine_client \ + libutils + +# Static proxy library for the binder interface. +# ======================================================== +include $(CLEAR_VARS) +LOCAL_MODULE := metricsd_binder_proxy +LOCAL_SHARED_LIBRARIES := libbinder libutils +LOCAL_SRC_FILES := aidl/android/brillo/metrics/IMetricsd.aidl +include $(BUILD_STATIC_LIBRARY) # Shared library for metrics. # ======================================================== @@ -100,6 +107,7 @@ LOCAL_RTTI_FLAG := -frtti LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include LOCAL_SHARED_LIBRARIES := $(libmetrics_shared_libraries) LOCAL_SRC_FILES := $(libmetrics_sources) +LOCAL_STATIC_LIBRARIES := metricsd_binder_proxy include $(BUILD_SHARED_LIBRARY) # CLI client for metrics. @@ -114,6 +122,7 @@ LOCAL_CPPFLAGS := $(metrics_CPPFLAGS) LOCAL_SHARED_LIBRARIES := $(libmetrics_shared_libraries) \ libmetrics LOCAL_SRC_FILES := $(metrics_client_sources) +LOCAL_STATIC_LIBRARIES := metricsd_binder_proxy include $(BUILD_EXECUTABLE) # Protobuf library for metricsd. @@ -144,6 +153,7 @@ LOCAL_RTTI_FLAG := -frtti LOCAL_SHARED_LIBRARIES := $(metrics_collector_shared_libraries) LOCAL_SRC_FILES := $(metrics_collector_common) \ metrics_collector_main.cc +LOCAL_STATIC_LIBRARIES := metricsd_binder_proxy include $(BUILD_EXECUTABLE) # metricsd daemon. @@ -158,9 +168,8 @@ LOCAL_CPPFLAGS := $(metrics_CPPFLAGS) LOCAL_INIT_RC := metricsd.rc LOCAL_REQUIRED_MODULES := \ metrics_collector -LOCAL_RTTI_FLAG := -frtti LOCAL_SHARED_LIBRARIES := $(metricsd_shared_libraries) -LOCAL_STATIC_LIBRARIES := metricsd_protos +LOCAL_STATIC_LIBRARIES := metricsd_protos metricsd_binder_proxy LOCAL_SRC_FILES := $(metricsd_common) \ metricsd_main.cc include $(BUILD_EXECUTABLE) @@ -173,10 +182,9 @@ LOCAL_CFLAGS := $(metrics_CFLAGS) LOCAL_CLANG := true LOCAL_CPP_EXTENSION := $(metrics_cpp_extension) LOCAL_CPPFLAGS := $(metrics_CPPFLAGS) -Wno-sign-compare -LOCAL_RTTI_FLAG := -frtti -LOCAL_SHARED_LIBRARIES := $(metricsd_shared_libraries) libmetrics +LOCAL_SHARED_LIBRARIES := $(metricsd_shared_libraries) LOCAL_SRC_FILES := $(metricsd_tests_sources) $(metricsd_common) -LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock metricsd_protos +LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock metricsd_protos metricsd_binder_proxy include $(BUILD_NATIVE_TEST) # Unit tests for metrics_collector. @@ -191,7 +199,7 @@ LOCAL_RTTI_FLAG := -frtti LOCAL_SHARED_LIBRARIES := $(metrics_collector_shared_libraries) LOCAL_SRC_FILES := $(metrics_collector_tests_sources) \ $(metrics_collector_common) -LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock +LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock metricsd_binder_proxy include $(BUILD_NATIVE_TEST) # Weave schema files diff --git a/metricsd/aidl/android/brillo/metrics/IMetricsd.aidl b/metricsd/aidl/android/brillo/metrics/IMetricsd.aidl new file mode 100644 index 000000000..a92ff46ee --- /dev/null +++ b/metricsd/aidl/android/brillo/metrics/IMetricsd.aidl @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.brillo.metrics; + +interface IMetricsd { + oneway void recordHistogram(String name, int sample, int min, int max, + int nbuckets); + oneway void recordLinearHistogram(String name, int sample, int max); + oneway void recordSparseHistogram(String name, int sample); + oneway void recordCrash(String type); +} diff --git a/metricsd/constants.h b/metricsd/constants.h index ee0c9cb5a..4815888cd 100644 --- a/metricsd/constants.h +++ b/metricsd/constants.h @@ -22,7 +22,6 @@ static const char kSharedMetricsDirectory[] = "/data/misc/metrics/"; static const char kMetricsdDirectory[] = "/data/misc/metricsd/"; static const char kMetricsCollectorDirectory[] = "/data/misc/metrics_collector/"; -static const char kMetricsEventsFileName[] = "uma-events"; static const char kMetricsGUIDFileName[] = "Sysinfo.GUID"; static const char kMetricsServer[] = "https://clients4.google.com/uma/v2"; static const char kConsentFileName[] = "enabled"; diff --git a/metricsd/include/metrics/metrics_library.h b/metricsd/include/metrics/metrics_library.h index d2e98c8df..37dda509e 100644 --- a/metricsd/include/metrics/metrics_library.h +++ b/metricsd/include/metrics/metrics_library.h @@ -24,9 +24,17 @@ #include <base/compiler_specific.h> #include <base/files/file_path.h> #include <base/macros.h> -#include <base/memory/scoped_ptr.h> +#include <binder/IServiceManager.h> #include <gtest/gtest_prod.h> // for FRIEND_TEST +namespace android { +namespace brillo { +namespace metrics { +class IMetricsd; +} // namespace metrics +} // namespace brillo +} // namespace android + class MetricsLibraryInterface { public: virtual void Init() = 0; @@ -152,6 +160,10 @@ class MetricsLibrary : public MetricsLibraryInterface { char* buffer, int buffer_size, bool* result); + // Connects to IMetricsd if the proxy does not exist or is not alive. + // Don't block if we fail to get the proxy for any reason. + bool CheckService(); + // Time at which we last checked if metrics were enabled. time_t cached_enabled_time_; @@ -161,7 +173,8 @@ class MetricsLibrary : public MetricsLibraryInterface { // True iff we should cache the enabled/disabled status. bool use_caching_; - base::FilePath uma_events_file_; + android::sp<android::IServiceManager> manager_; + android::sp<android::brillo::metrics::IMetricsd> metricsd_proxy_; base::FilePath consent_file_; DISALLOW_COPY_AND_ASSIGN(MetricsLibrary); diff --git a/metricsd/metrics_client.cc b/metricsd/metrics_client.cc index 946b36a86..5d7355582 100644 --- a/metricsd/metrics_client.cc +++ b/metricsd/metrics_client.cc @@ -21,11 +21,8 @@ #include "constants.h" #include "metrics/metrics_library.h" -#include "serialization/metric_sample.h" -#include "serialization/serialization_utils.h" enum Mode { - kModeDumpLogs, kModeSendSample, kModeSendEnumSample, kModeSendSparseSample, @@ -48,7 +45,6 @@ void ShowUsage() { " |min| > 0, |min| <= sample < |max|\n" " -c: return exit status 0 if user consents to stats, 1 otherwise,\n" " in guest mode always return 1\n" - " -d: dump cached logs to the console\n" " -e: send linear/enumeration histogram data\n" " -g: return exit status 0 if machine in guest mode, 1 otherwise\n" " -s: send a sparse histogram sample\n" @@ -139,59 +135,17 @@ static int IsGuestMode() { return metrics_lib.IsGuestMode() ? 0 : 1; } -static int DumpLogs() { - base::FilePath events_file = base::FilePath(metrics::kSharedMetricsDirectory) - .Append(metrics::kMetricsEventsFileName); - printf("Metrics from %s\n\n", events_file.value().data()); - - ScopedVector<metrics::MetricSample> metrics; - metrics::SerializationUtils::ReadMetricsFromFile(events_file.value(), - &metrics); - - for (ScopedVector<metrics::MetricSample>::const_iterator i = metrics.begin(); - i != metrics.end(); ++i) { - const metrics::MetricSample* sample = *i; - printf("name: %s\t", sample->name().c_str()); - printf("type: "); - - switch (sample->type()) { - case metrics::MetricSample::CRASH: - printf("CRASH"); - break; - case metrics::MetricSample::HISTOGRAM: - printf("HISTOGRAM"); - break; - case metrics::MetricSample::LINEAR_HISTOGRAM: - printf("LINEAR_HISTOGRAM"); - break; - case metrics::MetricSample::SPARSE_HISTOGRAM: - printf("SPARSE_HISTOGRAM"); - break; - case metrics::MetricSample::USER_ACTION: - printf("USER_ACTION"); - break; - } - - printf("\n"); - } - - return 0; -} - int main(int argc, char** argv) { enum Mode mode = kModeSendSample; bool secs_to_msecs = false; // Parse arguments int flag; - while ((flag = getopt(argc, argv, "abcdegstuv")) != -1) { + while ((flag = getopt(argc, argv, "abcegstuv")) != -1) { switch (flag) { case 'c': mode = kModeHasConsent; break; - case 'd': - mode = kModeDumpLogs; - break; case 'e': mode = kModeSendEnumSample; break; @@ -252,8 +206,6 @@ int main(int argc, char** argv) { return HasConsent(); case kModeIsGuestMode: return IsGuestMode(); - case kModeDumpLogs: - return DumpLogs(); default: ShowUsage(); return 0; diff --git a/metricsd/metrics_library.cc b/metricsd/metrics_library.cc index 686c92661..bc0aadd60 100644 --- a/metricsd/metrics_library.cc +++ b/metricsd/metrics_library.cc @@ -18,19 +18,21 @@ #include <base/logging.h> #include <base/strings/stringprintf.h> +#include <binder/IServiceManager.h> #include <errno.h> #include <sys/file.h> #include <sys/stat.h> +#include <utils/String16.h> #include <cstdio> #include <cstring> +#include "android/brillo/metrics/IMetricsd.h" #include "constants.h" -#include "serialization/metric_sample.h" -#include "serialization/serialization_utils.h" static const char kCrosEventHistogramName[] = "Platform.CrOSEvent"; static const int kCrosEventHistogramMax = 100; +static const char kMetricsServiceName[] = "android.brillo.metrics.IMetricsd"; /* Add new cros events here. * @@ -53,6 +55,10 @@ static const char *kCrosEventNames[] = { "TPM.EarlyResetDuringCommand", // 12 }; +using android::binder::Status; +using android::brillo::metrics::IMetricsd; +using android::String16; + MetricsLibrary::MetricsLibrary() {} MetricsLibrary::~MetricsLibrary() {} @@ -123,6 +129,17 @@ bool MetricsLibrary::IsGuestMode() { return result && (access("/var/run/state/logged-in", F_OK) == 0); } +bool MetricsLibrary::CheckService() { + if (metricsd_proxy_.get() && + android::IInterface::asBinder(metricsd_proxy_)->isBinderAlive()) + return true; + + const String16 name(kMetricsServiceName); + metricsd_proxy_ = android::interface_cast<IMetricsd>( + android::defaultServiceManager()->checkService(name)); + return metricsd_proxy_.get(); +} + bool MetricsLibrary::AreMetricsEnabled() { static struct stat stat_buffer; time_t this_check_time = time(nullptr); @@ -135,7 +152,6 @@ bool MetricsLibrary::AreMetricsEnabled() { void MetricsLibrary::Init() { base::FilePath dir = base::FilePath(metrics::kSharedMetricsDirectory); - uma_events_file_ = dir.Append(metrics::kMetricsEventsFileName); consent_file_ = dir.Append(metrics::kConsentFileName); cached_enabled_ = false; cached_enabled_time_ = 0; @@ -148,54 +164,52 @@ void MetricsLibrary::InitWithNoCaching() { } void MetricsLibrary::InitForTest(const base::FilePath& metrics_directory) { - uma_events_file_ = metrics_directory.Append(metrics::kMetricsEventsFileName); consent_file_ = metrics_directory.Append(metrics::kConsentFileName); cached_enabled_ = false; cached_enabled_time_ = 0; use_caching_ = true; } -bool MetricsLibrary::SendToUMA(const std::string& name, - int sample, - int min, - int max, - int nbuckets) { - return metrics::SerializationUtils::WriteMetricToFile( - *metrics::MetricSample::HistogramSample(name, sample, min, max, nbuckets) - .get(), - uma_events_file_.value()); +bool MetricsLibrary::SendToUMA( + const std::string& name, int sample, int min, int max, int nbuckets) { + return CheckService() && + metricsd_proxy_->recordHistogram(String16(name.c_str()), sample, min, + max, nbuckets) + .isOk(); } -bool MetricsLibrary::SendEnumToUMA(const std::string& name, int sample, +bool MetricsLibrary::SendEnumToUMA(const std::string& name, + int sample, int max) { - return metrics::SerializationUtils::WriteMetricToFile( - *metrics::MetricSample::LinearHistogramSample(name, sample, max).get(), - uma_events_file_.value()); + return CheckService() && + metricsd_proxy_->recordLinearHistogram(String16(name.c_str()), sample, + max) + .isOk(); } bool MetricsLibrary::SendBoolToUMA(const std::string& name, bool sample) { - return metrics::SerializationUtils::WriteMetricToFile( - *metrics::MetricSample::LinearHistogramSample(name, - sample ? 1 : 0, 2).get(), - uma_events_file_.value()); + return CheckService() && + metricsd_proxy_->recordLinearHistogram(String16(name.c_str()), + sample ? 1 : 0, 2) + .isOk(); } bool MetricsLibrary::SendSparseToUMA(const std::string& name, int sample) { - return metrics::SerializationUtils::WriteMetricToFile( - *metrics::MetricSample::SparseHistogramSample(name, sample).get(), - uma_events_file_.value()); + return CheckService() && + metricsd_proxy_->recordSparseHistogram(String16(name.c_str()), sample) + .isOk(); } bool MetricsLibrary::SendUserActionToUMA(const std::string& action) { - return metrics::SerializationUtils::WriteMetricToFile( - *metrics::MetricSample::UserActionSample(action).get(), - uma_events_file_.value()); + // Deprecated. + // TODO(bsimonnet): Delete this method entirely once all the callers are + // removed (b/25818567). + return true; } -bool MetricsLibrary::SendCrashToUMA(const char *crash_kind) { - return metrics::SerializationUtils::WriteMetricToFile( - *metrics::MetricSample::CrashSample(crash_kind).get(), - uma_events_file_.value()); +bool MetricsLibrary::SendCrashToUMA(const char* crash_kind) { + return CheckService() && + metricsd_proxy_->recordCrash(String16(crash_kind)).isOk(); } bool MetricsLibrary::SendCrosEventToUMA(const std::string& event) { diff --git a/metricsd/metrics_library_test.cc b/metricsd/metrics_library_test.cc index be8a4bb98..52fcce3a0 100644 --- a/metricsd/metrics_library_test.cc +++ b/metricsd/metrics_library_test.cc @@ -29,7 +29,6 @@ class MetricsLibraryTest : public testing::Test { virtual void SetUp() { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); lib_.InitForTest(temp_dir_.path()); - EXPECT_EQ(0, WriteFile(lib_.uma_events_file_, "", 0)); // Defeat metrics enabled caching between tests. lib_.cached_enabled_time_ = 0; } diff --git a/metricsd/metricsd_main.cc b/metricsd/metricsd_main.cc index eee8a94aa..f460268c7 100644 --- a/metricsd/metricsd_main.cc +++ b/metricsd/metricsd_main.cc @@ -14,34 +14,34 @@ * limitations under the License. */ +#include <thread> + #include <base/at_exit.h> #include <base/command_line.h> #include <base/files/file_path.h> #include <base/logging.h> +#include <base/metrics/statistics_recorder.h> #include <base/strings/string_util.h> #include <base/time/time.h> #include <brillo/flag_helper.h> #include <brillo/syslog_logging.h> #include "constants.h" +#include "uploader/bn_metricsd_impl.h" +#include "uploader/crash_counters.h" #include "uploader/upload_service.h" - int main(int argc, char** argv) { DEFINE_bool(foreground, false, "Don't daemonize"); // Upload the metrics once and exit. (used for testing) - DEFINE_bool(uploader_test, - false, - "run the uploader once and exit"); + DEFINE_bool(uploader_test, false, "run the uploader once and exit"); // Upload Service flags. - DEFINE_int32(upload_interval_secs, - 1800, + DEFINE_int32(upload_interval_secs, 1800, "Interval at which metrics_daemon sends the metrics. (needs " "-uploader)"); - DEFINE_string(server, - metrics::kMetricsServer, + DEFINE_string(server, metrics::kMetricsServer, "Server to upload the metrics to. (needs -uploader)"); DEFINE_string(private_directory, metrics::kMetricsdDirectory, "Path to the private directory used by metricsd " @@ -56,8 +56,8 @@ int main(int argc, char** argv) { brillo::FlagHelper::Init(argc, argv, "Brillo metrics daemon."); - int logging_location = (FLAGS_foreground ? brillo::kLogToStderr - : brillo::kLogToSyslog); + int logging_location = + (FLAGS_foreground ? brillo::kLogToStderr : brillo::kLogToSyslog); if (FLAGS_logtosyslog) logging_location = brillo::kLogToSyslog; @@ -76,10 +76,18 @@ int main(int argc, char** argv) { return errno; } - UploadService service( + std::shared_ptr<CrashCounters> counters(new CrashCounters); + + UploadService upload_service( FLAGS_server, base::TimeDelta::FromSeconds(FLAGS_upload_interval_secs), base::FilePath(FLAGS_private_directory), - base::FilePath(FLAGS_shared_directory)); + base::FilePath(FLAGS_shared_directory), counters); + + base::StatisticsRecorder::Initialize(); + + // Create and start the binder thread. + BnMetricsdImpl binder_service(counters); + std::thread binder_thread(&BnMetricsdImpl::Run, &binder_service); - service.Run(); + upload_service.Run(); } diff --git a/metricsd/serialization/metric_sample.cc b/metricsd/serialization/metric_sample.cc deleted file mode 100644 index 76a47c095..000000000 --- a/metricsd/serialization/metric_sample.cc +++ /dev/null @@ -1,209 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "serialization/metric_sample.h" - -#include <string> -#include <vector> - -#include "base/logging.h" -#include "base/strings/string_number_conversions.h" -#include "base/strings/string_split.h" -#include "base/strings/stringprintf.h" - -namespace metrics { - -MetricSample::MetricSample(MetricSample::SampleType sample_type, - const std::string& metric_name, - int sample, - int min, - int max, - int bucket_count) - : type_(sample_type), - name_(metric_name), - sample_(sample), - min_(min), - max_(max), - bucket_count_(bucket_count) { -} - -MetricSample::~MetricSample() { -} - -bool MetricSample::IsValid() const { - return name().find(' ') == std::string::npos && - name().find('\0') == std::string::npos && !name().empty(); -} - -std::string MetricSample::ToString() const { - if (type_ == CRASH) { - return base::StringPrintf("crash%c%s%c", - '\0', - name().c_str(), - '\0'); - } else if (type_ == SPARSE_HISTOGRAM) { - return base::StringPrintf("sparsehistogram%c%s %d%c", - '\0', - name().c_str(), - sample_, - '\0'); - } else if (type_ == LINEAR_HISTOGRAM) { - return base::StringPrintf("linearhistogram%c%s %d %d%c", - '\0', - name().c_str(), - sample_, - max_, - '\0'); - } else if (type_ == HISTOGRAM) { - return base::StringPrintf("histogram%c%s %d %d %d %d%c", - '\0', - name().c_str(), - sample_, - min_, - max_, - bucket_count_, - '\0'); - } else { - // The type can only be USER_ACTION. - CHECK_EQ(type_, USER_ACTION); - return base::StringPrintf("useraction%c%s%c", - '\0', - name().c_str(), - '\0'); - } -} - -int MetricSample::sample() const { - CHECK_NE(type_, USER_ACTION); - CHECK_NE(type_, CRASH); - return sample_; -} - -int MetricSample::min() const { - CHECK_EQ(type_, HISTOGRAM); - return min_; -} - -int MetricSample::max() const { - CHECK_NE(type_, CRASH); - CHECK_NE(type_, USER_ACTION); - CHECK_NE(type_, SPARSE_HISTOGRAM); - return max_; -} - -int MetricSample::bucket_count() const { - CHECK_EQ(type_, HISTOGRAM); - return bucket_count_; -} - -// static -scoped_ptr<MetricSample> MetricSample::CrashSample( - const std::string& crash_name) { - return scoped_ptr<MetricSample>( - new MetricSample(CRASH, crash_name, 0, 0, 0, 0)); -} - -// static -scoped_ptr<MetricSample> MetricSample::HistogramSample( - const std::string& histogram_name, - int sample, - int min, - int max, - int bucket_count) { - return scoped_ptr<MetricSample>(new MetricSample( - HISTOGRAM, histogram_name, sample, min, max, bucket_count)); -} - -// static -scoped_ptr<MetricSample> MetricSample::ParseHistogram( - const std::string& serialized_histogram) { - std::vector<std::string> parts; - base::SplitString(serialized_histogram, ' ', &parts); - - if (parts.size() != 5) - return scoped_ptr<MetricSample>(); - int sample, min, max, bucket_count; - if (parts[0].empty() || !base::StringToInt(parts[1], &sample) || - !base::StringToInt(parts[2], &min) || - !base::StringToInt(parts[3], &max) || - !base::StringToInt(parts[4], &bucket_count)) { - return scoped_ptr<MetricSample>(); - } - - return HistogramSample(parts[0], sample, min, max, bucket_count); -} - -// static -scoped_ptr<MetricSample> MetricSample::SparseHistogramSample( - const std::string& histogram_name, - int sample) { - return scoped_ptr<MetricSample>( - new MetricSample(SPARSE_HISTOGRAM, histogram_name, sample, 0, 0, 0)); -} - -// static -scoped_ptr<MetricSample> MetricSample::ParseSparseHistogram( - const std::string& serialized_histogram) { - std::vector<std::string> parts; - base::SplitString(serialized_histogram, ' ', &parts); - if (parts.size() != 2) - return scoped_ptr<MetricSample>(); - int sample; - if (parts[0].empty() || !base::StringToInt(parts[1], &sample)) - return scoped_ptr<MetricSample>(); - - return SparseHistogramSample(parts[0], sample); -} - -// static -scoped_ptr<MetricSample> MetricSample::LinearHistogramSample( - const std::string& histogram_name, - int sample, - int max) { - return scoped_ptr<MetricSample>( - new MetricSample(LINEAR_HISTOGRAM, histogram_name, sample, 0, max, 0)); -} - -// static -scoped_ptr<MetricSample> MetricSample::ParseLinearHistogram( - const std::string& serialized_histogram) { - std::vector<std::string> parts; - int sample, max; - base::SplitString(serialized_histogram, ' ', &parts); - if (parts.size() != 3) - return scoped_ptr<MetricSample>(); - if (parts[0].empty() || !base::StringToInt(parts[1], &sample) || - !base::StringToInt(parts[2], &max)) { - return scoped_ptr<MetricSample>(); - } - - return LinearHistogramSample(parts[0], sample, max); -} - -// static -scoped_ptr<MetricSample> MetricSample::UserActionSample( - const std::string& action_name) { - return scoped_ptr<MetricSample>( - new MetricSample(USER_ACTION, action_name, 0, 0, 0, 0)); -} - -bool MetricSample::IsEqual(const MetricSample& metric) { - return type_ == metric.type_ && name_ == metric.name_ && - sample_ == metric.sample_ && min_ == metric.min_ && - max_ == metric.max_ && bucket_count_ == metric.bucket_count_; -} - -} // namespace metrics diff --git a/metricsd/serialization/metric_sample.h b/metricsd/serialization/metric_sample.h deleted file mode 100644 index 5a4e4ae8a..000000000 --- a/metricsd/serialization/metric_sample.h +++ /dev/null @@ -1,131 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef METRICS_SERIALIZATION_METRIC_SAMPLE_H_ -#define METRICS_SERIALIZATION_METRIC_SAMPLE_H_ - -#include <string> - -#include "base/gtest_prod_util.h" -#include "base/macros.h" -#include "base/memory/scoped_ptr.h" - -namespace metrics { - -// This class is used by libmetrics (ChromeOS) to serialize -// and deserialize measurements to send them to a metrics sending service. -// It is meant to be a simple container with serialization functions. -class MetricSample { - public: - // Types of metric sample used. - enum SampleType { - CRASH, - HISTOGRAM, - LINEAR_HISTOGRAM, - SPARSE_HISTOGRAM, - USER_ACTION - }; - - ~MetricSample(); - - // Returns true if the sample is valid (can be serialized without ambiguity). - // - // This function should be used to filter bad samples before serializing them. - bool IsValid() const; - - // Getters for type and name. All types of metrics have these so we do not - // need to check the type. - SampleType type() const { return type_; } - const std::string& name() const { return name_; } - - // Getters for sample, min, max, bucket_count. - // Check the metric type to make sure the request make sense. (ex: a crash - // sample does not have a bucket_count so we crash if we call bucket_count() - // on it.) - int sample() const; - int min() const; - int max() const; - int bucket_count() const; - - // Returns a serialized version of the sample. - // - // The serialized message for each type is: - // crash: crash\0|name_|\0 - // user action: useraction\0|name_|\0 - // histogram: histogram\0|name_| |sample_| |min_| |max_| |bucket_count_|\0 - // sparsehistogram: sparsehistogram\0|name_| |sample_|\0 - // linearhistogram: linearhistogram\0|name_| |sample_| |max_|\0 - std::string ToString() const; - - // Builds a crash sample. - static scoped_ptr<MetricSample> CrashSample(const std::string& crash_name); - - // Builds a histogram sample. - static scoped_ptr<MetricSample> HistogramSample( - const std::string& histogram_name, - int sample, - int min, - int max, - int bucket_count); - // Deserializes a histogram sample. - static scoped_ptr<MetricSample> ParseHistogram(const std::string& serialized); - - // Builds a sparse histogram sample. - static scoped_ptr<MetricSample> SparseHistogramSample( - const std::string& histogram_name, - int sample); - // Deserializes a sparse histogram sample. - static scoped_ptr<MetricSample> ParseSparseHistogram( - const std::string& serialized); - - // Builds a linear histogram sample. - static scoped_ptr<MetricSample> LinearHistogramSample( - const std::string& histogram_name, - int sample, - int max); - // Deserializes a linear histogram sample. - static scoped_ptr<MetricSample> ParseLinearHistogram( - const std::string& serialized); - - // Builds a user action sample. - static scoped_ptr<MetricSample> UserActionSample( - const std::string& action_name); - - // Returns true if sample and this object represent the same sample (type, - // name, sample, min, max, bucket_count match). - bool IsEqual(const MetricSample& sample); - - private: - MetricSample(SampleType sample_type, - const std::string& metric_name, - const int sample, - const int min, - const int max, - const int bucket_count); - - const SampleType type_; - const std::string name_; - const int sample_; - const int min_; - const int max_; - const int bucket_count_; - - DISALLOW_COPY_AND_ASSIGN(MetricSample); -}; - -} // namespace metrics - -#endif // METRICS_SERIALIZATION_METRIC_SAMPLE_H_ diff --git a/metricsd/serialization/serialization_utils.cc b/metricsd/serialization/serialization_utils.cc deleted file mode 100644 index 102c9404a..000000000 --- a/metricsd/serialization/serialization_utils.cc +++ /dev/null @@ -1,265 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "serialization/serialization_utils.h" - -#include <sys/file.h> - -#include <string> -#include <vector> - -#include "base/files/file_path.h" -#include "base/files/file_util.h" -#include "base/files/scoped_file.h" -#include "base/logging.h" -#include "base/memory/scoped_ptr.h" -#include "base/memory/scoped_vector.h" -#include "base/strings/string_split.h" -#include "base/strings/string_util.h" -#include "serialization/metric_sample.h" - -#define READ_WRITE_ALL_FILE_FLAGS \ - (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) - -namespace metrics { -namespace { - -// Reads the next message from |file_descriptor| into |message|. -// -// |message| will be set to the empty string if no message could be read (EOF) -// or the message was badly constructed. -// -// Returns false if no message can be read from this file anymore (EOF or -// unrecoverable error). -bool ReadMessage(int fd, std::string* message) { - CHECK(message); - - int result; - int32_t message_size; - const int32_t message_hdr_size = sizeof(message_size); - // The file containing the metrics do not leave the device so the writer and - // the reader will always have the same endianness. - result = HANDLE_EINTR(read(fd, &message_size, sizeof(message_size))); - if (result < 0) { - DPLOG(ERROR) << "reading metrics message header"; - return false; - } - if (result == 0) { - // This indicates a normal EOF. - return false; - } - if (result < message_hdr_size) { - DLOG(ERROR) << "bad read size " << result << ", expecting " - << sizeof(message_size); - return false; - } - - // kMessageMaxLength applies to the entire message: the 4-byte - // length field and the content. - if (message_size > SerializationUtils::kMessageMaxLength) { - DLOG(ERROR) << "message too long : " << message_size; - if (HANDLE_EINTR(lseek(fd, message_size - 4, SEEK_CUR)) == -1) { - DLOG(ERROR) << "error while skipping message. abort"; - return false; - } - // Badly formatted message was skipped. Treat the badly formatted sample as - // an empty sample. - message->clear(); - return true; - } - - if (message_size < message_hdr_size) { - DLOG(ERROR) << "message too short : " << message_size; - return false; - } - - message_size -= message_hdr_size; // The message size includes itself. - char buffer[SerializationUtils::kMessageMaxLength]; - if (!base::ReadFromFD(fd, buffer, message_size)) { - DPLOG(ERROR) << "reading metrics message body"; - return false; - } - *message = std::string(buffer, message_size); - return true; -} - - -// Opens the metrics log file at |filename| in the given |mode|. -// -// Returns the file descriptor wrapped in a valid ScopedFD on success. -base::ScopedFD OpenMetricsFile(const std::string& filename, mode_t mode) { - struct stat stat_buf; - int result; - - result = stat(filename.c_str(), &stat_buf); - if (result < 0) { - if (errno != ENOENT) - DPLOG(ERROR) << filename << ": bad metrics file stat"; - - // Nothing to collect---try later. - return base::ScopedFD(); - } - if (stat_buf.st_size == 0) { - // Also nothing to collect. - return base::ScopedFD(); - } - base::ScopedFD fd(open(filename.c_str(), mode)); - if (fd.get() < 0) { - DPLOG(ERROR) << filename << ": cannot open"; - return base::ScopedFD(); - } - - return fd.Pass(); -} - - -// Parses the contents of the metrics log file descriptor |fd| into |metrics|. -void ReadAllMetricsFromFd(int fd, ScopedVector<MetricSample>* metrics) { - for (;;) { - std::string message; - - if (!ReadMessage(fd, &message)) - break; - - scoped_ptr<MetricSample> sample = SerializationUtils::ParseSample(message); - if (sample) - metrics->push_back(sample.release()); - } -} - -} // namespace - -scoped_ptr<MetricSample> SerializationUtils::ParseSample( - const std::string& sample) { - if (sample.empty()) - return scoped_ptr<MetricSample>(); - - std::vector<std::string> parts; - base::SplitString(sample, '\0', &parts); - // We should have two null terminated strings so split should produce - // three chunks. - if (parts.size() != 3) { - DLOG(ERROR) << "splitting message on \\0 produced " << parts.size() - << " parts (expected 3)"; - return scoped_ptr<MetricSample>(); - } - const std::string& name = parts[0]; - const std::string& value = parts[1]; - - if (base::LowerCaseEqualsASCII(name, "crash")) { - return MetricSample::CrashSample(value); - } else if (base::LowerCaseEqualsASCII(name, "histogram")) { - return MetricSample::ParseHistogram(value); - } else if (base::LowerCaseEqualsASCII(name, "linearhistogram")) { - return MetricSample::ParseLinearHistogram(value); - } else if (base::LowerCaseEqualsASCII(name, "sparsehistogram")) { - return MetricSample::ParseSparseHistogram(value); - } else if (base::LowerCaseEqualsASCII(name, "useraction")) { - return MetricSample::UserActionSample(value); - } else { - DLOG(ERROR) << "invalid event type: " << name << ", value: " << value; - } - return scoped_ptr<MetricSample>(); -} - -void SerializationUtils::ReadMetricsFromFile( - const std::string& filename, - ScopedVector<MetricSample>* metrics) { - base::ScopedFD fd(OpenMetricsFile(filename, O_RDONLY)); - if (!fd.is_valid()) { - return; - } - - // This processes all messages in the log. - ReadAllMetricsFromFd(fd.get(), metrics); -} - -void SerializationUtils::ReadAndTruncateMetricsFromFile( - const std::string& filename, - ScopedVector<MetricSample>* metrics) { - base::ScopedFD fd(OpenMetricsFile(filename, O_RDWR)); - if (!fd.is_valid()) { - return; - } - - int result = flock(fd.get(), LOCK_EX); - if (result < 0) { - DPLOG(ERROR) << filename << ": cannot lock"; - return; - } - - // This processes all messages in the log. When all messages are - // read and processed, or an error occurs, truncate the file to zero size. - ReadAllMetricsFromFd(fd.get(), metrics); - - result = ftruncate(fd.get(), 0); - if (result < 0) - DPLOG(ERROR) << "truncate metrics log"; - - result = flock(fd.get(), LOCK_UN); - if (result < 0) - DPLOG(ERROR) << "unlock metrics log"; -} - -bool SerializationUtils::WriteMetricToFile(const MetricSample& sample, - const std::string& filename) { - if (!sample.IsValid()) - return false; - - base::ScopedFD file_descriptor(open(filename.c_str(), - O_WRONLY | O_APPEND | O_CREAT, - READ_WRITE_ALL_FILE_FLAGS)); - - if (file_descriptor.get() < 0) { - DPLOG(ERROR) << filename << ": cannot open"; - return false; - } - - fchmod(file_descriptor.get(), READ_WRITE_ALL_FILE_FLAGS); - // Grab a lock to avoid chrome truncating the file - // underneath us. Keep the file locked as briefly as possible. - // Freeing file_descriptor will close the file and and remove the lock. - if (HANDLE_EINTR(flock(file_descriptor.get(), LOCK_EX)) < 0) { - DPLOG(ERROR) << filename << ": cannot lock"; - return false; - } - - std::string msg = sample.ToString(); - int32 size = msg.length() + sizeof(int32); - if (size > kMessageMaxLength) { - DLOG(ERROR) << "cannot write message: too long"; - return false; - } - - // The file containing the metrics samples will only be read by programs on - // the same device so we do not check endianness. - if (!base::WriteFileDescriptor(file_descriptor.get(), - reinterpret_cast<char*>(&size), - sizeof(size))) { - DPLOG(ERROR) << "error writing message length"; - return false; - } - - if (!base::WriteFileDescriptor( - file_descriptor.get(), msg.c_str(), msg.size())) { - DPLOG(ERROR) << "error writing message"; - return false; - } - - return true; -} - -} // namespace metrics diff --git a/metricsd/serialization/serialization_utils.h b/metricsd/serialization/serialization_utils.h deleted file mode 100644 index 655652d50..000000000 --- a/metricsd/serialization/serialization_utils.h +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef METRICS_SERIALIZATION_SERIALIZATION_UTILS_H_ -#define METRICS_SERIALIZATION_SERIALIZATION_UTILS_H_ - -#include <string> - -#include "base/memory/scoped_ptr.h" -#include "base/memory/scoped_vector.h" - -namespace metrics { - -class MetricSample; - -// Metrics helpers to serialize and deserialize metrics collected by -// ChromeOS. -namespace SerializationUtils { - -// Deserializes a sample passed as a string and return a sample. -// The return value will either be a scoped_ptr to a Metric sample (if the -// deserialization was successful) or a NULL scoped_ptr. -scoped_ptr<MetricSample> ParseSample(const std::string& sample); - -// Reads all samples from a file. The file contents remain unchanged. -void ReadMetricsFromFile(const std::string& filename, - ScopedVector<MetricSample>* metrics); - -// Reads all samples from a file and truncates the file when done. -void ReadAndTruncateMetricsFromFile(const std::string& filename, - ScopedVector<MetricSample>* metrics); - -// Serializes a sample and write it to filename. -// The format for the message is: -// message_size, serialized_message -// where -// * message_size is the total length of the message (message_size + -// serialized_message) on 4 bytes -// * serialized_message is the serialized version of sample (using ToString) -// -// NB: the file will never leave the device so message_size will be written -// with the architecture's endianness. -bool WriteMetricToFile(const MetricSample& sample, const std::string& filename); - -// Maximum length of a serialized message -static const int kMessageMaxLength = 1024; - -} // namespace SerializationUtils -} // namespace metrics - -#endif // METRICS_SERIALIZATION_SERIALIZATION_UTILS_H_ diff --git a/metricsd/serialization/serialization_utils_unittest.cc b/metricsd/serialization/serialization_utils_unittest.cc deleted file mode 100644 index 7a572de98..000000000 --- a/metricsd/serialization/serialization_utils_unittest.cc +++ /dev/null @@ -1,181 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "serialization/serialization_utils.h" - -#include <base/files/file_util.h> -#include <base/files/scoped_temp_dir.h> -#include <base/logging.h> -#include <base/strings/stringprintf.h> -#include <gtest/gtest.h> - -#include "serialization/metric_sample.h" - -namespace metrics { -namespace { - -class SerializationUtilsTest : public testing::Test { - protected: - SerializationUtilsTest() { - bool success = temporary_dir.CreateUniqueTempDir(); - if (success) { - base::FilePath dir_path = temporary_dir.path(); - filename = dir_path.value() + "chromeossampletest"; - filepath = base::FilePath(filename); - } - } - - void SetUp() override { base::DeleteFile(filepath, false); } - - void TestSerialization(MetricSample* sample) { - std::string serialized(sample->ToString()); - ASSERT_EQ('\0', serialized[serialized.length() - 1]); - scoped_ptr<MetricSample> deserialized = - SerializationUtils::ParseSample(serialized); - ASSERT_TRUE(deserialized); - EXPECT_TRUE(sample->IsEqual(*deserialized.get())); - } - - std::string filename; - base::ScopedTempDir temporary_dir; - base::FilePath filepath; -}; - -TEST_F(SerializationUtilsTest, CrashSerializeTest) { - TestSerialization(MetricSample::CrashSample("test").get()); -} - -TEST_F(SerializationUtilsTest, HistogramSerializeTest) { - TestSerialization( - MetricSample::HistogramSample("myhist", 13, 1, 100, 10).get()); -} - -TEST_F(SerializationUtilsTest, LinearSerializeTest) { - TestSerialization( - MetricSample::LinearHistogramSample("linearhist", 12, 30).get()); -} - -TEST_F(SerializationUtilsTest, SparseSerializeTest) { - TestSerialization(MetricSample::SparseHistogramSample("mysparse", 30).get()); -} - -TEST_F(SerializationUtilsTest, UserActionSerializeTest) { - TestSerialization(MetricSample::UserActionSample("myaction").get()); -} - -TEST_F(SerializationUtilsTest, IllegalNameAreFilteredTest) { - scoped_ptr<MetricSample> sample1 = - MetricSample::SparseHistogramSample("no space", 10); - scoped_ptr<MetricSample> sample2 = MetricSample::LinearHistogramSample( - base::StringPrintf("here%cbhe", '\0'), 1, 3); - - EXPECT_FALSE(SerializationUtils::WriteMetricToFile(*sample1.get(), filename)); - EXPECT_FALSE(SerializationUtils::WriteMetricToFile(*sample2.get(), filename)); - int64 size = 0; - - ASSERT_TRUE(!PathExists(filepath) || base::GetFileSize(filepath, &size)); - - EXPECT_EQ(0, size); -} - -TEST_F(SerializationUtilsTest, BadInputIsCaughtTest) { - std::string input( - base::StringPrintf("sparsehistogram%cname foo%c", '\0', '\0')); - EXPECT_EQ(NULL, MetricSample::ParseSparseHistogram(input).get()); -} - -TEST_F(SerializationUtilsTest, MessageSeparatedByZero) { - scoped_ptr<MetricSample> crash = MetricSample::CrashSample("mycrash"); - - SerializationUtils::WriteMetricToFile(*crash.get(), filename); - int64 size = 0; - ASSERT_TRUE(base::GetFileSize(filepath, &size)); - // 4 bytes for the size - // 5 bytes for crash - // 7 bytes for mycrash - // 2 bytes for the \0 - // -> total of 18 - EXPECT_EQ(size, 18); -} - -TEST_F(SerializationUtilsTest, MessagesTooLongAreDiscardedTest) { - // Creates a message that is bigger than the maximum allowed size. - // As we are adding extra character (crash, \0s, etc), if the name is - // kMessageMaxLength long, it will be too long. - std::string name(SerializationUtils::kMessageMaxLength, 'c'); - - scoped_ptr<MetricSample> crash = MetricSample::CrashSample(name); - EXPECT_FALSE(SerializationUtils::WriteMetricToFile(*crash.get(), filename)); - int64 size = 0; - ASSERT_TRUE(base::GetFileSize(filepath, &size)); - EXPECT_EQ(0, size); -} - -TEST_F(SerializationUtilsTest, ReadLongMessageTest) { - base::File test_file(filepath, - base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_APPEND); - std::string message(SerializationUtils::kMessageMaxLength + 1, 'c'); - - int32 message_size = message.length() + sizeof(int32); - test_file.WriteAtCurrentPos(reinterpret_cast<const char*>(&message_size), - sizeof(message_size)); - test_file.WriteAtCurrentPos(message.c_str(), message.length()); - test_file.Close(); - - scoped_ptr<MetricSample> crash = MetricSample::CrashSample("test"); - SerializationUtils::WriteMetricToFile(*crash.get(), filename); - - ScopedVector<MetricSample> samples; - SerializationUtils::ReadAndTruncateMetricsFromFile(filename, &samples); - ASSERT_EQ(size_t(1), samples.size()); - ASSERT_TRUE(samples[0] != NULL); - EXPECT_TRUE(crash->IsEqual(*samples[0])); -} - -TEST_F(SerializationUtilsTest, WriteReadTest) { - scoped_ptr<MetricSample> hist = - MetricSample::HistogramSample("myhist", 1, 2, 3, 4); - scoped_ptr<MetricSample> crash = MetricSample::CrashSample("mycrash"); - scoped_ptr<MetricSample> lhist = - MetricSample::LinearHistogramSample("linear", 1, 10); - scoped_ptr<MetricSample> shist = - MetricSample::SparseHistogramSample("mysparse", 30); - scoped_ptr<MetricSample> action = MetricSample::UserActionSample("myaction"); - - SerializationUtils::WriteMetricToFile(*hist.get(), filename); - SerializationUtils::WriteMetricToFile(*crash.get(), filename); - SerializationUtils::WriteMetricToFile(*lhist.get(), filename); - SerializationUtils::WriteMetricToFile(*shist.get(), filename); - SerializationUtils::WriteMetricToFile(*action.get(), filename); - ScopedVector<MetricSample> vect; - SerializationUtils::ReadAndTruncateMetricsFromFile(filename, &vect); - ASSERT_EQ(vect.size(), size_t(5)); - for (int i = 0; i < 5; i++) { - ASSERT_TRUE(vect[0] != NULL); - } - EXPECT_TRUE(hist->IsEqual(*vect[0])); - EXPECT_TRUE(crash->IsEqual(*vect[1])); - EXPECT_TRUE(lhist->IsEqual(*vect[2])); - EXPECT_TRUE(shist->IsEqual(*vect[3])); - EXPECT_TRUE(action->IsEqual(*vect[4])); - - int64 size = 0; - ASSERT_TRUE(base::GetFileSize(filepath, &size)); - ASSERT_EQ(0, size); -} - -} // namespace -} // namespace metrics diff --git a/metricsd/uploader/bn_metricsd_impl.cc b/metricsd/uploader/bn_metricsd_impl.cc new file mode 100644 index 000000000..1e3800232 --- /dev/null +++ b/metricsd/uploader/bn_metricsd_impl.cc @@ -0,0 +1,98 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "uploader/bn_metricsd_impl.h" + +#include <base/metrics/histogram.h> +#include <base/metrics/sparse_histogram.h> +#include <binder/IPCThreadState.h> +#include <binder/IServiceManager.h> +#include <utils/String16.h> +#include <utils/String8.h> + +using android::binder::Status; +using android::String16; + +static const char16_t kCrashTypeKernel[] = u"kernel"; +static const char16_t kCrashTypeUncleanShutdown[] = u"uncleanshutdown"; +static const char16_t kCrashTypeUser[] = u"user"; + +BnMetricsdImpl::BnMetricsdImpl(const std::shared_ptr<CrashCounters>& counters) + : counters_(counters) { + CHECK(counters_); +} + +void BnMetricsdImpl::Run() { + android::defaultServiceManager()->addService(getInterfaceDescriptor(), this); + android::ProcessState::self()->setThreadPoolMaxThreadCount(0); + android::IPCThreadState::self()->disableBackgroundScheduling(true); + android::IPCThreadState::self()->joinThreadPool(); +} + +Status BnMetricsdImpl::recordHistogram( + const String16& name, int sample, int min, int max, int nbuckets) { + base::HistogramBase* histogram = base::Histogram::FactoryGet( + android::String8(name).string(), min, max, nbuckets, + base::Histogram::kUmaTargetedHistogramFlag); + // |histogram| may be null if a client reports two contradicting histograms + // with the same name but different limits. + // FactoryGet will print a useful message if that is the case. + if (histogram) { + histogram->Add(sample); + } + return Status::ok(); +} + +Status BnMetricsdImpl::recordLinearHistogram(const String16& name, + int sample, + int max) { + base::HistogramBase* histogram = base::LinearHistogram::FactoryGet( + android::String8(name).string(), 1, max, max + 1, + base::Histogram::kUmaTargetedHistogramFlag); + // |histogram| may be null if a client reports two contradicting histograms + // with the same name but different limits. + // FactoryGet will print a useful message if that is the case. + if (histogram) { + histogram->Add(sample); + } + return Status::ok(); +} + +Status BnMetricsdImpl::recordSparseHistogram(const String16& name, int sample) { + base::HistogramBase* histogram = base::SparseHistogram::FactoryGet( + android::String8(name).string(), + base::Histogram::kUmaTargetedHistogramFlag); + // |histogram| may be null if a client reports two contradicting histograms + // with the same name but different limits. + // FactoryGet will print a useful message if that is the case. + if (histogram) { + histogram->Add(sample); + } + return Status::ok(); +} + +Status BnMetricsdImpl::recordCrash(const String16& type) { + if (type == kCrashTypeUser) { + counters_->IncrementUserCrashCount(); + } else if (type == kCrashTypeKernel) { + counters_->IncrementKernelCrashCount(); + } else if (type == kCrashTypeUncleanShutdown) { + counters_->IncrementUncleanShutdownCount(); + } else { + LOG(ERROR) << "Unknown crash type received: " << type; + } + return Status::ok(); +} diff --git a/metricsd/uploader/bn_metricsd_impl.h b/metricsd/uploader/bn_metricsd_impl.h new file mode 100644 index 000000000..c6e3d3681 --- /dev/null +++ b/metricsd/uploader/bn_metricsd_impl.h @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef METRICSD_UPLOADER_BN_METRICSD_IMPL_H_ +#define METRICSD_UPLOADER_BN_METRICSD_IMPL_H_ + +#include "android/brillo/metrics/BnMetricsd.h" +#include "uploader/crash_counters.h" + +class BnMetricsdImpl : public android::brillo::metrics::BnMetricsd { + public: + explicit BnMetricsdImpl(const std::shared_ptr<CrashCounters>& counters); + virtual ~BnMetricsdImpl() = default; + + // Starts the binder main loop. + void Run(); + + // Records a histogram. + android::binder::Status recordHistogram(const android::String16& name, + int sample, + int min, + int max, + int nbuckets) override; + + // Records a linear histogram. + android::binder::Status recordLinearHistogram(const android::String16& name, + int sample, + int max) override; + + // Records a sparse histogram. + android::binder::Status recordSparseHistogram(const android::String16& name, + int sample) override; + + // Records a crash. + android::binder::Status recordCrash(const android::String16& type) override; + + private: + std::shared_ptr<CrashCounters> counters_; +}; + +#endif // METRICSD_UPLOADER_BN_METRICSD_IMPL_H_ diff --git a/metricsd/uploader/crash_counters.cc b/metricsd/uploader/crash_counters.cc new file mode 100644 index 000000000..1478b9a63 --- /dev/null +++ b/metricsd/uploader/crash_counters.cc @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "uploader/crash_counters.h" + +CrashCounters::CrashCounters() + : kernel_crashes_(0), unclean_shutdowns_(0), user_crashes_(0) {} + +void CrashCounters::IncrementKernelCrashCount() { + kernel_crashes_++; +} + +unsigned int CrashCounters::GetAndResetKernelCrashCount() { + return kernel_crashes_.exchange(0); +} + +void CrashCounters::IncrementUncleanShutdownCount() { + unclean_shutdowns_++; +} + +unsigned int CrashCounters::GetAndResetUncleanShutdownCount() { + return unclean_shutdowns_.exchange(0); +} + +void CrashCounters::IncrementUserCrashCount() { + user_crashes_++; +} + +unsigned int CrashCounters::GetAndResetUserCrashCount() { + return user_crashes_.exchange(0); +} diff --git a/metricsd/uploader/crash_counters.h b/metricsd/uploader/crash_counters.h new file mode 100644 index 000000000..3fdbf3f2e --- /dev/null +++ b/metricsd/uploader/crash_counters.h @@ -0,0 +1,45 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef METRICSD_UPLOADER_CRASH_COUNTERS_H_ +#define METRICSD_UPLOADER_CRASH_COUNTERS_H_ + +#include <atomic> + +// This class is used to keep track of the crash counters. +// An instance of it will be used by both the binder thread (to increment the +// counters) and the uploader thread (to gather and reset the counters). +// As such, the internal counters are atomic uints to allow concurrent access. +class CrashCounters { + public: + CrashCounters(); + + void IncrementKernelCrashCount(); + unsigned int GetAndResetKernelCrashCount(); + + void IncrementUserCrashCount(); + unsigned int GetAndResetUserCrashCount(); + + void IncrementUncleanShutdownCount(); + unsigned int GetAndResetUncleanShutdownCount(); + + private: + std::atomic_uint kernel_crashes_; + std::atomic_uint unclean_shutdowns_; + std::atomic_uint user_crashes_; +}; + +#endif // METRICSD_UPLOADER_CRASH_COUNTERS_H_ diff --git a/metricsd/uploader/metrics_log.cc b/metricsd/uploader/metrics_log.cc index 1f16ca15d..a01b5da08 100644 --- a/metricsd/uploader/metrics_log.cc +++ b/metricsd/uploader/metrics_log.cc @@ -27,25 +27,25 @@ MetricsLog::MetricsLog() : MetricsLogBase("", 0, metrics::MetricsLogBase::ONGOING_LOG, "") { } -void MetricsLog::IncrementUserCrashCount() { +void MetricsLog::IncrementUserCrashCount(unsigned int count) { metrics::SystemProfileProto::Stability* stability( uma_proto()->mutable_system_profile()->mutable_stability()); int current = stability->other_user_crash_count(); - stability->set_other_user_crash_count(current + 1); + stability->set_other_user_crash_count(current + count); } -void MetricsLog::IncrementKernelCrashCount() { +void MetricsLog::IncrementKernelCrashCount(unsigned int count) { metrics::SystemProfileProto::Stability* stability( uma_proto()->mutable_system_profile()->mutable_stability()); int current = stability->kernel_crash_count(); - stability->set_kernel_crash_count(current + 1); + stability->set_kernel_crash_count(current + count); } -void MetricsLog::IncrementUncleanShutdownCount() { +void MetricsLog::IncrementUncleanShutdownCount(unsigned int count) { metrics::SystemProfileProto::Stability* stability( uma_proto()->mutable_system_profile()->mutable_stability()); int current = stability->unclean_system_shutdown_count(); - stability->set_unclean_system_shutdown_count(current + 1); + stability->set_unclean_system_shutdown_count(current + count); } bool MetricsLog::PopulateSystemProfile(SystemProfileSetter* profile_setter) { diff --git a/metricsd/uploader/metrics_log.h b/metricsd/uploader/metrics_log.h index 5e090701b..b76cd725b 100644 --- a/metricsd/uploader/metrics_log.h +++ b/metricsd/uploader/metrics_log.h @@ -14,8 +14,8 @@ * limitations under the License. */ -#ifndef METRICS_UPLOADER_METRICS_LOG_H_ -#define METRICS_UPLOADER_METRICS_LOG_H_ +#ifndef METRICSD_UPLOADER_METRICS_LOG_H_ +#define METRICSD_UPLOADER_METRICS_LOG_H_ #include <string> @@ -34,15 +34,20 @@ class MetricsLog : public metrics::MetricsLogBase { // SystemProfileSetter. MetricsLog(); - void IncrementUserCrashCount(); - void IncrementKernelCrashCount(); - void IncrementUncleanShutdownCount(); + // Increment the crash counters in the protobuf. + // These methods don't have to be thread safe as metrics logs are only + // accessed by the uploader thread. + void IncrementUserCrashCount(unsigned int count); + void IncrementKernelCrashCount(unsigned int count); + void IncrementUncleanShutdownCount(unsigned int count); // Populate the system profile with system information using setter. bool PopulateSystemProfile(SystemProfileSetter* setter); private: + friend class UploadServiceTest; FRIEND_TEST(UploadServiceTest, LogContainsAggregatedValues); + FRIEND_TEST(UploadServiceTest, LogContainsCrashCounts); FRIEND_TEST(UploadServiceTest, LogKernelCrash); FRIEND_TEST(UploadServiceTest, LogUncleanShutdown); FRIEND_TEST(UploadServiceTest, LogUserCrash); @@ -51,4 +56,4 @@ class MetricsLog : public metrics::MetricsLogBase { DISALLOW_COPY_AND_ASSIGN(MetricsLog); }; -#endif // METRICS_UPLOADER_METRICS_LOG_H_ +#endif // METRICSD_UPLOADER_METRICS_LOG_H_ diff --git a/metricsd/uploader/upload_service.cc b/metricsd/uploader/upload_service.cc index 3e0c503fd..ea8427ab9 100644 --- a/metricsd/uploader/upload_service.cc +++ b/metricsd/uploader/upload_service.cc @@ -33,8 +33,6 @@ #include <base/sha1.h> #include "constants.h" -#include "serialization/metric_sample.h" -#include "serialization/serialization_utils.h" #include "uploader/metrics_log.h" #include "uploader/sender_http.h" #include "uploader/system_profile_setter.h" @@ -44,21 +42,19 @@ const int UploadService::kMaxFailedUpload = 10; UploadService::UploadService(const std::string& server, const base::TimeDelta& upload_interval, const base::FilePath& private_metrics_directory, - const base::FilePath& shared_metrics_directory) + const base::FilePath& shared_metrics_directory, + const std::shared_ptr<CrashCounters> counters) : histogram_snapshot_manager_(this), sender_(new HttpSender(server)), failed_upload_count_(metrics::kFailedUploadCountName, private_metrics_directory), + counters_(counters), upload_interval_(upload_interval) { - metrics_file_ = - shared_metrics_directory.Append(metrics::kMetricsEventsFileName); staged_log_path_ = private_metrics_directory.Append(metrics::kStagedLogName); consent_file_ = shared_metrics_directory.Append(metrics::kConsentFileName); } int UploadService::OnInit() { - base::StatisticsRecorder::Initialize(); - system_profile_setter_.reset(new SystemProfileCache()); base::MessageLoop::current()->PostDelayedTask(FROM_HERE, @@ -70,7 +66,6 @@ int UploadService::OnInit() { } void UploadService::InitForTest(SystemProfileSetter* setter) { - base::StatisticsRecorder::Initialize(); system_profile_setter_.reset(setter); } @@ -102,8 +97,7 @@ void UploadService::UploadEvent() { return; } - // Previous upload successful, reading metrics sample from the file. - ReadMetrics(); + // Previous upload successful, stage another log. GatherHistograms(); StageCurrentLog(); @@ -143,76 +137,24 @@ void UploadService::Reset() { failed_upload_count_.Set(0); } -void UploadService::ReadMetrics() { - CHECK(!HasStagedLog()) << "cannot read metrics until the old logs have been " - << "discarded"; - - ScopedVector<metrics::MetricSample> vector; - metrics::SerializationUtils::ReadAndTruncateMetricsFromFile( - metrics_file_.value(), &vector); - - int i = 0; - for (ScopedVector<metrics::MetricSample>::iterator it = vector.begin(); - it != vector.end(); it++) { - metrics::MetricSample* sample = *it; - AddSample(*sample); - i++; - } - VLOG(1) << i << " samples read"; -} - -void UploadService::AddSample(const metrics::MetricSample& sample) { - base::HistogramBase* counter; - switch (sample.type()) { - case metrics::MetricSample::CRASH: - AddCrash(sample.name()); - break; - case metrics::MetricSample::HISTOGRAM: - counter = base::Histogram::FactoryGet( - sample.name(), sample.min(), sample.max(), sample.bucket_count(), - base::Histogram::kUmaTargetedHistogramFlag); - counter->Add(sample.sample()); - break; - case metrics::MetricSample::SPARSE_HISTOGRAM: - counter = base::SparseHistogram::FactoryGet( - sample.name(), base::HistogramBase::kUmaTargetedHistogramFlag); - counter->Add(sample.sample()); - break; - case metrics::MetricSample::LINEAR_HISTOGRAM: - counter = base::LinearHistogram::FactoryGet( - sample.name(), - 1, - sample.max(), - sample.max() + 1, - base::Histogram::kUmaTargetedHistogramFlag); - counter->Add(sample.sample()); - break; - case metrics::MetricSample::USER_ACTION: - GetOrCreateCurrentLog()->RecordUserAction(sample.name()); - break; - default: - break; - } -} - -void UploadService::AddCrash(const std::string& crash_name) { - if (crash_name == "user") { - GetOrCreateCurrentLog()->IncrementUserCrashCount(); - } else if (crash_name == "kernel") { - GetOrCreateCurrentLog()->IncrementKernelCrashCount(); - } else if (crash_name == "uncleanshutdown") { - GetOrCreateCurrentLog()->IncrementUncleanShutdownCount(); - } else { - DLOG(ERROR) << "crash name unknown" << crash_name; - } -} - void UploadService::GatherHistograms() { base::StatisticsRecorder::Histograms histograms; base::StatisticsRecorder::GetHistograms(&histograms); histogram_snapshot_manager_.PrepareDeltas( base::Histogram::kNoFlags, base::Histogram::kUmaTargetedHistogramFlag); + + // Gather and reset the crash counters, shared with the binder threads. + unsigned int kernel_crashes = counters_->GetAndResetKernelCrashCount(); + unsigned int unclean_shutdowns = counters_->GetAndResetUncleanShutdownCount(); + unsigned int user_crashes = counters_->GetAndResetUserCrashCount(); + + // Only create a log if the counters have changed. + if (kernel_crashes > 0 || unclean_shutdowns > 0 || user_crashes > 0) { + GetOrCreateCurrentLog()->IncrementKernelCrashCount(kernel_crashes); + GetOrCreateCurrentLog()->IncrementUncleanShutdownCount(unclean_shutdowns); + GetOrCreateCurrentLog()->IncrementUserCrashCount(user_crashes); + } } void UploadService::RecordDelta(const base::HistogramBase& histogram, diff --git a/metricsd/uploader/upload_service.h b/metricsd/uploader/upload_service.h index eed0d9de8..fe064b890 100644 --- a/metricsd/uploader/upload_service.h +++ b/metricsd/uploader/upload_service.h @@ -25,20 +25,12 @@ #include <brillo/daemons/daemon.h> #include "persistent_integer.h" +#include "uploader/crash_counters.h" #include "uploader/metrics_log.h" +#include "uploader/proto/chrome_user_metrics_extension.pb.h" #include "uploader/sender.h" #include "uploader/system_profile_cache.h" -namespace metrics { -class ChromeUserMetricsExtension; -class CrashSample; -class HistogramSample; -class LinearHistogramSample; -class MetricSample; -class SparseHistogramSample; -class UserActionSample; -} - class SystemProfileSetter; // Service responsible for uploading the metrics periodically to the server. @@ -57,22 +49,21 @@ class SystemProfileSetter; // - if the upload is successful, we discard the log (therefore // transitioning back to no staged log) // - if the upload fails, we keep the log to try again later. -// We do not try to read the metrics that are stored on -// the disk as we want to avoid storing the metrics in memory. // // * if no staged logs are present: -// Read all metrics from the disk, aggregate them and try to send them. +// Take a snapshot of the aggregated metrics, save it to disk and try to send +// it: // - if the upload succeeds, we discard the staged log (transitioning back // to the no staged log state) -// - if the upload fails, we keep the staged log in memory to retry -// uploading later. +// - if the upload fails, we continue and will retry to upload later. // class UploadService : public base::HistogramFlattener, public brillo::Daemon { public: UploadService(const std::string& server, const base::TimeDelta& upload_interval, const base::FilePath& private_metrics_directory, - const base::FilePath& shared_metrics_directory); + const base::FilePath& shared_metrics_directory, + const std::shared_ptr<CrashCounters> counters); // Initializes the upload service. int OnInit(); @@ -106,6 +97,7 @@ class UploadService : public base::HistogramFlattener, public brillo::Daemon { FRIEND_TEST(UploadServiceTest, EmptyLogsAreNotSent); FRIEND_TEST(UploadServiceTest, FailedSendAreRetried); FRIEND_TEST(UploadServiceTest, LogContainsAggregatedValues); + FRIEND_TEST(UploadServiceTest, LogContainsCrashCounts); FRIEND_TEST(UploadServiceTest, LogEmptyAfterUpload); FRIEND_TEST(UploadServiceTest, LogEmptyByDefault); FRIEND_TEST(UploadServiceTest, LogFromTheMetricsLibrary); @@ -125,15 +117,6 @@ class UploadService : public base::HistogramFlattener, public brillo::Daemon { // Resets the internal state. void Reset(); - // Reads all the metrics from the disk. - void ReadMetrics(); - - // Adds a generic sample to the current log. - void AddSample(const metrics::MetricSample& sample); - - // Adds a crash to the current log. - void AddCrash(const std::string& crash_name); - // Returns true iff metrics reporting is enabled. bool AreMetricsEnabled(); @@ -163,11 +146,11 @@ class UploadService : public base::HistogramFlattener, public brillo::Daemon { scoped_ptr<Sender> sender_; chromeos_metrics::PersistentInteger failed_upload_count_; scoped_ptr<MetricsLog> current_log_; + std::shared_ptr<CrashCounters> counters_; base::TimeDelta upload_interval_; base::FilePath consent_file_; - base::FilePath metrics_file_; base::FilePath staged_log_path_; bool testing_; diff --git a/metricsd/uploader/upload_service_test.cc b/metricsd/uploader/upload_service_test.cc index 9fc5e71d1..0e2ba8fba 100644 --- a/metricsd/uploader/upload_service_test.cc +++ b/metricsd/uploader/upload_service_test.cc @@ -20,12 +20,12 @@ #include <base/files/file_util.h> #include <base/files/scoped_temp_dir.h> #include <base/logging.h> +#include <base/metrics/sparse_histogram.h> +#include <base/metrics/statistics_recorder.h> #include <base/sys_info.h> #include "constants.h" -#include "metrics/metrics_library_mock.h" #include "persistent_integer.h" -#include "serialization/metric_sample.h" #include "uploader/metrics_log.h" #include "uploader/mock/mock_system_profile_setter.h" #include "uploader/mock/sender_mock.h" @@ -39,6 +39,10 @@ class UploadServiceTest : public testing::Test { protected: virtual void SetUp() { CHECK(dir_.CreateUniqueTempDir()); + // Make sure the statistics recorder is inactive (contains no metrics) then + // initialize it. + ASSERT_FALSE(base::StatisticsRecorder::IsActive()); + base::StatisticsRecorder::Initialize(); base::FilePath private_dir = dir_.path().Append("private"); base::FilePath shared_dir = dir_.path().Append("shared"); @@ -46,11 +50,12 @@ class UploadServiceTest : public testing::Test { EXPECT_TRUE(base::CreateDirectory(private_dir)); EXPECT_TRUE(base::CreateDirectory(shared_dir)); - metrics_lib_.InitForTest(shared_dir); ASSERT_EQ(0, base::WriteFile(shared_dir.Append(metrics::kConsentFileName), "", 0)); - upload_service_.reset( - new UploadService("", base::TimeDelta(), private_dir, shared_dir)); + counters_.reset(new CrashCounters); + + upload_service_.reset(new UploadService("", base::TimeDelta(), private_dir, + shared_dir, counters_)); upload_service_->sender_.reset(new SenderMock); upload_service_->InitForTest(new MockSystemProfileSetter); @@ -58,8 +63,17 @@ class UploadServiceTest : public testing::Test { upload_service_->Reset(); } - scoped_ptr<metrics::MetricSample> Crash(const std::string& name) { - return metrics::MetricSample::CrashSample(name); + void SendSparseHistogram(const std::string& name, int sample) { + base::HistogramBase* histogram = base::SparseHistogram::FactoryGet( + name, base::Histogram::kUmaTargetedHistogramFlag); + histogram->Add(sample); + } + + void SendHistogram( + const std::string& name, int sample, int min, int max, int nbuckets) { + base::HistogramBase* histogram = base::Histogram::FactoryGet( + name, min, max, nbuckets, base::Histogram::kUmaTargetedHistogramFlag); + histogram->Add(sample); } void SetTestingProperty(const std::string& name, const std::string& value) { @@ -71,57 +85,26 @@ class UploadServiceTest : public testing::Test { base::WriteFile(filepath, value.data(), value.size())); } + const metrics::SystemProfileProto_Stability GetCurrentStability() { + EXPECT_TRUE(upload_service_->current_log_); + + return upload_service_->current_log_->uma_proto()->system_profile().stability(); + } + base::ScopedTempDir dir_; scoped_ptr<UploadService> upload_service_; - MetricsLibrary metrics_lib_; scoped_ptr<base::AtExitManager> exit_manager_; + std::shared_ptr<CrashCounters> counters_; }; -// Tests that the right crash increments a values. -TEST_F(UploadServiceTest, LogUserCrash) { - upload_service_->AddSample(*Crash("user").get()); - - MetricsLog* log = upload_service_->current_log_.get(); - metrics::ChromeUserMetricsExtension* proto = log->uma_proto(); - - EXPECT_EQ(1, proto->system_profile().stability().other_user_crash_count()); -} - -TEST_F(UploadServiceTest, LogUncleanShutdown) { - upload_service_->AddSample(*Crash("uncleanshutdown")); - - EXPECT_EQ(1, upload_service_->current_log_ - ->uma_proto() - ->system_profile() - .stability() - .unclean_system_shutdown_count()); -} - -TEST_F(UploadServiceTest, LogKernelCrash) { - upload_service_->AddSample(*Crash("kernel")); - - EXPECT_EQ(1, upload_service_->current_log_ - ->uma_proto() - ->system_profile() - .stability() - .kernel_crash_count()); -} - -TEST_F(UploadServiceTest, UnknownCrashIgnored) { - upload_service_->AddSample(*Crash("foo")); - - // The log should be empty. - EXPECT_FALSE(upload_service_->current_log_); -} - TEST_F(UploadServiceTest, FailedSendAreRetried) { SenderMock* sender = new SenderMock(); upload_service_->sender_.reset(sender); sender->set_should_succeed(false); - upload_service_->AddSample(*Crash("user")); + SendSparseHistogram("hello", 1); upload_service_->UploadEvent(); EXPECT_EQ(1, sender->send_call_count()); std::string sent_string = sender->last_message(); @@ -137,7 +120,7 @@ TEST_F(UploadServiceTest, DiscardLogsAfterTooManyFailedUpload) { sender->set_should_succeed(false); - upload_service_->AddSample(*Crash("user")); + SendSparseHistogram("hello", 1); for (int i = 0; i < UploadService::kMaxFailedUpload; i++) { upload_service_->UploadEvent(); @@ -148,7 +131,7 @@ TEST_F(UploadServiceTest, DiscardLogsAfterTooManyFailedUpload) { EXPECT_FALSE(upload_service_->HasStagedLog()); // Log a new sample. The failed upload counter should be reset. - upload_service_->AddSample(*Crash("user")); + SendSparseHistogram("hello", 1); for (int i = 0; i < UploadService::kMaxFailedUpload; i++) { upload_service_->UploadEvent(); } @@ -165,7 +148,8 @@ TEST_F(UploadServiceTest, EmptyLogsAreNotSent) { } TEST_F(UploadServiceTest, LogEmptyByDefault) { - UploadService upload_service("", base::TimeDelta(), dir_.path(), dir_.path()); + UploadService upload_service("", base::TimeDelta(), dir_.path(), dir_.path(), + std::make_shared<CrashCounters>()); // current_log_ should be initialized later as it needs AtExitManager to exit // in order to gather system information from SysInfo. @@ -176,34 +160,28 @@ TEST_F(UploadServiceTest, CanSendMultipleTimes) { SenderMock* sender = new SenderMock(); upload_service_->sender_.reset(sender); - upload_service_->AddSample(*Crash("user")); + SendSparseHistogram("hello", 1); + upload_service_->UploadEvent(); std::string first_message = sender->last_message(); + SendSparseHistogram("hello", 2); - upload_service_->AddSample(*Crash("kernel")); upload_service_->UploadEvent(); EXPECT_NE(first_message, sender->last_message()); } TEST_F(UploadServiceTest, LogEmptyAfterUpload) { - upload_service_->AddSample(*Crash("user")); - - EXPECT_TRUE(upload_service_->current_log_); + SendSparseHistogram("hello", 2); upload_service_->UploadEvent(); EXPECT_FALSE(upload_service_->current_log_); } TEST_F(UploadServiceTest, LogContainsAggregatedValues) { - scoped_ptr<metrics::MetricSample> histogram = - metrics::MetricSample::HistogramSample("foo", 10, 0, 42, 10); - upload_service_->AddSample(*histogram.get()); - - scoped_ptr<metrics::MetricSample> histogram2 = - metrics::MetricSample::HistogramSample("foo", 11, 0, 42, 10); - upload_service_->AddSample(*histogram2.get()); + SendHistogram("foo", 11, 0, 42, 10); + SendHistogram("foo", 12, 0, 42, 10); upload_service_->GatherHistograms(); metrics::ChromeUserMetricsExtension* proto = @@ -211,6 +189,37 @@ TEST_F(UploadServiceTest, LogContainsAggregatedValues) { EXPECT_EQ(1, proto->histogram_event().size()); } +TEST_F(UploadServiceTest, LogContainsCrashCounts) { + // By default, there is no current log. + upload_service_->GatherHistograms(); + EXPECT_FALSE(upload_service_->current_log_); + + // If the user crash counter is incremented, we add the count to the current + // log. + counters_->IncrementUserCrashCount(); + upload_service_->GatherHistograms(); + EXPECT_EQ(1, GetCurrentStability().other_user_crash_count()); + + // If the kernel crash counter is incremented, we add the count to the current + // log. + counters_->IncrementKernelCrashCount(); + upload_service_->GatherHistograms(); + EXPECT_EQ(1, GetCurrentStability().kernel_crash_count()); + + // If the kernel crash counter is incremented, we add the count to the current + // log. + counters_->IncrementUncleanShutdownCount(); + counters_->IncrementUncleanShutdownCount(); + upload_service_->GatherHistograms(); + EXPECT_EQ(2, GetCurrentStability().unclean_system_shutdown_count()); + + // If no counter is incremented, the reported numbers don't change. + upload_service_->GatherHistograms(); + EXPECT_EQ(1, GetCurrentStability().other_user_crash_count()); + EXPECT_EQ(1, GetCurrentStability().kernel_crash_count()); + EXPECT_EQ(2, GetCurrentStability().unclean_system_shutdown_count()); +} + TEST_F(UploadServiceTest, ExtractChannelFromString) { EXPECT_EQ( SystemProfileCache::ProtoChannelFromString( @@ -234,13 +243,12 @@ TEST_F(UploadServiceTest, ValuesInConfigFileAreSent) { SetTestingProperty(metrics::kProductId, "hello"); SetTestingProperty(metrics::kProductVersion, "1.2.3.4"); - scoped_ptr<metrics::MetricSample> histogram = - metrics::MetricSample::SparseHistogramSample("myhistogram", 1); + SendSparseHistogram("hello", 1); + // Reset to create the new log with the profile setter. upload_service_->system_profile_setter_.reset( new SystemProfileCache(true, dir_.path())); upload_service_->Reset(); - upload_service_->AddSample(*histogram.get()); upload_service_->UploadEvent(); EXPECT_EQ(1, sender->send_call_count()); @@ -281,21 +289,6 @@ TEST_F(UploadServiceTest, SessionIdIncrementedAtInitialization) { EXPECT_EQ(cache.profile_.session_id, session_id + 1); } -// Test that we can log metrics from the metrics library and have the uploader -// upload them. -TEST_F(UploadServiceTest, LogFromTheMetricsLibrary) { - SenderMock* sender = new SenderMock(); - upload_service_->sender_.reset(sender); - - upload_service_->UploadEvent(); - EXPECT_EQ(0, sender->send_call_count()); - - metrics_lib_.SendEnumToUMA("testname", 2, 10); - upload_service_->UploadEvent(); - - EXPECT_EQ(1, sender->send_call_count()); -} - // The product id must be set for metrics to be uploaded. // If it is not set, the system profile cache should fail to initialize. TEST_F(UploadServiceTest, ProductIdMandatory) { |