-
Notifications
You must be signed in to change notification settings - Fork 43
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
De-dupe screenshots #282
De-dupe screenshots #282
Conversation
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪Test errors
Expand to view the tests failures
|
src/reporters/json.ts
Outdated
@@ -285,6 +298,7 @@ export default class JSONReporter extends BaseReporter { | |||
url, | |||
status, | |||
metrics, | |||
screenshot_ref: screenshotRef, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does the screenshot get constructed from the individual blocks? Uptime UI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question, maybe there, maybe in Kibana.
Another thought is it may be worth pre-rendering small thumbnails. Assembling 20 of these for a list of thumbnails may be too much. Ideally we'd have a caching CDN
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If kibana in cloud already uses HTTP/2, We can just do individual reqs as we would yield better caching for both screenshots and filmstrips moving forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this out of the payload
field and into synthetics.screenshot_ref
. Statistics on these fields could be useful, especially in establishing cache-hit ratio, which will be useful for debugging customer storage issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just made the change ^^
Closing in favor of https://github.com/elastic/synthetics/pull/290/files |
WIP for #266
Beats side is in: elastic/beats#25808