Added top_os.pipe to tinybird and bumped all pipes to version 1
#22290
Conversation
WalkthroughThe update applies changes across several analytics pipeline files in the ghost/tinybird-dedicated repository. In each modified pipe, a new SQL case statement has been introduced to derive the operating system (os) from the user agent string, categorizing it as 'windows', 'mac', 'ios', 'android', or 'Unknown'. This new field is added to the SELECT and GROUP BY clauses or used in aggregation tokens. Additionally, conditional SQL clauses checking for a defined os have been implemented to enable filtering based on the operating system. Version numbers in multiple files have been incremented from 0 to 1 to reflect these changes. Furthermore, a new pipeline file, top_os.pipe, has been created to aggregate and display operating system statistics, incorporating parameters for date range and pagination. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (12)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
ghost/tinybird-dedicated/pipes/kpis.pipe (1)
166-166:⚠️ Potential issueMissing OS field in GROUP BY clause for single-day case.
The GROUP BY clause for the single-day case doesn't include the
osfield, while it's been added to the SELECT clause. This inconsistency could lead to aggregation issues since the output includesosbut doesn't group by it.- group by toStartOfHour(timestamp), session_id, site_uuid, member_status, device, browser, location, source, pathname + group by toStartOfHour(timestamp), session_id, site_uuid, member_status, device, browser, os, location, source, pathname
🧹 Nitpick comments (1)
ghost/tinybird-dedicated/pipes/top_os.pipe (1)
1-45: New pipe implementation looks good, with two minor issuesThe new top_os.pipe follows the same structure and conventions as other similar pipes, which is excellent for consistency. The SQL query correctly groups by OS and calculates visits and pageviews.
There are two minor issues to address:
Line 28: The default date filter is set to 1 year (
interval -1 year), which differs from the 7-day default mentioned in the description and used in other pipes.Line 38: The description for the OS filter incorrectly says "Browser to filter on" instead of "Operating System to filter on".
- and date >= timestampAdd(today(), interval -1 year) + and date >= timestampAdd(today(), interval -7 day)- {% if defined(os) %} and os = {{ String(os, description="Browser to filter on", required=False) }} {% end %} + {% if defined(os) %} and os = {{ String(os, description="Operating System to filter on", required=False) }} {% end %}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
ghost/tinybird-dedicated/pipes/analytics_pages.pipe(4 hunks)ghost/tinybird-dedicated/pipes/analytics_sessions.pipe(3 hunks)ghost/tinybird-dedicated/pipes/analytics_sources.pipe(4 hunks)ghost/tinybird-dedicated/pipes/kpis.pipe(6 hunks)ghost/tinybird-dedicated/pipes/top_browsers.pipe(2 hunks)ghost/tinybird-dedicated/pipes/top_devices.pipe(2 hunks)ghost/tinybird-dedicated/pipes/top_locations.pipe(1 hunks)ghost/tinybird-dedicated/pipes/top_os.pipe(1 hunks)ghost/tinybird-dedicated/pipes/top_pages.pipe(2 hunks)ghost/tinybird-dedicated/pipes/top_sources.pipe(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- ghost/tinybird-dedicated/pipes/top_locations.pipe
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Database tests (Node 20.11.1, sqlite3)
- GitHub Check: Regression tests (Node 20.11.1, sqlite3)
- GitHub Check: Database tests (Node 22.13.1, mysql8)
- GitHub Check: Regression tests (Node 20.11.1, mysql8)
- GitHub Check: Database tests (Node 20.11.1, mysql8)
- GitHub Check: Database tests (Node 18.12.1, mysql8)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Unit tests (Node 22.13.1)
- GitHub Check: Unit tests (Node 20.11.1)
- GitHub Check: Lint
- GitHub Check: Unit tests (Node 18.12.1)
- GitHub Check: i18n
🔇 Additional comments (23)
ghost/tinybird-dedicated/pipes/top_browsers.pipe (2)
1-1: Version increment is consistent with PR objectivesThe version has been properly incremented from 0 to 1 as stated in the PR description.
38-38: OS filter implementation follows existing patternThe new OS filter follows the same conditional pattern used for other filters in this file. The parameter is correctly defined with an appropriate description and marked as optional.
ghost/tinybird-dedicated/pipes/analytics_sources.pipe (3)
1-1: Version increment is consistent with PR objectivesThe version has been properly incremented from 0 to 1 as stated in the PR description.
63-73: OS detection implementation is appropriateThe case statement for OS detection follows the same pattern as the existing device and browser detection. The implementation is intentionally simplistic as mentioned in the PR objectives, since the focus is on change management processes rather than robust OS detection.
94-94: OS field correctly added to both SELECT and GROUP BY clausesThe OS field has been properly added to both the SELECT clause and GROUP BY clause, which is necessary for correct aggregation in the analytics pipeline.
Also applies to: 103-103
ghost/tinybird-dedicated/pipes/top_sources.pipe (2)
1-1: Version increment is consistent with PR objectivesThe version has been properly incremented from 0 to 1 as stated in the PR description.
38-38: OS filter implementation follows existing patternThe new OS filter follows the same conditional pattern used for other filters in this file. The parameter is correctly defined with an appropriate description and marked as optional.
ghost/tinybird-dedicated/pipes/top_pages.pipe (2)
1-1: Version increment is consistent with PR objectivesThe version has been properly incremented from 0 to 1 as stated in the PR description.
42-42: OS filter implementation follows existing patternThe new OS filter follows the same conditional pattern used for other filters in this file. The parameter is correctly defined with an appropriate description and marked as optional.
ghost/tinybird-dedicated/pipes/analytics_sessions.pipe (3)
1-1: Version incrementing looks goodThe version has been properly updated from 0 to 1 to reflect the changes made to the pipe.
67-78: OS detection implementation looks appropriateThe implementation for parsing operating systems from user agent strings is straightforward and follows the same pattern as the existing browser detection logic. As mentioned in the PR objectives, this implementation is intentionally simplistic as the main goal is to understand change control in Tinybird.
100-100: OS aggregation added correctlyThe OS field has been properly added to the analytics_sessions_1 node using the same anySimpleState pattern as other fields.
ghost/tinybird-dedicated/pipes/analytics_pages.pipe (3)
1-1: Version incrementing looks goodThe version has been properly updated from 0 to 1 to reflect the changes made to the pipe.
63-74: OS detection implementation is consistentThe OS detection logic is consistent with the implementation in analytics_sessions.pipe, applying the same pattern matching approach for identifying operating systems from user agent strings.
88-88: OS field properly added to SELECT and GROUP BY clausesThe OS field has been correctly added to both the SELECT statement and GROUP BY clause, enabling proper aggregation and filtering by operating system.
Also applies to: 99-99
ghost/tinybird-dedicated/pipes/top_devices.pipe (2)
1-1: Version incrementing looks goodThe version has been properly updated from 0 to 1 to reflect the changes made to the pipe.
39-39: OS filter condition added correctlyThe conditional clause for filtering by operating system follows the same pattern as the existing filters and is properly implemented.
ghost/tinybird-dedicated/pipes/kpis.pipe (6)
1-1: Version increment appropriately reflects the added functionality.The version number has been bumped from 0 to 1, which aligns with the PR objective of updating all pipes to version 1 for the OS filtering experiment.
128-138: OS detection implementation is straightforward and consistent with existing patterns.The case statement for OS detection follows the same pattern as the browser detection above it. While the implementation is intentionally simplistic (as noted in the PR objectives), it provides the necessary functionality for this experiment.
155-155: OS field correctly added to SELECT clause for single-day case.The addition of the
osfield to the SELECT clause for the single-day case ensures the OS information is included in the results.
174-174: OS field correctly added to SELECT clause for multi-day case.The addition of the
osfield to the SELECT clause for the multi-day case ensures the OS information is included in the results.
192-192: OS field correctly added to GROUP BY clause for multi-day case.The GROUP BY clause for the multi-day case now includes the
osfield, which is necessary for proper aggregation.
214-214: OS filtering capability correctly implemented.The conditional filter for the
osvariable follows the same pattern as other filters in the file. This enables filtering analytics data by operating system, which is one of the main objectives of this PR.
ref https://linear.app/ghost/issue/ANAL-128/experiment-update-the-implementation-to-also-store-os-for-each-event
top_os.jsonpipe to aggregate pageviews by operating system, and updates all other relevant pipes so they can be filtered/grouped by operating system