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

fix(events-explorer): events explorer improvements #14648

Merged
merged 14 commits into from
Mar 9, 2023
Merged
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
33 changes: 11 additions & 22 deletions frontend/src/queries/examples.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const Events: EventsQuery = {
properties: [
{ type: PropertyFilterType.Event, key: '$browser', operator: PropertyOperator.Exact, value: 'Chrome' },
],
after: '-24h',
limit: 100,
}

Expand Down Expand Up @@ -310,38 +311,26 @@ const TimeToSeeDataWaterfall: TimeToSeeDataWaterfallNode = {
},
}

const HogQL: HogQLQuery = {
const HogQLRaw: HogQLQuery = {
kind: NodeKind.HogQLQuery,
query:
' select event,\n' +
' properties.$geoip_country_name as `Country Name`,\n' +
' count() as `Event count`\n' +
' from events\n' +
' where timestamp > now() - interval 1 month\n' +
' group by event,\n' +
' properties.$geoip_country_name\n' +
' order by count() desc\n' +
' limit 100',
}

const HogQLTable: DataTableNode = {
kind: NodeKind.DataTableNode,
full: true,
source: {
kind: NodeKind.HogQLQuery,
query: ` select event,
query: ` select event,
person.properties.email,
properties.$browser,
count()
from events
where timestamp > now () - interval 1 month
where timestamp > now () - interval 1 day
and person.properties.email is not null
group by event,
properties.$browser,
person.properties.email
order by count() desc
limit 100`,
},
}

const HogQLTable: DataTableNode = {
kind: NodeKind.DataTableNode,
full: true,
source: HogQLRaw,
}

export const examples: Record<string, Node> = {
Expand All @@ -365,7 +354,7 @@ export const examples: Record<string, Node> = {
TimeToSeeDataSessionsJSON,
TimeToSeeDataWaterfall,
TimeToSeeDataJSON,
HogQL,
HogQLRaw,
HogQLTable,
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/queries/nodes/DataNode/dataNodeLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ describe('dataNodeLogic', () => {

// Autoload is running in the background and will fire in 5 seconds. Check that there's a background script for this.
expect(logic.cache.autoLoadInterval).toBeTruthy()
jest.advanceTimersByTime(6000)
jest.advanceTimersByTime(31000)
await expectLogic(logic)
.toDispatchActions(['loadNewData', 'loadNewDataSuccess'])
.toMatchValues({
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/queries/nodes/DataNode/dataNodeLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export interface DataNodeLogicProps {
query: DataNode
}

const AUTOLOAD_INTERVAL = 5000
const AUTOLOAD_INTERVAL = 30000

export const dataNodeLogic = kea<dataNodeLogicType>([
path(['queries', 'nodes', 'dataNodeLogic']),
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/queries/nodes/DataTable/renderColumn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { TZLabel } from 'lib/components/TZLabel'
import { Property } from 'lib/components/Property'
import { urls } from 'scenes/urls'
import { PersonHeader } from 'scenes/persons/PersonHeader'
import { DataTableNode, HasPropertiesNode, QueryContext } from '~/queries/schema'
import { DataTableNode, EventsQueryPersonColumn, HasPropertiesNode, QueryContext } from '~/queries/schema'
import { isEventsQuery, isHogQLQuery, isPersonsNode, isTimeToSeeDataSessionsQuery } from '~/queries/utils'
import { combineUrl, router } from 'kea-router'
import { CopyToClipboardInline } from 'lib/components/CopyToClipboard'
Expand Down Expand Up @@ -151,9 +151,9 @@ export function renderColumn(
}
return <Property value={eventRecord.person?.properties?.[propertyKey]} />
} else if (key === 'person' && isEventsQuery(query.source)) {
const personRecord = value as PersonType
return !!personRecord.distinct_ids.length ? (
<Link to={urls.person(personRecord.distinct_ids[0])}>
const personRecord = value as EventsQueryPersonColumn
return !!personRecord.distinct_id ? (
<Link to={urls.person(personRecord.distinct_id)}>
<PersonHeader noLink withIcon person={personRecord} />
</Link>
) : (
Expand Down
10 changes: 9 additions & 1 deletion frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,15 @@ export interface EventsQueryResponse {
results: any[][]
hasMore?: boolean
}

export interface EventsQueryPersonColumn {
uuid: string
created_at: string
properties: {
name?: string
email?: string
}
distinct_id: string
}
export interface EventsQuery extends DataNode {
kind: NodeKind.EventsQuery
/** Return a limited set of data. Required. */
Expand Down
22 changes: 15 additions & 7 deletions frontend/src/scenes/persons/PersonHeader.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { PersonActorType, PersonType } from '~/types'
import './PersonHeader.scss'
import { Link } from 'lib/lemon-ui/Link'
import { urls } from 'scenes/urls'
Expand All @@ -8,14 +7,16 @@ import { PERSON_DEFAULT_DISPLAY_NAME_PROPERTIES } from 'lib/constants'
import { midEllipsis } from 'lib/utils'
import clsx from 'clsx'

type PersonPropType = { properties?: Record<string, any>; distinct_ids?: string[]; distinct_id?: string }

export interface PersonHeaderProps {
person?: Pick<PersonType, 'properties' | 'distinct_ids'> | null
person?: PersonPropType | null
withIcon?: boolean
noLink?: boolean
noEllipsis?: boolean
}

export function asDisplay(person: PersonType | PersonActorType | null | undefined, maxLength?: number): string {
export function asDisplay(person: PersonPropType | null | undefined, maxLength?: number): string {
if (!person) {
return 'Unknown'
}
Expand All @@ -28,13 +29,17 @@ export function asDisplay(person: PersonType | PersonActorType | null | undefine
const customIdentifier: string =
typeof propertyIdentifier !== 'string' ? JSON.stringify(propertyIdentifier) : propertyIdentifier

const display: string | undefined = (customIdentifier || person.distinct_ids?.[0])?.trim()
const display: string | undefined = (customIdentifier || person.distinct_id || person.distinct_ids?.[0])?.trim()

return display ? midEllipsis(display, maxLength || 40) : 'Person without ID'
}

export const asLink = (person: Partial<PersonType> | null | undefined): string | undefined =>
person?.distinct_ids?.length ? urls.person(person.distinct_ids[0]) : undefined
export const asLink = (person?: PersonPropType | null): string | undefined =>
person?.distinct_id
? urls.person(person.distinct_id)
: person?.distinct_ids?.length
? urls.person(person.distinct_ids[0])
: undefined

export function PersonHeader(props: PersonHeaderProps): JSX.Element {
const href = asLink(props.person)
Expand All @@ -52,7 +57,10 @@ export function PersonHeader(props: PersonHeaderProps): JSX.Element {
{props.noLink || !href ? (
content
) : (
<Link to={href} data-attr={`goto-person-email-${props.person?.distinct_ids?.[0]}`}>
<Link
to={href}
data-attr={`goto-person-email-${props.person?.distinct_id || props.person?.distinct_ids?.[0]}`}
>
{content}
</Link>
)}
Expand Down
33 changes: 15 additions & 18 deletions posthog/api/test/__snapshots__/test_query.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,19 @@
OFFSET 0
'
---
# name: TestQuery.test_select_event_person
'
/* user_id:0 request:_snapshot_ */
SELECT event,
distinct_id,
distinct_id
FROM events
WHERE and(equals(team_id, 2), less(timestamp, '2020-01-10 12:14:05.000000'), greater(timestamp, '2020-01-09 12:00:00.000000'))
ORDER BY event ASC
LIMIT 101
OFFSET 0
'
---
# name: TestQuery.test_select_hogql_expressions
'
/* user_id:0 request:_snapshot_ */
Expand All @@ -361,27 +374,11 @@
# name: TestQuery.test_select_hogql_expressions.1
'
/* user_id:0 request:_snapshot_ */
SELECT tuple(uuid, event, properties, timestamp, team_id, events.distinct_id, elements_chain, events.created_at, events__pdi.person_id, events__pdi__person.created_at, events__pdi__person.properties___name, events__pdi__person.properties___email),
SELECT tuple(uuid, event, properties, timestamp, team_id, distinct_id, elements_chain, created_at),
event
FROM events
INNER JOIN
(SELECT argMax(person_distinct_id2.person_id, version) AS person_id,
distinct_id
FROM person_distinct_id2
WHERE equals(team_id, 2)
GROUP BY distinct_id
HAVING equals(argMax(is_deleted, version), 0)) AS events__pdi ON equals(events.distinct_id, events__pdi.distinct_id)
INNER JOIN
(SELECT argMax(person.created_at, version) AS created_at,
argMax(replaceRegexpAll(JSONExtractRaw(properties, 'name'), '^"|"$', ''), version) AS properties___name,
argMax(replaceRegexpAll(JSONExtractRaw(properties, 'email'), '^"|"$', ''), version) AS properties___email,
id
FROM person
WHERE equals(team_id, 2)
GROUP BY id
HAVING equals(argMax(is_deleted, version), 0)) AS events__pdi__person ON equals(events__pdi.person_id, events__pdi__person.id)
WHERE and(equals(team_id, 2), less(timestamp, '2020-01-10 12:14:05.000000'), greater(timestamp, '2020-01-09 12:00:00.000000'))
ORDER BY tuple(uuid, event, properties, timestamp, team_id, events.distinct_id, elements_chain, events.created_at, events__pdi.person_id, events__pdi__person.created_at, events__pdi__person.properties___name, events__pdi__person.properties___email) ASC
ORDER BY tuple(uuid, event, properties, timestamp, team_id, distinct_id, elements_chain, created_at) ASC
LIMIT 101
OFFSET 0
'
Expand Down
37 changes: 37 additions & 0 deletions posthog/api/test/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,43 @@ def test_property_filter_aggregations(self):
response = self.client.post(f"/api/projects/{self.team.id}/query/", query.dict()).json()
self.assertEqual(len(response["results"]), 1)

@snapshot_clickhouse_queries
def test_select_event_person(self):
with freeze_time("2020-01-10 12:00:00"):
person = _create_person(
properties={"name": "Tom", "email": "tom@posthog.com"},
distinct_ids=["2", "some-random-uid"],
team=self.team,
immediate=True,
)
_create_event(team=self.team, event="sign up", distinct_id="2", properties={"key": "test_val1"})
with freeze_time("2020-01-10 12:11:00"):
_create_event(team=self.team, event="sign out", distinct_id="2", properties={"key": "test_val2"})
with freeze_time("2020-01-10 12:12:00"):
_create_event(team=self.team, event="sign out", distinct_id="3", properties={"key": "test_val2"})
with freeze_time("2020-01-10 12:13:00"):
_create_event(
team=self.team, event="sign out", distinct_id="4", properties={"key": "test_val3", "path": "a/b/c"}
)
flush_persons_and_events()

with freeze_time("2020-01-10 12:14:00"):
query = EventsQuery(select=["event", "person", "person"])
response = self.client.post(f"/api/projects/{self.team.id}/query/", query.dict()).json()
self.assertEqual(len(response["results"]), 4)
self.assertEqual(response["results"][0][1], {"distinct_id": "4"})
self.assertEqual(response["results"][1][1], {"distinct_id": "3"})
self.assertEqual(response["results"][1][2], {"distinct_id": "3"})
expected_user = {
"uuid": str(person.uuid),
"properties": {"name": "Tom", "email": "tom@posthog.com"},
"distinct_id": "2",
"created_at": "2020-01-10T12:00:00Z",
}
self.assertEqual(response["results"][2][1], expected_user)
self.assertEqual(response["results"][3][1], expected_user)
self.assertEqual(response["results"][3][2], expected_user)

@also_test_with_materialized_columns(event_properties=["key"])
@snapshot_clickhouse_queries
def test_full_hogql_query(self):
Expand Down
69 changes: 36 additions & 33 deletions posthog/models/event/events_query.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import json
from datetime import timedelta
from typing import List, Optional, Tuple
from typing import Dict, List, Optional

from dateutil.parser import isoparse
from django.db.models import Prefetch
from django.utils.timezone import now

from posthog.api.element import ElementSerializer
Expand All @@ -14,6 +15,7 @@
from posthog.hogql.query import execute_hogql_query
from posthog.models import Action, Person, Team
from posthog.models.element import chain_to_elements
from posthog.models.person.util import get_persons_by_distinct_ids
from posthog.schema import EventsQuery, EventsQueryResponse
from posthog.utils import relative_date_parse

Expand All @@ -31,10 +33,6 @@
"distinct_id",
"elements_chain",
"created_at",
"person_id",
"person.created_at",
"person.properties.name",
"person.properties.email",
]


Expand All @@ -58,10 +56,8 @@ def run_events_query(
if col == "*":
select_input.append(f"tuple({', '.join(SELECT_STAR_FROM_EVENTS_FIELDS)})")
elif col == "person":
# Select just enough person fields to show the name/email in the UI. Put it back into a dict later.
select_input.append(
"tuple(distinct_id, person_id, person.created_at, person.properties.name, person.properties.email)"
)
# This will be expanded into a followup query
select_input.append("distinct_id")
else:
select_input.append(col)

Expand Down Expand Up @@ -154,33 +150,40 @@ def run_events_query(
new_result["elements"] = ElementSerializer(
chain_to_elements(new_result["elements_chain"]), many=True
).data
new_result["person"] = {
"id": new_result["person_id"],
"created_at": new_result["person.created_at"],
"properties": {
"name": new_result["person.properties.name"],
"email": new_result["person.properties.email"],
},
"distinct_ids": [new_result["distinct_id"]],
}
del new_result["person_id"]
del new_result["person.created_at"]
del new_result["person.properties.name"]
del new_result["person.properties.email"]
query_result.results[index][star_idx] = new_result

# Convert person field from tuple to dict in each result
if "person" in select_input_raw:
if "person" in select_input_raw and len(query_result.results) > 0:
# Make a query into postgres to fetch person
person_idx = select_input_raw.index("person")
for index, result in enumerate(query_result.results):
person_tuple: Tuple = result[person_idx]
query_result.results[index] = list(result)
query_result.results[index][person_idx] = {
"id": person_tuple[1],
"created_at": person_tuple[2],
"properties": {"name": person_tuple[3], "email": person_tuple[4]},
"distinct_ids": [person_tuple[0]],
}
distinct_ids = list(set(event[person_idx] for event in query_result.results))
persons = get_persons_by_distinct_ids(team.pk, distinct_ids)
persons = persons.prefetch_related(Prefetch("persondistinctid_set", to_attr="distinct_ids_cache"))
distinct_to_person: Dict[str, Person] = {}
for person in persons:
if person:
for person_distinct_id in person.distinct_ids:
distinct_to_person[person_distinct_id] = person

# Loop over all columns in case there is more than one "person" column
for column_index, column in enumerate(select_input_raw):
if column != "person":
continue
for index, result in enumerate(query_result.results):
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 not super clear on what all is going on in these things yet 🙈

This is an in-memory loop not an N+1 query?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are all in memory loops.

To get it to work, just select more than "last 24h"

Copy link
Member

Choose a reason for hiding this comment

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

To get it to work, just select more than "last 24h"

🙈 🤣

Then I get

Screenshot 2023-03-09 at 10 28 49

Copy link
Member

Choose a reason for hiding this comment

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

return !!personRecord.distinct_ids.length ? (

when

{
    "uuid": "01869ed9-85a3-0000-f0dc-c6dbf095aef6",
    "created_at": "2023-03-01T20:24:59.852Z",
    "properties": {
        "name": null,
        "email": "paul@posthog.com"
    },
    "distinct_id": "a0JNkovhz54zqTBN2pBKf8jqA488qudRnugUzVshN2E"
}

Copy link
Member

Choose a reason for hiding this comment

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

Dumping in

else if (key === 'person' && isEventsQuery(query.source)) {
        const personRecord = value as PersonType
        let distinctId = undefined
        if (personRecord.distinct_ids && personRecord.distinct_ids.length > 0) {
            distinctId = personRecord.distinct_ids[0]
        } else if (personRecord.distinct_id) {
            distinctId = personRecord.id
        }
        return distinctId ? (
            <Link to={urls.person(distinctId)}>
                <PersonHeader noLink withIcon person={personRecord} />
            </Link>
        ) : (
            <PersonHeader noLink withIcon person={value} />
        )
    }

and the table displays (although the type system is unhappy)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

whoops. Should be fixed now 🙈

distinct_id: str = result[column_index]
query_result.results[index] = list(result)
if distinct_to_person.get(distinct_id):
person = distinct_to_person[distinct_id]
properties = person.properties or {}
query_result.results[index][column_index] = {
"uuid": person.uuid,
"created_at": person.created_at,
"properties": {"name": properties.get("name"), "email": properties.get("email")},
"distinct_id": distinct_id,
}
else:
query_result.results[index][column_index] = {
"distinct_id": distinct_id,
}

received_extra_row = len(query_result.results) == limit # limit was +=1'd above
return EventsQueryResponse(
Expand Down