From 77ef671c41c2e587f34e156834ffa35b135bc866 Mon Sep 17 00:00:00 2001
From: David Chen <dwchen@google.com>
Date: Fri, 23 Feb 2018 18:23:42 -0800
Subject: [PATCH] Updates jank metrics in statsd to include uid.

We need the uid to easily know which app to blame for producing the
frame with excessively long render time. Also updates the errors so
it's more obvious if the error is in parsing versus the other checks.

Test: Test that statsd builds and verified CTS test still passes.
Change-Id: Ib6518f2d9fe6f9c78d548b6dcbdb67a0f211ff5c
---
 cmds/statsd/src/atoms.proto                |  5 ++-
 cmds/statsd/src/metrics/MetricsManager.cpp | 42 ++++++++++++++++++----
 libs/hwui/JankTracker.cpp                  |  2 +-
 3 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/cmds/statsd/src/atoms.proto b/cmds/statsd/src/atoms.proto
index 2b02025f0a73c..9b01e4bbbdb20 100644
--- a/cmds/statsd/src/atoms.proto
+++ b/cmds/statsd/src/atoms.proto
@@ -763,8 +763,11 @@ message CallStateChanged {
  *   frameworks/base/libs/hwui/JankTracker.cpp
  */
 message DaveyOccurred {
+    // The UID that logged this atom.
+    optional int32 uid = 1;
+
     // Amount of time it took to render the frame. Should be >=700ms.
-    optional int64 jank_duration_millis = 1;
+    optional int64 jank_duration_millis = 2;
 }
 
 /**
diff --git a/cmds/statsd/src/metrics/MetricsManager.cpp b/cmds/statsd/src/metrics/MetricsManager.cpp
index e75b710cc9db9..fe61b99048251 100644
--- a/cmds/statsd/src/metrics/MetricsManager.cpp
+++ b/cmds/statsd/src/metrics/MetricsManager.cpp
@@ -198,31 +198,59 @@ void MetricsManager::onLogEvent(const LogEvent& event) {
         // Uid is 3rd from last field and must match the caller's uid,
         // unless that caller is statsd itself (statsd is allowed to spoof uids).
         long appHookUid = event.GetLong(event.size()-2, &err);
+        if (err != NO_ERROR ) {
+            VLOG("APP_BREADCRUMB_REPORTED had error when parsing the uid");
+            return;
+        }
         int32_t loggerUid = event.GetUid();
-        if (err != NO_ERROR || (loggerUid != appHookUid && loggerUid != AID_STATSD)) {
-            VLOG("AppHook has invalid uid: claimed %ld but caller is %d", appHookUid, loggerUid);
+        if (loggerUid != appHookUid && loggerUid != AID_STATSD) {
+            VLOG("APP_BREADCRUMB_REPORTED has invalid uid: claimed %ld but caller is %d",
+                 appHookUid, loggerUid);
             return;
         }
 
         // Label is 2nd from last field and must be from [0, 15].
         long appHookLabel = event.GetLong(event.size()-1, &err);
-        if (err != NO_ERROR || appHookLabel < 0 || appHookLabel > 15) {
-            VLOG("AppHook does not have valid label %ld", appHookLabel);
+        if (err != NO_ERROR ) {
+            VLOG("APP_BREADCRUMB_REPORTED had error when parsing the label field");
+            return;
+        } else if (appHookLabel < 0 || appHookLabel > 15) {
+            VLOG("APP_BREADCRUMB_REPORTED does not have valid label %ld", appHookLabel);
             return;
         }
 
         // The state must be from 0,3. This part of code must be manually updated.
         long appHookState = event.GetLong(event.size(), &err);
-        if (err != NO_ERROR || appHookState < 0 || appHookState > 3) {
-            VLOG("AppHook does not have valid state %ld", appHookState);
+        if (err != NO_ERROR ) {
+            VLOG("APP_BREADCRUMB_REPORTED had error when parsing the state field");
+            return;
+        } else if (appHookState < 0 || appHookState > 3) {
+            VLOG("APP_BREADCRUMB_REPORTED does not have valid state %ld", appHookState);
             return;
         }
     } else if (event.GetTagId() == android::util::DAVEY_OCCURRED) {
         // Daveys can be logged from any app since they are logged in libs/hwui/JankTracker.cpp.
         // Check that the davey duration is reasonable. Max length check is for privacy.
         status_t err = NO_ERROR;
+
+        // Uid is the first field provided.
+        long jankUid = event.GetLong(1, &err);
+        if (err != NO_ERROR ) {
+            VLOG("Davey occurred had error when parsing the uid");
+            return;
+        }
+        int32_t loggerUid = event.GetUid();
+        if (loggerUid != jankUid && loggerUid != AID_STATSD) {
+            VLOG("DAVEY_OCCURRED has invalid uid: claimed %ld but caller is %d", jankUid,
+                 loggerUid);
+            return;
+        }
+
         long duration = event.GetLong(event.size(), &err);
-        if (err != NO_ERROR || duration > 100000) {
+        if (err != NO_ERROR ) {
+            VLOG("Davey occurred had error when parsing the duration");
+            return;
+        } else if (duration > 100000) {
             VLOG("Davey duration is unreasonably long: %ld", duration);
             return;
         }
diff --git a/libs/hwui/JankTracker.cpp b/libs/hwui/JankTracker.cpp
index ab27a0d00246f..cf29e434a3515 100644
--- a/libs/hwui/JankTracker.cpp
+++ b/libs/hwui/JankTracker.cpp
@@ -165,7 +165,7 @@ void JankTracker::finishFrame(const FrameInfo& frame) {
         ALOGI("%s", ss.str().c_str());
         // Just so we have something that counts up, the value is largely irrelevant
         ATRACE_INT(ss.str().c_str(), ++sDaveyCount);
-        android::util::stats_write(android::util::DAVEY_OCCURRED, ns2ms(totalDuration));
+        android::util::stats_write(android::util::DAVEY_OCCURRED, getuid(), ns2ms(totalDuration));
     }
 }