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

Report page load asset size #66224

Merged
merged 45 commits into from
Jun 5, 2020

Conversation

dmlemeshko
Copy link
Member

@dmlemeshko dmlemeshko commented May 12, 2020

Summary

Using puppeteer and CDP client to get size of each resource loaded during app navigation.

Since the goal is to determine which assets are always loaded regardless of the application, we load several apps across Kibana. We only report the assets that appear on in all the apps, ignoring the app-specific ones.

Only a few requests contain "content-length" header, we cannot use puppeteer-only approach with request interception. We use CDP event listeners as an alternative.

Still response in CDP has encodedDataLength and we can get it via puppeteer.
Turned out, there is a DevTools bug that encodedDataLength includes headers size, so I'm using approach suggested here

Update: It turned out that the proper way to get size is to sum data length chunks for the same request-id.

Start Kibana/ES: x-pack: node scripts/functional_tests_server.js --config test/page_load_metrics/config.ts

Capture and print out metrics (local run): node scripts/page_load_metrics.js --kibana-url=http://localhost:5620

#65524

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@dmlemeshko dmlemeshko changed the title [skip ci ] report all file sizes loaded on Kibana page [skip ci] report all file sizes loaded on Kibana page May 12, 2020
@dmlemeshko dmlemeshko changed the title [skip ci] report all file sizes loaded on Kibana page report all file sizes loaded on Kibana page May 19, 2020
package.json Outdated Show resolved Hide resolved
scripts/performance_tests.js Outdated Show resolved Hide resolved
src/dev/performance/ingest_metrics.ts Outdated Show resolved Hide resolved
src/dev/performance/navigation.ts Outdated Show resolved Hide resolved
src/dev/performance/navigation.ts Outdated Show resolved Hide resolved
src/dev/performance/ingest_metrics.ts Outdated Show resolved Hide resolved
@spalger
Copy link
Contributor

spalger commented May 29, 2020

@elasticmachine merge upstream

@dmlemeshko dmlemeshko added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels May 29, 2020
@dmlemeshko dmlemeshko requested review from spalger and mshustov May 29, 2020 07:31
@dmlemeshko dmlemeshko marked this pull request as ready for review May 29, 2020 07:31
@dmlemeshko dmlemeshko requested a review from a team as a code owner May 29, 2020 07:31
@dmlemeshko dmlemeshko requested a review from spalger June 4, 2020 17:37
export interface NavigationOptions {
headless: boolean;
appConfig: { url: string; username: string; password: string };
screenshotsDir?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we please require this option and only define the default in the CLI?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in fec3c4e

dataLength: number;
}

function joinPath(pathA: string, pathB: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue with Url.resolve() is that it resets the path if the second segment starts with a /, it's not really a join equivalent.

Comment on lines +25 to +27
esArchiver: {
directory: resolve(__dirname, 'es_archives'),
},
Copy link
Contributor

Choose a reason for hiding this comment

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

We should define screenshots.directory here too.

@spalger spalger removed the backport:skip This commit does not require backporting label Jun 4, 2020
@dmlemeshko
Copy link
Member Author

dmlemeshko commented Jun 5, 2020

@spalger I found the reason for flakiness:
Navigating to http://localhost:5620/app/maps actually may have 2 results:
new map page: http://localhost:5620/app/maps#/map?_g=(filters:!(),refreshInterval:(pause:!f,value:0),time:(from:now-15m,to:now))&_a=(filters:!(),query:(language:kuery,query:''))
landing page: http://localhost:5620/app/maps#/?_g=()

The 2nd result happens rarely, but I was able to get it locally running the same script. I updated the locator in d6ee29b to be the common between those 2 pages.

@dmlemeshko dmlemeshko requested a review from spalger June 5, 2020 13:06
@spalger
Copy link
Contributor

spalger commented Jun 5, 2020

@spalger I found the reason for flakiness:
Navigating to http://localhost:5620/app/maps actually may have 2 results:
new map page: http://localhost:5620/app/maps#/map?_g=(filters:!(),refreshInterval:(pause:!f,value:0),time:(from:now-15m,to:now))&_a=(filters:!(),query:(language:kuery,query:''))
landing page: http://localhost:5620/app/maps#/?_g=()

The 2nd result happens rarely, but I was able to get it locally running the same script. I updated the locator in d6ee29b to be the common between those 2 pages.

@elastic/kibana-gis Is this expected behavior?

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@nreese
Copy link
Contributor

nreese commented Jun 5, 2020

@elastic/kibana-gis Is this expected behavior?

Yes. the route http://localhost:5620/app/maps resolves to maps listing page when one or more maps saved object exists. The route http://localhost:5620/app/maps routes to a new map when there are no maps saved objects. The thinking was, why send users to an empty listing page that then requires them to click "New map". Why not just directly send them to a new map.

@dmlemeshko dmlemeshko merged commit 6f57fa0 into elastic:master Jun 5, 2020
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Jun 5, 2020
* performance_tests: draft version

* update cli runner and script

* ingest metrics

* save asset type for plugins

* Update src/dev/performance/ingest_metrics.ts

Co-authored-by: Spencer <[email protected]>

* follow review comments

* fix size calc, add FTR config, move src to kbn/test

* fix import, remove unused interface

* Update packages/kbn-test/src/page_load_metrics/capture_page_load_metrics.ts

Co-authored-by: Spencer <[email protected]>

* start chromium with no-sandbox

* add logging

* check page contains expected element, cut apps to 5

* fix locator & typo

* Update packages/kbn-test/src/page_load_metrics/navigation.ts

Co-authored-by: Spencer <[email protected]>

* Update navigation.ts

* Update navigation.ts

* bump puppeteer version

* fix typo

* update navigation script

* update config file

* update yarn.lock

* fix

* take screenshot on failure

* update screenshot title

* fix screenshot saving and error

* invalid locator

* Revert "invalid locator"

This reverts commit 3007539.

* run script in a loop 10 times

* Revert "run script in a loop 10 times"

This reverts commit 6cfa219.

* path config value directly

* fix screenshots directory setup

* update maps locator, common for landing and new map

Co-authored-by: Spencer <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Mikhail Shustov <[email protected]>
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Jun 5, 2020
* performance_tests: draft version

* update cli runner and script

* ingest metrics

* save asset type for plugins

* Update src/dev/performance/ingest_metrics.ts

Co-authored-by: Spencer <[email protected]>

* follow review comments

* fix size calc, add FTR config, move src to kbn/test

* fix import, remove unused interface

* Update packages/kbn-test/src/page_load_metrics/capture_page_load_metrics.ts

Co-authored-by: Spencer <[email protected]>

* start chromium with no-sandbox

* add logging

* check page contains expected element, cut apps to 5

* fix locator & typo

* Update packages/kbn-test/src/page_load_metrics/navigation.ts

Co-authored-by: Spencer <[email protected]>

* Update navigation.ts

* Update navigation.ts

* bump puppeteer version

* fix typo

* update navigation script

* update config file

* update yarn.lock

* fix

* take screenshot on failure

* update screenshot title

* fix screenshot saving and error

* invalid locator

* Revert "invalid locator"

This reverts commit 3007539.

* run script in a loop 10 times

* Revert "run script in a loop 10 times"

This reverts commit 6cfa219.

* path config value directly

* fix screenshots directory setup

* update maps locator, common for landing and new map

Co-authored-by: Spencer <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Mikhail Shustov <[email protected]>
# Conflicts:
#	Jenkinsfile
dmlemeshko added a commit that referenced this pull request Jun 5, 2020
* performance_tests: draft version

* update cli runner and script

* ingest metrics

* save asset type for plugins

* Update src/dev/performance/ingest_metrics.ts

Co-authored-by: Spencer <[email protected]>

* follow review comments

* fix size calc, add FTR config, move src to kbn/test

* fix import, remove unused interface

* Update packages/kbn-test/src/page_load_metrics/capture_page_load_metrics.ts

Co-authored-by: Spencer <[email protected]>

* start chromium with no-sandbox

* add logging

* check page contains expected element, cut apps to 5

* fix locator & typo

* Update packages/kbn-test/src/page_load_metrics/navigation.ts

Co-authored-by: Spencer <[email protected]>

* Update navigation.ts

* Update navigation.ts

* bump puppeteer version

* fix typo

* update navigation script

* update config file

* update yarn.lock

* fix

* take screenshot on failure

* update screenshot title

* fix screenshot saving and error

* invalid locator

* Revert "invalid locator"

This reverts commit 3007539.

* run script in a loop 10 times

* Revert "run script in a loop 10 times"

This reverts commit 6cfa219.

* path config value directly

* fix screenshots directory setup

* update maps locator, common for landing and new map

Co-authored-by: Spencer <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Mikhail Shustov <[email protected]>

Co-authored-by: Spencer <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Mikhail Shustov <[email protected]>
dmlemeshko added a commit that referenced this pull request Jun 6, 2020
* Report page load asset size (#66224)

* performance_tests: draft version

* update cli runner and script

* ingest metrics

* save asset type for plugins

* Update src/dev/performance/ingest_metrics.ts

Co-authored-by: Spencer <[email protected]>

* follow review comments

* fix size calc, add FTR config, move src to kbn/test

* fix import, remove unused interface

* Update packages/kbn-test/src/page_load_metrics/capture_page_load_metrics.ts

Co-authored-by: Spencer <[email protected]>

* start chromium with no-sandbox

* add logging

* check page contains expected element, cut apps to 5

* fix locator & typo

* Update packages/kbn-test/src/page_load_metrics/navigation.ts

Co-authored-by: Spencer <[email protected]>

* Update navigation.ts

* Update navigation.ts

* bump puppeteer version

* fix typo

* update navigation script

* update config file

* update yarn.lock

* fix

* take screenshot on failure

* update screenshot title

* fix screenshot saving and error

* invalid locator

* Revert "invalid locator"

This reverts commit 3007539.

* run script in a loop 10 times

* Revert "run script in a loop 10 times"

This reverts commit 6cfa219.

* path config value directly

* fix screenshots directory setup

* update maps locator, common for landing and new map

Co-authored-by: Spencer <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Mikhail Shustov <[email protected]>
# Conflicts:
#	Jenkinsfile

* [page_load_metrics] fix path for discover

* fix url formatting

* fix screenshot title
@dmlemeshko
Copy link
Member Author

7.x 8e50e59
7.8 b6d6889

gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 8, 2020
* master: (57 commits)
  Add app arch team as owner of datemath package (elastic#66880)
  [Observability] Landing page for Observability (elastic#67467)
  [SIEM] Fix timeline buildGlobalQuery (elastic#68320)
  Optimize saved objects getScopedClient and HTTP API (elastic#68221)
  [Maps] Fix mb-style interpolate style rule (elastic#68413)
  update script to always download node (elastic#68421)
  [SECURITY SOLEIL] Fix selection of event type when no siem index signal created (elastic#68291)
  [DOCS] Adds note about configuring File Data Visualizer (elastic#68407)
  [DOCS] Adds link from remote clusters to index patterns (elastic#68406)
  [QA] slack notify on failure (elastic#68126)
  upgrade eslint-plugin-react-hooks from 2.3.0 to 4.0.4 (elastic#68295)
  moving to jira to a gold license (elastic#67178)
  [DOCS] Revises doc on adding data (elastic#68038)
  [APM] Add ThemeProvider to support dark mode (elastic#68242)
  Make welcome screen disabling first action in loginIfPrompted (elastic#68238)
  [QA] Code coverage: unskip tests, collect tests results, exclude bundles from report (elastic#64477)
  [ML] Functional tests - disable flaky regression and classification creation test
  [Alerting] change eventLog ILM requests to absolute URLs (elastic#68331)
  Report page load asset size (elastic#66224)
  [SIEM][CASE] Change SIEM to Security (elastic#68365)
  ...
spalger added a commit to spalger/kibana that referenced this pull request Jul 9, 2020
spalger added a commit that referenced this pull request Jul 15, 2020
* Revert "Report page load asset size (#66224)"

This reverts commit 6f57fa0.

* [kbn/optimizer] report sizes of assets produced by optimizer

* coalese the fast-glob versions we're using to prevent additional installs

* update kbn/pm dist

* Revert "update kbn/pm dist"

This reverts commit 68e24f0.

* Revert "coalese the fast-glob versions we're using to prevent additional installs"

This reverts commit 4201fb6.

* remove fast-glob, just recursivly call readdirSync()

* update integration tests to use new chunk filename

Co-authored-by: spalger <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
spalger pushed a commit to spalger/kibana that referenced this pull request Jul 15, 2020
…#71319)

* Revert "Report page load asset size (elastic#66224)"

This reverts commit 6f57fa0.

* [kbn/optimizer] report sizes of assets produced by optimizer

* coalese the fast-glob versions we're using to prevent additional installs

* update kbn/pm dist

* Revert "update kbn/pm dist"

This reverts commit 68e24f0.

* Revert "coalese the fast-glob versions we're using to prevent additional installs"

This reverts commit 4201fb6.

* remove fast-glob, just recursivly call readdirSync()

* update integration tests to use new chunk filename

Co-authored-by: spalger <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
# Conflicts:
#	packages/kbn-optimizer/src/integration_tests/basic_optimization.test.ts
#	yarn.lock
@dmlemeshko dmlemeshko deleted the ftr/report-all-file-sizes branch January 31, 2022 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants