diff --git a/libwvdrmengine/cdm/core/src/oemcrypto_adapter_dynamic.cpp b/libwvdrmengine/cdm/core/src/oemcrypto_adapter_dynamic.cpp index be1b2042..1525f176 100644 --- a/libwvdrmengine/cdm/core/src/oemcrypto_adapter_dynamic.cpp +++ b/libwvdrmengine/cdm/core/src/oemcrypto_adapter_dynamic.cpp @@ -434,7 +434,7 @@ class WatchDog { } // Check to see if the failure file was created before that last abort. - void CheckForPreviousFailure(wvcdm::metrics::CryptoMetrics* metrics) { + void CheckForPreviousFailure(wvcdm::metrics::OemCryptoDynamicAdapterMetrics* metrics) { wvcdm::FileSystem file_system; std::string filename = FailureFilename(); if (!file_system.Exists(filename)) return; @@ -448,7 +448,7 @@ class WatchDog { if (size == size_read && flag) { LOGE("Previous L3 Init failed."); if (metrics == nullptr) return; - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_L3_INITIALIZATION_FAILED); } } @@ -574,15 +574,12 @@ class Adapter { OEMCryptoResult Initialize() { /* - * To avoid changing the function signature and function contract - declare - * a one-off metrics group to collect detailed information about how - * oemcrypto was intialized. - * - * TODO(blueeyes): Refactor this to allow Initialize to provide the - * details to the caller or to use the metrics instance provided by - * the caller. + * To avoid changing the function signature and function contract, use a + * reference to a singleton object for the metrics collected from the + * dynamic adapter. */ - wvcdm::metrics::CryptoMetrics metrics; + wvcdm::metrics::OemCryptoDynamicAdapterMetrics& metrics = + wvcdm::metrics::GetDynamicAdapterMetricsInstance(); level1_ = FunctionPointers(); // start with all null pointers. level3_ = FunctionPointers(); // start with all null pointers. @@ -592,20 +589,20 @@ class Adapter { watcher->StartThread(); OEMCryptoResult result = watcher->WaitForStatusAndCleanUp(); if (Level3_IsInApp()) { - metrics.oemcrypto_initialization_mode_ - .Record(wvcdm::metrics::OEMCrypto_INITIALIZED_USING_IN_APP); + metrics.SetInitializationMode( + wvcdm::metrics::OEMCrypto_INITIALIZED_USING_IN_APP); return result; } if (force_level3()) { LOGW("Test code. User requested falling back to L3"); - metrics.oemcrypto_initialization_mode_ - .Record(wvcdm::metrics::OEMCrypto_INITIALIZED_FORCING_L3); + metrics.SetInitializationMode( + wvcdm::metrics::OEMCrypto_INITIALIZED_FORCING_L3); return result; } std::string library_name; if (!wvcdm::Properties::GetOEMCryptoPath(&library_name)) { LOGW("L1 library not specified. Falling back to L3"); - metrics.oemcrypto_initialization_mode_.Record( + metrics.OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_NO_L1_LIBRARY_PATH); return result; } @@ -613,7 +610,7 @@ class Adapter { if (level1_library_ == NULL) { LOGW("Could not load %s. Falling back to L3. %s", library_name.c_str(), dlerror()); - metrics.oemcrypto_initialization_mode_.Record( + metrics.OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_L1_OPEN_FAILED); return result; } @@ -628,7 +625,7 @@ class Adapter { return result; } - bool LoadLevel1(wvcdm::metrics::CryptoMetrics* metrics) { + bool LoadLevel1(wvcdm::metrics::OemCryptoDynamicAdapterMetrics* metrics) { if (metrics == nullptr) { return false; } @@ -640,25 +637,25 @@ class Adapter { LOOKUP_ALL(8, APIVersion, OEMCrypto_APIVersion); LOOKUP_ALL(8, Terminate, OEMCrypto_Terminate); if (!level1_valid_) { - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_INVALID_L1); return false; } OEMCryptoResult st = level1_.Initialize(); if (st != OEMCrypto_SUCCESS) { LOGW("Could not initialize L1. Falling Back to L3."); - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_COULD_NOT_INITIALIZE_L1); return false; } level1_.version = level1_.APIVersion(); - metrics->oemcrypto_l1_api_version_.Record(level1_.version); - metrics->oemcrypto_l1_min_api_version_.Record(kMinimumVersion); + metrics->SetL1ApiVersion(level1_.version); + metrics->SetL1MinApiVersion(kMinimumVersion); if (level1_.version < kMinimumVersion) { LOGW("liboemcrypto.so is version %d, not %d. Falling Back to L3.", level1_.version, kMinimumVersion); - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_WRONG_L1_VERSION); level1_.Terminate(); return false; @@ -733,7 +730,7 @@ class Adapter { // If we have a valid keybox, initialization is done. We're good. if (OEMCrypto_SUCCESS == level1_.IsKeyboxValid()) { - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L1_WITH_KEYBOX); return true; } @@ -742,7 +739,7 @@ class Adapter { // will have to be caught in the future when provisioning fails. if (level1_.version > 11 && (level1_.GetProvisioningMethod() == OEMCrypto_OEMCertificate)) { - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L1_WITH_PROVISIONING_3_0); return true; } @@ -759,10 +756,10 @@ class Adapter { // because we still want to test OEMCrypto in that configuration. LOGE("OEMCrypto uses cert as identification, but cdm does not!"); LOGE("This will not work on a production device."); - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L1_CERTIFICATE_MIX); } else { - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L1_WITH_CERTIFICATE); } return true; @@ -772,7 +769,7 @@ class Adapter { if (!wvcdm::Properties::GetFactoryKeyboxPath(&filename)) { LOGW("Bad Level 1 Keybox. Falling Back to L3."); level1_.Terminate(); - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_BAD_KEYBOX); return false; } @@ -781,7 +778,7 @@ class Adapter { if (size <= 0 || !file) { LOGW("Could not open %s. Falling Back to L3.", filename.c_str()); level1_.Terminate(); - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_COULD_NOT_OPEN_FACTORY_KEYBOX); return false; } @@ -792,12 +789,12 @@ class Adapter { LOGE("Could NOT install keybox from %s. Falling Back to L3.", filename.c_str()); level1_.Terminate(); - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_COULD_NOT_INSTALL_KEYBOX); return false; } LOGI("Installed keybox from %s", filename.c_str()); - metrics->oemcrypto_initialization_mode_.Record( + metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode( wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L1_INSTALLED_KEYBOX); return true; } diff --git a/libwvdrmengine/cdm/metrics/include/metrics_collections.h b/libwvdrmengine/cdm/metrics/include/metrics_collections.h index 213baadd..e720408b 100644 --- a/libwvdrmengine/cdm/metrics/include/metrics_collections.h +++ b/libwvdrmengine/cdm/metrics/include/metrics_collections.h @@ -146,11 +146,6 @@ class CryptoMetrics { ValueMetric oemcrypto_supports_usage_table_; CounterMetric oemcrypto_update_usage_table_; EventMetric oemcrypto_wrap_keybox_; - - /* Internal OEMCrypto Metrics */ - ValueMetric oemcrypto_initialization_mode_; - ValueMetric oemcrypto_l1_api_version_; - ValueMetric oemcrypto_l1_min_api_version_; }; // This class contains session-scoped metrics. All properties and @@ -196,6 +191,45 @@ class SessionMetrics { CryptoMetrics crypto_metrics_; }; +// This class contains metrics for the OEMCrypto Dynamic Adapter. They are +// separated from other metrics because they need to be encapsulated in a +// singleton object. This is because the dynamic adapter uses the OEMCrypto +// function signatures and contract and cannot be extended to inject +// dependencies. +// +// Operations for this metrics class are serialized since these particular +// metrics may be accessed by a separate thread during intialize even as +// the metric may be serialized. +class OemCryptoDynamicAdapterMetrics { + public: + explicit OemCryptoDynamicAdapterMetrics(); + + // Set methods for OEMCrypto metrics. + void SetInitializationMode(OEMCryptoInitializationMode mode); + void SetL1ApiVersion(uint32_t version); + void SetL1MinApiVersion(uint32_t version); + + // Serialize the session metrics to the provided |metric_group|. + // |metric_group| is owned by the caller and must not be null. + void Serialize(drm_metrics::MetricsGroup* metric_group); + + // Clears the existing metric values. + void Clear(); + + private: + Lock adapter_lock_; + ValueMetric oemcrypto_initialization_mode_; + ValueMetric oemcrypto_l1_api_version_; + ValueMetric oemcrypto_l1_min_api_version_; +}; + +// This will fetch the singleton instance for dynamic adapter metrics. +// This method is safe only if we use C++ 11. In C++ 11, static function-local +// initialization is guaranteed to be threadsafe. We return the reference to +// avoid non-guaranteed destructor order problems. Effectively, the destructor +// is never run for the created instance. +OemCryptoDynamicAdapterMetrics& GetDynamicAdapterMetricsInstance(); + // This class contains engine-scoped metrics. All properties and // statistics related to operations within the engine, but outside // the scope of a session are recorded here. diff --git a/libwvdrmengine/cdm/metrics/include/value_metric.h b/libwvdrmengine/cdm/metrics/include/value_metric.h index 6de010ee..5c0704ef 100644 --- a/libwvdrmengine/cdm/metrics/include/value_metric.h +++ b/libwvdrmengine/cdm/metrics/include/value_metric.h @@ -86,6 +86,12 @@ class ValueMetric : public MetricSerializable { // Get the current value of the metric. const T& GetValue() { return value_; } + // Clears the indicators that the metric or error was set. + void Clear() { + has_value_ = false; + has_error_ = false; + } + private: std::string metric_name_; T value_; diff --git a/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp b/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp index 8a8085ae..c70f2da4 100644 --- a/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp +++ b/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp @@ -234,14 +234,8 @@ CryptoMetrics::CryptoMetrics() : "oemcrypto_error"), oemcrypto_wrap_keybox_( "/drm/widevine/oemcrypto/wrap_keybox/time", - "oemcrypto_error"), - oemcrypto_initialization_mode_( - "/drm/widevine/oemcrypto/initialization_mode"), - oemcrypto_l1_api_version_( - "/drm/widevine/oemcrypto/l1_api_version"), - oemcrypto_l1_min_api_version_( - "/drm/widevine/oemcrypto/l1_min_api_version") - {} + "oemcrypto_error") { +} void CryptoMetrics::Serialize(MetricsGroup* metrics) { ProtoMetricSerializer serializer(metrics); @@ -304,11 +298,6 @@ void CryptoMetrics::Serialize(MetricsGroup* metrics) { oemcrypto_supports_usage_table_.Serialize(&serializer); oemcrypto_update_usage_table_.Serialize(&serializer); oemcrypto_wrap_keybox_.Serialize(&serializer); - - /* Internal OEMCrypto Metrics */ - oemcrypto_initialization_mode_.Serialize(&serializer); - oemcrypto_l1_api_version_.Serialize(&serializer); - oemcrypto_l1_min_api_version_.Serialize(&serializer); } SessionMetrics::SessionMetrics() : @@ -341,6 +330,60 @@ void SessionMetrics::SerializeSessionMetrics(MetricsGroup* metric_group) { cdm_session_restore_usage_session_.Serialize(&serializer); } + +OemCryptoDynamicAdapterMetrics::OemCryptoDynamicAdapterMetrics() : + oemcrypto_initialization_mode_( + "/drm/widevine/oemcrypto/initialization_mode"), + oemcrypto_l1_api_version_( + "/drm/widevine/oemcrypto/l1_api_version"), + oemcrypto_l1_min_api_version_( + "/drm/widevine/oemcrypto/l1_min_api_version") { +} + +void OemCryptoDynamicAdapterMetrics::SetInitializationMode( + OEMCryptoInitializationMode mode) { + AutoLock lock(adapter_lock_); + oemcrypto_initialization_mode_.Record(mode); +} + +void OemCryptoDynamicAdapterMetrics::SetL1ApiVersion(uint32_t version) { + AutoLock lock(adapter_lock_); + oemcrypto_l1_api_version_.Record(version); +} + +void OemCryptoDynamicAdapterMetrics::SetL1MinApiVersion(uint32_t version) { + AutoLock lock(adapter_lock_); + oemcrypto_l1_min_api_version_.Record(version); +} + +void OemCryptoDynamicAdapterMetrics::Serialize( + drm_metrics::MetricsGroup* metric_group) { + AutoLock lock(adapter_lock_); + ProtoMetricSerializer serializer(metric_group); + + oemcrypto_initialization_mode_.Serialize(&serializer); + oemcrypto_l1_api_version_.Serialize(&serializer); + oemcrypto_l1_min_api_version_.Serialize(&serializer); +} + +void OemCryptoDynamicAdapterMetrics::Clear() { + AutoLock lock(adapter_lock_); + + oemcrypto_initialization_mode_.Clear(); + oemcrypto_l1_api_version_.Clear(); + oemcrypto_l1_min_api_version_.Clear(); +} + +// This method returns a reference. This means that the destructor is never +// executed for the returned object. +OemCryptoDynamicAdapterMetrics& GetDynamicAdapterMetricsInstance() { + // This is safe in C++ 11 since the initialization is guaranteed to run + // only once regardless of multi-threaded access. + static OemCryptoDynamicAdapterMetrics* adapter_metrics = + new OemCryptoDynamicAdapterMetrics(); + return *adapter_metrics; +} + EngineMetrics::EngineMetrics() : cdm_engine_add_key_( "/drm/widevine/cdm_engine/add_key/time", @@ -401,7 +444,7 @@ EngineMetrics::EngineMetrics() : } EngineMetrics::~EngineMetrics() { - AutoLock kock(session_metrics_lock_); + AutoLock lock(session_metrics_lock_); std::vector::iterator i; if (!session_metrics_list_.empty()) { LOGV("EngineMetrics::~EngineMetrics. Session count: %d", @@ -435,6 +478,11 @@ void EngineMetrics::Serialize(drm_metrics::MetricsGroup* metric_group, bool clear_serialized_sessions) { AutoLock lock(session_metrics_lock_); + // Serialize the most recent metrics from the OemCyrpto dynamic adapter. + OemCryptoDynamicAdapterMetrics& adapter_metrics = + GetDynamicAdapterMetricsInstance(); + adapter_metrics.Serialize(metric_group); + SerializeEngineMetrics(metric_group); std::vector::iterator i; for (i = session_metrics_list_.begin(); i != session_metrics_list_.end(); diff --git a/libwvdrmengine/cdm/test/wv_cdm_metrics_test.cpp b/libwvdrmengine/cdm/test/wv_cdm_metrics_test.cpp index fb8655d8..169cc280 100644 --- a/libwvdrmengine/cdm/test/wv_cdm_metrics_test.cpp +++ b/libwvdrmengine/cdm/test/wv_cdm_metrics_test.cpp @@ -53,15 +53,17 @@ TEST_F(WvContentDecryptionModuleMetricsTest, EngineOnlyMetrics) { ASSERT_TRUE(metrics.ParseFromString(serialized_metrics)); EXPECT_THAT(metrics.metric_size(), Eq(0)); ASSERT_THAT(metrics.metric_sub_group_size(), Eq(1)); - ASSERT_THAT(metrics.metric_sub_group(0).metric_size(), Ge(3)); + ASSERT_THAT(metrics.metric_sub_group(0).metric_size(), Ge(6)); EXPECT_THAT(metrics.metric_sub_group(0).metric_sub_group_size(), Eq(0)); EXPECT_THAT(metrics.metric_sub_group(0).metric(0).name(), - StrEq("/drm/widevine/cdm_engine/version")); + StrEq("/drm/widevine/oemcrypto/initialization_mode")); + // Can't check the initialization mode value. Different devices will have + // different values. EXPECT_THAT( - metrics.metric_sub_group(0).metric(2).name(), + metrics.metric_sub_group(0).metric(5).name(), StrEq("/drm/widevine/cdm_engine/" "get_provisioning_request/time/count{error:0}")); - EXPECT_THAT(metrics.metric_sub_group(0).metric(2).value().int_value(), Eq(1)); + EXPECT_THAT(metrics.metric_sub_group(0).metric(5).value().int_value(), Eq(1)); } @@ -85,14 +87,14 @@ TEST_F(WvContentDecryptionModuleMetricsTest, EngineAndSessionMetrics) { // The outer container will never have metrics. EXPECT_THAT(metrics.metric_size(), Eq(0)); ASSERT_THAT(metrics.metric_sub_group_size(), Eq(1)); - ASSERT_THAT(metrics.metric_sub_group(0).metric_size(), Ge(3)); + ASSERT_THAT(metrics.metric_sub_group(0).metric_size(), Ge(6)); // Validate engine-level metrics. EXPECT_THAT(metrics.metric_sub_group(0).metric(0).name(), - StrEq("/drm/widevine/cdm_engine/version")); - EXPECT_THAT(metrics.metric_sub_group(0).metric(2).name(), + StrEq("/drm/widevine/oemcrypto/initialization_mode")); + EXPECT_THAT(metrics.metric_sub_group(0).metric(5).name(), StrEq("/drm/widevine/cdm_engine/open_session/count{error:7}")); - EXPECT_THAT(metrics.metric_sub_group(0).metric(2).value().int_value(), Eq(1)); + EXPECT_THAT(metrics.metric_sub_group(0).metric(5).value().int_value(), Eq(1)); // Validate a session-level metric. EXPECT_THAT(metrics.metric_sub_group(0).metric_sub_group_size(), Eq(1)); @@ -131,12 +133,12 @@ TEST_F(WvContentDecryptionModuleMetricsTest, MultipleEngineMetric) { for (int i = 0; i < metrics.metric_sub_group_size(); i++) { // Validate the engine-level metric. - ASSERT_THAT(metrics.metric_sub_group(i).metric_size(), Ge(3)); + ASSERT_THAT(metrics.metric_sub_group(i).metric_size(), Ge(6)); EXPECT_THAT(metrics.metric_sub_group(i).metric(0).name(), - StrEq("/drm/widevine/cdm_engine/version")); - EXPECT_THAT(metrics.metric_sub_group(i).metric(2).name(), + StrEq("/drm/widevine/oemcrypto/initialization_mode")); + EXPECT_THAT(metrics.metric_sub_group(i).metric(5).name(), StrEq("/drm/widevine/cdm_engine/open_session/count{error:7}")); - EXPECT_THAT(metrics.metric_sub_group(i).metric(2).value().int_value(), Eq(1)); + EXPECT_THAT(metrics.metric_sub_group(i).metric(5).value().int_value(), Eq(1)); // Validate a session-level metric. EXPECT_THAT(metrics.metric_sub_group(i).metric_sub_group_size(), Eq(1));