fix(hogql): fix recodings in actors response #20953
Conversation
|
Size Change: 0 B Total Size: 821 kB ℹ️ View Unchanged
|
| return ( | ||
| {"session_id": session_id, "events": recordings_lookup[session_id]} | ||
| for session_id in (event[2] for event in event_results) | ||
| if session_id in recordings_lookup | ||
| for session_id in recordings_lookup.keys() |
There was a problem hiding this comment.
Wouldn't this simply return everything in lookup for each row in results? 🤔
I don't remember it exactly, but I think going via event we should only pick the matching recordings for each event row.
There was a problem hiding this comment.
Hmm in case of funnels this has a list of "matched_recordings" that include person ids with matched session ids. Doing the event lookup results in 10 recordings, where in reality there are only 3 for this example:
[
(datetime.datetime(2024, 3, 14, 22, 33, 13, 749000, tzinfo=<UTC>), UUID('018e3f19-1b02-76bd-8d6f-4fd5f5ef5826'), '018e3f0e-0031-781d-a8d4-5cf3487503f3', '018e3f18-fae2-7c29-8520-f5a57c19e267'),
(datetime.datetime(2024, 3, 14, 15, 25, 40, 59000, tzinfo=<UTC>), UUID('018e3d91-a94b-76eb-a322-b73324a07a8f'), '018e3d69-1372-78c8-adae-e1ac913a2dba', '018e3d91-a85f-7a4c-af5d-692adbaca3cf'),
(datetime.datetime(2024, 3, 15, 11, 27, 51, 141000, tzinfo=<UTC>), UUID('018e41de-4b54-7c70-9d24-98e343ab91ab'), '018e4193-7a80-74a4-9df3-c8806ee08cba', '018e41de-4a0a-7229-9695-90e4a52d571b'),
(datetime.datetime(2024, 3, 15, 11, 27, 50, 837000, tzinfo=<UTC>), UUID('018e41de-4a2c-741d-b6b9-dab1c3b1540f'), '018e4193-7a80-74a4-9df3-c8806ee08cba', '018e41de-4a0a-7229-9695-90e4a52d571b'),
(datetime.datetime(2024, 3, 15, 11, 41, 41, 817000, tzinfo=<UTC>), UUID('018e41ea-f81e-7b31-b2c6-a507306f988a'), '018e4193-7a80-74a4-9df3-c8806ee08cba', '018e41dd-c212-7eee-9780-f21aac398718'),
(datetime.datetime(2024, 3, 15, 11, 40, 20, 300000, tzinfo=<UTC>), UUID('018e41e9-b9bf-700d-80f3-fb95ef7346a6'), '018e4193-7a80-74a4-9df3-c8806ee08cba', '018e41de-620f-7b69-8162-aae854f2d949'),
(datetime.datetime(2024, 3, 15, 11, 40, 20, 11000, tzinfo=<UTC>), UUID('018e41e9-b89c-7720-9c77-187700096c22'), '018e4193-7a80-74a4-9df3-c8806ee08cba', '018e41de-620f-7b69-8162-aae854f2d949'),
(datetime.datetime(2024, 3, 15, 11, 39, 49, 859000, tzinfo=<UTC>), UUID('018e41e9-42c9-7b8d-8969-46732fbcdcb2'), '018e4193-7a80-74a4-9df3-c8806ee08cba', '018e41de-620f-7b69-8162-aae854f2d949'),
(datetime.datetime(2024, 3, 15, 11, 39, 49, 696000, tzinfo=<UTC>), UUID('018e41e9-4234-7c97-81ec-631045381b4e'), '018e4193-7a80-74a4-9df3-c8806ee08cba', '018e41de-620f-7b69-8162-aae854f2d949'),
(datetime.datetime(2024, 3, 15, 11, 39, 47, 494000, tzinfo=<UTC>), UUID('018e41e9-3997-762f-9f4f-04131349e240'), '018e4193-7a80-74a4-9df3-c8806ee08cba', '018e41de-620f-7b69-8162-aae854f2d949')
]These are the valid session ids here:
{'018e3f0e-0031-781d-a8d4-5cf3487503f3', '018e4193-7a80-74a4-9df3-c8806ee08cba', '018e3d69-1372-78c8-adae-e1ac913a2dba'}But maybe I don't understand how this should work in general.
There was a problem hiding this comment.
Tried locally and I see what you mean. I tried to debug what the difference is... I think the funnels gives events with same session id, maybe I didn't have that case in paths (and locally I see a single session there).
I just pushed a commit that simply uses a set to only return a single recording in your example. This way it can be left as is otherwise.
| return ( | ||
| {"session_id": session_id, "events": recordings_lookup[session_id]} | ||
| for session_id in (event[2] for event in event_results) | ||
| if session_id in recordings_lookup | ||
| for session_id in recordings_lookup.keys() |
There was a problem hiding this comment.
Tried locally and I see what you mean. I tried to debug what the difference is... I think the funnels gives events with same session id, maybe I didn't have that case in paths (and locally I see a single session there).
I just pushed a commit that simply uses a set to only return a single recording in your example. This way it can be left as is otherwise.
|
Thank you @webjunkie ! (Tried locally and still works as I'd expect) |
Problem
It wasn't possible to open a recording in a HogQL persons modal.
Changes
This PR:
prepare_actorsmethod also for theactorcolumnrecordings_lookuponly instead of mapping each event to a recording inget_recording, which was also renamed toserialize_recordingsHow did you test this code?
Tried locally with a funnel insight