[Heartbeat] Dedupe screenshots / Extra Args by andrewvc · Pull Request #25808 · elastic/beats

andrewvc · 2021-05-20T22:11:05Z

Counterpart of elastic/synthetics#282
Fixes: #26188

Adds support for screenshot blocks as mentioned in the synthetics PR, and also switches over to using the new flags / capabilities system. This is done by using --rich-events by default as a synthetics flag, and also exposing a new synthetics_args option for synthetics, allowing users to use configs like:

- type: browser
  enabled: true
  id: my-monitor
  name: My Monitor
  synthetics_args: ["--capability", "trace", "metrics", "filmstrips", "ssblocks"]
  source:
    inline:
      script:
        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]');
        });
  schedule: "@every 1m"

This PR also moves some of the logic from browser.go to suite.go, which was also renamed to be cleaner and more in keeping with idiomatic go. This also made adding a small test possible. We still need a better story around E2E testing, but for now this will suffice.

Finally, this adds mappings for screenshot_ref objects as well as blocks, and routes them to the correct dataset. It also improves tests for enrichment of events to help test this.

elasticmachine · 2021-05-20T22:11:07Z

Pinging @elastic/uptime (Team:Uptime)

elasticmachine · 2021-05-20T23:10:12Z

💚 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 #25808 updated
Start Time: 2021-06-08T02:22:32.702+0000
Duration: 78 min 2 sec
Commit: 452c53a

Test stats 🧪

Test	Results
Failed	0
Passed	3359
Skipped	82
Total	3441

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	3359
Skipped	82
Total	3441

vigneshshanmugam

LGTM, tested and it works as expected.

We might need to change one more thing to get it merged. adding step/block to the screenshot datastream index.

andrewvc · 2021-05-27T02:53:50Z

PR now adds mapping and properly routes to correct dataset. Still needs tests

mergify · 2021-06-01T07:45:39Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b dedupe upstream/dedupe
git merge upstream/master
git push upstream dedupe

vigneshshanmugam · 2021-06-04T22:40:21Z

heartbeat/_meta/fields.common.yml

+            - name: blocks
+              type: group
+              fields: 
+                - name: hash


Need to add top and left here.

Good catch!

I don't see the top & left yet in the new changes.

vigneshshanmugam · 2021-06-04T22:40:52Z

heartbeat/_meta/fields.common.yml

+          fields:
+            - name: width
+              type: integer
+              description: Width of the full screenshot in pixels.


Shall we remove the use of full here? Full seems like full page screenshot but we are capture only the viewport width and height.

Hmmm, I intended it to mean not of an individual block. I'll clarify that.

vigneshshanmugam · 2021-06-04T22:47:25Z

x-pack/heartbeat/monitors/browser/synthexec/synthexec.go

 	}

+	// Default capabilities
+	capabilities = append(capabilities, "trace", "metrics", "filmstrips", "ssblocks")


I am not sure if its the right way to do it.

I was thinking we should make it configurable by introducing Capability flag in the config.go in addition to params and then using it here for experimentation.

moving forward once the feature is built in to the UI, we can add it to --rich-events in synthetics.

WDYT?

Ah, I see what you mean. That makes sense, will make those changes!

x-pack/heartbeat/monitors/browser/synthexec/synthexec.go

mergify · 2021-06-07T20:14:18Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b dedupe upstream/dedupe
git merge upstream/master
git push upstream dedupe

andrewvc · 2021-06-07T21:52:17Z

x-pack/heartbeat/monitors/browser/suite.go

+// or more contributor license agreements. Licensed under the Elastic License;
+// you may not use this file except in compliance with the Elastic License.
+
+package browser


This file isn't new, it's just moved. Enough of it changed due to renaming the ss var to s that git thinks it's all new.

vigneshshanmugam

LGTM after the top and left changes.

CHANGELOG.next.asciidoc

vigneshshanmugam · 2021-06-07T22:36:55Z

heartbeat/_meta/fields.common.yml

+            - name: blocks
+              type: group
+              fields: 
+                - name: hash


I don't see the top & left yet in the new changes.

vigneshshanmugam · 2021-06-07T22:38:55Z

x-pack/heartbeat/monitors/browser/suite_test.go

+		},
+		{
+			"kitchen sink",
+			&Config{SyntheticsArgs: []string{"--capability", "trace"}, Sandbox: true},


I am curios to see a test with capability > 2 and other flags as part of synthetic args. Reason being variadic args are prone to errors.

Co-authored-by: Vignesh Shanmugam <vignesh.shanmugam22@gmail.com>

andrewvc · 2021-06-07T22:51:01Z

@vigneshshanmugam I believe I've addressed your concerns. I left out top and left on the mapping intentionally. We'll probably never query them, so it makes sense to save a field. I've added some notes to the fields yml indicating why this is.

vigneshshanmugam · 2021-06-07T22:55:40Z

@andrewvc Spot on, Sounds good to me ++.

andrewvc · 2021-06-08T03:50:30Z

@Mergifyio backport 7.x

(cherry picked from commit db6738e) # Conflicts: # heartbeat/_meta/fields.common.yml # heartbeat/docs/fields.asciidoc # heartbeat/include/fields.go # x-pack/heartbeat/include/fields.go

mergify · 2021-06-08T03:56:14Z

Command backport 7.x: success

Backports have been created

#26192 [Heartbeat] Dedupe screenshots / Extra Args (backport #25808) has been created for branch 7.x

mergify · 2021-06-08T03:56:15Z

Command backport 7.14: failure

No backport have been created

Backport to branch 7.14 failed: Branch not found

* [Heartbeat] Dedupe screenshots / Extra Args (#25808) (cherry picked from commit db6738e) # Conflicts: # heartbeat/_meta/fields.common.yml # heartbeat/docs/fields.asciidoc # heartbeat/include/fields.go # x-pack/heartbeat/include/fields.go * Fix merge * Update fields docs Co-authored-by: Andrew Cholakian <andrew@andrewvc.com>

POC for dedupe of screenshots

91fcabf

andrewvc added the Team:obs-ds-hosted-services Label for the Observability Hosted Services team label May 20, 2021

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels May 20, 2021

andrewvc mentioned this pull request May 20, 2021

De-dupe screenshots elastic/synthetics#282

Closed

Always use _create for screenshots

709a7f9

vigneshshanmugam mentioned this pull request May 24, 2021

De-duplicate images elastic/synthetics#285

Closed

vigneshshanmugam reviewed May 25, 2021

View reviewed changes

andrewvc added 2 commits May 26, 2021 14:16

Handle new screenshot types

7e831a6

Add mappings

81c6b1c

andrewvc requested a review from a team as a code owner May 27, 2021 02:43

andrewvc changed the title ~~POC for dedupe of screenshots~~ Dedupe screenshots May 27, 2021

andrewvc added 9 commits June 1, 2021 17:59

Merge remote-tracking branch 'origin/master' into dedupe

05a01f2

Support capabilities flags

38084d8

cleanup output

e29d661

Remove annoying error message

343adb1

Cleanup

247ae18

Merge remote-tracking branch 'origin/master' into dedupe

442078d

Update fields.go

b1666ce

Add some tests, update

759139b

Clean up capabilities

c1ff28a

andrewvc added enhancement Heartbeat v7.14.0 labels Jun 4, 2021

andrewvc changed the title ~~Dedupe screenshots~~ [Heartbeat] Dedupe screenshots Jun 4, 2021

vigneshshanmugam reviewed Jun 4, 2021

View reviewed changes

andrewvc added 2 commits June 7, 2021 16:46

Switch to extra args

609bc66

Merge remote-tracking branch 'origin/master' into dedupe

9cdcb08

andrewvc changed the title ~~[Heartbeat] Dedupe screenshots~~ [Heartbeat] Dedupe screenshots / Extra Args Jun 7, 2021

andrewvc commented Jun 7, 2021

View reviewed changes

andrewvc self-assigned this Jun 7, 2021

andrewvc added 2 commits June 7, 2021 17:25

Update fields.go

0998ddd

Update docs

076f77f

vigneshshanmugam approved these changes Jun 7, 2021

View reviewed changes

andrewvc and others added 2 commits June 7, 2021 17:49

Update CHANGELOG.next.asciidoc

31f7934

Co-authored-by: Vignesh Shanmugam <vignesh.shanmugam22@gmail.com>

Include PR feedback

a825a7c

andrewvc added 3 commits June 7, 2021 17:56

Merge remote-tracking branch 'andrewvc/dedupe' into dedupe

d3e5b33

Update fields

b8153f6

Fix syntax

452c53a

andrewvc merged commit db6738e into elastic:master Jun 8, 2021

mergify bot mentioned this pull request Jun 8, 2021

[Heartbeat] Dedupe screenshots / Extra Args (backport #25808) #26192

Merged

vigneshshanmugam mentioned this pull request Jun 10, 2021

[Heartbeat] Fix broken invocation of synth package #26228

Merged

3 tasks

Conversation

andrewvc commented May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented May 20, 2021

Uh oh!

elasticmachine commented May 20, 2021 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

💚 Flaky test report

Test stats 🧪

Uh oh!

vigneshshanmugam left a comment

Choose a reason for hiding this comment

Uh oh!

andrewvc commented May 27, 2021

Uh oh!

mergify bot commented Jun 1, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mergify bot commented Jun 7, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vigneshshanmugam left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewvc commented Jun 7, 2021

Uh oh!

vigneshshanmugam commented Jun 7, 2021

Uh oh!

andrewvc commented Jun 8, 2021

Uh oh!

mergify bot commented Jun 8, 2021

Uh oh!

mergify bot commented Jun 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andrewvc commented May 20, 2021 •

edited

Loading

elasticmachine commented May 20, 2021 •

edited by jenkins-beats-ci bot

Loading

vigneshshanmugam left a comment •

edited

Loading