Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Synthetics] Increase project API payload limit #142140

Merged
merged 4 commits into from
Oct 4, 2022

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Sep 28, 2022

Summary

Increase synthetics monitors project push API payload limit to 20MB !!

@shahzad31 shahzad31 force-pushed the increase-payload-size branch from c10c008 to 1b9b2a2 Compare September 29, 2022 12:15
@shahzad31 shahzad31 marked this pull request as ready for review September 29, 2022 13:44
@shahzad31 shahzad31 requested review from a team as code owners September 29, 2022 13:44
@shahzad31 shahzad31 self-assigned this Sep 29, 2022
@shahzad31 shahzad31 added release_note:skip Skip the PR/issue when compiling release notes v8.5.0 v8.6.0 labels Sep 29, 2022
Copy link
Contributor

@awahab07 awahab07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the PR by pushing browser monitors with various payloads. Kibana server did error out when payload exceeded the limit. However, on my machine, Kibana server times out when payload is closer to (but less than) 20 MB.

Monitor 1 Payload Monitor 1 Bundle Monitor 2 Pyalod Monitor 2 Bundle Result Observation
25MB 18.80MB - - image Restricted by Kibana Server
20MB 15.04MB - - image Restricted by Kibana Server
18MB 13.54MB - - image Timed out
15MB 11.28MB - - - Pushed successfully 👍
8MB 6.02MB 8MB 6.02MB - Pushed successfully 👍
10MB 7.52MB 8MB 6.02MB image Timed out
9MB 6.77MB 8MB 6.02MB - Pushed successfully 👍

Based on the above results, it LGTM.

How did I test

Since @elastic/synthetics also restricts pushing a monitor with > 800KB bundle size (per monitor), I changed the limit to 20 MB when testing a single monitor and 10MB when testing two monitors. I changed here.

I used the following node script to generate a random string payload in MBs

// helpers/generate-payload.js
const fs = require('fs');
const path = require('path');
const size = Number(process.argv[2]);

if(isNaN(size) || size <= 0) {
  console.error(`Error: Provide a valid size in MBs e.g. \`node ../generate-payload.js 20\` for 20 MB payload.`);
}

const payload = genPayload(size * 1024 * 1024);
const exportScript = `export const payload = '${payload}';`;
const outputPath = path.resolve(__dirname, `./payload-${size}MB.ts`);
fs.writeFileSync(outputPath, exportScript, { flag: 'w+' });
console.info(`Successfully written payload of ${size}MB to ${outputPath}`);

function genPayload(length) {
  const characters       = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
  const charactersLength = characters.length;

  let result = '';
  for ( let i = 0; i < length; i++ ) {
    result += characters.charAt(Math.floor(Math.random() * charactersLength));
  }
  return result;
}

e.g. Usage

$node <dir-path>/synthetics/examples/push-project-07/helpers/generate-payload.js 9

# Successfully written payload of 9MB to <dir-path>/synthetics/examples/push-project-07/helpers/payload-9MB.ts

The monitor script

// journeys/payload-01.journey.ts
import { journey, step, monitor, expect } from '@elastic/synthetics';

import { payload } from '../helpers/payload-9MB';

journey('Payload Test 01', ({ page, params }) => {
  // Only relevant for the push command to create
  // monitors in Kibana
  monitor.use({
    id: 'payload-test-001',
    schedule: 10,
  });

  step('Goto Amazon', async () => {
    await page.goto(params.url);
  });

  step('Search', async () => {
    await page.locator('#twotabsearchtextbox').type(payload);
  });
});

@botelastic botelastic bot added Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Sep 30, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@@ -13,6 +13,8 @@ import { API_URLS } from '../../../common/constants';
import { getAllLocations } from '../../synthetics_service/get_all_locations';
import { ProjectMonitorFormatter } from '../../synthetics_service/project_monitor/project_monitor_formatter';

const MAX_PAYLOAD_SIZE = 1048576 * 20; // 20MB
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this is MiB not MB

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not test locally, code changes make sense. Thanks @shahzad31 !

@@ -20,6 +20,7 @@ import {
} from '@kbn/core/server';
import { schema } from '@kbn/config-schema';
import { map$ } from '@kbn/std';
import { RouteConfigOptions } from '@kbn/core-http-server';
Copy link
Contributor

@jloleysens jloleysens Oct 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit+question: looks like we can combine this type import with @kbn/core/server?

not actually sure which to use now that there are two sources from which the same types can be imported, I guess we should trend toward using the types from the package @kbn/core-http-server?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will leave it up to the experts on this :)
I guess once everything migrates to packages, this will be auto resolved?

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
bfetch 71 72 +1
Unknown metric groups

API count

id before after diff
bfetch 80 81 +1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @shahzad31

@shahzad31 shahzad31 merged commit 6824718 into elastic:main Oct 4, 2022
@shahzad31 shahzad31 deleted the increase-payload-size branch October 4, 2022 12:46
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 4, 2022
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 4, 2022
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.5.0 v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants