Repository: elastic/synthetics (ts)

Issue:
Disable (ignore) throttling settings
For the launch of Synthetics, there is an issue where we need to disable throttling (this will be added in again in the future).

ACs:
* Synthetics should ignore any throttling settings when running the test (i.e. run with throttling disabled)
* When `push` is used, include a message informing the user
  * This needs to be generic enough because throttling will be enabled again at some future point (in the case of the service, this will be independent of the Synthetics version they are running `push` from)
  * This should be added alongside a warning triangle (similar to how we currently warn about frequencies being rounded
  * The copy will include the URL where they can find out more information
  * Copy suggestion (@florent-leborgne does this seem ok to you?):
  > ⚠️ Throttling may not be active when the tests run - see https://github.com/elastic/synthetics/blob/main/THROTTLING.md for more details.

Linking to the Kibana issue: https://github.com/elastic/kibana/issues/156512

Pull request description:
feat: disable throttling and emit warning
+ fix #756
+ Disables the Network emulation throttling settings via Chrome devtools in the browser CDP session and also provides warnings to the user at different places when users tries
   - Run tests locally with `--throttling` flag
   - Run tests locally with `--no-throttling` flag
   - Push monitors with `--throttling` and `--no-throttling` flag.
   - Push monitors with `monitor.use({throttling: {} )` or `monitor.use({throttling: false})`  on journey level. 
   - Push monitors when `monitor.use({throttling: {}))` set on file level. 
 + Cleaned up the old throttling formatting code as its not required anymore since we have disabled throttling altogether. 

Patch:
diff --git a/THROTTLING.md b/THROTTLING.md
deleted file mode 100644
index 566b2a8..0000000
--- a/THROTTLING.md
+++ /dev/null
@@ -1,1 +0,0 @@
-⚠️ Placeholder for throttling details in Elastic's global managed testing infrastructure (the service) and for Private Locations
diff --git a/docs/throttling.md b/docs/throttling.md
new file mode 100644
index 0000000..3a43087
--- /dev/null
+++ b/docs/throttling.md
@@ -0,0 +1,2 @@
+## Throttling
+
diff --git a/src/cli.ts b/src/cli.ts
index a87a40a..be5bbbd 100644
--- a/src/cli.ts
+++ b/src/cli.ts
@@ -43,6 +43,7 @@ import {
   loadSettings,
   validateSettings,
   catchIncorrectSettings,
+  warnIfThrottled,
 } from './push';
 import {
   formatLocations,
@@ -118,10 +119,13 @@ program
   .option(
     '--throttling <config>',
     'JSON object to throttle network conditions for download and upload throughput in megabits/second and latency in milliseconds. Ex: { "download": 10, "upload": 5, "latency": 200 }.',
-    parseThrottling,
-    {}
+    parseThrottling
+  )
+  .option(
+    '--no-throttling',
+    'Turns off default network throttling.',
+    parseThrottling
   )
-  .option('--no-throttling', 'Turns off default network throttling.')
   .addOption(playwrightOpts)
   .version(version)
   .description('Run synthetic tests')
@@ -207,6 +211,9 @@ program
       validateSettings(options);
       await catchIncorrectSettings(settings, options);
       const monitors = runner.buildMonitors(options);
+      if ((options as CliArgs).throttling == null) {
+        warnIfThrottled(monitors);
+      }
       monitors.push(...(await createLightweightMonitors(workDir, options)));
       await push(monitors, options);
     } catch (e) {
diff --git a/src/core/gatherer.ts b/src/core/gatherer.ts
index 2ac9f75..3303467 100644
--- a/src/core/gatherer.ts
+++ b/src/core/gatherer.ts
@@ -46,7 +46,7 @@ export class Gatherer {
 
   static async setupDriver(options: RunOptions): Promise<Driver> {
     log('Gatherer: setup driver');
-    const { wsEndpoint, playwrightOptions, networkConditions } = options;
+    const { wsEndpoint, playwrightOptions } = options;
 
     if (Gatherer.browser == null) {
       if (wsEndpoint) {
@@ -74,7 +74,9 @@ export class Gatherer {
       playwrightOptions?.navigationTimeout ?? DEFAULT_TIMEOUT
     );
 
-    Gatherer.setNetworkConditions(context, networkConditions);
+    // TODO: Network throttling via chrome devtools emulation is disabled for now.
+    // See docs/throttling.md for more details.
+    // Gatherer.setNetworkConditions(context, networkConditions);
     if (playwrightOptions?.testIdAttribute) {
       selectors.setTestIdAttribute(playwrightOptions.testIdAttribute);
     }
diff --git a/src/dsl/monitor.ts b/src/dsl/monitor.ts
index bb61413..7cad26d 100644
--- a/src/dsl/monitor.ts
+++ b/src/dsl/monitor.ts
@@ -40,7 +40,9 @@ export type SyntheticsLocationsType = keyof typeof LocationsMap;
 export const SyntheticsLocations = Object.keys(
   LocationsMap
 ) as SyntheticsLocationsType[];
-export const ALLOWED_SCHEDULES = [1, 3, 5, 10, 15, 20, 30, 60, 120, 240] as const;
+export const ALLOWED_SCHEDULES = [
+  1, 3, 5, 10, 15, 20, 30, 60, 120, 240,
+] as const;
 
 export type MonitorConfig = {
   id?: string;
@@ -51,6 +53,11 @@ export type MonitorConfig = {
   enabled?: boolean;
   locations?: SyntheticsLocationsType[];
   privateLocations?: string[];
+  /**
+   * @deprecated This option is ignored.
+   * Network throttling via chrome devtools is ignored at the moment.
+   * See https://github.com/elastic/synthetics/blob/main/docs/throttling.md for more details.
+   */
   throttling?: boolean | ThrottlingOptions;
   screenshot?: ScreenshotOptions;
   params?: Params;
diff --git a/src/helpers.ts b/src/helpers.ts
index 7a71183..1f51633 100644
--- a/src/helpers.ts
+++ b/src/helpers.ts
@@ -300,6 +300,9 @@ export function getNetworkConditions(
   };
 }
 
+export const THROTTLING_WARNING_MSG = `Throttling may not be active when the tests run - see
+https://github.com/elastic/synthetics/blob/main/docs/throttling.md for more details`;
+
 const dstackTraceLimit = 10;
 
 // Uses the V8 Stacktrace API to get the function location
diff --git a/src/options.ts b/src/options.ts
index 9f6260e..8397dc9 100644
--- a/src/options.ts
+++ b/src/options.ts
@@ -26,8 +26,8 @@
 import merge from 'deepmerge';
 import { createOption } from 'commander';
 import { readConfig } from './config';
-import { getNetworkConditions, DEFAULT_THROTTLING_OPTIONS } from './helpers';
-import type { CliArgs, RunOptions, ThrottlingOptions } from './common_types';
+import type { CliArgs, RunOptions } from './common_types';
+import { THROTTLING_WARNING_MSG, warn } from './helpers';
 
 export function normalizeOptions(cliArgs: CliArgs): RunOptions {
   const options: RunOptions = {
@@ -110,28 +110,6 @@ export function normalizeOptions(cliArgs: CliArgs): RunOptions {
    * Get the default monitor config from synthetics.config.ts file
    */
   const monitor = config.monitor;
-  if (cliArgs.throttling) {
-    const throttleConfig = merge.all([
-      DEFAULT_THROTTLING_OPTIONS,
-      toObject(monitor?.throttling),
-      toObject(cliArgs.throttling),
-    ]);
-    if (monitor?.throttling !== false) {
-      options.throttling = throttleConfig;
-      options.networkConditions = getNetworkConditions(throttleConfig);
-    } else {
-      /**
-       * If the throttling is disabled via Project monitor config, use it a source
-       * of truth and disable it for pushing those monitors.
-       */
-      options.throttling = false;
-    }
-  } else {
-    /**
-     * Do not apply throttling when `--no-throttling` flag is passed
-     */
-    options.throttling = false;
-  }
 
   options.schedule = cliArgs.schedule ?? monitor?.schedule;
   options.locations = cliArgs.locations ?? monitor?.locations;
@@ -141,6 +119,7 @@ export function normalizeOptions(cliArgs: CliArgs): RunOptions {
   return options;
 }
 
+/* eslint-disable-next-line @typescript-eslint/no-unused-vars */
 function toObject(value: boolean | Record<string, any>): Record<string, any> {
   const defaulVal = {};
   if (typeof value === 'boolean') {
@@ -150,35 +129,14 @@ function toObject(value: boolean | Record<string, any>): Record<string, any> {
 }
 
 /**
- * Parses the throttling CLI settings and also
- * adapts to the format to keep the backwards compatability
- * - Accepts old format `<5d/3u/20l>`
- * - Processess new format otherwise `{download: 5, upload: 3, latency: 20}`
+ * Parses the throttling CLI settings with `{download: 5, upload: 3, latency:
+ * 20}` format
+ *
+ * Since throttling is disabled for now, we warn the users if throttling/nothrottling
+ * flag is passed to the CLI
  */
-export function parseThrottling(value: string, prev?: string) {
-  const THROTTLING_REGEX = /([0-9]{1,}u)|([0-9]{1,}d)|([0-9]{1,}l)/gm;
-  if (THROTTLING_REGEX.test(value)) {
-    const throttling: ThrottlingOptions = {};
-    const conditions = value.split('/');
-
-    conditions.forEach(condition => {
-      const setting = condition.slice(0, condition.length - 1);
-      const token = condition.slice(-1);
-      switch (token) {
-        case 'd':
-          throttling.download = Number(setting);
-          break;
-        case 'u':
-          throttling.upload = Number(setting);
-          break;
-        case 'l':
-          throttling.latency = Number(setting);
-          break;
-      }
-    });
-    return throttling;
-  }
-  return JSON.parse(value || prev);
+export function parseThrottling() {
+  warn(THROTTLING_WARNING_MSG);
 }
 
 export function getCommonCommandOpts() {
diff --git a/src/push/index.ts b/src/push/index.ts
index 93736ee..f6ffb07 100644
--- a/src/push/index.ts
+++ b/src/push/index.ts
@@ -42,6 +42,7 @@ import {
   indent,
   done,
   getMonitorManagementURL,
+  THROTTLING_WARNING_MSG,
 } from '../helpers';
 import type { PushOptions, ProjectSettings } from '../common_types';
 import { findSyntheticsConfig, readConfig } from '../config';
@@ -280,3 +281,11 @@ export async function pushLegacy(
 
   done(`Pushed: ${grey(getMonitorManagementURL(options.url))}`);
 }
+
+// prints warning if any of the monitors has throttling settings enabled during push
+export function warnIfThrottled(monitors: Monitor[]) {
+  const throttled = monitors.some(monitor => monitor.config.throttling != null);
+  if (throttled) {
+    warn(THROTTLING_WARNING_MSG);
+  }
+}


Test patch:
diff --git a/__tests__/cli.test.ts b/__tests__/cli.test.ts
index 514832a..115c024 100644
--- a/__tests__/cli.test.ts
+++ b/__tests__/cli.test.ts
@@ -27,12 +27,7 @@ import { join } from 'path';
 import { devices } from 'playwright-chromium';
 import { Server } from './utils/server';
 import { CLIMock } from './utils/test-config';
-import {
-  DEFAULT_THROTTLING_OPTIONS,
-  getNetworkConditions,
-  megabitsToBytes,
-  safeNDJSONParse,
-} from '../src/helpers';
+import { THROTTLING_WARNING_MSG, safeNDJSONParse } from '../src/helpers';
 
 describe('CLI', () => {
   let server: Server;
@@ -134,25 +129,6 @@ describe('CLI', () => {
         params: { url: 'dev' },
       });
     });
-
-    it('loads a configuration file when passing a config param', async () => {
-      const cli = new CLIMock(false)
-        .stdin(`step('fake step', async () => {})`)
-        .args([
-          '--reporter',
-          'json',
-          '--inline',
-          '--config',
-          join(FIXTURES_DIR, 'synthetics.config.ts'),
-        ])
-        .run({ cwd: FIXTURES_DIR });
-      await cli.waitFor('synthetics/metadata');
-      expect(await cli.exitCode).toBe(0);
-      const [output] = safeNDJSONParse(cli.output());
-      expect(output.payload).toMatchObject({
-        network_conditions: { latency: 11 },
-      });
-    });
   });
 
   it('run suites and exit with 0', async () => {
@@ -274,34 +250,6 @@ describe('CLI', () => {
     expect(await cli.exitCode).toBe(0);
   });
 
-  it('pass dynamic config to journey params', async () => {
-    const getMetadata = async () => {
-      const cli = new CLIMock()
-        .args([
-          join(FIXTURES_DIR, 'fake.journey.ts'),
-          '--reporter',
-          'json',
-          '--config',
-          join(FIXTURES_DIR, 'synthetics.config.ts'),
-        ])
-        .run();
-      await cli.waitFor('synthetics/metadata');
-      expect(await cli.exitCode).toBe(0);
-      return cli.output();
-    };
-
-    let [output] = safeNDJSONParse([await getMetadata()]);
-    expect(output.payload).toMatchObject({
-      network_conditions: { latency: 11 },
-    });
-
-    process.env['NODE_ENV'] = 'development';
-    [output] = safeNDJSONParse([await getMetadata()]);
-    expect(output.payload).toMatchObject({
-      network_conditions: { latency: 10 },
-    });
-  });
-
   it('params wins over config params', async () => {
     const cli = new CLIMock()
       .args([
@@ -447,88 +395,30 @@ describe('CLI', () => {
     });
   });
 
-  describe('throttling', () => {
+  describe('Throttling', () => {
     let cliArgs: Array<string>;
-
     beforeAll(async () => {
-      cliArgs = [
-        join(FIXTURES_DIR, 'example.journey.ts'),
-        '--params',
-        JSON.stringify(serverParams),
-        '--reporter',
-        'json',
-      ];
+      cliArgs = ['--reporter', 'json', '--inline'];
     });
 
-    it('applies --no-throttling', async () => {
-      const cli = new CLIMock().args(cliArgs.concat(['--no-throttling'])).run();
-      await cli.waitFor('synthetics/metadata');
-      const journeyStartOutput = JSON.parse(cli.output());
-      expect(await cli.exitCode).toBe(0);
-      expect(journeyStartOutput.payload).toBeUndefined();
-    });
-
-    it('applies default throttling', async () => {
-      const cli = new CLIMock().args(cliArgs).run();
-      await cli.waitFor('synthetics/metadata');
-      const journeyStartOutput = JSON.parse(cli.output());
-      expect(await cli.exitCode).toBe(0);
-      expect(journeyStartOutput.payload).toHaveProperty(
-        'network_conditions',
-        getNetworkConditions(DEFAULT_THROTTLING_OPTIONS)
-      );
-    });
-
-    it('applies custom throttling', async () => {
+    it('warns on --no-throttling', async () => {
       const cli = new CLIMock()
-        .args(
-          cliArgs.concat([
-            '--throttling',
-            JSON.stringify({
-              download: 3,
-              upload: 1,
-              latency: 30,
-            }),
-          ])
-        )
-        .run();
-      await cli.waitFor('synthetics/metadata');
-      const journeyStartOutput = JSON.parse(cli.output());
-      expect(await cli.exitCode).toBe(0);
-      expect(journeyStartOutput.payload).toHaveProperty('network_conditions', {
-        downloadThroughput: megabitsToBytes(3),
-        uploadThroughput: megabitsToBytes(1),
-        latency: 30,
-        offline: false,
-      });
-    });
-
-    it('supports older format', async () => {
-      const cli = new CLIMock()
-        .args(cliArgs.concat(['--throttling', '17u/30l/3d']))
+        .stdin(`step('fake step', async () => {})`)
+        .args(cliArgs.concat('--no-throttling'))
         .run();
-      await cli.waitFor('synthetics/metadata');
-      const journeyStartOutput = JSON.parse(cli.output());
       expect(await cli.exitCode).toBe(0);
-      expect(journeyStartOutput.payload).toHaveProperty('network_conditions', {
-        downloadThroughput: megabitsToBytes(3),
-        uploadThroughput: megabitsToBytes(17),
-        latency: 30,
-        offline: false,
-      });
+      const output = cli.stderr();
+      expect(output).toContain(THROTTLING_WARNING_MSG);
     });
 
-    it('uses default throttling when specific params are not provided', async () => {
+    it('warns on --throttling setting', async () => {
       const cli = new CLIMock()
-        .args(cliArgs.concat(['--throttling', JSON.stringify({ download: 2 })]))
+        .stdin(`step('fake step', async () => {})`)
+        .args(cliArgs.concat('--throttling', JSON.stringify({ upload: 10 })))
         .run();
-      await cli.waitFor('synthetics/metadata');
-      const journeyStartOutput = JSON.parse(cli.output());
       expect(await cli.exitCode).toBe(0);
-      expect(journeyStartOutput.payload).toHaveProperty('network_conditions', {
-        ...getNetworkConditions(DEFAULT_THROTTLING_OPTIONS),
-        downloadThroughput: megabitsToBytes(2),
-      });
+      const output = cli.stderr();
+      expect(output).toContain(THROTTLING_WARNING_MSG);
     });
   });
 
diff --git a/__tests__/core/gatherer.test.ts b/__tests__/core/gatherer.test.ts
index 367c073..27927ee 100644
--- a/__tests__/core/gatherer.test.ts
+++ b/__tests__/core/gatherer.test.ts
@@ -199,7 +199,7 @@ describe('Gatherer', () => {
     });
   });
 
-  describe('Network emulation', () => {
+  describe.skip('Network emulation', () => {
     const networkConditions = {
       downloadThroughput: megabitsToBytes(3),
       uploadThroughput: megabitsToBytes(1),
diff --git a/__tests__/fixtures/synthetics.config.ts b/__tests__/fixtures/synthetics.config.ts
index 8040482..dc08c7a 100644
--- a/__tests__/fixtures/synthetics.config.ts
+++ b/__tests__/fixtures/synthetics.config.ts
@@ -31,16 +31,12 @@ module.exports = env => {
     params: {
       url: 'dev',
     },
-    monitor: {
-      throttling: { latency: 10 },
-    },
     playwrightOptions: {
       ...devices['Galaxy S9+'],
     },
   };
-  if (env !== 'development' && config.params && config.monitor) {
+  if (env !== 'development' && config.params) {
     config.params.url = 'non-dev';
-    config.monitor.throttling = { latency: 11 };
   }
   return config;
 };
diff --git a/__tests__/options.test.ts b/__tests__/options.test.ts
index 621e126..5efa57f 100644
--- a/__tests__/options.test.ts
+++ b/__tests__/options.test.ts
@@ -24,7 +24,7 @@
  */
 
 import { CliArgs } from '../src/common_types';
-import { normalizeOptions, parseThrottling } from '../src/options';
+import { normalizeOptions } from '../src/options';
 import { join } from 'path';
 
 describe('options', () => {
@@ -76,32 +76,16 @@ describe('options', () => {
   });
 
   it('normalize monitor configs', () => {
-    expect(normalizeOptions({ throttling: false })).toMatchObject({
-      throttling: false,
-    });
-
     expect(
-      normalizeOptions({ throttling: { download: 50 }, schedule: 3 })
+      normalizeOptions({
+        schedule: 3,
+        privateLocations: ['test'],
+        locations: ['australia_east'],
+      })
     ).toMatchObject({
       schedule: 3,
-      throttling: {
-        download: 50,
-        upload: 3,
-        latency: 20,
-      },
-    });
-  });
-
-  it('parse throttling', () => {
-    expect(parseThrottling('{}')).toEqual({});
-    expect(parseThrottling('{"download": 20, "upload": 10}')).toEqual({
-      download: 20,
-      upload: 10,
-    });
-    expect(parseThrottling('100l/41u/9d')).toEqual({
-      download: 9,
-      upload: 41,
-      latency: 100,
+      privateLocations: ['test'],
+      locations: ['australia_east'],
     });
   });
 });
diff --git a/__tests__/push/index.test.ts b/__tests__/push/index.test.ts
index ca0d215..2083178 100644
--- a/__tests__/push/index.test.ts
+++ b/__tests__/push/index.test.ts
@@ -31,6 +31,7 @@ import { formatDuplicateError } from '../../src/push';
 import { createKibanaTestServer } from '../utils/kibana-test-server';
 import { Server } from '../utils/server';
 import { CLIMock } from '../utils/test-config';
+import { THROTTLING_WARNING_MSG } from '../../src/helpers';
 
 describe('Push', () => {
   const PROJECT_DIR = join(__dirname, 'new-project');
@@ -61,14 +62,6 @@ describe('Push', () => {
     await rm(PROJECT_DIR, { recursive: true, force: true });
   });
 
-  // TODO: Fix this failing test.
-  // Fails on andrew's box with "Aborted (missing synthetics config file), Project not set up correctly.·
-  // Run 'npx @elastic/synthetics init' to create project with default settings.
-  it.skip('error when auth is ignored', async () => {
-    const output = await runPush([]);
-    expect(output).toContain(`required option '--auth <auth>' not specified`);
-  });
-
   it('error when project is not setup', async () => {
     const output = await runPush();
     expect(output).toContain(
@@ -76,6 +69,12 @@ describe('Push', () => {
     );
   });
 
+  it('error when auth is ignored', async () => {
+    await fakeProjectSetup({}, {});
+    const output = await runPush([]);
+    expect(output).toContain(`required option '--auth <auth>' not specified`);
+  });
+
   it('error on empty project id', async () => {
     await fakeProjectSetup({}, {});
     const output = await runPush();
@@ -156,6 +155,22 @@ journey('duplicate name', () => monitor.use({ schedule: 15 }));`
     await rm(dupJourney, { force: true });
   });
 
+  it('warn if throttling config is set', async () => {
+    await fakeProjectSetup(
+      { id: 'test-project' },
+      { locations: ['test-loc'], schedule: 3 }
+    );
+    const testJourney = join(PROJECT_DIR, 'test.journey.ts');
+    await writeFile(
+      testJourney,
+      `import {journey, monitor} from '../../../';
+  journey('journey 1', () => monitor.use({ throttling: {latency: 20} }));`
+    );
+    const output = await runPush();
+    await rm(testJourney, { force: true });
+    expect(output).toContain(THROTTLING_WARNING_MSG);
+  });
+
   it('errors on duplicate lightweight monitors', async () => {
     await fakeProjectSetup(
       { id: 'test-project' },