Merge pull request #13 from adam-p/lite-diagnostics

Add a "lite" diagnostic dump for more frequent logging
Psiphon-Inc · Aug 24, 2021 · 7b843a5 · 7b843a5
2 parents 8e0441b + f0c4289
commit 7b843a5
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 7 deletions.
diff --git a/psicash.cpp b/psicash.cpp
@@ -477,7 +477,7 @@ Result<string> PsiCash::GetRewardedActivityData() const {
     return json_data;
 }
 
-json PsiCash::GetDiagnosticInfo() const {
+json PsiCash::GetDiagnosticInfo(bool lite) const {
     // NOTE: Do not put personal identifiers in this package.
     // TODO: This is still enough info to uniquely identify the user (combined with the
     // PsiCash DB). So maybe avoiding direct PII does not achieve anything, and we should
@@ -492,7 +492,6 @@ json PsiCash::GetDiagnosticInfo() const {
     j["isAccount"] = IsAccount();
     j["balance"] = Balance();
     j["serverTimeDiff"] = user_data_->GetServerTimeDiff().count(); // in milliseconds
-    j["purchasePrices"] = GetPurchasePrices();
 
     // Include a sanitized version of the purchases
     j["purchases"] = json::array();
@@ -501,6 +500,12 @@ json PsiCash::GetDiagnosticInfo() const {
                                   {"distinguisher", p.distinguisher}});
     }
 
+    // The purchase prices are about 800 bytes of the 1000 bytes in a typical diangnostic
+    // dump, and they're generally not useful.
+    if (!lite) {
+        j["purchasePrices"] = GetPurchasePrices();
+    }
+
     return j;
 }
 

diff --git a/psicash.hpp b/psicash.hpp
@@ -287,9 +287,12 @@ class PsiCash {
     error::Result<std::string> GetRewardedActivityData() const;
 
     // TODO: This return value might be a problem for direct C++ consumers (vs glue).
+    /// If `lite` is true, the diagnostic info will be smaller -- on the order of 200 bytes.
+    /// If `lite` false, the diagnostic info will be larger -- on the order of 1k bytes.
+    /// The smaller package is suitable for more frequent logging.
     /// Returns a JSON object suitable for serializing that can be included in a
     /// feedback diagnostic data package.
-    nlohmann::json GetDiagnosticInfo() const;
+    nlohmann::json GetDiagnosticInfo(bool lite) const;
 
     //
     // API Server Requests

diff --git a/psicash_test.cpp b/psicash_test.cpp
@@ -1101,7 +1101,19 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         "test":false,
         "validTokenTypes":[]
         })|"_json;
-        auto j = pc.GetDiagnosticInfo();
+        auto j = pc.GetDiagnosticInfo(/*lite=*/false);
+        ASSERT_EQ(j, want);
+
+        want = R"|({
+        "balance":0,
+        "isAccount":false,
+        "isLoggedOutAccount":false,
+        "purchases":[],
+        "serverTimeDiff":0,
+        "test":false,
+        "validTokenTypes":[]
+        })|"_json;
+        j = pc.GetDiagnosticInfo(/*lite=*/true);
         ASSERT_EQ(j, want);
     }
 
@@ -1121,7 +1133,19 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         "test":true,
         "validTokenTypes":[]
         })|"_json;
-        auto j = pc.GetDiagnosticInfo();
+        auto j = pc.GetDiagnosticInfo(/*lite=*/false);
+        ASSERT_EQ(j, want);
+
+        want = R"|({
+        "balance":0,
+        "isAccount":false,
+        "isLoggedOutAccount":false,
+        "purchases":[],
+        "serverTimeDiff":0,
+        "test":true,
+        "validTokenTypes":[]
+        })|"_json;
+        j = pc.GetDiagnosticInfo(/*lite=*/true);
         ASSERT_EQ(j, want);
 
         pc.user_data().SetBalance(12345);
@@ -1140,7 +1164,18 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         "test":true,
         "validTokenTypes":["a","b","c"]
         })|"_json;
-        j = pc.GetDiagnosticInfo();
+        j = pc.GetDiagnosticInfo(/*lite=*/false);
+        ASSERT_EQ(j, want);
+        want = R"|({
+        "balance":12345,
+        "isAccount":true,
+        "isLoggedOutAccount":false,
+        "purchases":[{"class":"tc2","distinguisher":"d2"}],
+        "serverTimeDiff":0,
+        "test":true,
+        "validTokenTypes":["a","b","c"]
+        })|"_json;
+        j = pc.GetDiagnosticInfo(/*lite=*/true);
         ASSERT_EQ(j, want);
 
         pc.user_data().DeleteUserData(/*is_logged_out_account=*/true);
@@ -1154,7 +1189,18 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         "test":true,
         "validTokenTypes":[]
         })|"_json;
-        j = pc.GetDiagnosticInfo();
+        j = pc.GetDiagnosticInfo(/*lite=*/false);
+        ASSERT_EQ(j, want);
+        want = R"|({
+        "balance":0,
+        "isAccount":true,
+        "isLoggedOutAccount":true,
+        "purchases":[],
+        "serverTimeDiff":0,
+        "test":true,
+        "validTokenTypes":[]
+        })|"_json;
+        j = pc.GetDiagnosticInfo(/*lite=*/true);
         ASSERT_EQ(j, want);
     }
 }