-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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: heatmap storage and API #21623
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few things - @pauldambra let me know if you want me to implement the changes suggested
🤣 we both did it... good test of how consistent a change would be if we both did it 🤣 |
paging @fuziontech 😊 plz can we do these things to the clickhouse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reminder here @pauldambra 🙏 I needed the bump from last week so I appreciate that. This looks great - only one request to add a few kafka virtual columns, but that's totally optional.
posthog/heatmaps/sql.py
Outdated
pointer_target_fixed Bool, | ||
current_url VARCHAR, | ||
type LowCardinality(String), | ||
_timestamp DateTime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something else that would be lovely to add here are the following virtual columns:
_offset
_partition
We've really wished to have these on tables in the past - would be super cool to start adding them here.
-- * width | ||
-- we'll almost never query this by session id | ||
-- so from least to most cardinality that's | ||
ORDER BY (type, team_id, toDate(timestamp), current_url, viewport_width) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love it, this is perfect. Well done 🙏
posthog/heatmaps/sql.py
Outdated
pointer_target_fixed, | ||
current_url, | ||
type, | ||
_timestamp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing here:
something else that would be lovely to add here are the following virtual columns:
_offset
_partition
We've really wished to have these on tables in the past - would be super cool to start adding them here.
posthog/heatmaps/sql.py
Outdated
f"TRUNCATE TABLE IF EXISTS {HEATMAPS_DATA_TABLE()} ON CLUSTER '{settings.CLICKHOUSE_CLUSTER}'" | ||
) | ||
|
||
INSERT_SINGLE_HEATMAP_EVENT = """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a scary bit of text - as long as it's only ever used for testing we are good!
@benjackwhite I have confirmed the migration and ingestion with these latest changes locally (well, I was on partition 0 but the offset was populating) am in and out all morning but feel free to merge without waiting for me - or I'll do it this afternoon |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Splitting parts out of #21487 which was a good spike PR but needs breaking up now
This adds