feat: post process screenshots for deduping #290

vigneshshanmugam · 2021-05-26T06:17:25Z

fix De-duplicate images #285
Captures screenshot for each step and writes them to the cached .synthetics/<pid>/screenshots folder in JSON format with step and screenshot info. The pid stuff is to make sure each process (workers in future for leveraging parallelism) is writing screenshots to individual folders to avoid mutation.
Written screenshots are processed in to 8 blocks which results in 64 images blobs for 1280*720 viewport (Default) and wrote to heartbeat as screenshot/block and the whole screenshot reference is written as step/screenshot_reference which can be queried from the Uptime UI to construct all the images from the hash of individual block.

// screenshot block
{
  "type": "screenshot/block",
  "_id": "20f2cdc9e8302e94ff8d3dc09d3c359683cbcec5",
  "@timestamp": 1622076105043985.8,
  "blob": "UklGRlAAAABXRUJQVlA4IEQAAAAwBgCdASqgAFoAPm02mUmkIyKhIIgAgA2JaW7hdJAAT22IvEFRz2xF4gqOe2IvEFRz2xF4gqOe2ItgAP7/dlECgAAAAA==",
  "blob_mime": "image/webp",
  "package_version": "1.0.0-beta.2"
}
// screenshot ref

{
  "type": "step/screenshot_ref",
  "@timestamp": 1600300800000000,
  "journey": {
    "name": "j1",
    "id": "j1"
  },
  "step": {
    "name": "launch app",
    "index": 1
  },
  "root_fields": {
    "screenshot_ref": {
      "width": 1280,
      "height": 720,
      "blocks": [
        {
          "hash": "aee19c1f32131ed90e6dd34074a6e220ad97baf6",
          "top": 0,
          "left": 0,
          "width": 160,
          "height": 90
        },
        {
          "hash": "aee19c1f32131ed90e6dd34074a6e220ad97baf6",
          "top": 0,
          "left": 160,
          "width": 160,
          "height": 90
        }
      ]
    }
  },
  "package_version": "0.0.1"
}

apmmachine · 2021-05-26T06:25:51Z

💚 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

Build Cause: Pull request #290 updated
Start Time: 2021-06-03T18:33:11.759+0000
Duration: 15 min 54 sec
Commit: 088e24d

Test stats 🧪

Test	Results
Failed	0
Passed	102
Skipped	0
Total	102

Trends 🧪

vigneshshanmugam · 2021-05-26T14:35:12Z

src/reporters/json.ts

+      const left = col * blockWidth;
+      const buf = await img
+        .extract({ top, left, width: blockWidth, height: blockHeight })
+        .toFormat(sharp.format.webp)


self note:

check the size of individual blocks using different format.

Run benchmarks on this function.

webp even though smaller than png by ~20ms average would yield more savings. Will leave it as webp for now.

Processing 50 images
webp - 14.36 seconds
png - 13.119 seconds

Benchmarks script to play with

!(async () => { console.time('screenshots'); for (let i = 0; i < 50; i++) { await gatherScreenshots( 'path/to/.synthetics/pid/screenshots' ); } console.timeEnd('screenshots'); })();

src/reporters/json.ts

vigneshshanmugam · 2021-05-27T05:35:28Z

src/reporters/base.ts

-       */
-      this.stream.flush();
-    }
+    this.runner.on('end', () => this.stream.flush());


We don't need to take care of closing the stream as the process exists automatically which closes the underlying stream once program runs to completion. This is better than figuring out when to close and prematurely closing it than leaving it which does not harm.

andrewvc

Love the improvements here left some comments on some ways it could work even better

src/reporters/json.ts

andrewvc

This PR is essentially LGTM but I'm leaving it in request changes state because we can't merge it as is without breaking things for existing users. If we were to merge and release this today screenshots will not work with current versions of kibana. We really need a system of feature flags or something similar to heartbeat to determine which features are enabled. Heartbeat at least follows the stack versioning, and as a result can inform the code here of how to act.

I think we will also need to support the old way of screenshots working based on whatever system of flags or capabilities we come up with. I've opened #292 to track

andrewvc

There's a bug somewhere in this PR that affects handling of erroring test-cases. Running the following: cat t.js | node dist/cli.js --network --json --inline --screenshots | jq .type will fail early, not outputting journey/end and other events, where the contents of t.js are:

        step("load homepage", async () => {
            await page.goto('https://www.elastic.co');
        });
        step("hover over products menu", async () => {
            await page.hover('css=[data-nav-item=products]');
        });
        step("failme", async () => {
            await page.hhover('css=[data-nav-item=products]');
        });

vigneshshanmugam · 2021-06-02T04:26:10Z

@andrewvc Could be this #290 (comment), Will check tomorrow.

andrewvc · 2021-06-02T13:56:02Z

Not sure as to the root cause

vigneshshanmugam · 2021-06-02T20:09:35Z

Fixed the issue - We do async processing of the events in the microtasks which are being cut of by the CLI node process. Now we wait for the ack from the reporter once its written to the file descriptor before closing the process - 36a42e5

…enshots

vigneshshanmugam · 2021-06-03T00:05:55Z

src/reporters/json.ts

+      const left = col * blockWidth;
+      const buf = await img
+        .extract({ top, left, width: blockWidth, height: blockHeight })
+        .jpeg()


changed to jpeg for now, we can play with webp once we decide a solution in the Kibana.

@justinkambic FYI

src/common_types.ts

…enshots

andrewvc · 2021-06-03T19:31:18Z

__tests__/reporters/json.test.ts

+    expect(callback).not.toHaveBeenCalled();
+  });
+
+  it('idempotent on constructing screenshots blocks', async () => {


andrewvc

LGTM

vigneshshanmugam commented May 26, 2021

View reviewed changes

vigneshshanmugam added 3 commits May 26, 2021 17:32

feat: post process screenshots for deduping

91834d5

add types and change webp

0f52d8f

add comments and fix dir check

5ad799d

vigneshshanmugam force-pushed the post-process-screenshots branch from c6e5089 to 5ad799d Compare May 27, 2021 00:32

vigneshshanmugam marked this pull request as ready for review May 27, 2021 00:32

vigneshshanmugam commented May 27, 2021

View reviewed changes

src/reporters/json.ts Show resolved Hide resolved

write id to the stream

eb6dbf4

vigneshshanmugam commented May 27, 2021

View reviewed changes

src/reporters/json.ts Show resolved Hide resolved

add tests and remove abrupt closing

b8f6056

vigneshshanmugam commented May 27, 2021

View reviewed changes

andrewvc requested changes May 27, 2021

View reviewed changes

src/reporters/json.ts Outdated Show resolved Hide resolved

src/reporters/json.ts Outdated Show resolved Hide resolved

src/reporters/json.ts Show resolved Hide resolved

src/reporters/json.ts Show resolved Hide resolved

src/reporters/json.ts Show resolved Hide resolved

address review and idempotent test

2f77142

andrewvc mentioned this pull request May 27, 2021

Add flags/capabilities system #292

Closed

andrewvc requested changes May 27, 2021

View reviewed changes

andrewvc requested changes Jun 2, 2021

View reviewed changes

ensure events are reported

36a42e5

vigneshshanmugam added 2 commits June 2, 2021 16:56

fallback to old way

77d6dfd

Merge remote-tracking branch 'upstream/master' into post-process-scre…

64f90ac

…enshots

vigneshshanmugam commented Jun 3, 2021

View reviewed changes

src/common_types.ts Show resolved Hide resolved

vigneshshanmugam added the [zube]: In Review label Jun 3, 2021

vigneshshanmugam added 2 commits June 3, 2021 11:26

Merge remote-tracking branch 'upstream/master' into post-process-scre…

1713b40

…enshots

add ssblocks to capability

088e24d

vigneshshanmugam requested a review from andrewvc June 3, 2021 18:33

andrewvc reviewed Jun 3, 2021

View reviewed changes

andrewvc approved these changes Jun 3, 2021

View reviewed changes

vigneshshanmugam merged commit 3ccd2c1 into elastic:master Jun 3, 2021

vigneshshanmugam deleted the post-process-screenshots branch June 3, 2021 19:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: post process screenshots for deduping #290

feat: post process screenshots for deduping #290

vigneshshanmugam commented May 26, 2021 •

edited

apmmachine commented May 26, 2021 •

edited

Build stats

Test stats 🧪

Trends 🧪

vigneshshanmugam May 26, 2021 •

edited

vigneshshanmugam May 26, 2021

vigneshshanmugam May 26, 2021

vigneshshanmugam May 27, 2021

andrewvc left a comment

andrewvc left a comment •

edited

andrewvc left a comment

vigneshshanmugam commented Jun 2, 2021

andrewvc commented Jun 2, 2021

vigneshshanmugam commented Jun 2, 2021

vigneshshanmugam Jun 3, 2021

vigneshshanmugam Jun 3, 2021

andrewvc Jun 3, 2021

andrewvc left a comment

feat: post process screenshots for deduping #290

feat: post process screenshots for deduping #290

Conversation

vigneshshanmugam commented May 26, 2021 • edited

apmmachine commented May 26, 2021 • edited

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

vigneshshanmugam May 26, 2021 • edited

Choose a reason for hiding this comment

vigneshshanmugam May 26, 2021

Choose a reason for hiding this comment

vigneshshanmugam May 26, 2021

Choose a reason for hiding this comment

vigneshshanmugam May 27, 2021

Choose a reason for hiding this comment

andrewvc left a comment

Choose a reason for hiding this comment

andrewvc left a comment • edited

Choose a reason for hiding this comment

andrewvc left a comment

Choose a reason for hiding this comment

vigneshshanmugam commented Jun 2, 2021

andrewvc commented Jun 2, 2021

vigneshshanmugam commented Jun 2, 2021

vigneshshanmugam Jun 3, 2021

Choose a reason for hiding this comment

vigneshshanmugam Jun 3, 2021

Choose a reason for hiding this comment

andrewvc Jun 3, 2021

Choose a reason for hiding this comment

andrewvc left a comment

Choose a reason for hiding this comment

vigneshshanmugam commented May 26, 2021 •

edited

apmmachine commented May 26, 2021 •

edited

vigneshshanmugam May 26, 2021 •

edited

andrewvc left a comment •

edited