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: report reason for recording start #1452

Merged
merged 10 commits into from
Oct 15, 2024
Merged

Conversation

pauldambra
Copy link
Member

@pauldambra pauldambra commented Oct 3, 2024

in this private slack thread https://posthog.slack.com/archives/C06R7TKD508/p1727988694238459

A user has

  • 10% sampling set on replay
  • a batch export of events that identify interesting sessions
  • every session has a session id regardless of it was recorded
  • so the user has to check every session id one at a time to see which was recorded

adding this as an event has cost implication for users (if small)
and it is tricky to pick up a definitive start reasons for some flows as start is idempotent and we call it more than once

so, let's add a session super property so folk can look for that

Copy link

vercel bot commented Oct 3, 2024

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

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Oct 15, 2024 0:00am

Copy link

github-actions bot commented Oct 3, 2024

Size Change: +5.23 kB (+0.19%)

Total Size: 2.83 MB

Filename Size Change
dist/array.full.js 354 kB +581 B (+0.16%)
dist/array.full.no-external.js 353 kB +581 B (+0.16%)
dist/array.js 169 kB +581 B (+0.35%)
dist/array.no-external.js 168 kB +581 B (+0.35%)
dist/main.js 170 kB +581 B (+0.34%)
dist/module.full.js 354 kB +581 B (+0.16%)
dist/module.full.no-external.js 353 kB +581 B (+0.16%)
dist/module.js 169 kB +581 B (+0.35%)
dist/module.no-external.js 168 kB +581 B (+0.35%)
ℹ️ View Unchanged
Filename Size
dist/all-external-dependencies.js 192 kB
dist/exception-autocapture.js 11.1 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

@pauldambra pauldambra marked this pull request as ready for review October 4, 2024 11:31
@pauldambra pauldambra added the bump minor Bump minor version when this PR gets merged label Oct 4, 2024
@pauldambra
Copy link
Member Author

i'm going to make this draft... turns out to be really tricky to only trigger once on "normal" start since we might need to wait for decide or not

also makes me think it'd be useful in a recording/session to be able to distinguish between client and browser navigation generally

@posthog-bot
Copy link
Collaborator

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@pauldambra pauldambra marked this pull request as ready for review October 15, 2024 11:58
@pauldambra pauldambra requested a review from a team October 15, 2024 11:58
@pauldambra pauldambra merged commit 5e20147 into main Oct 15, 2024
13 checks passed
@pauldambra pauldambra deleted the feat/pd/report_session_started branch October 15, 2024 12:45
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 stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants