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

core(trace): enable JS samples for advanced workflows #15542

Merged
merged 7 commits into from
Dec 19, 2023

Conversation

adamraine
Copy link
Member

@adamraine adamraine commented Oct 16, 2023

Feature request from internal chat. Lighthouse doesn't currently use JS samples for anything meaningful, but they are still useful for advanced workflows looking at the trace (e.g. "View Trace" button in DT). Considering we eventually want to use these JS samples for things like 3P attribution, it shouldn't hurt to enable samples for these use cases.

Closes #15599

@adamraine adamraine requested a review from a team as a code owner October 16, 2023 21:39
@adamraine adamraine requested review from connorjclark and removed request for a team October 16, 2023 21:40
@adamraine
Copy link
Member Author

adamraine commented Oct 17, 2023

Seems like PROTOCOL_TIMEOUT happens more often with this PR. Most of the errors seem to be on Debugger.disable in the byte-efficiency smoke test.

Edit: ope should have just looked at #12835

This reverts commit 55eac1e.
@connorjclark
Copy link
Collaborator

connorjclark commented Dec 19, 2023

Is this change still producing issues? The current CI failure looks like a previously-accepted-to-be-flaky test.

@adamraine adamraine merged commit f05700e into main Dec 19, 2023
29 checks passed
@adamraine adamraine deleted the enable-js-samples-for-debug branch December 19, 2023 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable JS Sampling in Lighthouse Timespan mode in DevTools
3 participants