Support GCF through a 'initialization completed promise' #341

gaofanmichael · 2017-10-16T23:38:33Z

This PR is to support Google Cloud Function (GCF) through initialization stackdriver debugger through a initialization of a promise and resolve the promise after breakpointfetch being scheduled.

src/agent/debuglet.ts

@@ -314,6 +319,14 @@ export class Debuglet extends EventEmitter {

    });
  }
+  initializationPromise() {


src/agent/debuglet.ts

+  initializationPromise() {
+    this.promiseInitialized_ = true;
+    return new Promise<void>((resolve) => {
+      this.promiseResolve_ = () => {


src/agent/debuglet.ts

@@ -115,6 +115,8 @@ export class Debuglet extends EventEmitter {
  private project_: string|null;
  private controller_: Controller;
  private completedBreakpointMap_: {[key: string]: boolean};
+  private promiseInitialized_: boolean;
+  private promiseResolve_: (value?: void|PromiseLike<void>|undefined) => void;


src/agent/debuglet.ts

@@ -117,6 +118,9 @@ export class Debuglet extends EventEmitter {
  private controller_: Controller;
  private completedBreakpointMap_: {[key: string]: boolean};

+  private promise_: Promise<void>|null;
+  private promiseResolve_: (value?: void|PromiseLike<void>|undefined) => void;


src/agent/debuglet.ts

@@ -322,6 +329,18 @@ export class Debuglet extends EventEmitter {

    });
  }
+  checkReady() {
+    const diff = process.hrtime(this.promiseResolvedTimestamp_);


src/agent/debuglet.ts

+    if (diff[0] < PROMISE_RESOLVE_CUT_OFF_IN_SECONDS) {
+      return Promise.resolve();
+    }
+    this.promise_ = new Promise<void>((resolve) => {


test/test-debuglet.ts

+      debuglet.once('registered', function reg(id: string) {
+        assert.equal(id, DEBUGGEE_ID);
+        debugPromise.then(() => {
+          setTimeout(function() {


src/agent/debuglet.ts

@@ -175,6 +179,9 @@ export class Debuglet extends EventEmitter {

    /** @private {Object.<string, Boolean>} */
    this.completedBreakpointMap_ = {};
+
+    /** @private {Array<number>} */


src/agent/debuglet.ts

@@ -175,6 +179,9 @@ export class Debuglet extends EventEmitter {

    /** @private {Object.<string, Boolean>} */
    this.completedBreakpointMap_ = {};
+
+    /** @private {Array<number>} */
+    this.promiseResolvedTimestamp_ = [0, 0];


src/agent/debuglet.ts

+// debug agent return a promise when checkReady is called. The value is selected
+// between Stackdriver debugger hanging get duration (40s) and TCP default
+// time-out (https://tools.ietf.org/html/rfc5482).
+const PROMISE_RESOLVE_CUT_OFF_IN_SECONDS = 60;


src/agent/debuglet.ts

@@ -174,6 +183,9 @@ export class Debuglet extends EventEmitter {

    /** @private {Object.<string, Boolean>} */
    this.completedBreakpointMap_ = {};
+
+    /** @private {Array<number>} */
+    this.promiseResolvedTimestamp_ = Date.now();


src/agent/debuglet.ts

+    if (this.promise_) return this.promise_;
+    const diff = Date.now() - this.promiseResolvedTimestamp_;
+    if (diff < PROMISE_RESOLVE_CUT_OFF_IN_SECONDS * 1000) {
+      return Promise.resolve();


src/agent/debuglet.ts

@@ -597,6 +622,11 @@ export class Debuglet extends EventEmitter {
                }
                that.scheduleBreakpointFetch_(
                    that.config_.breakpointUpdateIntervalSec);
+                if (that.promise_) {


src/agent/debuglet.ts

@@ -597,6 +622,11 @@ export class Debuglet extends EventEmitter {
                }
                that.scheduleBreakpointFetch_(
                    that.config_.breakpointUpdateIntervalSec);
+                if (that.promise_) {
+                  that.promiseResolve_();
+                  that.promise_ = null;


src/agent/debuglet.ts

@@ -175,6 +179,9 @@ export class Debuglet extends EventEmitter {

    /** @private {Object.<string, Boolean>} */
    this.completedBreakpointMap_ = {};
+
+    /** @private {Array<number>} */


src/agent/debuglet.ts

@@ -174,6 +183,9 @@ export class Debuglet extends EventEmitter {

    /** @private {Object.<string, Boolean>} */
    this.completedBreakpointMap_ = {};
+
+    /** @private {Array<number>} */
+    this.promiseResolvedTimestamp_ = Date.now();


src/agent/debuglet.ts

@@ -62,6 +62,12 @@ const BREAKPOINT_ACTION_MESSAGE =
    'The only currently supported breakpoint actions' +
    ' are CAPTURE and LOG.';

+// PROMISE_RESOLVE_CUT_OFF_IN_SECONDS is a heuristic that we set to force the


src/agent/debuglet.ts

@@ -62,6 +62,12 @@ const BREAKPOINT_ACTION_MESSAGE =
    'The only currently supported breakpoint actions' +
    ' are CAPTURE and LOG.';

+// PROMISE_RESOLVE_CUT_OFF_IN_SECONDS is a heuristic that we set to force the
+// debug agent return a promise when checkReady is called. The value is selected


src/agent/debuglet.ts

@@ -322,6 +333,17 @@ export class Debuglet extends EventEmitter {

    });
  }
+  checkReady() {
+    const diff = Date.now() - this.promiseResolvedTimestamp;
+    if (diff > PROMISE_RESOLVE_CUT_OFF_IN_MILLISECONDS) {


src/agent/debuglet.ts

@@ -114,6 +120,9 @@ export class Debuglet extends EventEmitter {
  private controller_: Controller;
  private completedBreakpointMap_: {[key: string]: boolean};

+  private promise: Promise<void>;


src/agent/debuglet.ts

+  private promiseResolve: (() => void)|null;
+  private promiseResolvedTimestamp: number;
+  constructor() {
+    this.promiseResolvedTimestamp = -Infinity;


src/agent/debuglet.ts

+  constructor() {
+    this.promiseResolvedTimestamp = -Infinity;
+  }
+  isReady() {


src/agent/debuglet.ts

+    return this.promise;
+  }
+
+  makeReady() {


src/agent/debuglet.ts

+ * old one if it steals. makeReady will resolve the promise at appropriate
+ * code location (in our case is after listActiveBreakpoints executed).
+ */
+export class IsReadyManager {


src/agent/debuglet.ts

+// isReadyManager. The value is selected between Stackdriver debugger hanging
+// get duration (40s) and TCP default time-out
+// (https://tools.ietf.org/html/rfc5482).
+const PROMISE_RESOLVE_CUT_OFF_IN_MILLISECONDS = 60 * 1000;


src/agent/debuglet.ts

+// PROMISE_RESOLVE_CUT_OFF_IN_MILLISECONDS is a heuristic that we set to force
+// the debug agent to return a promise when isReady is called in
+// isReadyManager. The value is selected between Stackdriver debugger hanging
+// get duration (40s) and TCP default time-out


src/agent/debuglet.ts

+  isReady() {
+    const diff = Date.now() - this.promiseResolvedTimestamp;
+    if (diff > PROMISE_RESOLVE_CUT_OFF_IN_MILLISECONDS) {
+      this.promise = new Promise<void>((resolve) => {


src/agent/debuglet.ts

+ * listActiveBreakpoints executed).
+ */
+export class CachedPromise {
+  private promise: Promise<void>;


src/agent/debuglet.ts

+/**
+ * CachedPromise class is designed to support debug agent in Google Cloud
+ * Function (GCF). As GCF is a serverless environment and we wanted to make
+ * sure debug agent could always capture the snapshots. We designed this class


src/agent/debuglet.ts

+ * CachedPromise class is designed to support debug agent in Google Cloud
+ * Function (GCF). As GCF is a serverless environment and we wanted to make
+ * sure debug agent could always capture the snapshots. We designed this class
+ * so that GCF could wait the ListActiveBreakpoitns event completed before


src/agent/debuglet.ts

+ * throttling CPU to 0. CachedPromise has two member functions.
+ * get() is to create or return an existing promise. resolve() will resolve
+ * the promise at appropriate code location (in our case is after
+ * listActiveBreakpoints executed).


src/agent/debuglet.ts

+      this.promise = new Promise<void>((resolve) => {
+        this.promiseResolve = () => {
+          resolve();
+        };


src/agent/debuglet.ts

@@ -114,6 +160,7 @@ export class Debuglet extends EventEmitter {
  private controller_: Controller;
  private completedBreakpointMap_: {[key: string]: boolean};

+  private cachedPromise: CachedPromise;


src/agent/debuglet.ts

@@ -322,6 +372,9 @@ export class Debuglet extends EventEmitter {

    });
  }
+  isReady() {


test/test-debuglet.ts

@@ -74,6 +74,19 @@ function verifyBreakpointRejection(re: RegExp, body: {breakpoint: any}) {
  const hasCorrectDescription = status.description.format.match(re);
  return status.isError && hasCorrectDescription;
 }
+describe('CachedPromise', function() {


test/test-debuglet.ts

@@ -74,6 +74,19 @@ function verifyBreakpointRejection(re: RegExp, body: {breakpoint: any}) {
  const hasCorrectDescription = status.description.format.match(re);
  return status.isError && hasCorrectDescription;
 }
+describe('CachedPromise', function() {
+  it('CachedPromise can proceed execute when resolve is called', function(done) {


test/test-debuglet.ts

+    this.timeout(2000);
+    let cachedPromise = new CachedPromise(60 * 1000);
+    let promise = cachedPromise.get();
+    setTimeout(function(){


jinwoo

LGTM. Not sure about other people's opinions.

src/agent/debuglet.ts

+      });
+    }
+    assert(this.promise);
+    return this.promise as Promise<void>;


kjin

Logic looks good... left a few comments on naming and comments.

src/agent/debuglet.ts

+  private promise: Promise<void>|undefined;
+  private promiseResolve: (() => void)|null;
+  private promiseResolvedTimestamp = -Infinity;
+  private refresh: number;


src/agent/debuglet.ts

+  private promiseResolvedTimestamp = -Infinity;
+  private refresh: number;
+  constructor(refresh: number) {
+    this.refresh = refresh;


src/agent/debuglet.ts

@@ -106,6 +112,44 @@ const formatBreakpoints = function(
          .join('\n');
 };

+/**
+ * CachedPromise class is designed to support debug agent in Google Cloud


src/debuggee.ts

-  private description: string;
-  private agentVersion?: string;
-  private sourceContexts?: Array<{[key: string]: any}>;
+  uniquifier: string;


test/test-debuglet.ts

+describe('CachedPromise', function() {
+  it('CachedPromise can proceed execute when resolve is called', (done) => {
+    this.timeout(2000);
+    let cachedPromise = new CachedPromise(60 * 1000);


request changes were made

test/test-debuglet.ts

@@ -87,6 +87,28 @@ describe('CachedPromise', function() {
      done();
    });
  });
+
+  it('CachedPromise should return the same promise if not staled', (done) => {


src/agent/debuglet.ts

- * listActiveBreakpoints is executed).
+ * CachedPromise stores a promise for a limited time. Its member function get()
+ * will initially create a promise, or create a promise when previous promise
+ * stales, then return that promise. resolve() will resolve the stored promise.
 */
 export class CachedPromise {


src/agent/debuglet.ts

@@ -368,6 +365,15 @@ export class Debuglet extends EventEmitter {

    });
  }
+
+  /**
+   * isReady is designed to support debug agent on Google Cloud Function (GCF).


kjin

Looks good on my end. w/ nits

test/test-debuglet.ts

@@ -75,6 +75,53 @@ function verifyBreakpointRejection(re: RegExp, body: {breakpoint: any}) {
  return status.isError && hasCorrectDescription;
 }

+describe('CachedPromise', function() {
+  it('CachedPromise can proceed execute when resolve is called', (done) => {


src/agent/debuglet.ts

@@ -324,6 +367,18 @@ export class Debuglet extends EventEmitter {
  }

  /**
+   * isReady is designed to support debug agent on Google Cloud Function (GCF).


gaofanmichael · 2017-11-07T18:33:23Z

Opened another PR #358
Close this one.

gaofanmichael requested review from ofrobots, kjin and DominicKramer October 16, 2017 23:38

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 16, 2017

jmdobry added the in progress label Oct 16, 2017

kjin reviewed Oct 18, 2017

View reviewed changes

gaofanmichael force-pushed the gcf branch 3 times, most recently from 31e0cb2 to aac1617 Compare October 25, 2017 21:49

kjin previously requested changes Oct 25, 2017

View reviewed changes

jinwoo reviewed Oct 25, 2017

View reviewed changes

gaofanmichael force-pushed the gcf branch from e0360b4 to affd558 Compare October 26, 2017 00:04

kjin reviewed Oct 27, 2017

View reviewed changes

jinwoo reviewed Oct 27, 2017

View reviewed changes

DominicKramer reviewed Oct 27, 2017

View reviewed changes

jinwoo reviewed Oct 27, 2017

View reviewed changes

ofrobots reviewed Oct 28, 2017

View reviewed changes

jinwoo reviewed Oct 31, 2017

View reviewed changes

gaofanmichael force-pushed the gcf branch from cf31750 to afdc565 Compare October 31, 2017 21:10

jinwoo reviewed Oct 31, 2017

View reviewed changes

src/agent/debuglet.ts Outdated

});

}

assert(this.promise);

return this.promise as Promise<void>;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

kjin reviewed Nov 1, 2017

View reviewed changes

gaofanmichael force-pushed the gcf branch from 3f89e95 to 28bd0b7 Compare November 2, 2017 02:48

kjin reviewed Nov 2, 2017

View reviewed changes

kjin approved these changes Nov 2, 2017

View reviewed changes

gaofanmichael added 4 commits November 2, 2017 14:11

implementation

af29230

modify test

1e6ef73

Change promise resolving location

893e910

change function name

ef84619

gaofanmichael added 11 commits November 2, 2017 14:11

Add time difference

a052153

change process.hrtime to Date.now

28e13fb

fix test

64d041f

Revise promiseResolvedTimestamp

4e524da

address comments

da81e14

make IsReadyManager class

e6b58cd

rename to CachedPromise

e3070a4

make some debuggee properties public

fecc998

address comments

aa4eb98

resolve comments etc

1f85485

revise tests

e13d87f

gaofanmichael force-pushed the gcf branch from c4f7727 to e13d87f Compare November 2, 2017 21:11

gaofanmichael closed this Nov 7, 2017

jmdobry removed the in progress label Nov 7, 2017

kjin mentioned this pull request Nov 7, 2017

feat: Enable isReady for debugger for GCF #358

Merged

Support GCF through a 'initialization completed promise' #341

Support GCF through a 'initialization completed promise' #341

Conversation

gaofanmichael commented Oct 16, 2017 • edited Loading

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jinwoo left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

gaofanmichael commented Oct 16, 2017 •

edited

Loading