From 35f90c6993c5d9412eac2752d93c58d46871beb8 Mon Sep 17 00:00:00 2001 From: Holden Warriner Date: Mon, 25 Mar 2024 13:44:12 -0400 Subject: [PATCH] Add Cobalt Telemetry for the Loader App (#2667) A LoaderAppMetrics Starboard extension is added so that measurements taken by the Loader App can be reliably accessed by Cobalt once loaded. This PR just adds one metric for now, for b/329458881, but this pattern and extension can be used for b/329445690 and other Loader App metrics. b/329458881 Change-Id: I940d07b058d197afa2ad8b1160eba469f1055f8a (cherry picked from commit bf9373676f97022af808a07c45e30b67f883a304) --- cobalt/browser/application.cc | 19 ++++++ starboard/extension/extension_test.cc | 22 +++++++ starboard/extension/loader_app_metrics.h | 64 +++++++++++++++++++ starboard/linux/shared/BUILD.gn | 2 + .../linux/shared/system_get_extensions.cc | 5 ++ .../shared/starboard/loader_app_metrics.cc | 50 +++++++++++++++ .../shared/starboard/loader_app_metrics.h | 28 ++++++++ third_party/crashpad/wrapper/wrapper.cc | 34 ++++++++++ .../histograms/metadata/cobalt/enums.xml | 13 ++++ .../histograms/metadata/cobalt/histograms.xml | 11 ++++ 10 files changed, 248 insertions(+) create mode 100644 starboard/extension/loader_app_metrics.h create mode 100644 starboard/shared/starboard/loader_app_metrics.cc create mode 100644 starboard/shared/starboard/loader_app_metrics.h diff --git a/cobalt/browser/application.cc b/cobalt/browser/application.cc index ebf95f7ba587..c6dcde15d4c8 100644 --- a/cobalt/browser/application.cc +++ b/cobalt/browser/application.cc @@ -23,6 +23,7 @@ #include "base/command_line.h" #include "base/lazy_instance.h" #include "base/logging.h" +#include "base/metrics/histogram_functions.h" #include "base/metrics/statistics_recorder.h" #include "base/metrics/user_metrics.h" #include "base/optional.h" @@ -84,6 +85,7 @@ #include "starboard/event.h" #include "starboard/extension/crash_handler.h" #include "starboard/extension/installation_manager.h" +#include "starboard/extension/loader_app_metrics.h" #include "starboard/system.h" #include "starboard/time.h" #include "url/gurl.h" @@ -621,6 +623,22 @@ void AddCrashLogApplicationState(base::ApplicationState state) { application_state); } +void RecordLoaderAppMetrics() { + auto metrics_extension = + static_cast( + SbSystemGetExtension(kStarboardExtensionLoaderAppMetricsName)); + if (metrics_extension && + strcmp(metrics_extension->name, + kStarboardExtensionLoaderAppMetricsName) == 0 && + metrics_extension->version >= 1) { + base::UmaHistogramEnumeration( + "Cobalt.LoaderApp.CrashpadInstallationStatus", + metrics_extension->GetCrashpadInstallationStatus()); + LOG(INFO) << "Recorded sample for " + << "Cobalt.LoaderApp.CrashpadInstallationStatus"; + } +} + } // namespace // Static user logs @@ -1029,6 +1047,7 @@ Application::Application(const base::Closure& quit_closure, bool should_preload, #endif // ENABLE_DEBUG_COMMAND_LINE_SWITCHES AddCrashLogApplicationState(base::kApplicationStateStarted); + RecordLoaderAppMetrics(); } Application::~Application() { diff --git a/starboard/extension/extension_test.cc b/starboard/extension/extension_test.cc index 4308cb8608a1..caa0cceb08c6 100644 --- a/starboard/extension/extension_test.cc +++ b/starboard/extension/extension_test.cc @@ -24,6 +24,7 @@ #include "starboard/extension/ifa.h" #include "starboard/extension/installation_manager.h" #include "starboard/extension/javascript_cache.h" +#include "starboard/extension/loader_app_metrics.h" #include "starboard/extension/media_session.h" #include "starboard/extension/memory_mapped_file.h" #include "starboard/extension/platform_info.h" @@ -503,5 +504,26 @@ TEST(ExtensionTest, PlayerSetMaxVideoInputSize) { << "Extension struct should be a singleton"; } +TEST(ExtensionTest, LoaderAppMetrics) { + typedef StarboardExtensionLoaderAppMetricsApi ExtensionApi; + const char* kExtensionName = kStarboardExtensionLoaderAppMetricsName; + + const ExtensionApi* extension_api = + static_cast(SbSystemGetExtension(kExtensionName)); + if (!extension_api) { + return; + } + + EXPECT_STREQ(extension_api->name, kExtensionName); + EXPECT_EQ(extension_api->version, 1u); + EXPECT_NE(extension_api->SetCrashpadInstallationStatus, nullptr); + EXPECT_NE(extension_api->GetCrashpadInstallationStatus, nullptr); + + const ExtensionApi* second_extension_api = + static_cast(SbSystemGetExtension(kExtensionName)); + EXPECT_EQ(second_extension_api, extension_api) + << "Extension struct should be a singleton"; +} + } // namespace extension } // namespace starboard diff --git a/starboard/extension/loader_app_metrics.h b/starboard/extension/loader_app_metrics.h new file mode 100644 index 000000000000..abdb120a2de7 --- /dev/null +++ b/starboard/extension/loader_app_metrics.h @@ -0,0 +1,64 @@ +// Copyright 2024 The Cobalt Authors. All Rights Reserved. +// +// 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 STARBOARD_EXTENSION_LOADER_APP_METRICS_H_ +#define STARBOARD_EXTENSION_LOADER_APP_METRICS_H_ + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +#define kStarboardExtensionLoaderAppMetricsName \ + "dev.cobalt.extension.LoaderAppMetrics" + +// These values are persisted to logs. Entries should not be renumbered and +// numeric values should never be reused. Must be kept in sync with the +// corresponding definition in +// tools/metrics/histograms/metadata/cobalt/enums.xml. +typedef enum CrashpadInstallationStatus { + // The enumerators below this point were added in version 1 or later. + kUnknown = 0, + kSucceeded = 1, + kFailedCrashpadHandlerBinaryNotFound = 2, + kFailedDatabaseInitializationFailed = 3, + kFailedSignalHandlerInstallationFailed = 4, + kMaxValue = kFailedSignalHandlerInstallationFailed +} CrashpadInstallationStatus; + +typedef struct StarboardExtensionLoaderAppMetricsApi { + // Name should be the string |kStarboardExtensionLoaderAppMetricsName|. + // This helps to validate that the extension API is correct. + const char* name; + + // This specifies the version of the API that is implemented. + uint32_t version; + + // The fields below this point were added in version 1 or later. + + // The accessors and mutators below are assumed to be called from the same + // thread: Cobalt's application thread. + + void (*SetCrashpadInstallationStatus)(CrashpadInstallationStatus status); + + CrashpadInstallationStatus (*GetCrashpadInstallationStatus)(); + +} StarboardExtensionLoaderAppMetricsApi; + +#ifdef __cplusplus +} // extern "C" +#endif + +#endif // STARBOARD_EXTENSION_LOADER_APP_METRICS_H_ diff --git a/starboard/linux/shared/BUILD.gn b/starboard/linux/shared/BUILD.gn index 507a764834f4..d5f906d9aa84 100644 --- a/starboard/linux/shared/BUILD.gn +++ b/starboard/linux/shared/BUILD.gn @@ -293,6 +293,8 @@ static_library("starboard_platform_sources") { "//starboard/shared/starboard/file_storage/storage_get_record_size.cc", "//starboard/shared/starboard/file_storage/storage_open_record.cc", "//starboard/shared/starboard/file_storage/storage_read_record.cc", + "//starboard/shared/starboard/loader_app_metrics.cc", + "//starboard/shared/starboard/loader_app_metrics.h", "//starboard/shared/starboard/log_mutex.cc", "//starboard/shared/starboard/log_mutex.h", "//starboard/shared/starboard/log_raw_dump_stack.cc", diff --git a/starboard/linux/shared/system_get_extensions.cc b/starboard/linux/shared/system_get_extensions.cc index efc9169f829d..0e1bf0662dba 100644 --- a/starboard/linux/shared/system_get_extensions.cc +++ b/starboard/linux/shared/system_get_extensions.cc @@ -21,6 +21,7 @@ #include "starboard/extension/enhanced_audio.h" #include "starboard/extension/free_space.h" #include "starboard/extension/ifa.h" +#include "starboard/extension/loader_app_metrics.h" #include "starboard/extension/memory_mapped_file.h" #include "starboard/extension/platform_service.h" #include "starboard/extension/time_zone.h" @@ -33,6 +34,7 @@ #include "starboard/shared/posix/memory_mapped_file.h" #include "starboard/shared/starboard/application.h" #include "starboard/shared/starboard/crash_handler.h" +#include "starboard/shared/starboard/loader_app_metrics.h" #if SB_IS(EVERGREEN_COMPATIBLE) #include "starboard/elf_loader/evergreen_config.h" #endif @@ -86,5 +88,8 @@ const void* SbSystemGetExtension(const char* name) { return starboard::shared::GetIfaApi(); } #endif // SB_API_VERSION < 14 + if (strcmp(name, kStarboardExtensionLoaderAppMetricsName) == 0) { + return starboard::shared::starboard::GetLoaderAppMetricsApi(); + } return NULL; } diff --git a/starboard/shared/starboard/loader_app_metrics.cc b/starboard/shared/starboard/loader_app_metrics.cc new file mode 100644 index 000000000000..6b7b8d7d6477 --- /dev/null +++ b/starboard/shared/starboard/loader_app_metrics.cc @@ -0,0 +1,50 @@ +// Copyright 2024 The Cobalt Authors. All Rights Reserved. +// +// 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 "starboard/shared/starboard/loader_app_metrics.h" + +#include "starboard/extension/loader_app_metrics.h" + +namespace starboard { +namespace shared { +namespace starboard { + +namespace { + +// Thread safety isn't needed for this global variable since the extension's +// interface specifies that all accesses and mutations must be from the same +// thread. +static CrashpadInstallationStatus g_crashpad_installation_status; + +void SetCrashpadInstallationStatus(CrashpadInstallationStatus status) { + g_crashpad_installation_status = status; +} + +CrashpadInstallationStatus GetCrashpadInstallationStatus() { + return g_crashpad_installation_status; +} + +const StarboardExtensionLoaderAppMetricsApi kLoaderAppMetricsApi = { + kStarboardExtensionLoaderAppMetricsName, 1, &SetCrashpadInstallationStatus, + &GetCrashpadInstallationStatus}; + +} // namespace + +const void* GetLoaderAppMetricsApi() { + return &kLoaderAppMetricsApi; +} + +} // namespace starboard +} // namespace shared +} // namespace starboard diff --git a/starboard/shared/starboard/loader_app_metrics.h b/starboard/shared/starboard/loader_app_metrics.h new file mode 100644 index 000000000000..ede4cef01b75 --- /dev/null +++ b/starboard/shared/starboard/loader_app_metrics.h @@ -0,0 +1,28 @@ +// Copyright 2024 The Cobalt Authors. All Rights Reserved. +// +// 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 STARBOARD_SHARED_STARBOARD_LOADER_APP_METRICS_H_ +#define STARBOARD_SHARED_STARBOARD_LOADER_APP_METRICS_H_ + +namespace starboard { +namespace shared { +namespace starboard { + +const void* GetLoaderAppMetricsApi(); + +} // namespace starboard +} // namespace shared +} // namespace starboard + +#endif // STARBOARD_SHARED_STARBOARD_LOADER_APP_METRICS_H_ diff --git a/third_party/crashpad/wrapper/wrapper.cc b/third_party/crashpad/wrapper/wrapper.cc index 93eb5ae42d6a..b29701b62d4f 100644 --- a/third_party/crashpad/wrapper/wrapper.cc +++ b/third_party/crashpad/wrapper/wrapper.cc @@ -26,6 +26,7 @@ #include "starboard/common/system_property.h" #include "starboard/configuration_constants.h" #include "starboard/directory.h" +#include "starboard/extension/loader_app_metrics.h" #include "starboard/file.h" #include "starboard/system.h" #include "third_party/crashpad/snapshot/sanitized/sanitization_information.h" @@ -187,6 +188,18 @@ std::map GetPlatformInfo() { return platform_info; } +void RecordStatus(CrashpadInstallationStatus status) { + auto metrics_extension = + static_cast( + SbSystemGetExtension(kStarboardExtensionLoaderAppMetricsName)); + if (metrics_extension && + strcmp(metrics_extension->name, + kStarboardExtensionLoaderAppMetricsName) == 0 && + metrics_extension->version >= 1) { + metrics_extension->SetCrashpadInstallationStatus(status); + } +} + } // namespace void InstallCrashpadHandler(bool start_at_crash, @@ -197,6 +210,8 @@ void InstallCrashpadHandler(bool start_at_crash, if (!SbFileExists(handler_path.value().c_str())) { LOG(ERROR) << "crashpad_handler not at expected location of " << handler_path.value(); + RecordStatus( + CrashpadInstallationStatus::kFailedCrashpadHandlerBinaryNotFound); return; } @@ -214,6 +229,8 @@ void InstallCrashpadHandler(bool start_at_crash, if (!InitializeCrashpadDatabase(database_directory_path)) { LOG(ERROR) << "Failed to initialize Crashpad database"; + RecordStatus( + CrashpadInstallationStatus::kFailedDatabaseInitializationFailed); // As we investigate b/329458881 we may find that it's safe to continue with // installation of the Crashpad handler here with the hope that the handler, @@ -225,6 +242,7 @@ void InstallCrashpadHandler(bool start_at_crash, client->SetUnhandledSignals({}); +<<<<<<< HEAD if (start_at_crash) client->StartHandlerAtCrash(handler_path, database_directory_path, @@ -243,9 +261,25 @@ void InstallCrashpadHandler(bool start_at_crash, default_arguments, false, false); +======= + if (!client->StartHandlerAtCrash(handler_path, + database_directory_path, + default_metrics_dir, + kUploadUrl, + ca_certificates_path, + default_annotations, + default_arguments)) { + LOG(ERROR) << "Failed to install the signal handler"; + RecordStatus( + CrashpadInstallationStatus::kFailedSignalHandlerInstallationFailed); + return; + } +>>>>>>> bf9373676f9 (Add Cobalt Telemetry for the Loader App (#2667)) ::crashpad::SanitizationInformation sanitization_info = {0, 0, 0, 1}; client->SendSanitizationInformationToHandler(sanitization_info); + + RecordStatus(CrashpadInstallationStatus::kSucceeded); } bool AddEvergreenInfoToCrashpad(EvergreenInfo evergreen_info) { diff --git a/tools/metrics/histograms/metadata/cobalt/enums.xml b/tools/metrics/histograms/metadata/cobalt/enums.xml index feb372a0520c..a4229ae41f78 100644 --- a/tools/metrics/histograms/metadata/cobalt/enums.xml +++ b/tools/metrics/histograms/metadata/cobalt/enums.xml @@ -68,6 +68,19 @@ https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histogra +<<<<<<< HEAD +======= + + + Possible status of Crashpad installation by the Loader App + + + + + + + +>>>>>>> bf9373676f9 (Add Cobalt Telemetry for the Loader App (#2667)) diff --git a/tools/metrics/histograms/metadata/cobalt/histograms.xml b/tools/metrics/histograms/metadata/cobalt/histograms.xml index d6af96002295..43cd9df32c98 100644 --- a/tools/metrics/histograms/metadata/cobalt/histograms.xml +++ b/tools/metrics/histograms/metadata/cobalt/histograms.xml @@ -198,6 +198,17 @@ Always run the pretty print utility on this file after editing: + + + + hwarriner@google.com + cobalt-team@google.com + + Status of Crashpad installation by the Loader App. + + + \ No newline at end of file