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

Enable rrweb recording only in cli #98

Closed
wants to merge 2 commits into from

Conversation

Que3216
Copy link
Contributor

@Que3216 Que3216 commented Sep 23, 2022

The CLI code also sets some settings:

        await frame.evaluate(`
        window["METICULOUS_RECORDING_TOKEN"] = "${recordingToken}";
        window["METICULOUS_APP_COMMIT_HASH"] = "${appCommitHash}";
        window["METICULOUS_FORCE_RECORDING"] = true;
        window["METICULOUS_UPLOAD_INTERVAL_MS"] = ${uploadIntervalMs};
        window["METICULOUS_ENABLE_RRWEB_PLUGIN_NODE_DATA"] = true;
      `);

However we can't easily make changes to these, since it'll depend on the customer updating their version of the CLI. This can cause issues when the CLI and passive snippet make assumptions about their configuration options or defaults and get out of sync. Therefore maybe we should move to:

        await frame.evaluate(`
        window["METICULOUS_MANUAL_RECORDING"] = true;
        window["METICULOUS_RECORDING_TOKEN"] = "${recordingToken}";
        window["METICULOUS_APP_COMMIT_HASH"] = "${appCommitHash}";
        window["METICULOUS_UPLOAD_INTERVAL_MS"] = ${uploadIntervalMs};
      `);

And then we set the CLI defaults within the snippet. Eventually we could publish separate snippets for passive vs manual, and so reduce the size of the passive snippet.

This PR moves in this direction (CLI settings for snippet set in snippet not in CLI), and also disables rrweb recording in the passive snippet. This is because rrweb often produces > 2mb of data, which causes the session to be abandoned.

@Que3216 Que3216 requested a review from leafty September 23, 2022 11:05
@Que3216
Copy link
Contributor Author

Que3216 commented Sep 23, 2022

Oops pushed wrong changes, will update later

@Que3216 Que3216 closed this Sep 23, 2022
@leafty leafty deleted the qsh/enable-rrweb-recording-only-in-cli branch October 17, 2022 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant