Skip to content

[WIP] Support funnel by hosts #3378

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

reynaldichernando
Copy link

Addresses #3372

  • Add hosts input to funnel report form
  • Adjust column query logic to use switch case
  • Adjust query to support hostname filtering

The bulk of the change is in the query, in the first level I'm querying all the necessary field which is available for filtering which currently are url_path, event_name, and the newly added hostname.

The join query is done on the first level because we need the hostname value from session table, plus instead of performing more join down the levels, we are simply querying off of previous levels.

Copy link

vercel bot commented Apr 30, 2025

@reynaldichernando is attempting to deploy a commit to the umami-software Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR adds hostname filtering capability to the funnel report feature, allowing users to track user journeys across different domains or subdomains.

Key changes:

  • Added 'hosts' option in FunnelStepAddForm.tsx dropdown menu for hostname filtering
  • Modified getFunnel.ts to include hostname data through session table joins
  • Implemented switch case logic to handle different filter types (URL, event, hostname)
  • Warning: ClickHouse query implementation needs updates to properly support hostname filtering
  • Performance consideration: First-level join strategy may need optimization for large datasets

The implementation provides the requested functionality but requires attention to the ClickHouse query portion and potential performance impacts before being production-ready.

💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!

2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +111 to 114
and l.created_at between l.created_at and ${getAddIntervalQuery(
`l.created_at `,
`${windowMinutes} minute`,
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Self-referential date comparison could cause incorrect funnel tracking. Should compare against the original event's timestamp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant