fix: Breakpoint expiry now uses create time (#71)

Updated the internal agent code and breakpoint json model to support
the `createTimeUnixMsec` field which is used in the Firebase Backend
version of the agent. When determining the breakpoint expiry this field
will now be used.

Fixes #66

Author: jasonborg

src/agent/jvm_breakpoint.cc

@@ -205,12 +205,12 @@ void JvmBreakpoint::Initialize() {
 
   // Schedule breakpoint cancellation.
   time_t expiration_time_base;
-  if (definition_->create_time == kUnspecifiedTimestamp) {
+  if (model_util::GetCreateTimestamp(*definition_) == kUnspecifiedTimestamp) {
     // It really shouldn't happen, but if it does start computing expiration
     // time from this moment.
     expiration_time_base = scheduler_->CurrentTime();
   } else {
-    expiration_time_base = definition_->create_time.seconds;
+    expiration_time_base = model_util::GetCreateTimestamp(*definition_).seconds;
   }
 
   int32_t expiration_seconds = absl::GetFlag(FLAGS_breakpoint_expiration_sec);

src/agent/model.h

@@ -144,7 +144,16 @@ struct BreakpointModel {
   std::string log_message_format;
   LogLevel log_level = LogLevel::INFO;
   bool is_final_state = false;
+
+  // The Cloud Debugger GCP Backend version of the service uses a field called
+  // 'create_time' to represent the breakpoint creation time. The OSS Snapshot
+  // Debugger Firebase Backend version of the service uses a field called
+  // 'create_time_unix_msec'. For simplicity when serializing we treat them
+  // seperately and they will either get serialized or not depending on if they
+  // have a real value or not.
   TimestampModel create_time;
+  TimestampModel create_time_unix_msec;
+
   std::unique_ptr<StatusMessageModel> status;
   std::vector<std::unique_ptr<StackFrameModel>> stack;
   std::vector<std::unique_ptr<VariableModel>> evaluated_expressions;

src/agent/model_json.cc

@@ -270,6 +270,11 @@ static void SerializeTimestamp(
   (*root) = Json::Value(value);
 }
 
+static void SerializeTimestampUnixMsec(const TimestampModel& model,
+                                       Json::Value* root) {
+  int64_t total_millis = model.seconds * 1000 + model.nanos / (1000 * 1000);
+  (*root) = Json::Value(total_millis);
+}
 
 static void SerializeModel(
     const StatusMessageModel& model,
@@ -374,6 +379,23 @@ TimestampModel DeserializeTimestamp(const Json::Value& root) {
   return timestamp;
 }
 
+TimestampModel DeserializeTimestampUnixMsec(const Json::Value& root) {
+  if (!root.isInt64()) {
+    return kUnspecifiedTimestamp;
+  }
+
+  int64_t total_millis = root.asInt64();
+
+  if (total_millis == 0) {
+    return kUnspecifiedTimestamp;
+  }
+
+  TimestampModel timestamp;
+  timestamp.seconds = total_millis / 1000;
+  timestamp.nanos = (total_millis % 1000) * 1000 * 1000;
+
+  return timestamp;
+}
 
 template <>
 std::unique_ptr<StatusMessageModel> DeserializeModel<StatusMessageModel>(
@@ -558,6 +580,11 @@ static void SerializeModel(
     SerializeTimestamp(model.create_time, &(*root)["createTime"]);
   }
 
+  if (model.create_time_unix_msec != kUnspecifiedTimestamp) {
+    SerializeTimestampUnixMsec(model.create_time_unix_msec,
+                               &(*root)["createTimeUnixMsec"]);
+  }
+
   if (model.status != nullptr) {
     SerializeModel(*model.status, &(*root)["status"]);
   }
@@ -611,9 +638,16 @@ std::unique_ptr<BreakpointModel> DeserializeModel<BreakpointModel>(
   // Final state flag.
   model->is_final_state = JsonCppGetBool(root, "isFinalState", false);
 
-  // Breakpoint creation time.
+  // Breakpoint creation time. This field is from the Cloud Debugger GCP Backend
+  // version of the service, and is a string in RFC3339 format.
   model->create_time = DeserializeTimestamp(root["createTime"]);
 
+  // Breakpoint creation time. This field is from the OSS Snapshot Debugger
+  // Firebase Backend version of the service, and is an integer representing
+  // unix epoch time in milliseconds.
+  model->create_time_unix_msec =
+      DeserializeTimestampUnixMsec(root["createTimeUnixMsec"]);
+
   // Breakpoint status.
   if (root.isMember("status")) {
     model->status = DeserializeModel<StatusMessageModel>(root["status"]);

src/agent/model_util.h

@@ -418,6 +418,7 @@ class BreakpointBuilder {
     set_is_final_state(source.is_final_state);
 
     set_create_time(source.create_time);
+    set_create_time_unix_msec(source.create_time_unix_msec);
 
     if (source.status != nullptr) {
       set_status(StatusMessageBuilder(*source.status).build());
@@ -501,6 +502,11 @@ class BreakpointBuilder {
     return *this;
   }
 
+  BreakpointBuilder& set_create_time_unix_msec(TimestampModel timestamp) {
+    data_->create_time_unix_msec = timestamp;
+    return *this;
+  }
+
   BreakpointBuilder& clear_status() {
     return set_status(nullptr);
   }
@@ -660,7 +666,6 @@ class ErrorOr {
   FormatMessageModel error_message_;
 };
 
-
 inline bool operator== (const TimestampModel& t1, const TimestampModel& t2) {
   return (t1.seconds == t2.seconds) && (t1.nanos == t2.nanos);
 }
@@ -670,6 +675,16 @@ inline bool operator!= (const TimestampModel& t1, const TimestampModel& t2) {
   return !(t1 == t2);
 }
 
+namespace model_util {
+
+inline TimestampModel GetCreateTimestamp(const BreakpointModel& model) {
+  return (model.create_time != kUnspecifiedTimestamp)
+             ? model.create_time
+             : model.create_time_unix_msec;
+}
+
+}  // namespace model_util
+
 inline bool operator== (const DurationModel& d1, const DurationModel& d2) {
   return (d1.seconds == d2.seconds) && (d1.nanos == d2.nanos);
 }

tests/agent/jvm_breakpoint_test.cc

@@ -575,6 +575,19 @@ TEST_F(JvmBreakpointTest, BreakpointExpirationWithCreatedTime) {
   scheduler_.Process();
 }
 
+TEST_F(JvmBreakpointTest, BreakpointExpirationWithCreatedTimeUnixMsec) {
+  Create(BreakpointBuilder(*breakpoint_template_)
+             .set_create_time_unix_msec(
+                 TimestampBuilder::Build(simulated_time_sec_))
+             .build());
+
+  EXPECT_CALL(breakpoints_manager_, CompleteBreakpoint("test_breakpoint_id"))
+      .WillOnce(Return());
+
+  simulated_time_sec_ += absl::GetFlag(FLAGS_breakpoint_expiration_sec);
+  scheduler_.Process();
+}
+
 TEST_F(JvmBreakpointTest, BreakpointExpirationNoCreatedTime) {
   Create(BreakpointBuilder(*breakpoint_template_).build());
 

tests/agent/model_json_test.cc

@@ -136,6 +136,7 @@ static bool IsEqual(
          (model1.log_level == model2.log_level) &&
          (model1.is_final_state == model2.is_final_state) &&
          (model1.create_time == model2.create_time) &&
+         (model1.create_time_unix_msec == model2.create_time_unix_msec) &&
          IsEqual(model1.status, model2.status) &&
          IsEqual(model1.stack, model2.stack) &&
          IsEqual(model1.evaluated_expressions, model2.evaluated_expressions) &&
@@ -388,6 +389,19 @@ TEST_F(ModelJsonTest, BreakpointCreateTime) {
 }
 
 
+TEST_F(ModelJsonTest, BreakpointCreateTimeUnixMsec) {
+  std::unique_ptr<BreakpointModel> breakpoint = CreateFullBreakpoint();
+
+  breakpoint->create_time_unix_msec.seconds = 1444163838L;
+  breakpoint->create_time_unix_msec.nanos = 893000000;
+  SerializationLoop(*breakpoint);
+
+  breakpoint->create_time_unix_msec.seconds = 3489578;
+  breakpoint->create_time_unix_msec.nanos = TimestampModel().nanos;
+  SerializationLoop(*breakpoint);
+}
+
+
 TEST_F(ModelJsonTest, UserId_Empty) {
   std::unique_ptr<BreakpointModel> breakpoint = CreateFullBreakpoint();
   breakpoint->evaluated_user_id.reset(new UserIdModel());

tests/agent/model_util_test.cc

@@ -26,6 +26,8 @@
 namespace devtools {
 namespace cdbg {
 
+using testing::Eq;
+
 class ModelUtilTest : public ::testing::Test {
  protected:
   void SetUp() override {
@@ -440,6 +442,50 @@ TEST_F(ModelUtilTest, BreakpointCreateTime) {
 }
 
 
+TEST_F(ModelUtilTest, BreakpointCreateTimeUnixMsec) {
+  TimestampModel test_timestamp;
+  test_timestamp.seconds = 1444163838L;
+  test_timestamp.nanos = 123456789;  // Internal precision is milliseconds.
+                                     // Will be truncated to '123'.
+  CheckBuilder(
+      "{"
+      "   'id' : 'A'"
+      "}",
+      BreakpointBuilder().set_id("A").set_create_time_unix_msec(
+          kUnspecifiedTimestamp));
+
+  CheckBuilder(
+      "{"
+      "   'id' : 'A',"
+      "   'createTimeUnixMsec' : 1444163838123"
+      "}",
+      BreakpointBuilder().set_id("A").set_create_time_unix_msec(
+          test_timestamp));
+}
+
+TEST_F(ModelUtilTest, GetCreateTimestamp) {
+  TimestampModel test_timestamp;
+  test_timestamp.seconds = 1444163838L;
+  test_timestamp.nanos = 123456789;
+
+  auto bp1 = BreakpointBuilder()
+                 .set_create_time(test_timestamp)
+                 .set_create_time_unix_msec(kUnspecifiedTimestamp)
+                 .build();
+  auto bp2 = BreakpointBuilder()
+                 .set_create_time(kUnspecifiedTimestamp)
+                 .set_create_time_unix_msec(test_timestamp)
+                 .build();
+  auto bp3 = BreakpointBuilder()
+                 .set_create_time(kUnspecifiedTimestamp)
+                 .set_create_time_unix_msec(kUnspecifiedTimestamp)
+                 .build();
+
+  EXPECT_THAT(model_util::GetCreateTimestamp(*bp1), Eq(test_timestamp));
+  EXPECT_THAT(model_util::GetCreateTimestamp(*bp2), Eq(test_timestamp));
+  EXPECT_THAT(model_util::GetCreateTimestamp(*bp3), Eq(kUnspecifiedTimestamp));
+}
+
 TEST_F(ModelUtilTest, SetBreakpointLabels) {
   std::map<std::string, std::string> labels;
   labels["key1"] = "value1";