feat(data-warehouse): add logs/retry debug and sectioned webhook tab#60704
Conversation
|
Size Change: -4.6 kB (-0.01%) Total Size: 80.9 MB 📦 View Changed
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Pull request overview
Adds a sectioned webhook source tab and embeds hog function log/retry tooling so users can debug webhook activity from the data warehouse source page.
Changes:
- Adds webhook section state synced via
?section=. - Reworks the webhook tab into Overview, Configuration, and Activity sections.
- Adds a standalone
WebhookLogsSectionwith log status, event links, and retry actions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
products/data_warehouse/frontend/scenes/SourceScene/tabs/webhookTabLogic.ts |
Adds section constants, reducer state, and URL synchronization. |
products/data_warehouse/frontend/scenes/SourceScene/tabs/WebhookTab.tsx |
Introduces the sidebar layout, section-specific content, and collapsible enabled events. |
products/data_warehouse/frontend/scenes/SourceScene/tabs/WebhookLogsSection.tsx |
Adds reusable logs and retry UI for webhook-backed hog functions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
products/data_warehouse/frontend/scenes/SourceScene/tabs/WebhookLogsSection.tsx:114-126
The status computation only inspects the **last** log entry for the terminal-state strings. The `eventIdByInvocationId` selector in `hogFunctionLogsLogic` uses `.find()` across all entries for the same strings — precisely because they're not guaranteed to be the last message. If any log line is emitted after "Function completed" (debug output, cleanup), the status will show "Running" for a completed request.
```suggestion
const status = useMemo<WebhookLogStatusType>((): WebhookLogStatusType => {
if (thisRetry === 'pending') {
return 'running'
}
if (record.entries.some((e) => e.message.includes('Function completed') || e.message.includes('Execution successful'))) {
return 'success'
}
if (record.entries.some((e) => e.level === 'ERROR')) {
return 'failure'
}
return 'running'
}, [record, thisRetry])
```
### Issue 2 of 3
products/data_warehouse/frontend/scenes/SourceScene/tabs/WebhookLogsSection.tsx:107-110
`hogFunctionLogsLogic(logicProps)` is called twice — once for `useValues` and once for `useActions`. The parent `WebhookLogsSection` already shows the cleaner pattern of extracting the logic instance first. This is superfluous repetition.
```suggestion
const logic = hogFunctionLogsLogic(logicProps)
const { retries, selectingMany, selectedForRetry, eventIdByInvocationId } = useValues(logic)
const { retryInvocations, setSelectingMany, setSelectedForRetry } = useActions(logic)
```
### Issue 3 of 3
products/data_warehouse/frontend/scenes/SourceScene/tabs/WebhookLogsSection.tsx:88
The lambda `(message) => renderHogFunctionMessage(message)` is a redundant wrapper — the function can be passed by reference directly.
```suggestion
renderMessage={renderHogFunctionMessage}
```
Reviews (1): Last reviewed commit: "feat(data-warehouse): add logs/retry deb..." | Re-trigger Greptile |
|
It looks like the code of |
MCP UI Apps size report
|
PR overviewThis PR adds debugging improvements around data warehouse logs and retry behavior, and updates the webhook tab to be organized into sections. It also touches external data source schema discovery paths used by the data warehouse integration flow. The PR has made progress with 2 issues already addressed, but 1 security issue remains open. The remaining gap allows users who are not eligible for Custom sources to invoke the database schema discovery path with a Custom source type, causing the server to validate credentials and make server-side requests to manifest URLs. Applying the same availability check used during source creation to the schema discovery action would close the bypass. Open issues (1)
Fixed/addressed: 2 · PR risk: 6/10 |
Migration SQL ChangesHey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:
|
There was a problem hiding this comment.
Supply Chain Security Review
✅ Approve — 1 finding in 1 file
Dependency changes are routine version bumps of established packages. The recency-flagged hogql_parser_rs 1.3.82 is a first-party package built from source in this repo (publish = false). The PR fixes 29 known vulnerabilities (node-forge, express, turbo, happy-dom, xmldom, etc.) and removes unused packages. Workflow changes use SHA-pinned actions with minimal permissions.
Tag @mendral-app with feedback or questions. View session
Restructure the data warehouse webhook source tab into Overview / Configuration / Activity sidebar sections (mirroring the schema config scene), collapse the "Listening to N events" list by default, and add a logs + retry panel that reuses the hog function LogsViewer and retry logic for debugging failed webhook requests. renderHogFunctionMessage lives in its own module so importers don't pull hogFunctionTestLogic (which calls getCurrentTeamId at module load) into their tree, which used to break unrelated jest suites. Generated-By: PostHog Code Task-Id: b030b433-e0f1-4871-baf1-59fcdb6f4f32
97f3928 to
c8ec6c6
Compare
✅ Hobby deploy smoke test: PASSEDHobby deployment smoke test passed successfully. |
|
Heads-up: the PR #60442 ( |
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
Problem
The data warehouse webhook source tab had grown into one long scrolling page mixing status, configuration, mapped tables, metrics and a danger zone. Two pain points:
Changes
Restructures the webhook tab to mirror the schema config scene's layout and reuse existing hog function debug tooling.
?section=so a specific view (e.g. Activity for debugging) can be shared.WebhookLogsSectionembeds the sameLogsViewerused on hog function destination pages (keyed by the webhook's hog function id), with a per-row status column and per-row + batch Retry actions, reusinghogFunctionLogsLogic. It's a slimmed-down standalone version ofHogFunctionLogswithout the configuration/test-logic dependencies that don't apply here.LemonCollapsepanel that expands to the full tag list.No new feature flags — the hog function logs/metrics/retry UI is already stable and was partially reused here (metrics) before this change.
How did you test this code?
I'm an agent. The frontend
node_modulesweren't installed in my environment, so I could not runtsc,kea-typegen, or the linter locally — these need to pass in CI (includingtypegen:check, since the generated*Type.tsfile is gitignored and regenerated by CI). No automated tests were added. The changes still need a manual pass in the running app against a real webhook source to confirm the section navigation, collapsible events, and logs/retry behavior.Automatic notifications
Docs update
No docs changes.
🤖 Agent context
Authored with Claude Code (Opus 4.7). Approach: explored the existing webhook tab, the reusable hog function logs/metrics/retry components, and the schema config scene's section layout before implementing.
Decisions:
SourceSceneand restructure its body with the schema scene's sidebar-section pattern, rather than promoting it to a standalone routed scene — lighter change, no new routes.LogsViewer+hogFunctionLogsLogicdirectly instead ofHogFunctionLogs, since that wrapper depends onhogFunctionConfigurationLogic/hogFunctionTestLogicwhich aren't mounted on this page.Human review required; not self-merged.