feat: use network events from pw #488

vigneshshanmugam · 2022-04-12T06:05:02Z

bring back the original PR along with couple of improvements feat: use events from PW for network data #372 fix: handle page close during response event #398
fixes Synthetics Doesn’t Handle Popups (new tab or new window) #253
We use the network events from the Playwright context instead of events from CDP which can only listen for the first page request and ignores all subsequent requests. With this change, all requests inside all frames, windows gets captured with this change and also we get header and body size for each individual request.
Timing calculation becomes a lot simpler to deal with and don't require much understanding of the chrome devtools code which we relied on before.
Changed logic around emulating the subsequent pages for network throttling as the previous implementation had bug and did not account for pages that are closed during the navigation.
Barriers are added for all the async operations that happens while observing for network events.

Missing data

We don't have a queuing time exposed from the PW API as Queueing time is pretty specific to chromium and not exposed for other browsers like FF and Webkit and its in the order of 1-2ms max when tested -
Bare uncompressed resource size is not exposed for each request instead we have header and body sizes for both request and responses.
http.version data is not exposed, but we can add that as a follow up once PW adds support for it and we are not using it on the waterfall UI.

apmmachine · 2022-04-12T06:12:23Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-04-12T15:52:30.434+0000
Duration: 14 min 54 sec

Test stats 🧪

Test	Results
Failed	0
Passed	148
Skipped	2
Total	150

🤖 GitHub comments

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

/test : Re-trigger the build.

lucasfcosta

Code LGTM, just had small nits.

Also, can you confirm the testing method below is what we need in order to consider this change to have been E2E tested?

Create an HTML file with an a tag that opens a popup with another HTML page in a new tab.
This new popup must contain some code to immediately close it and take some time to reach the networkidle state (ideally)
The agent should not throw an error.
There should be network data for the opened popup (it should show any script requests from that popup) (easy to see with --rich-events).

I've followed testing steps at #253 with slight modifications:

popup1.html

<html>
  <head>
  </head>
  <body>
    Test popup page 1
    <p><a href="popup2.html" onclick="return !window.open(this.href, 'Test Page', 'width=500,height=500')" target="_blank" id="linkWithPopup">Click</a> here (id=linkWithPopup) to open - Test Page - popup window</p>
    <p><a href="popup2.html" target="_blank" id="linkWithPopupNoNewWindow">Click</a> here (id=linkWithPopupNoNewWindow) to open - Test Page - new tab (not new window)</p>
  </body>
</html>

popup2.html

<html>
  <head>
    <script src="https://google.com"></script>
  </head>
  <body>
    Test popup page 2
  </body>
</html>

example.journey.ts

// Replace the import here for the original package or the built branch to test
const { journey, step } = require('../dist/index');
const { deepStrictEqual } = require('assert');
const { join } = require('path');

journey('Popup Test', async ({ page }) => {
  step('Popup Landing Page', async () => {
    const path = 'file://' + join(__dirname, '.', 'popup1.html');
    await page.goto(path);
    await page.waitForSelector('text=Test popup page 1');
  });

  step('Open Popup', async () => {
    const [popup] = await Promise.all([
      page.waitForEvent('popup'),
      page.click('#linkWithPopup'),
    ]);

    // Test 2 - new tab (not new popup window)
    //await page.click('#linkWithPopupNoNewWindow');

    await popup.waitForSelector('text=Test popup page 2');
  });

  step('test', async () => {
    await new Promise(resolve => setTimeout(resolve, 5000));
  });
});

Then, to run and check network data:

synthetics --rich-events ./ | jq -c 'select(.type == "journey/network_info")'

Do you think that would be a satisfactory test?

src/core/gatherer.ts

lucasfcosta · 2022-04-12T13:59:21Z

src/core/gatherer.ts

+          );
+        // Guard aganist pages that gets closed before the emulation kicks in
+        Promise.race([
+          new Promise<void>(resolve => page.on('close', () => resolve())),


Suggested change

new Promise<void>(resolve => page.on('close', () => resolve())),

new Promise<void>(resolve => page.on('close', resolve)),

Its done to satisfy the void type. Otherwise Page object will be resolved which is not the intention here.

src/core/gatherer.ts

vigneshshanmugam · 2022-04-12T15:14:45Z

@lucasfcosta Thanks for the review, yeah the test details provided by Paul should suffice for the popup scenario and we dont need to explicity test for the barrier case as the unit tests catches that.

But we need an E2E with HB and Kibana to check if everything looks fine on the Kibana side as the PR touches lots of network data.

lucasfcosta · 2022-04-12T15:47:34Z

Btw just E2E tested with Kibana and I was able to see the screenshot from the popup page as well as the network request it did to Google because of the script tag I've added.

feat: use network events from pw

f8aaea4

fix tls logic

6d5b4f4

vigneshshanmugam requested review from dominiqueclarke and lucasfcosta April 12, 2022 06:19

lucasfcosta reviewed Apr 12, 2022

View reviewed changes

address feedback

3ef75bc

lucasfcosta approved these changes Apr 12, 2022

View reviewed changes

vigneshshanmugam merged commit 2ae1448 into elastic:main Apr 12, 2022

vigneshshanmugam deleted the network-events-pw branch April 12, 2022 16:58

vigneshshanmugam mentioned this pull request Apr 12, 2022

chore: add test for cached res timings #489

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use network events from pw #488

feat: use network events from pw #488

vigneshshanmugam commented Apr 12, 2022

apmmachine commented Apr 12, 2022 •

edited

Loading

Build stats

Test stats 🧪

lucasfcosta left a comment •

edited

Loading

lucasfcosta Apr 12, 2022

vigneshshanmugam Apr 12, 2022

vigneshshanmugam commented Apr 12, 2022

lucasfcosta commented Apr 12, 2022

	new Promise<void>(resolve => page.on('close', () => resolve())),
	new Promise<void>(resolve => page.on('close', resolve)),

feat: use network events from pw #488

feat: use network events from pw #488

Conversation

vigneshshanmugam commented Apr 12, 2022

Missing data

apmmachine commented Apr 12, 2022 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

lucasfcosta left a comment • edited Loading

Choose a reason for hiding this comment

lucasfcosta Apr 12, 2022

Choose a reason for hiding this comment

vigneshshanmugam Apr 12, 2022

Choose a reason for hiding this comment

vigneshshanmugam commented Apr 12, 2022

lucasfcosta commented Apr 12, 2022

apmmachine commented Apr 12, 2022 •

edited

Loading

lucasfcosta left a comment •

edited

Loading