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: show event ticks in player modal #27819

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

daibhin
Copy link
Contributor

@daibhin daibhin commented Jan 23, 2025

Problem

There was no way to highlight $exception events when viewing recordings from the error tracking page

Changes

  • Remove matching_events as it didn't seem to be used anywhere or could be replaced with matchingEventsMatchType
  • Allow openSessionPlayer to take the matchingEventsMatchType argument
  • Add an eventName property to the HogQLX RecordingButton which creates a 'name' match type on the ViewRecordingButton component. Cannot directly pass the matchingEventsMatchType prop because HogQLX does not support complex object arguments

@daibhin daibhin requested a review from a team January 23, 2025 11:39
const { closeSessionPlayer } = useActions(sessionPlayerModalLogic())

// activeSessionRecording?.matching_events should always be a single element array
// but, we're filtering and using flatMap just in case
const eventUUIDs =
Copy link
Member

Choose a reason for hiding this comment

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

i'm surprised this is safe... but i don't have enough recordings locally to test

there are two matching modes iirc

  1. simple events -> done locally
  2. events with filters -> done as a follow-up query

could this path be necessary for that second type?

Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

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

so long as the matching events still work if you filter e.g. pageview by current url on the home page / playlist page then i'm happy

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.

2 participants