Repository: elastic/synthetics
Pull request #724 (2023-03-24 17:45:16)

feat: consolidate schedule options for project monitors
Relates to https://github.com/elastic/kibana/issues/142653

apmmachine: 
## :broken_heart: Build Failed


<!-- BUILD BADGES-->
> _the below badges are clickable and redirect to their specific view in the CI or DOCS_
[![Pipeline View](https://img.shields.io/badge/pipeline-pipeline%20-green)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-724/detail/PR-724/1//pipeline) [![Test View](https://img.shields.io/badge/test-test-green)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-724/detail/PR-724/1//tests) [![Changes](https://img.shields.io/badge/changes-changes-green)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-724/detail/PR-724/1//changes) [![Artifacts](https://img.shields.io/badge/artifacts-artifacts-yellow)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-724/detail/PR-724/1//artifacts) [![preview](https://img.shields.io/badge/docs-preview-yellowgreen)](http://synthetics_724.docs-preview.app.elstc.co/diff)  [![preview](https://img.shields.io/badge/elastic-observability-blue)](https://ci-stats.elastic.co/app/apm/services/apm-ci/transactions/view?rangeFrom=2023-03-24T17:35:30.630Z&rangeTo=2023-03-24T17:55:30.630Z&transactionName=BUILD+apm-agent-rum%2Felastic-synthetics%2FPR-%7Bnumber%7D&transactionType=job&latencyAggregationType=avg&traceId=8401f305d788d4747f44e90bc51daab6&transactionId=fd6bd477541289ed)

<!-- BUILD SUMMARY-->
<details><summary>Expand to view the summary</summary>
<p>

#### Build stats




* Start Time: 2023-03-24T17:45:30.630+0000


* Duration: 5 min 5 sec





</p>
</details>

<!-- TEST RESULTS IF ANY-->


<!-- STEPS ERRORS IF ANY -->














### Steps errors [![2](https://img.shields.io/badge/2%20-red)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-724/detail/PR-724/1//pipeline)
  <details><summary>Expand to view the steps failures</summary>
  <p>


  
  
  
  
##### `Install precommit hooks`
<ul>
<li>Took 0 min 1 sec . View more details <a href="https://apm-ci.elastic.co//blue/rest/organizations/jenkins/pipelines/apm-agent-rum/pipelines/elastic-synthetics/pipelines/PR-724/runs/1/steps/229/log/?start=0">here</a></li>
<li>Description: <code>          ## Install with the hooks therefore ~/.cache/pre-commit will be created with the repos          pre-commit install --install-hooks        </code></l1>
</ul>
  
  
  
  
##### `Install precommit hooks`
<ul>
<li>Took 0 min 1 sec . View more details <a href="https://apm-ci.elastic.co//blue/rest/organizations/jenkins/pipelines/apm-agent-rum/pipelines/elastic-synthetics/pipelines/PR-724/runs/1/steps/233/log/?start=0">here</a></li>
<li>Description: <code>          ## Install with the hooks therefore ~/.cache/pre-commit will be created with the repos          pre-commit install --install-hooks        </code></l1>
</ul>
  
  </p>
  </details>



## :robot: GitHub comments

<details><summary>Expand to view the GitHub comments</summary>
<p>

To re-run your PR in the CI, just comment with:



- `/test` : Re-trigger the build.




</p>
</details>

<!--COMMENT_GENERATED_WITH_ID_comment.id-->

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 1da7f16..c9a5383 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,6 +1,6 @@
 repos:
--   repo: git@github.com:pre-commit/pre-commit-hooks
-    rev: v2.2.3
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.4.0
     hooks:
     -   id: check-case-conflict
     -   id: check-executables-have-shebangs
diff --git a/package-lock.json b/package-lock.json
index 68540fc..3459b73 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
 {
   "name": "@elastic/synthetics",
-  "version": "1.0.0-beta.42",
+  "version": "1.0.0-beta.43",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "@elastic/synthetics",
-      "version": "1.0.0-beta.42",
+      "version": "1.0.0-beta.43",
       "license": "MIT",
       "dependencies": {
         "archiver": "^5.3.1",
diff --git a/package.json b/package.json
index b528a86..ebef00d 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@elastic/synthetics",
-  "version": "1.0.0-beta.42",
+  "version": "1.0.0-beta.43",
   "description": "Elastic synthetic monitoring agent",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",
diff --git a/src/core/gatherer.ts b/src/core/gatherer.ts
index 958a7c0..f78a2e2 100644
--- a/src/core/gatherer.ts
+++ b/src/core/gatherer.ts
@@ -72,6 +72,12 @@ export class Gatherer {
     const page = await context.newPage();
     const client = await context.newCDPSession(page);
     const request = await apiRequest.newContext({ ...playwrightOptions });
+
+    // Register sig int handler to close the browser
+    process.on('SIGINT', async () => {
+      await Gatherer.closeBrowser();
+      process.exit(130);
+    });
     return { browser: Gatherer.browser, context, page, client, request };
   }
 
@@ -108,6 +114,14 @@ export class Gatherer {
     }
   }
 
+  static async closeBrowser() {
+    log(`Gatherer: closing browser`);
+    if (Gatherer.browser) {
+      await Gatherer.browser.close();
+      Gatherer.browser = null;
+    }
+  }
+
   /**
    * Starts recording all events related to the v8 devtools protocol
    * https://chromedevtools.github.io/devtools-protocol/v8/
@@ -131,10 +145,6 @@ export class Gatherer {
   }
 
   static async stop() {
-    log(`Gatherer: closing browser`);
-    if (Gatherer.browser) {
-      await Gatherer.browser.close();
-      Gatherer.browser = null;
-    }
+    await Gatherer.closeBrowser();
   }
 }
diff --git a/src/core/index.ts b/src/core/index.ts
index ce806e6..b6bb81c 100644
--- a/src/core/index.ts
+++ b/src/core/index.ts
@@ -47,7 +47,7 @@ export const journey = wrapFnWithLocation(
     options: JourneyOptions | string,
     callback: JourneyCallback
   ) => {
-    log(`register journey: ${JSON.stringify(options)}`);
+    log(`Journey register: ${JSON.stringify(options)}`);
     if (typeof options === 'string') {
       options = { name: options, id: options };
     }
@@ -59,7 +59,7 @@ export const journey = wrapFnWithLocation(
 
 export const step = wrapFnWithLocation(
   (location: Location, name: string, callback: VoidCallback) => {
-    log(`register step: ${name}`);
+    log(`Step register: ${name}`);
     return runner.currentJourney?.addStep(name, callback, location);
   }
 );
diff --git a/src/core/logger.ts b/src/core/logger.ts
index d123ef2..bb28e4d 100644
--- a/src/core/logger.ts
+++ b/src/core/logger.ts
@@ -41,5 +41,5 @@ export function log(msg) {
     msg = JSON.stringify(msg);
   }
   const time = dim(cyan(`at ${parseInt(String(now()))} ms `));
-  console.log(time + italic(grey(msg)));
+  process.stderr.write(time + italic(grey(msg)) + '\n');
 }
diff --git a/src/dsl/monitor.ts b/src/dsl/monitor.ts
index e60cfb5..bb61413 100644
--- a/src/dsl/monitor.ts
+++ b/src/dsl/monitor.ts
@@ -40,7 +40,7 @@ export type SyntheticsLocationsType = keyof typeof LocationsMap;
 export const SyntheticsLocations = Object.keys(
   LocationsMap
 ) as SyntheticsLocationsType[];
-export const ALLOWED_SCHEDULES = [1, 3, 5, 10, 15, 30, 60] as const;
+export const ALLOWED_SCHEDULES = [1, 3, 5, 10, 15, 20, 30, 60, 120, 240] as const;
 
 export type MonitorConfig = {
   id?: string;
diff --git a/src/plugins/network.ts b/src/plugins/network.ts
index e21e8e5..c1fd53f 100644
--- a/src/plugins/network.ts
+++ b/src/plugins/network.ts
@@ -180,31 +180,25 @@ export class NetworkManager {
           networkEntry.response.mimeType = contentType.split(';')[0];
       })
     );
+    this._addBarrier(
+      page,
+      response.serverAddr().then(server => {
+        networkEntry.response.remoteIPAddress = server?.ipAddress;
+        networkEntry.response.remotePort = server?.port;
+      })
+    );
+    this._addBarrier(
+      page,
+      response.securityDetails().then(details => {
+        if (details) networkEntry.response.securityDetails = details;
+      })
+    );
   }
 
   private async _onRequestCompleted(request: Request) {
     const networkEntry = this._findNetworkEntry(request);
     if (!networkEntry) return;
 
-    const page = request.frame().page();
-    const response = await request.response();
-    // response can be null if the request failed
-    if (response != null) {
-      this._addBarrier(
-        page,
-        response.serverAddr().then(server => {
-          networkEntry.response.remoteIPAddress = server?.ipAddress;
-          networkEntry.response.remotePort = server?.port;
-        })
-      );
-      this._addBarrier(
-        page,
-        response.securityDetails().then(details => {
-          if (details) networkEntry.response.securityDetails = details;
-        })
-      );
-    }
-
     networkEntry.loadEndTime = epochTimeInSeconds();
     const timing = request.timing();
     const { loadEndTime, requestSentTime } = networkEntry;
@@ -281,6 +275,7 @@ export class NetworkManager {
       total,
     };
 
+    const page = request.frame().page();
     // For aborted/failed requests sizes will not be present
     this._addBarrier(
       page,
@@ -301,7 +296,14 @@ export class NetworkManager {
   }
 
   async stop() {
-    await Promise.all(this._barrierPromises);
+    /**
+     * Waiting for all network events is error prone and might hang the tests
+     * from getting closed forever when there are upstream bugs in browsers or
+     * Playwright. So we log and drop these events once the test run is completed
+     */
+    if (this._barrierPromises.size > 0) {
+      log(`Plugins: dropping ${this._barrierPromises.size} network events}`);
+    }
     const context = this.driver.context;
     context.off('request', this._onRequest.bind(this));
     context.off('response', this._onResponse.bind(this));
diff --git a/src/push/monitor.ts b/src/push/monitor.ts
index dd71b6a..fc80da2 100644
--- a/src/push/monitor.ts
+++ b/src/push/monitor.ts
@@ -260,7 +260,7 @@ export function parseSchedule(schedule: string) {
     const scheduleValue = parseInt(value, 10);
     switch (format) {
       case 's':
-        minutes += Math.ceil(scheduleValue / 60);
+        minutes += Math.floor(scheduleValue / 60) || 1;
         break;
       case 'm':
         minutes += scheduleValue;


Test patch:
diff --git a/__tests__/plugins/network.test.ts b/__tests__/plugins/network.test.ts
index a3f0e6b..f1281fe 100644
--- a/__tests__/plugins/network.test.ts
+++ b/__tests__/plugins/network.test.ts
@@ -311,15 +311,7 @@ describe('network', () => {
       res.writeHead(200, {
         'Content-Type': 'text/html',
       });
-      res.end(`
-        <script>
-      var delayedPromise = new Promise(r => setTimeout(r, 300));
-      setTimeout(() => {
-        var x = new XMLHttpRequest();
-        x.open("POST", "chunked.txt");
-        x.send(null);
-      }, 0);
-      </script>`);
+      res.end('');
     });
     server.route('/chunked.txt', async (req, res) => {
       res.writeHead(200, {
@@ -330,11 +322,16 @@ describe('network', () => {
       // response is never ended
     });
 
-    driver.page.on('console', (msg: any) => {
-      console.log('console', msg.text());
-    });
     await driver.page.goto(server.PREFIX + '/index');
-    await driver.page.evaluate(() => (window as any).delayedPromise);
+    await Promise.all([
+      driver.page.evaluate(() => {
+        // hangs forever
+        const x = new XMLHttpRequest();
+        x.open('GET', 'chunked.txt', true);
+        x.send();
+      }, [server.PREFIX + 'chunked.txt']),
+      delay(1000),
+    ]);
     const netinfo = await network.stop();
     await Gatherer.stop();
     expect(netinfo.length).toBe(2);
diff --git a/__tests__/push/__snapshots__/index.test.ts.snap b/__tests__/push/__snapshots__/index.test.ts.snap
index 25c1512..1ec22f7 100644
--- a/__tests__/push/__snapshots__/index.test.ts.snap
+++ b/__tests__/push/__snapshots__/index.test.ts.snap
@@ -25,7 +25,7 @@ Run 'npx @elastic/synthetics init' to create project with default settings.
 exports[`Push error on invalid schedule 1`] = `
 "Aborted. Invalid synthetics project settings.
 
-Set default schedule(12) to one of the allowed values - 1,3,5,10,15,30,60
+Set default schedule(12) to one of the allowed values - 1,3,5,10,15,20,30,60,120,240
 
 Run 'npx @elastic/synthetics init' to create project with default settings.
 "
diff --git a/__tests__/push/monitor.test.ts b/__tests__/push/monitor.test.ts
index 5d2f374..f325dee 100644
--- a/__tests__/push/monitor.test.ts
+++ b/__tests__/push/monitor.test.ts
@@ -113,15 +113,19 @@ describe('Monitors', () => {
     );
     expect(parseSchedule('@every 4s')).toBe(1);
     expect(parseSchedule('@every 70s')).toBe(1);
-    expect(parseSchedule('@every 121s')).toBe(3);
+    expect(parseSchedule('@every 121s')).toBe(1);
     expect(parseSchedule('@every 1m')).toBe(1);
+    expect(parseSchedule('@every 181s')).toBe(3);
     expect(parseSchedule('@every 2m10s')).toBe(3);
     expect(parseSchedule('@every 4m25s')).toBe(5);
     expect(parseSchedule('@every 16m')).toBe(15);
-    expect(parseSchedule('@every 25m')).toBe(15);
+    expect(parseSchedule('@every 24m')).toBe(20);
+    expect(parseSchedule('@every 30m')).toBe(30);
     expect(parseSchedule('@every 45m')).toBe(30);
     expect(parseSchedule('@every 1h2m')).toBe(60);
-    expect(parseSchedule('@every 10h2m10s')).toBe(60);
+    expect(parseSchedule('@every 110m')).toBe(60);
+    expect(parseSchedule('@every 3h50m')).toBe(120);
+    expect(parseSchedule('@every 10h2m10s')).toBe(240);
   });
 
   it('parse alert config option', async () => {