spike: web performance

[pauldambra]

Problem

see RFC

Users want to see network requests and performance facts alongside other information

Changes

• adds a schema to store un-aggregated performance facts
  • this is intended to be queried by session id and/or pageview id and return time ordered facts for that id combination
  • in future we can build aggregating materialized views from this to provide dashboards for performance info
• updates the session recording player to show these network logs alongside recordings
  • this uses spark lines to show relative amount of wall time taken by requests
• adds an ingestion route for new the $performance_event events

works with

• feat: Adds performance capture posthog-js#488 which adds use of PerformanceObserver to capture events
• and feat: page view ids posthog-js#487 which added pageview_ids to captured events

todo

• update ingress rules https://github.com/PostHog/charts-clickhouse/blob/2cd44dd84d7913255b6c53e863a6941f1dda063f/charts/posthog/templates/ingress.yaml
• update middleware shortcircuiting feat(capture): short-circuit most middlewares before event capture #13319
• separate ingestion as feat: Added performance API #13452
• separate schema change so we can merge that in
• get posthog-js change in with capture performance turned off feat: Adds performance capture posthog-js#488
• turn off capture performance for posthog chore: turn off web performance alpha #13387
• add server side config for web performance feat: capture performance opt in #13348
• do we want to measure clickhouse lag for performance events cf fix(query-performance): Make clickhouse_lag celery metric cheap #13397

How did you test this code?

locally only so far

[pauldambra]

migrations were silently failing because I was ordering by mis-spelled column names 🤷

[macobo]

Problems:

1. Looking up (only) via pageview_id with this ORDER BY is really really expensive.
2. Looking up via session_id requires inspecting more granules than needed.

Re 1, could we always include session_id in the where clause when looking up by pageview_id?

Re 2, could we also add a filter on origin_timestamp to all the point queries?

[macobo]

Sending these events in via the main topic is not a good idea or it sharing the rest of the ingestion pipeline.

Similar to session recordings, having ingesting these events be batched/buffered/create persons/update properties/be processed by plugins touches a very complex part of an application you don't want to be touching. While adding this here speeds you up short-term you're creating problems for yourself and #team-pipeline as soon as it needs to get untangled.

Also the capture endpoint has custom partitioning logic that's not relevant for performance event.

This needs a rethink: I suggest removing it from this PR or putting it behind a flag completely, with the understanding that this will be reviewed by the proper team.

[xvello]

Completely flyby and could warrant its own RFC, but we're growing a pretty big list of urls that point to the same endpoint. We'll need to update the ingress rules and the middleware short circuit to account for this new url.

For operational simplicity (and, anecdotally, for routing performance), could we agree on consolidating all new intake urls into a common /i/ path (/p -> /i/p), so than we can easily send every /i/* request to the event pool's get_event and call it a day?

Longer term, I'd like us to move the existing endpoints too, but that's RFC material for later (how to coordinate SDK and backend releases, although nginx rewrite rules can help address the long tail).

[posthog-bot]

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.

[benjackwhite]

Closing as all the code is implemented in other PRs now 👍