feat: Support EventMetadata filter for EventsQuery/TrendsQuery by danielbachhuber · Pull Request #30232 · PostHog/posthog

danielbachhuber · 2025-03-20T15:59:45Z

See #29881
See https://posthog.slack.com/archives/C0368RPHLQH/p1742417106336409

Changes

Adds support for filtering EventsQuery or TrendsQuery by a new EventMetadata filter:

CleanShot.2025-03-21.at.10.27.10.mp4

The happy path works but I'm not sure how many gotchas I might be missing.

How did you test this code?

Added backend tests.

greptile-apps

PR Summary

This PR adds support for filtering events by metadata properties in EventsQuery and TrendsQuery, enabling direct filtering on fields like distinct_id and timestamp.

Key changes:

Added new EventMetadata filter type in frontend/src/types.ts with corresponding taxonomic group support
Implemented metadata property handling in posthog/hogql/property.py for direct field access
Added test coverage in test_events_query_runner.py and test_trends_query_runner.py for metadata filtering
Updated property validation in posthog/models/property/property.py to support event_metadata type
Extended taxonomic filter logic with new metadata options and icons in taxonomicFilterLogic.tsx

_{12 file(s) reviewed, 2 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-03-20T16:00:30Z

frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx

+                        options: eventMetadataOptions,
+                        getIcon: (option: Record<string, string>) => option.icon,
+                        getName: (option: Record<string, string>) => option.name,
+                        getValue: (option: Record<string, string>) => option.value,
+                        getPopoverHeader: () => 'Event metadata',


logic: Missing type validation for option.icon - could cause runtime errors if icon is not a valid React element

greptile-apps · 2025-03-20T16:01:21Z

posthog/hogql/property.py

+        return _expr_to_compare_op(
+            expr=ast.Field(chain=[property.key]),
+            value=value,
+            operator=operator,
+            team=team,
+            property=property,
+            is_json_field=False,
+        )


logic: Missing boolean value handling for event metadata fields

mariusandra

Cool stuff, left some thoughts inline.

mariusandra · 2025-03-20T19:10:24Z

frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx

+                    { icon: <IconLogomark />, name: 'Distinct ID', value: 'distinct_id' },
+                    { icon: <IconLogomark />, name: 'Timestamp', value: 'timestamp' },


Can you also add event here? Perhaps also person_id if it works well.

Thinking out loud...

While I think it makes total sense to have a new event metadata TaxonomicFilterGroupType for this new tab/pill... I wonder if this is the right PropertyFilterType to add. It feels a bit restrictive. More specifically, I wonder if EventMetadataPropertyFilter could be merged with HogQLPropertyFilter.

Looking at this list of options here, we effectively have expression operator value, where the expression comes from a predefined list of SQL expressions. There's no backend validation... so it's effectively a generic SQLValueFilter or something like that.

The existing HogQLPropertyFilter does the expression part as its key. We could just add operator and value into it... or if making a new property filter, to make it more abstract in its name.

Can you also add event here? Perhaps also person_id if it works well.

Added with 94d3026. Both work fine.

The existing HogQLPropertyFilter does the expression part as its key. We could just add operator and value into it... or if making a new property filter, to make it more abstract in its name.

Doesn't that mean we'd need to apply this logic and this logic in the client?

Doesn't that mean we'd need to apply this logic and this logic in the client?

Hmm... well, no... it'd work exactly like the filter you now added works, just renamed to HogQLPropertyValueFilter or something more abstract.

My worry is that people will want to start filtering for all sorts of other and custom expressions. However I do like that this is scoped to just events, making some things a lot easier than if it'd be totally abstract...

posthog/hogql/property.py

aspicer · 2025-03-20T20:51:34Z

Do we want to call this "$group_2" in the customer facing UI? I don't think we use that terminology anywhere and it's quite an implementation detail to expose.

"Organization ID" might make more sense? I'm not sure we want to surface that group naming anywhere?

posthog-bot · 2025-03-20T20:52:12Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

aspicer · 2025-03-20T20:53:35Z

This looks awesome. Much overdue.

posthog-bot · 2025-03-20T21:02:30Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions · 2025-03-20T21:14:21Z

Size Change: -194 kB (-1.95%)

Total Size: 9.76 MB

Filename	Size	Change
`frontend/dist/toolbar.js`	9.76 MB	-194 kB (-1.95%)

_{compressed-size-action}

danielbachhuber · 2025-03-20T23:46:29Z

Do we want to call this "$group_2" in the customer facing UI? I don't think we use that terminology anywhere and it's quite an implementation detail to expose.

"Organization ID" might make more sense? I'm not sure we want to surface that group naming anywhere?

@aspicer My intent was to ease this transition:

Specifically, when the user selects "organization" in the UI and then sees $group_2 in the filter.

It looks like getCoreFilterDefinition() is where the label is fetched:

posthog/frontend/src/lib/components/PropertyKeyInfo.tsx

Line 37 in 55db33c

const coreDefinition = getCoreFilterDefinition(value, type)

posthog/frontend/src/lib/taxonomy.tsx

Lines 1915 to 1921 in 55db33c

    
           export function getCoreFilterDefinition( 
        
               value: string | PropertyFilterValue | undefined, 
        
               type: TaxonomicFilterGroupType 
        
           ): CoreFilterDefinition | null { 
        
               if (value == undefined) { 
        
                   return null 
        
               }

We could fetch the group type mapping labels dynamically but I don't love the idea of adding a call to a logic. It seems like too many of our components have random dependencies on logics.

I do think $group_2 is the value we should store in the filter, though (not group_type).

Thoughts?

mariusandra · 2025-03-21T08:06:04Z

It's definitely better for the user to see a clearly labelled "Organization ID" rather than a cryptic "$group_2", so I think it's worth taking whatever extra steps are needed to make it work. We should have all the group names in memory normally as well.

mariusandra

I had a play around. While it did filter when I entered stuff to the fields, there's no autocomplete:

Plus the timestamp field is not treated as a datetime field (no date/time controls, before/after, etc)

This is what I'd expect:

posthog-bot · 2025-03-24T13:13:14Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

danielbachhuber · 2025-03-24T13:15:39Z

I'm willing to 🙈 on the part of it all going through property definitions, however I'd see if there's any way to avoid those "loading"/"missing" states (those kea actions in one of the last gifs).

@mariusandra I think I fixed this with fcfba12 and 0fda1bc (at least, I don't see the erroneous property_definitions?limit=50&properties=event&type=event API request anymore).

Perhaps avoid going through checkOrLoadPropertyDefinition altogether?

I tried this initially with e5f6405 but had to change it in bef7375 because of some bug (don't recall exactly). Is there some alternative you had in mind?

posthog-bot · 2025-03-24T13:23:46Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

mariusandra

The person_id ~~filter~~ dropdown doesn't work:

and I left a few other comments inline.

As for skipping checkOrLoadPropertyDefinition, well, just prepopulate propertyDefinitionStorage with the metadata properties and that should be enough, no? One trick you can use is to rename the propertyDefinitionStorage reducer to rawPropertyDefinitionStorage, and then add a new selector called propertyDefinitionStorage that merges rawPropertyDefinitionStorage and eventMetadataPropertyDefinitions into one object.

frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx

posthog/api/event.py

danielbachhuber · 2025-03-24T16:11:28Z

The person_id ~~filter~~ dropdown doesn't work:

@mariusandra Good catch, fixed with fdf4a12

As for skipping checkOrLoadPropertyDefinition, well, just prepopulate propertyDefinitionStorage with the metadata properties and that should be enough, no? One trick you can use is to rename the propertyDefinitionStorage reducer to rawPropertyDefinitionStorage, and then add a new selector called propertyDefinitionStorage that merges rawPropertyDefinitionStorage and eventMetadataPropertyDefinitions into one object.

Ah, that's quite nice. Thanks for the suggestion! Updated with 070a82d

danielbachhuber · 2025-03-24T17:44:30Z

I think I fixed this with fcfba12 and 0fda1bc (at least, I don't see the erroneous property_definitions?limit=50&properties=event&type=event API request anymore).

Kind of ugly, but I also had to do 1efb7ee. Otherwise, the group filters end up with empty values:

mariusandra

This has been a big undertaking and thanks for putting up with all my comments and remarks. However I think we made it! 🙌

I still left one comment inline about merging the objects in taxonomy.tsx, however the rest LGTM! So merge whenever you're ready! 👍

mariusandra · 2025-03-25T08:19:06Z

frontend/src/lib/taxonomy.tsx

+    event_metadata: {
+        person_id: {
+            label: 'Person ID',
+            description: 'The ID of the person, depending on the person properties mode.',
+            examples: ['16ff262c4301e5-0aa346c03894bc-39667c0e-1aeaa0-16ff262c431767'],
+        },
+        // Other values are assigned below
+    },


Hmm... I'm not too sure, but it looks like it was added in this PR

Looking at the code:

... it does seem like "distinct_id" was explicitly moved out of the "event_properties" tab because it wasn't a property, but it was more like (event) "metadata". So I think the intent is the same. As for where is it used, I could only find the CORE_FILTER_DEFINITIONS_BY_GROUP.event_properties.distinct_id = CORE_FILTER_DEFINITIONS_BY_GROUP.metadata.distinct_id line right now.

I think it's safe to merge the two... unless we get any immediate typescript errors, probably the worst that can happen is some missing descriptions, which can be fixed later... I also think it's worth merging, as otherwise people will lose time wondering the same question 2 years down the line :D.

posthog/api/event.py

This reverts commit bd93bec.

danielbachhuber added 3 commits March 20, 2025 08:18

First pass at event metadata filter support

3fde145

Dynamically add group types

e5f9453

Display the icon

da79dfc

danielbachhuber requested review from aspicer, mariusandra and thmsobrmlr March 20, 2025 15:59

greptile-apps bot reviewed Mar 20, 2025

View reviewed changes

danielbachhuber added 2 commits March 20, 2025 09:32

Fix type issue

e8206dd

Fix other type issue

39297e5

mariusandra reviewed Mar 20, 2025

View reviewed changes

danielbachhuber added 3 commits March 20, 2025 13:00

Consolidate logic

b68ac92

Add event and person_id

94d3026

Hacky workaround for type issue

4df1198

danielbachhuber requested a review from mariusandra March 20, 2025 20:42

Update UI snapshots for chromium (1)

8710c8b

Update UI snapshots for chromium (1)

55db33c

mariusandra requested changes Mar 21, 2025

View reviewed changes

danielbachhuber added 6 commits March 21, 2025 04:21

Merge branch 'master' into crm/event-metadata

249c180

Reorder

b090c7b

Persist the group type name as the label

226c663

Display definition popover

747f92d

Monkey-patch eventMetadata into property definitions

e5f6405

Special case behavior for the group type name

43e6b22

danielbachhuber and others added 4 commits March 24, 2025 06:07

Ensure propertyDefinitionType is passed through in more places

0fda1bc

Just a 'Type'

df418a0

Handle is_column=false

4a92888

Update UI snapshots for chromium (1)

5a67627

Update UI snapshots for chromium (1)

76a00e9

danielbachhuber requested a review from mariusandra March 24, 2025 13:45

mariusandra requested changes Mar 24, 2025

View reviewed changes

frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx Show resolved Hide resolved

posthog/api/event.py Outdated Show resolved Hide resolved

danielbachhuber added 3 commits March 24, 2025 08:53

Handle person_id auto-complete

fdf4a12

Encode the query parameter

e09495d

Refactor to merge at access

070a82d

danielbachhuber requested a review from mariusandra March 24, 2025 16:14

danielbachhuber added 2 commits March 24, 2025 09:22

Merge branch 'master' into crm/event-metadata

a66f92d

Pass group type index through so values aren't nooped

1efb7ee

mariusandra approved these changes Mar 25, 2025

View reviewed changes

danielbachhuber added 7 commits March 25, 2025 05:00

Merge branch 'master' into crm/event-metadata

e8fb656

Move person_id into metadata

1aea097

Always apply the not null comparison

9bf1d91

Fix type issue

a728bcc

Use static values to prevent racy behavior

bd93bec

Revert "Use static values to prevent racy behavior"

a14adf5

This reverts commit bd93bec.

Update tests for non-deterministic return order

495346e

danielbachhuber merged commit 9d0f8ee into master Mar 25, 2025
111 of 112 checks passed

danielbachhuber deleted the crm/event-metadata branch March 25, 2025 13:42

danielbachhuber mentioned this pull request Mar 26, 2025

fix(crm): Ensure filter values display when group_type_index=0 #30438

Merged

danielbachhuber mentioned this pull request Apr 3, 2025

feat: Add 'Event metadata' as a breakdown filter #30784

Merged

		{ icon: <IconLogomark />, name: 'Distinct ID', value: 'distinct_id' },
		{ icon: <IconLogomark />, name: 'Timestamp', value: 'timestamp' },

Conversation

danielbachhuber commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

How did you test this code?

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

greptile-apps bot Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

mariusandra left a comment

Choose a reason for hiding this comment

Uh oh!

mariusandra Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

mariusandra Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

danielbachhuber Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

mariusandra Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aspicer commented Mar 20, 2025

Uh oh!

posthog-bot commented Mar 20, 2025

📸 UI snapshots have been updated

Uh oh!

aspicer commented Mar 20, 2025

Uh oh!

posthog-bot commented Mar 20, 2025

📸 UI snapshots have been updated

Uh oh!

github-actions bot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielbachhuber commented Mar 20, 2025

Uh oh!

mariusandra commented Mar 21, 2025

Uh oh!

mariusandra left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

posthog-bot commented Mar 24, 2025

📸 UI snapshots have been updated

Uh oh!

danielbachhuber commented Mar 24, 2025

Uh oh!

posthog-bot commented Mar 24, 2025

📸 UI snapshots have been updated

Uh oh!

mariusandra left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

danielbachhuber commented Mar 24, 2025

Uh oh!

danielbachhuber commented Mar 24, 2025

Uh oh!

mariusandra left a comment

Choose a reason for hiding this comment

Uh oh!

mariusandra Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

danielbachhuber commented Mar 20, 2025 •

edited

Loading

mariusandra Mar 21, 2025 •

edited

Loading

github-actions bot commented Mar 20, 2025 •

edited

Loading

mariusandra left a comment •

edited

Loading

mariusandra left a comment •

edited

Loading