diff --git a/metricsd/Android.mk b/metricsd/Android.mk index 35537537f..2149a4bd4 100644 --- a/metricsd/Android.mk +++ b/metricsd/Android.mk @@ -28,6 +28,7 @@ metrics_collector_common := \ collectors/cpu_usage_collector.cc \ collectors/disk_usage_collector.cc \ metrics_collector.cc \ + metrics_collector_service_trampoline.cc \ persistent_integer.cc metricsd_common := \ @@ -69,6 +70,7 @@ metrics_includes := external/gtest/include \ $(LOCAL_PATH)/include libmetrics_shared_libraries := libchrome libbinder libbrillo libutils metrics_collector_shared_libraries := $(libmetrics_shared_libraries) \ + libbrillo-binder \ libbrillo-dbus \ libbrillo-http \ libchrome-dbus \ @@ -77,6 +79,8 @@ metrics_collector_shared_libraries := $(libmetrics_shared_libraries) \ librootdev \ libweaved +metrics_collector_static_libraries := libmetricscollectorservice + metricsd_shared_libraries := \ libbinder \ libbrillo \ @@ -86,7 +90,7 @@ metricsd_shared_libraries := \ libupdate_engine_client \ libutils -# Static proxy library for the binder interface. +# Static proxy library for the metricsd binder interface. # ======================================================== include $(CLEAR_VARS) LOCAL_MODULE := metricsd_binder_proxy @@ -94,6 +98,21 @@ LOCAL_SHARED_LIBRARIES := libbinder libutils LOCAL_SRC_FILES := aidl/android/brillo/metrics/IMetricsd.aidl include $(BUILD_STATIC_LIBRARY) +# Static library for the metrics_collector binder interface. +# ========================================================== +include $(CLEAR_VARS) +LOCAL_MODULE := libmetricscollectorservice +LOCAL_SHARED_LIBRARIES := libbinder libbrillo-binder libchrome libutils +LOCAL_CPP_EXTENSION := $(metrics_cpp_extension) +LOCAL_C_INCLUDES := $(LOCAL_PATH)/include +LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include +LOCAL_SRC_FILES := \ + aidl/android/brillo/metrics/IMetricsCollectorService.aidl \ + metrics_collector_service_impl.cc \ + metrics_collector_service_client.cc +LOCAL_RTTI_FLAG := -fno-rtti +include $(BUILD_STATIC_LIBRARY) + # Shared library for metrics. # ======================================================== include $(CLEAR_VARS) @@ -151,7 +170,8 @@ 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 +LOCAL_STATIC_LIBRARIES := metricsd_binder_proxy \ + $(metrics_collector_static_libraries) include $(BUILD_EXECUTABLE) # metricsd daemon. @@ -197,7 +217,8 @@ 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 metricsd_binder_proxy +LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock metricsd_binder_proxy \ + $(metrics_collector_static_libraries) include $(BUILD_NATIVE_TEST) # Weave schema files diff --git a/metricsd/aidl/android/brillo/metrics/IMetricsCollectorService.aidl b/metricsd/aidl/android/brillo/metrics/IMetricsCollectorService.aidl new file mode 100644 index 000000000..49f484fd8 --- /dev/null +++ b/metricsd/aidl/android/brillo/metrics/IMetricsCollectorService.aidl @@ -0,0 +1,21 @@ +/* + * 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 IMetricsCollectorService { + oneway void notifyUserCrash(); +} diff --git a/metricsd/include/metrics/metrics_collector_service_client.h b/metricsd/include/metrics/metrics_collector_service_client.h new file mode 100644 index 000000000..c800eae10 --- /dev/null +++ b/metricsd/include/metrics/metrics_collector_service_client.h @@ -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. + */ + +// Client interface to IMetricsCollectorService. + +#ifndef METRICS_METRICS_COLLECTOR_SERVICE_CLIENT_H_ +#define METRICS_METRICS_COLLECTOR_SERVICE_CLIENT_H_ + +#include "android/brillo/metrics/IMetricsCollectorService.h" + +class MetricsCollectorServiceClient { + public: + MetricsCollectorServiceClient() = default; + ~MetricsCollectorServiceClient() = default; + + // Initialize. Returns true if OK, or false if IMetricsCollectorService + // is not registered. + bool Init(); + + // Called by crash_reporter to report a userspace crash event. Returns + // true if successfully called the IMetricsCollectorService method of the + // same name, or false if the service was not registered at Init() time. + bool notifyUserCrash(); + + private: + // IMetricsCollectorService binder proxy + android::sp + metrics_collector_service_; +}; + +#endif // METRICS_METRICS_COLLECTOR_SERVICE_CLIENT_H_ diff --git a/metricsd/metrics_collector.cc b/metricsd/metrics_collector.cc index 5468b9f25..b5c22892d 100644 --- a/metricsd/metrics_collector.cc +++ b/metricsd/metrics_collector.cc @@ -33,6 +33,7 @@ #include #include "constants.h" +#include "metrics_collector_service_trampoline.h" using base::FilePath; using base::StringPrintf; @@ -46,11 +47,6 @@ using std::vector; namespace { -const char kCrashReporterInterface[] = "org.chromium.CrashReporter"; -const char kCrashReporterUserCrashSignal[] = "UserCrash"; -const char kCrashReporterMatchRule[] = - "type='signal',interface='%s',path='/',member='%s'"; - const int kSecondsPerMinute = 60; const int kMinutesPerHour = 60; const int kHoursPerDay = 24; @@ -130,6 +126,10 @@ int MetricsCollector::Run() { version_cumulative_cpu_use_->Set(0); } + // Start metricscollectorservice via trampoline + MetricsCollectorServiceTrampoline metricscollectorservice_trampoline(this); + metricscollectorservice_trampoline.Run(); + return brillo::DBusDaemon::Run(); } @@ -223,28 +223,6 @@ int MetricsCollector::OnInit() { bus_->AssertOnDBusThread(); CHECK(bus_->SetUpAsyncOperations()); - if (bus_->is_connected()) { - const std::string match_rule = - base::StringPrintf(kCrashReporterMatchRule, - kCrashReporterInterface, - kCrashReporterUserCrashSignal); - - bus_->AddFilterFunction(&MetricsCollector::MessageFilter, this); - - DBusError error; - dbus_error_init(&error); - bus_->AddMatch(match_rule, &error); - - if (dbus_error_is_set(&error)) { - LOG(ERROR) << "Failed to add match rule \"" << match_rule << "\". Got " - << error.name << ": " << error.message; - return EX_SOFTWARE; - } - } else { - LOG(ERROR) << "DBus isn't connected."; - return EX_UNAVAILABLE; - } - device_ = weaved::Device::CreateInstance( bus_, base::Bind(&MetricsCollector::UpdateWeaveState, base::Unretained(this))); @@ -268,23 +246,6 @@ int MetricsCollector::OnInit() { } void MetricsCollector::OnShutdown(int* return_code) { - if (!testing_ && bus_->is_connected()) { - const std::string match_rule = - base::StringPrintf(kCrashReporterMatchRule, - kCrashReporterInterface, - kCrashReporterUserCrashSignal); - - bus_->RemoveFilterFunction(&MetricsCollector::MessageFilter, this); - - DBusError error; - dbus_error_init(&error); - bus_->RemoveMatch(match_rule, &error); - - if (dbus_error_is_set(&error)) { - LOG(ERROR) << "Failed to remove match rule \"" << match_rule << "\". Got " - << error.name << ": " << error.message; - } - } brillo::DBusDaemon::OnShutdown(return_code); } @@ -340,36 +301,6 @@ void MetricsCollector::UpdateWeaveState() { } } -// static -DBusHandlerResult MetricsCollector::MessageFilter(DBusConnection* connection, - DBusMessage* message, - void* user_data) { - int message_type = dbus_message_get_type(message); - if (message_type != DBUS_MESSAGE_TYPE_SIGNAL) { - DLOG(WARNING) << "unexpected message type " << message_type; - return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; - } - - // Signal messages always have interfaces. - const std::string interface(dbus_message_get_interface(message)); - const std::string member(dbus_message_get_member(message)); - DLOG(INFO) << "Got " << interface << "." << member << " D-Bus signal"; - - MetricsCollector* daemon = static_cast(user_data); - - DBusMessageIter iter; - dbus_message_iter_init(message, &iter); - if (interface == kCrashReporterInterface) { - CHECK_EQ(member, kCrashReporterUserCrashSignal); - daemon->ProcessUserCrash(); - } else { - // Ignore messages from the bus itself. - return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; - } - - return DBUS_HANDLER_RESULT_HANDLED; -} - void MetricsCollector::ProcessUserCrash() { // Counts the active time up to now. UpdateStats(TimeTicks::Now(), Time::Now()); diff --git a/metricsd/metrics_collector.h b/metricsd/metrics_collector.h index 69747d0be..422ed7c39 100644 --- a/metricsd/metrics_collector.h +++ b/metricsd/metrics_collector.h @@ -63,6 +63,10 @@ class MetricsCollector : public brillo::DBusDaemon { // Returns the active time since boot (uptime minus sleep time) in seconds. static double GetActiveTime(); + // Updates the active use time and logs time between user-space + // process crashes. Called via MetricsCollectorServiceTrampoline. + void ProcessUserCrash(); + protected: // Used also by the unit tests. static const char kComprDataSizeName[]; @@ -108,11 +112,6 @@ class MetricsCollector : public brillo::DBusDaemon { int value; // value from /proc/meminfo }; - // D-Bus filter callback. - static DBusHandlerResult MessageFilter(DBusConnection* connection, - DBusMessage* message, - void* user_data); - // Enables metrics reporting. void OnEnableMetrics(const std::weak_ptr& cmd); @@ -122,10 +121,6 @@ class MetricsCollector : public brillo::DBusDaemon { // Updates the weave device state. void UpdateWeaveState(); - // Updates the active use time and logs time between user-space - // process crashes. - void ProcessUserCrash(); - // Updates the active use time and logs time between kernel crashes. void ProcessKernelCrash(); diff --git a/metricsd/metrics_collector_service_client.cc b/metricsd/metrics_collector_service_client.cc new file mode 100644 index 000000000..08aaa4a70 --- /dev/null +++ b/metricsd/metrics_collector_service_client.cc @@ -0,0 +1,50 @@ +/* + * 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. + */ + +// Client interface to IMetricsCollectorService. + +#include "metrics/metrics_collector_service_client.h" + +#include +#include +#include + +#include "android/brillo/metrics/IMetricsCollectorService.h" + +namespace { +const char kMetricsCollectorServiceName[] = + "android.brillo.metrics.IMetricsCollectorService"; +} + +bool MetricsCollectorServiceClient::Init() { + const android::String16 name(kMetricsCollectorServiceName); + metrics_collector_service_ = android::interface_cast< + android::brillo::metrics::IMetricsCollectorService>( + android::defaultServiceManager()->checkService(name)); + + if (metrics_collector_service_ == nullptr) + LOG(ERROR) << "Unable to lookup service " << kMetricsCollectorServiceName; + + return metrics_collector_service_ != nullptr; +} + +bool MetricsCollectorServiceClient::notifyUserCrash() { + if (metrics_collector_service_ == nullptr) + return false; + + metrics_collector_service_->notifyUserCrash(); + return true; +} diff --git a/metricsd/metrics_collector_service_impl.cc b/metricsd/metrics_collector_service_impl.cc new file mode 100644 index 000000000..dbb0578aa --- /dev/null +++ b/metricsd/metrics_collector_service_impl.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 "metrics_collector_service_impl.h" + +#include +#include +#include +#include + +#include "metrics_collector_service_trampoline.h" + +using namespace android; + +BnMetricsCollectorServiceImpl::BnMetricsCollectorServiceImpl( + MetricsCollectorServiceTrampoline* metrics_collector_service_trampoline) { + metrics_collector_service_trampoline_ = metrics_collector_service_trampoline; +} + +void BnMetricsCollectorServiceImpl::Run() { + status_t status = + defaultServiceManager()->addService(getInterfaceDescriptor(), this); + CHECK(status == OK) << "libmetricscollectorservice: failed to add service"; + binder_watcher_.reset(new ::brillo::BinderWatcher); + CHECK(binder_watcher_->Init()) << "Binder FD watcher init failed"; +} + +android::binder::Status BnMetricsCollectorServiceImpl::notifyUserCrash() { + metrics_collector_service_trampoline_->ProcessUserCrash(); + return android::binder::Status::ok(); +} diff --git a/metricsd/metrics_collector_service_impl.h b/metricsd/metrics_collector_service_impl.h new file mode 100644 index 000000000..bdcab5084 --- /dev/null +++ b/metricsd/metrics_collector_service_impl.h @@ -0,0 +1,62 @@ +/* + * 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_METRICS_COLLECTOR_SERVICE_IMPL_H_ +#define METRICSD_METRICS_COLLECTOR_SERVICE_IMPL_H_ + +// metrics_collector binder service implementation. Constructed by +// MetricsCollectorServiceTrampoline, which we use to call back into +// MetricsCollector. The trampoline isolates us from the -frtti code of +// metrics_collector / libbrillo. + +#include "android/brillo/metrics/BnMetricsCollectorService.h" + +#include + +#include +#include + +class MetricsCollectorServiceTrampoline; + +//#include "metrics_collector_service_trampoline.h" + +class BnMetricsCollectorServiceImpl + : public android::brillo::metrics::BnMetricsCollectorService { + public: + // Passed a this pointer from the MetricsCollectorServiceTrampoline + // object that constructs us. + explicit BnMetricsCollectorServiceImpl( + MetricsCollectorServiceTrampoline* metrics_collector_service_trampoline); + + virtual ~BnMetricsCollectorServiceImpl() = default; + + // Starts the binder main loop. + void Run(); + + // Called by crash_reporter to report a userspace crash event. We relay + // this to MetricsCollector using the trampoline. + android::binder::Status notifyUserCrash(); + + private: + // Trampoline object that constructs us, we use this to call MetricsCollector + // methods via the trampoline. + MetricsCollectorServiceTrampoline* metrics_collector_service_trampoline_; + + // BinderWatcher object we construct for handling Binder traffic + std::unique_ptr binder_watcher_; +}; + +#endif // METRICSD_METRICS_COLLECTOR_SERVICE_IMPL_H_ diff --git a/metricsd/metrics_collector_service_trampoline.cc b/metricsd/metrics_collector_service_trampoline.cc new file mode 100644 index 000000000..12b80a192 --- /dev/null +++ b/metricsd/metrics_collector_service_trampoline.cc @@ -0,0 +1,34 @@ +/* + * 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 "metrics_collector_service_trampoline.h" +#include "metrics_collector.h" +#include "metrics_collector_service_impl.h" + +MetricsCollectorServiceTrampoline::MetricsCollectorServiceTrampoline( + MetricsCollector* metrics_collector) { + metrics_collector_ = metrics_collector; +} + +void MetricsCollectorServiceTrampoline::Run() { + // Start metricscollectorservice binder service + metrics_collector_service.reset(new BnMetricsCollectorServiceImpl(this)); + metrics_collector_service->Run(); +} + +void MetricsCollectorServiceTrampoline::ProcessUserCrash() { + metrics_collector_->ProcessUserCrash(); +} diff --git a/metricsd/metrics_collector_service_trampoline.h b/metricsd/metrics_collector_service_trampoline.h new file mode 100644 index 000000000..5da9fa519 --- /dev/null +++ b/metricsd/metrics_collector_service_trampoline.h @@ -0,0 +1,57 @@ +/* + * 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_METRICS_COLLECTOR_SERVICE_TRAMPOLINE_H_ +#define METRICSD_METRICS_COLLECTOR_SERVICE_TRAMPOLINE_H_ + +// Trampoline between the -fno-rtti compile of libmetricsservice and the +// -frtti compile of metrics_collector. MetricsCollectorServiceTrampoline +// is called from MetricsCollector to run the IMetricsCollectorService +// server, and acts as a go-between for calls from server back to +// MetricsCollector. + +#include + +#include "metrics_collector_service_impl.h" + +// Forward declaration of MetricsCollector. Don't include the header file +// for the class here, as it pulls in -frtti stuff. +class MetricsCollector; + +class MetricsCollectorServiceTrampoline { + public: + // Constructor take a this pointer from the MetricsCollector class that + // constructs these objects. + explicit MetricsCollectorServiceTrampoline( + MetricsCollector* metrics_collector); + + // Initialize and run the IMetricsCollectorService + void Run(); + + // Called from IMetricsCollectorService to trampoline into the + // MetricsCollector method of the same name. + void ProcessUserCrash(); + + private: + // The MetricsCollector object that constructs us, for which we act as + // the go-between for MetricsCollectorServiceImpl use. + MetricsCollector* metrics_collector_; + + // The IMetricsCollectorService implementation we construct. + std::unique_ptr metrics_collector_service; +}; + +#endif // METRICSD_METRICS_COLLECTOR_SERVICE_TRAMPOLINE_H_ diff --git a/metricsd/metrics_collector_test.cc b/metricsd/metrics_collector_test.cc index 956e56bdf..5fb3ac898 100644 --- a/metricsd/metrics_collector_test.cc +++ b/metricsd/metrics_collector_test.cc @@ -115,37 +115,6 @@ class MetricsCollectorTest : public testing::Test { StrictMock metrics_lib_; }; -TEST_F(MetricsCollectorTest, MessageFilter) { - // Ignore calls to SendToUMA. - EXPECT_CALL(metrics_lib_, SendToUMA(_, _, _, _, _)).Times(AnyNumber()); - - DBusMessage* msg = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL); - DBusHandlerResult res = - MetricsCollector::MessageFilter(/* connection */ nullptr, msg, &daemon_); - EXPECT_EQ(DBUS_HANDLER_RESULT_NOT_YET_HANDLED, res); - DeleteDBusMessage(msg); - - vector signal_args; - msg = NewDBusSignalString("/", - "org.chromium.CrashReporter", - "UserCrash", - signal_args); - res = MetricsCollector::MessageFilter(/* connection */ nullptr, msg, &daemon_); - EXPECT_EQ(DBUS_HANDLER_RESULT_HANDLED, res); - DeleteDBusMessage(msg); - - signal_args.clear(); - signal_args.push_back("randomstate"); - signal_args.push_back("bob"); // arbitrary username - msg = NewDBusSignalString("/", - "org.chromium.UnknownService.Manager", - "StateChanged", - signal_args); - res = MetricsCollector::MessageFilter(/* connection */ nullptr, msg, &daemon_); - EXPECT_EQ(DBUS_HANDLER_RESULT_NOT_YET_HANDLED, res); - DeleteDBusMessage(msg); -} - TEST_F(MetricsCollectorTest, SendSample) { ExpectSample("Dummy.Metric", 3); daemon_.SendSample("Dummy.Metric", /* sample */ 3,