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

feat: default replay partial compression on #1445

Merged
merged 3 commits into from
Oct 1, 2024

Conversation

pauldambra
Copy link
Member

@pauldambra pauldambra commented Sep 30, 2024

Partial compression of large rrweb events will make them more reliably ingestable

We've had this on for days now, and some users have tested it

Let's default it on


Clicked around in PH app and saved ~1MB in 2 minutes of recording

Copy link

vercel bot commented Sep 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Sep 30, 2024 10:29pm

@pauldambra pauldambra added the bump minor Bump minor version when this PR gets merged label Sep 30, 2024
@pauldambra pauldambra requested a review from a team September 30, 2024 21:05
Copy link

github-actions bot commented Sep 30, 2024

Size Change: +495 B (+0.02%)

Total Size: 2.81 MB

Filename Size Change
dist/array.full.js 351 kB +55 B (+0.02%)
dist/array.full.no-external.js 350 kB +55 B (+0.02%)
dist/array.js 167 kB +55 B (+0.03%)
dist/array.no-external.js 166 kB +55 B (+0.03%)
dist/main.js 167 kB +55 B (+0.03%)
dist/module.full.js 351 kB +55 B (+0.02%)
dist/module.full.no-external.js 350 kB +55 B (+0.02%)
dist/module.js 167 kB +55 B (+0.03%)
dist/module.no-external.js 166 kB +55 B (+0.03%)
ℹ️ View Unchanged
Filename Size
dist/all-external-dependencies.js 191 kB
dist/exception-autocapture.js 10.5 kB
dist/external-scripts-loader.js 2.35 kB
dist/recorder-v2.js 111 kB
dist/recorder.js 111 kB
dist/surveys-preview.js 59.8 kB
dist/surveys.js 66 kB
dist/tracing-headers.js 8.36 kB
dist/web-vitals.js 10.3 kB

compressed-size-action

Comment on lines +149 to +152
const originalSize = estimateSize(event)
if (originalSize < PARTIAL_COMPRESSION_THRESHOLD) {
return event
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we pretty frequently get small enough snapshots that they're bigger when compressed so we can leave them be

@kitsirota
Copy link

Hi @pauldambra,

We were going from 1.165.0 to 1.178.0 with a production release today, but we ended up being blocked by the console stack traces relating to undefined length across any event or page change. I can confirm that without any changes to the our codebase, this PR results in endless console errors.

We actually had to roll a release back this afternoon, but after debugging this further we finally narrowed down the issue to this PR. The errors here show up on every pageview, event, navigation, etc.
posthog-error

The workaround to proceed was to adjust our posthog.init statement to add compress_events: false
posthog.init(POSTHOG_KEY, { api_host: POSTHOG_API_HOST, ui_host: POSTHOG_UI_HOST, debug: POSTHOG_DEBUG, disable_surveys: POSTHOG_DISABLE_SURVEYS, secure_cookie: POSTHOG_SECURE_COOKIE, capture_pageview: POSTHOG_RECORD_PAGEVIEWS, capture_pageleave: POSTHOG_CAPTURE_PAGELEAVE, session_recording: { compress_events: false, }, })

Have you received any other reports of this issue? Ive been going through different package versions for a while and the exceptions are showing up with this PR and all releases from 1.166.0

@pauldambra
Copy link
Member Author

hey @kitsirota

We've not had any reports of that and it's been the default for a month now.

Are you able to open a ticket from in-app support? We'd need to take a look at the site and see what was happening.

There was a similar looking bug but that was fixed in 1.166.2 so shouldn't be possible in 1.178.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump minor Bump minor version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants