Add session metric pruning.

[ Merge of http://go/wvgerrit/65983 ]

Add support to drop closed metrics in order to save space for
long-running applications (and CdmEngine instances). The code now keeps
only a limited number of metrics collections after the session is closed.
As a session (and its metrics session) is closed, the oldest, closed metrics
session is dropped. This means those metrics will not be reported nor
accessible in the client.

Bug: http://b/118664842

Test: CDM Unit tests. Android Unit Test. Ran GPLay Manually.
Change-Id: I27d6e61a8fe4148ad1ef2a433c8e5f4cdd84cc72

libwvdrmengine/cdm/core/src/cdm_engine.cpp

@@ -194,6 +194,7 @@ CdmResponseType CdmEngine::CloseSession(const CdmSessionId& session_id) {
     LOGE("CdmEngine::CloseSession: session not found = %s", session_id.c_str());
     return SESSION_NOT_FOUND_1;
   }
+  metrics_.ConsolidateSessions();
   return NO_ERROR;
 }
 

libwvdrmengine/cdm/metrics/include/metrics_collections.h

@@ -90,6 +90,10 @@ const int kLicenseTypeFieldNumber =
 
 }  // anonymous namespace
 
+// The maximum number of completed sessions that can be stored. More than this
+// will cause some metrics to be discarded.
+const int kMaxCompletedSessions = 40;
+
 // This enum defines the conditions encountered during OEMCrypto Initialization
 // in oemcrypto_adapter_dynamic.
 typedef enum OEMCryptoInitializationMode {
@@ -356,22 +360,19 @@ class EngineMetrics {
   // be called when the SessionMetrics instance is no longer in use.
   void RemoveSession(CdmSessionId session_id);
 
+  // Looks for session metrics that have been marked as completed. These metrics
+  // may be merged or discarded if there are too many completed session metric
+  // instances.
+  void ConsolidateSessions();
+
   // Returns a pointer to the crypto metrics belonging to the engine instance.
   // The CryptoMetrics instance is still owned by this object and will exist
   // until this object is deleted.
   CryptoMetrics *GetCryptoMetrics() { return &crypto_metrics_; }
 
-  // Serialize engine and session metrics into a serialized MetricsGroup
-  // instance and output that instance to the provided |metric_group|.
-  // |metric_group| is owned by the caller and must NOT be null.
-  // |completed_only| indicates that this call should only publish
-  //     SessionMetrics instances that are marked as completed.
-  // |clear_sessions| indicates that this call should clear sessions metrics
-  //     for those sessions that were serialized. This allows atomic
-  //     serialization and closing of session-level metrics.
-  // void Serialize(drm_metrics::MetricsGroup* metric_group, bool
-  // completed_only,
-  //               bool clear_serialized_sessions);
+  // Serialize engine and session metrics into a serialized WvCdmMetrics
+  // instance and output that instance to the provided |engine_metrics|.
+  // |engine_metrics| is owned by the caller and must NOT be null.
   void Serialize(drm_metrics::WvCdmMetrics *engine_metrics) const;
 
   void SetAppPackageName(const std::string &app_package_name);
@@ -421,7 +422,10 @@ class EngineMetrics {
 
  private:
   mutable std::mutex session_metrics_lock_;
-  std::vector<metrics::SessionMetrics *> session_metrics_list_;
+  std::vector<std::shared_ptr<metrics::SessionMetrics>>
+      active_session_metrics_list_;
+  std::vector<std::shared_ptr<metrics::SessionMetrics>>
+      completed_session_metrics_list_;
   // This is used to populate the engine lifespan metric
   metrics::TimerMetric life_span_internal_;
   CryptoMetrics crypto_metrics_;

libwvdrmengine/cdm/metrics/src/metrics_collections.cpp

@@ -13,13 +13,15 @@ using ::google::protobuf::RepeatedPtrField;
 using ::wvcdm::metrics::EventMetric;
 
 namespace {
+
 // Helper struct for comparing session ids.
 struct CompareSessionIds {
   const std::string &target_;
 
   CompareSessionIds(const wvcdm::CdmSessionId &target) : target_(target){};
 
-  bool operator()(const wvcdm::metrics::SessionMetrics *metrics) const {
+  bool operator()(const std::shared_ptr<wvcdm::metrics::SessionMetrics> metrics)
+      const {
     return metrics->GetSessionId() == target_;
   }
 };
@@ -258,31 +260,60 @@ EngineMetrics::EngineMetrics() {
 
 EngineMetrics::~EngineMetrics() {
   std::unique_lock<std::mutex> lock(session_metrics_lock_);
-  std::vector<SessionMetrics *>::iterator i;
-  if (!session_metrics_list_.empty()) {
-    LOGV("EngineMetrics::~EngineMetrics. Session count: %d",
-         session_metrics_list_.size());
+  if (!active_session_metrics_list_.empty()
+      || !completed_session_metrics_list_.empty()) {
+    LOGV("EngineMetrics::~EngineMetrics. Session counts: "
+         "active %d. completed %d.", active_session_metrics_list_.size(),
+         completed_session_metrics_list_.size());
   }
-  for (i = session_metrics_list_.begin(); i != session_metrics_list_.end();
-       i++) {
-    delete *i;
-  }
-  session_metrics_list_.clear();
 }
 
 SessionMetrics *EngineMetrics::AddSession() {
   std::unique_lock<std::mutex> lock(session_metrics_lock_);
-  SessionMetrics *metrics = new SessionMetrics();
-  session_metrics_list_.push_back(metrics);
-  return metrics;
+  active_session_metrics_list_.push_back(std::make_shared<SessionMetrics>());
+  return active_session_metrics_list_.back().get();
 }
 
 void EngineMetrics::RemoveSession(wvcdm::CdmSessionId session_id) {
   std::unique_lock<std::mutex> lock(session_metrics_lock_);
-  session_metrics_list_.erase(
-      std::remove_if(session_metrics_list_.begin(), session_metrics_list_.end(),
+  active_session_metrics_list_.erase(
+      std::remove_if(active_session_metrics_list_.begin(),
+                     active_session_metrics_list_.end(),
                      CompareSessionIds(session_id)),
-      session_metrics_list_.end());
+      active_session_metrics_list_.end());
+  completed_session_metrics_list_.erase(
+      std::remove_if(completed_session_metrics_list_.begin(),
+                     completed_session_metrics_list_.end(),
+                     CompareSessionIds(session_id)),
+      completed_session_metrics_list_.end());
+}
+
+void EngineMetrics::ConsolidateSessions() {
+  auto completed_filter =
+      [] (const std::shared_ptr<SessionMetrics>& session_metrics) {
+        return session_metrics->IsCompleted();
+      };
+
+  std::unique_lock<std::mutex> lock(session_metrics_lock_);
+  std::copy_if(active_session_metrics_list_.begin(),
+               active_session_metrics_list_.end(),
+               std::back_inserter(completed_session_metrics_list_),
+               completed_filter);
+  active_session_metrics_list_.erase(
+      std::remove_if(active_session_metrics_list_.begin(),
+                     active_session_metrics_list_.end(),
+                     completed_filter),
+      active_session_metrics_list_.end());
+
+  // TODO(b/118664842): Add support to merge older metrics into one
+  // consolidated metric.
+  int excess_completed = completed_session_metrics_list_.size()
+                         - kMaxCompletedSessions;
+  if (excess_completed > 0) {
+    completed_session_metrics_list_.erase(
+        completed_session_metrics_list_.begin(),
+        completed_session_metrics_list_.begin() + excess_completed);
+  }
 }
 
 void EngineMetrics::Serialize(WvCdmMetrics *wv_metrics) const {
@@ -299,8 +330,12 @@ void EngineMetrics::Serialize(WvCdmMetrics *wv_metrics) const {
   }
   SerializeEngineMetrics(engine_metrics);
 
-  for (std::vector<metrics::SessionMetrics *>::const_iterator it =
-       session_metrics_list_.begin(); it != session_metrics_list_.end(); it++) {
+  for (auto it = completed_session_metrics_list_.begin();
+       it != completed_session_metrics_list_.end(); it++) {
+    (*it)->Serialize(wv_metrics->add_session_metrics());
+  }
+  for (auto it = active_session_metrics_list_.begin();
+       it != active_session_metrics_list_.end(); it++) {
     (*it)->Serialize(wv_metrics->add_session_metrics());
   }
 }

libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp

@@ -14,9 +14,14 @@
 #include "metrics.pb.h"
 #include "wv_cdm_types.h"
 
-using drm_metrics::MetricsGroup;
+using drm_metrics::WvCdmMetrics;
 using google::protobuf::TextFormat;
 
+namespace {
+const char kSessionId1[] = "session_id_1";
+const char kSessionId2[] = "session_id_2";
+}  // anonymous namespace
+
 namespace wvcdm {
 namespace metrics {
 
@@ -29,216 +34,204 @@ TEST_F(EngineMetricsTest, AllEngineMetrics) {
   EngineMetrics engine_metrics;
 
   // Set some values in all of the engine metrics.
-  engine_metrics.cdm_engine_add_key_.Record(1.0, KEY_ADDED);
-  engine_metrics.cdm_engine_close_session_.Record(1.0, NO_ERROR);
-  engine_metrics.cdm_engine_decrypt_.Record(1.0, NO_ERROR);
-  engine_metrics.cdm_engine_find_session_for_key_.Record(1.0, false);
-  engine_metrics.cdm_engine_generate_key_request_.Record(1.0, NO_ERROR);
+  engine_metrics.cdm_engine_add_key_.Record(1.0, KEY_ADDED, kLicenseTypeRelease);
+  engine_metrics.cdm_engine_close_session_.Increment(NO_ERROR);
+  engine_metrics.cdm_engine_decrypt_.Record(1.0, NO_ERROR,
+                                            metrics::Pow2Bucket(8));
+  engine_metrics.cdm_engine_find_session_for_key_.Increment(false);
+  engine_metrics.cdm_engine_generate_key_request_.Record(1.0, NO_ERROR, kLicenseTypeRelease);
   engine_metrics.cdm_engine_get_provisioning_request_.Record(1.0, NO_ERROR);
   engine_metrics.cdm_engine_get_usage_info_.Record(1.0, NO_ERROR);
   engine_metrics.cdm_engine_handle_provisioning_response_.Record(1.0, NO_ERROR);
-  engine_metrics.cdm_engine_life_span_.Record(1.0);
-  engine_metrics.cdm_engine_open_key_set_session_.Record(1.0, NO_ERROR);
-  engine_metrics.cdm_engine_open_session_.Record(1.0, NO_ERROR);
+  engine_metrics.cdm_engine_open_key_set_session_.Increment(NO_ERROR);
+  engine_metrics.cdm_engine_open_session_.Increment(NO_ERROR);
   engine_metrics.cdm_engine_query_key_status_.Record(1.0, NO_ERROR);
-  engine_metrics.cdm_engine_release_all_usage_info_.Record(1.0, NO_ERROR);
-  engine_metrics.cdm_engine_release_usage_info_.Record(1.0, NO_ERROR);
-  engine_metrics.cdm_engine_remove_keys_.Record(1.0, NO_ERROR);
+  engine_metrics.cdm_engine_release_all_usage_info_.Increment(NO_ERROR);
+  engine_metrics.cdm_engine_release_usage_info_.Increment(NO_ERROR);
+  engine_metrics.cdm_engine_remove_keys_.Increment(NO_ERROR);
   engine_metrics.cdm_engine_restore_key_.Record(1.0, NO_ERROR);
-  engine_metrics.cdm_engine_unprovision_.Record(1.0, NO_ERROR, kSecurityLevelL1);
+  engine_metrics.cdm_engine_unprovision_.Increment(NO_ERROR, kSecurityLevelL1);
 
-  drm_metrics::MetricsGroup actual_metrics;
-  engine_metrics.Serialize(&actual_metrics, true, false);
+  // Also set and serialize the oemcrypto dynamic adapter metrics.
+  OemCryptoDynamicAdapterMetrics adapter_metrics;
+  adapter_metrics.SetInitializationMode(OEMCrypto_INITIALIZED_FORCING_L3);
+  adapter_metrics.SetL1ApiVersion(2);
+  adapter_metrics.SetL1MinApiVersion(1);
 
-  // For each EventMetric, 2 metrics get serialized since only one sample was
-  // taken. So, the total number of serialized metrics are 2*17.
-  ASSERT_EQ(2 * 17, actual_metrics.metric_size());
-  EXPECT_EQ(0, actual_metrics.metric_sub_group_size());
+  WvCdmMetrics actual_metrics;
+  engine_metrics.Serialize(&actual_metrics);
+  adapter_metrics.Serialize(actual_metrics.mutable_engine_metrics());
+
+  ASSERT_TRUE(actual_metrics.has_engine_metrics());
+  EXPECT_EQ(0, actual_metrics.session_metrics_size());
 
   // Spot check some metrics.
-  EXPECT_EQ("/drm/widevine/cdm_engine/add_key/time/count{error:2}",
-            actual_metrics.metric(0).name());
-  EXPECT_EQ("/drm/widevine/cdm_engine/close_session/count{error:0}",
-            actual_metrics.metric(3).name());
-  EXPECT_EQ("/drm/widevine/cdm_engine/decrypt/time/mean{error:0}",
-            actual_metrics.metric(5).name());
-  EXPECT_EQ(1.0, actual_metrics.metric(5).value().double_value());
+  ASSERT_EQ(1,
+            actual_metrics.engine_metrics().cdm_engine_add_key_time_us_size());
+  EXPECT_EQ(2, actual_metrics.engine_metrics()
+                             .cdm_engine_add_key_time_us(0)
+                             .attributes().error_code());
+  ASSERT_EQ(1,
+            actual_metrics.engine_metrics().cdm_engine_close_session_size());
+  EXPECT_EQ(0, actual_metrics.engine_metrics()
+                             .cdm_engine_close_session(0)
+                             .attributes().error_code());
+  ASSERT_EQ(1,
+            actual_metrics.engine_metrics().cdm_engine_decrypt_time_us_size());
+  EXPECT_EQ(0, actual_metrics.engine_metrics()
+                             .cdm_engine_decrypt_time_us(0)
+                             .attributes().error_code());
+  EXPECT_EQ(1.0, actual_metrics.engine_metrics()
+                               .cdm_engine_decrypt_time_us(0)
+                               .mean());
+  EXPECT_EQ(OEMCrypto_INITIALIZED_FORCING_L3,
+            actual_metrics.engine_metrics().oemcrypto_initialization_mode()
+                          .int_value());
 }
 
 TEST_F(EngineMetricsTest, EngineAndCryptoMetrics) {
   EngineMetrics engine_metrics;
 
   // Set some values in some of the engine metrics and some crypto metrics.
-  engine_metrics.cdm_engine_add_key_.Record(1.0, KEY_ADDED);
-  engine_metrics.cdm_engine_close_session_.Record(1.0, NO_ERROR);
+  engine_metrics.cdm_engine_add_key_.Record(1.0, KEY_ADDED, kLicenseTypeRelease);
+  engine_metrics.cdm_engine_close_session_.Increment(UNKNOWN_ERROR);
   CryptoMetrics* crypto_metrics = engine_metrics.GetCryptoMetrics();
 
+  crypto_metrics->crypto_session_get_device_unique_id_.Increment(true);
   crypto_metrics->crypto_session_generic_decrypt_
       .Record(2.0, NO_ERROR, Pow2Bucket(1025), kEncryptionAlgorithmAesCbc128);
-  crypto_metrics->crypto_session_get_device_unique_id_
-      .Record(4.0, false);
 
-  drm_metrics::MetricsGroup actual_metrics;
-  engine_metrics.Serialize(&actual_metrics, true, false);
+  WvCdmMetrics actual_metrics;
+  engine_metrics.Serialize(&actual_metrics);
 
-  // For each EventMetric, 2 metrics get serialized since only one sample was
-  // taken. So, the total number of serialized metrics are 2*4 since we
-  // touched 4 metrics.
-  ASSERT_EQ(2 * 4, actual_metrics.metric_size());
-  EXPECT_EQ(0, actual_metrics.metric_sub_group_size());
+  ASSERT_TRUE(actual_metrics.has_engine_metrics());
+  EXPECT_EQ(0, actual_metrics.session_metrics_size());
 
   // Spot check some metrics.
-  EXPECT_EQ("/drm/widevine/cdm_engine/add_key/time/count{error:2}",
-            actual_metrics.metric(0).name());
-  EXPECT_EQ(
-      "/drm/widevine/crypto_session/generic_decrypt/time/count"
-      "{error:0&length:1024&encryption_algorithm:1}",
-      actual_metrics.metric(4).name());
-  EXPECT_EQ(
-      "/drm/widevine/crypto_session/get_device_unique_id/count{success:0}",
-      actual_metrics.metric(7).name());
-  EXPECT_EQ(4.0, actual_metrics.metric(7).value().double_value());
+  ASSERT_EQ(1,
+            actual_metrics.engine_metrics().cdm_engine_add_key_time_us_size());
+  EXPECT_EQ(2, actual_metrics.engine_metrics()
+                             .cdm_engine_add_key_time_us(0)
+                             .attributes().error_code());
+  ASSERT_EQ(1,
+            actual_metrics.engine_metrics().cdm_engine_close_session_size());
+  EXPECT_EQ(UNKNOWN_ERROR, actual_metrics.engine_metrics()
+                                         .cdm_engine_close_session(0)
+                                         .attributes().error_code());
+  ASSERT_EQ(1, actual_metrics.engine_metrics().crypto_metrics()
+                             .crypto_session_get_device_unique_id_size());
+  EXPECT_EQ(1, actual_metrics.engine_metrics().crypto_metrics()
+                             .crypto_session_get_device_unique_id(0)
+                             .count());
+  EXPECT_EQ(true, actual_metrics.engine_metrics().crypto_metrics()
+                                .crypto_session_get_device_unique_id(0)
+                                .attributes().error_code_bool());
+  ASSERT_EQ(1, actual_metrics.engine_metrics().crypto_metrics()
+                             .crypto_session_generic_decrypt_time_us_size());
+  EXPECT_EQ(2.0, actual_metrics.engine_metrics().crypto_metrics()
+                             .crypto_session_generic_decrypt_time_us(0)
+                             .mean());
+  EXPECT_EQ(NO_ERROR, actual_metrics.engine_metrics().crypto_metrics()
+                                    .crypto_session_generic_decrypt_time_us(0)
+                                    .attributes().error_code());
 }
 
 TEST_F(EngineMetricsTest, EmptyEngineMetrics) {
   EngineMetrics engine_metrics;
 
-  drm_metrics::MetricsGroup actual_metrics;
-  engine_metrics.Serialize(&actual_metrics, true, false);
+  WvCdmMetrics actual_metrics;
+  engine_metrics.Serialize(&actual_metrics);
 
-  EXPECT_EQ(0, actual_metrics.metric_size());
-  EXPECT_EQ(0, actual_metrics.metric_sub_group_size());
+  // Empty engine metics will still produce an engine metrics record.
+  EXPECT_TRUE(actual_metrics.has_engine_metrics());
+  EXPECT_EQ(0, actual_metrics.session_metrics_size());
 }
 
-TEST_F(EngineMetricsTest, EngineMetricsWithCompletedSessions) {
+TEST_F(EngineMetricsTest, EngineMetricsWithSessions) {
   EngineMetrics engine_metrics;
 
   // Set a values in an engine metric and in a crypto metric.
-  engine_metrics.cdm_engine_add_key_.Record(1.0, KEY_ADDED);
+  engine_metrics.cdm_engine_add_key_.Record(1.0, KEY_ADDED, kLicenseTypeRelease);
   engine_metrics.GetCryptoMetrics()
       ->crypto_session_load_certificate_private_key_.Record(2.0, true);
 
   // Create two sessions and record some metrics.
   SessionMetrics* session_metrics_1 = engine_metrics.AddSession();
-  session_metrics_1->SetSessionId("session_id_1");
+  session_metrics_1->SetSessionId(kSessionId1);
   SessionMetrics* session_metrics_2 = engine_metrics.AddSession();
-  session_metrics_2->SetSessionId("session_id_2");
+  session_metrics_2->SetSessionId(kSessionId2);
   // Record a CryptoMetrics metric in the session.
   session_metrics_2->GetCryptoMetrics()->crypto_session_generic_decrypt_
       .Record(2.0, NO_ERROR, Pow2Bucket(1025), kEncryptionAlgorithmAesCbc128);
-  session_metrics_2->SetSessionId("session_id_2");
-  // Mark only session 2 as completed.
-  session_metrics_2->SetCompleted();
 
-  drm_metrics::MetricsGroup actual_metrics;
-  engine_metrics.Serialize(&actual_metrics, true, false);
-
-  // Validate metric counts.
-  // For each EventMetric, 2 metrics get serialized since only one sample was
-  // taken. So, the total number of serialized metrics are 2*2 since we
-  // touched 2 metrics.
-  ASSERT_EQ(2 * 2, actual_metrics.metric_size());
-  ASSERT_EQ(1, actual_metrics.metric_sub_group_size());
-  ASSERT_EQ(3, actual_metrics.metric_sub_group(0).metric_size());
+  WvCdmMetrics actual_metrics;
+  engine_metrics.Serialize(&actual_metrics);
 
   // Spot check some metrics.
-  EXPECT_EQ("/drm/widevine/cdm_engine/add_key/time/count{error:2}",
-            actual_metrics.metric(0).name());
-  EXPECT_EQ("/drm/widevine/crypto_session/load_certificate_private_key"
-            "/time/count{success:1}",
-            actual_metrics.metric(2).name());
-  EXPECT_EQ("/drm/widevine/cdm_session/session_id",
-            actual_metrics.metric_sub_group(0).metric(0).name());
-  EXPECT_EQ(
-      "session_id_2",
-      actual_metrics.metric_sub_group(0).metric(0).value().string_value());
-  EXPECT_EQ(
-      "/drm/widevine/crypto_session/generic_decrypt/time/count"
-      "{error:0&length:1024&encryption_algorithm:1}",
-      actual_metrics.metric_sub_group(0).metric(1).name());
+  ASSERT_TRUE(actual_metrics.has_engine_metrics());
+  EXPECT_EQ(1,
+            actual_metrics.engine_metrics().cdm_engine_add_key_time_us_size());
+  EXPECT_EQ(2, actual_metrics.session_metrics_size());
+  EXPECT_EQ(kSessionId1,
+            actual_metrics.session_metrics(0).session_id().string_value());
+  EXPECT_EQ(kSessionId2,
+            actual_metrics.session_metrics(1).session_id().string_value());
+  EXPECT_EQ(1, actual_metrics.session_metrics(1).crypto_metrics()
+                   .crypto_session_generic_decrypt_time_us_size());
 }
 
-TEST_F(EngineMetricsTest, EngineMetricsSerializeAllSessions) {
+TEST_F(EngineMetricsTest, EngineMetricsConsolidateSessions) {
   EngineMetrics engine_metrics;
 
-  // Create two sessions and record some metrics.
-  SessionMetrics* session_metrics_1 = engine_metrics.AddSession();
-  session_metrics_1->SetSessionId("session_id_1");
-  SessionMetrics* session_metrics_2 = engine_metrics.AddSession();
-  session_metrics_2->SetSessionId("session_id_2");
-  // Mark only session 2 as completed.
-  session_metrics_2->SetCompleted();
+  // Create one more session than the max allowed closed sessions.
+  std::vector<SessionMetrics*> session_metrics;
+  for (int i = 0; i < kMaxCompletedSessions + 1; i++) {
+    session_metrics.push_back(engine_metrics.AddSession());
+    session_metrics.back()->SetSessionId(std::to_string(i));
+  }
 
-  drm_metrics::MetricsGroup actual_metrics;
-  engine_metrics.Serialize(&actual_metrics, false, false);
+  // Mark sessions as completed.
+  for (auto it = session_metrics.begin(); it != session_metrics.end(); it++) {
+    (*it)->SetCompleted();
+  }
 
-  // Validate metric counts.
-  // No Engine-level metrics were recorded.
-  ASSERT_EQ(0, actual_metrics.metric_size());
-  // Two sub groups, 1 per session.
-  ASSERT_EQ(2, actual_metrics.metric_sub_group_size());
-  ASSERT_EQ(1, actual_metrics.metric_sub_group(0).metric_size());
+  engine_metrics.ConsolidateSessions();
 
-  // Spot check some metrics.
-  EXPECT_EQ("/drm/widevine/cdm_session/session_id",
-            actual_metrics.metric_sub_group(0).metric(0).name());
-  EXPECT_EQ(
-      "session_id_1",
-      actual_metrics.metric_sub_group(0).metric(0).value().string_value());
-  EXPECT_EQ("/drm/widevine/cdm_session/session_id",
-            actual_metrics.metric_sub_group(1).metric(0).name());
-  EXPECT_EQ(
-      "session_id_2",
-      actual_metrics.metric_sub_group(1).metric(0).value().string_value());
+  // Consolidate the metrics now. These should now be consolidated down to
+  // kMaxCompletedSessions.
+  WvCdmMetrics actual_metrics;
+  engine_metrics.Serialize(&actual_metrics);
+  ASSERT_EQ(kMaxCompletedSessions, actual_metrics.session_metrics_size());
 }
 
-TEST_F(EngineMetricsTest, EngineMetricsRemoveSessions) {
+TEST_F(EngineMetricsTest, EngineMetricsConsolidateSessionsNoClosedSessions) {
   EngineMetrics engine_metrics;
 
-  // Create two sessions and record some metrics.
-  SessionMetrics* session_metrics_1 = engine_metrics.AddSession();
-  session_metrics_1->SetSessionId("session_id_1");
-  SessionMetrics* session_metrics_2 = engine_metrics.AddSession();
-  session_metrics_2->SetSessionId("session_id_2");
-  // Mark only session 2 as completed.
-  session_metrics_2->SetCompleted();
+  // Create one more session than the max allowed closed sessions.
+  std::vector<SessionMetrics*> session_metrics;
+  for (int i = 0; i < kMaxCompletedSessions + 1; i++) {
+    session_metrics.push_back(engine_metrics.AddSession());
+    session_metrics.back()->SetSessionId(std::to_string(i));
+  }
 
-  // Serialize all metrics, don't remove any.
-  drm_metrics::MetricsGroup actual_metrics;
-  engine_metrics.Serialize(&actual_metrics, false, false);
+  // Consolidate sessions will do nothing since no sessions are closed.
+  engine_metrics.ConsolidateSessions();
 
-  // Validate metric counts.
-  // Two sub groups, 1 per session.
-  ASSERT_EQ(2, actual_metrics.metric_sub_group_size());
+  // Serializing now will include all sessions since none are closed.
+  WvCdmMetrics actual_metrics;
+  engine_metrics.Serialize(&actual_metrics);
+  ASSERT_EQ(kMaxCompletedSessions + 1, actual_metrics.session_metrics_size());
+}
 
-  // Serialize completed metrics, remove them.
-  actual_metrics.Clear();
-  engine_metrics.Serialize(&actual_metrics, true, true);
-  // Validate metric counts.
-  // Only one, completed session should exist.
-  ASSERT_EQ(1, actual_metrics.metric_sub_group_size());
-  ASSERT_EQ(1, actual_metrics.metric_sub_group(0).metric_size());
-  EXPECT_EQ(
-      "session_id_2",
-      actual_metrics.metric_sub_group(0).metric(0).value().string_value());
+TEST_F(EngineMetricsTest, EngineMetricsConsolidateSessionsNoSessions) {
+  EngineMetrics engine_metrics;
 
-  // Serialize all metrics, remove them.
-  actual_metrics.Clear();
-  engine_metrics.Serialize(&actual_metrics, false, true);
-  // Validate metric counts.
-  // Only one, non-complete session should exist.
-  ASSERT_EQ(1, actual_metrics.metric_sub_group_size());
-  ASSERT_EQ(1, actual_metrics.metric_sub_group(0).metric_size());
-  EXPECT_EQ(
-      "session_id_1",
-      actual_metrics.metric_sub_group(0).metric(0).value().string_value());
+  // Consolidate sessions will do nothing since there are no sessions
+  engine_metrics.ConsolidateSessions();
 
-  // Serialize all metrics, don't remove any.
-  actual_metrics.Clear();
-  engine_metrics.Serialize(&actual_metrics, false, false);
-  // Validate metric counts.
-  // There should be no more metric subgroups left.
-  ASSERT_EQ(0, actual_metrics.metric_sub_group_size());
+  WvCdmMetrics actual_metrics;
+  engine_metrics.Serialize(&actual_metrics);
+  ASSERT_EQ(0, actual_metrics.session_metrics_size());
 }
 
 class SessionMetricsTest : public ::testing::Test {
@@ -246,48 +239,44 @@ class SessionMetricsTest : public ::testing::Test {
 
 TEST_F(SessionMetricsTest, AllSessionMetrics) {
   SessionMetrics session_metrics;
-  session_metrics.SetSessionId("session_id 1");
-
+  session_metrics.SetSessionId(kSessionId1);
   session_metrics.cdm_session_life_span_.Record(1.0);
   session_metrics.cdm_session_renew_key_.Record(1.0, NO_ERROR);
-  session_metrics.cdm_session_restore_offline_session_.Record(1.0, NO_ERROR);
-  session_metrics.cdm_session_restore_usage_session_.Record(1.0, NO_ERROR);
+  session_metrics.cdm_session_restore_offline_session_.Increment(NO_ERROR);
+  session_metrics.cdm_session_restore_usage_session_.Increment(NO_ERROR);
+  session_metrics.cdm_session_license_request_latency_ms_.Record(
+      2.0, kKeyRequestTypeInitial);
 
   // Record a CryptoMetrics metric in the session.
   session_metrics.GetCryptoMetrics()->crypto_session_generic_decrypt_
       .Record(2.0, NO_ERROR, Pow2Bucket(1025), kEncryptionAlgorithmAesCbc128);
 
-  MetricsGroup actual_metrics;
+  WvCdmMetrics::SessionMetrics actual_metrics;
   session_metrics.Serialize(&actual_metrics);
 
-  ASSERT_EQ(11, actual_metrics.metric_size());
-  EXPECT_EQ(0, actual_metrics.metric_sub_group_size());
-
-  // Spot check some metrics.
-  EXPECT_EQ("/drm/widevine/cdm_session/session_id",
-            actual_metrics.metric(0).name());
-  EXPECT_EQ("/drm/widevine/cdm_session/life_span",
-            actual_metrics.metric(1).name());
-  EXPECT_EQ("/drm/widevine/cdm_session/renew_key/time/mean{error:0}",
-            actual_metrics.metric(4).name());
-  EXPECT_EQ(1.0, actual_metrics.metric(4).value().double_value());
-  EXPECT_EQ("/drm/widevine/crypto_session/generic_decrypt/time/count"
-            "{error:0&length:1024&encryption_algorithm:1}",
-            actual_metrics.metric(9).name());
+  // Spot check metrics.
+  EXPECT_STREQ(kSessionId1,
+               actual_metrics.session_id().string_value().c_str());
+  EXPECT_EQ(1.0, actual_metrics.cdm_session_life_span_ms().double_value());
+  EXPECT_EQ(1, actual_metrics.cdm_session_renew_key_time_us_size());
+  EXPECT_EQ(1, actual_metrics.cdm_session_restore_offline_session_size());
+  EXPECT_EQ(1, actual_metrics.cdm_session_restore_usage_session_size());
+  EXPECT_EQ(1, actual_metrics.crypto_metrics()
+                             .crypto_session_generic_decrypt_time_us_size());
 }
 
 TEST_F(SessionMetricsTest, EmptySessionMetrics) {
   SessionMetrics session_metrics;
 
-  MetricsGroup actual_metrics;
+  WvCdmMetrics::SessionMetrics actual_metrics;
   session_metrics.Serialize(&actual_metrics);
 
-  // Session metric always has a session id.
-  ASSERT_EQ(1, actual_metrics.metric_size());
-  EXPECT_EQ("/drm/widevine/cdm_session/session_id",
-            actual_metrics.metric(0).name());
-  EXPECT_EQ("", actual_metrics.metric(0).value().string_value());
-  EXPECT_EQ(0, actual_metrics.metric_sub_group_size());
+  EXPECT_TRUE(actual_metrics.has_crypto_metrics());
+  EXPECT_FALSE(actual_metrics.has_session_id());
+  EXPECT_FALSE(actual_metrics.has_cdm_session_life_span_ms());
+  EXPECT_EQ(0, actual_metrics.cdm_session_renew_key_time_us_size());
+  EXPECT_EQ(0, actual_metrics.cdm_session_restore_offline_session_size());
+  EXPECT_EQ(0, actual_metrics.cdm_session_restore_usage_session_size());
 }
 
 class CryptoMetricsTest : public ::testing::Test {
@@ -297,10 +286,9 @@ TEST_F(CryptoMetricsTest, AllCryptoMetrics) {
   CryptoMetrics crypto_metrics;
 
   // Crypto session metrics.
-  crypto_metrics.crypto_session_delete_all_usage_reports_
-      .Record(1.0, NO_ERROR);
+  crypto_metrics.crypto_session_delete_all_usage_reports_.Increment(NO_ERROR);
   crypto_metrics.crypto_session_delete_multiple_usage_information_
-      .Record(1.0, NO_ERROR);
+      .Increment(NO_ERROR);
   crypto_metrics.crypto_session_generic_decrypt_
       .Record(2.0, NO_ERROR, Pow2Bucket(1025), kEncryptionAlgorithmAesCbc128);
   crypto_metrics.crypto_session_generic_encrypt_
@@ -309,42 +297,39 @@ TEST_F(CryptoMetricsTest, AllCryptoMetrics) {
       .Record(2.0, NO_ERROR, Pow2Bucket(1025), kSigningAlgorithmHmacSha256);
   crypto_metrics.crypto_session_generic_verify_
       .Record(2.0, NO_ERROR, Pow2Bucket(1025), kSigningAlgorithmHmacSha256);
-  crypto_metrics.crypto_session_get_device_unique_id_.Record(1.0, true);
-  crypto_metrics.crypto_session_get_security_level_
-      .Record(1.0, kSecurityLevelL1);
-  crypto_metrics.crypto_session_get_system_id_.Record(1.0, true, 1234);
-  crypto_metrics.crypto_session_get_token_.Record(1.0, true);
+  crypto_metrics.crypto_session_get_device_unique_id_.Increment(true);
+  crypto_metrics.crypto_session_get_token_.Increment(true);
   crypto_metrics.crypto_session_life_span_.Record(1.0);
   crypto_metrics.crypto_session_load_certificate_private_key_
       .Record(1.0, true);
   crypto_metrics.crypto_session_open_.Record(1.0, NO_ERROR, kLevelDefault);
   crypto_metrics.crypto_session_update_usage_information_
       .Record(1.0, NO_ERROR);
-  crypto_metrics.crypto_session_usage_information_support_.Record(1.0, true);
+  crypto_metrics.crypto_session_usage_information_support_.Record(true);
+  crypto_metrics.crypto_session_security_level_.Record(kSecurityLevelL2);
 
   // Oem crypto metrics.
-  crypto_metrics.oemcrypto_api_version_.Record(1.0, 123, kLevelDefault);
+  crypto_metrics.oemcrypto_api_version_.Record(123);
   crypto_metrics.oemcrypto_close_session_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
+      .Increment(OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_copy_buffer_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED,
-              kLevelDefault, Pow2Bucket(1025));
+      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, Pow2Bucket(1025));
   crypto_metrics.oemcrypto_deactivate_usage_entry_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
+      .Increment(OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_decrypt_cenc_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, Pow2Bucket(1025));
   crypto_metrics.oemcrypto_delete_usage_entry_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
+      .Increment(OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_delete_usage_table_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
+      .Increment(OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_derive_keys_from_session_key_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_force_delete_usage_entry_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
+      .Increment(OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_generate_derived_keys_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_generate_nonce_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
+      .Increment(OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_generate_rsa_signature_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, Pow2Bucket(1025));
   crypto_metrics.oemcrypto_generate_signature_
@@ -358,87 +343,44 @@ TEST_F(CryptoMetricsTest, AllCryptoMetrics) {
   crypto_metrics.oemcrypto_generic_verify_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, Pow2Bucket(1025));
   crypto_metrics.oemcrypto_get_device_id_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, kLevelDefault);
-  crypto_metrics.oemcrypto_get_hdcp_capability_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, kLevelDefault);
+      .Increment(OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_get_key_data_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED,
-              Pow2Bucket(1025), kLevelDefault);
-  crypto_metrics.oemcrypto_get_max_number_of_sessions_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, kLevelDefault);
-  crypto_metrics.oemcrypto_get_number_of_open_sessions_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, kLevelDefault);
-  crypto_metrics.oemcrypto_get_oem_public_certificate_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
-
-  crypto_metrics.oemcrypto_get_provisioning_method_
-      .Record(1.0, OEMCrypto_Keybox, kLevelDefault);
-
-  crypto_metrics.oemcrypto_get_random_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, Pow2Bucket(1025));
+  crypto_metrics.oemcrypto_max_number_of_sessions_.Record(7);
+  crypto_metrics.oemcrypto_number_of_open_sessions_.Record(5);
+  crypto_metrics.oemcrypto_get_oem_public_certificate_
+      .Increment(OEMCrypto_ERROR_INIT_FAILED);
+  crypto_metrics.oemcrypto_provisioning_method_.Record(OEMCrypto_Keybox);
+  crypto_metrics.oemcrypto_get_random_
+      .Increment(OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_initialize_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
-  crypto_metrics.oemcrypto_install_keybox_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, kLevelDefault);
-  crypto_metrics.oemcrypto_is_anti_rollback_hw_present_
-      .Record(1.0, true, kLevelDefault);
-
-  crypto_metrics.oemcrypto_is_keybox_valid_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, kLevelDefault);
+  crypto_metrics.oemcrypto_is_anti_rollback_hw_present_.Record(true);
+  crypto_metrics.oemcrypto_is_keybox_valid_.Record(true);
   crypto_metrics.oemcrypto_load_device_rsa_key_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_load_keys_.Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
-  crypto_metrics.oemcrypto_load_test_keybox_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
-  crypto_metrics.oemcrypto_load_test_rsa_key_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
-  crypto_metrics.oemcrypto_open_session_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, kLevelDefault);
   crypto_metrics.oemcrypto_refresh_keys_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
-  crypto_metrics.oemcrypto_report_usage_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
+  crypto_metrics.oemcrypto_report_usage_.Increment(OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_rewrap_device_rsa_key_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
   crypto_metrics.oemcrypto_rewrap_device_rsa_key_30_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
-  crypto_metrics.oemcrypto_security_level_
-      .Record(1.0, kSecurityLevelL2, kLevelDefault);
-  crypto_metrics.oemcrypto_security_patch_level_
-      .Record(1.0, 123, kLevelDefault);
+  crypto_metrics.oemcrypto_security_patch_level_.Record(123);
   crypto_metrics.oemcrypto_select_key_
       .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
-  crypto_metrics.oemcrypto_supports_usage_table_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED, kLevelDefault);
   crypto_metrics.oemcrypto_update_usage_table_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
-  crypto_metrics.oemcrypto_wrap_keybox_
-      .Record(1.0, OEMCrypto_ERROR_INIT_FAILED);
+      .Increment(OEMCrypto_ERROR_INIT_FAILED);
 
-  // Internal OEMCrypto Metrics
-  crypto_metrics.oemcrypto_initialization_mode_
-      .Record(1.0, OEMCrypto_INITIALIZED_FORCING_L3);
-  crypto_metrics.oemcrypto_l1_api_version_.Record(1.0, 12, 123);
-
-  MetricsGroup actual_metrics;
+  WvCdmMetrics::CryptoMetrics actual_metrics;
   crypto_metrics.Serialize(&actual_metrics);
 
-  // 61 EventMetric instances, 2 values each.
-  ASSERT_EQ(122, actual_metrics.metric_size());
-
   // Spot check some metrics.
-  EXPECT_EQ(
-      "/drm/widevine/crypto_session/delete_all_usage_reports/count"
-      "{error:0}",
-      actual_metrics.metric(0).name());
-  EXPECT_EQ(1, actual_metrics.metric(0).value().int_value());
-  EXPECT_EQ(
-      "/drm/widevine/oemcrypto/l1_api_version/mean{version:12&min_version:123}",
-      actual_metrics.metric(121).name());
-  EXPECT_EQ(1.0, actual_metrics.metric(121).value().double_value());
-
-  // No subgroups should exist.
-  EXPECT_EQ(0, actual_metrics.metric_sub_group_size());
+  EXPECT_EQ(1, actual_metrics.crypto_session_delete_all_usage_reports_size());
+  EXPECT_EQ(1, actual_metrics.crypto_session_delete_all_usage_reports_size());
+  EXPECT_EQ(123, actual_metrics.oemcrypto_api_version().int_value());
+  EXPECT_EQ(1, actual_metrics.oemcrypto_generic_sign_time_us_size());
 }
 
 }  // namespace metrics

libwvdrmengine/cdm/test/Android.mk

@@ -88,6 +88,10 @@ test_src_dir := ../core/test
 test_main := ../core/test/test_main.cpp
 include $(LOCAL_PATH)/integration-test.mk
 
+test_name := metrics_collections_unittest
+test_src_dir := ../metrics/test
+include $(LOCAL_PATH)/unit-test.mk
+
 test_name := policy_engine_constraints_unittest
 test_src_dir := ../core/test
 test_main := ../core/test/test_main.cpp