feat(data-warehouse): implement the aws cloudtrail import source - #110792
Conversation
|
😎 Merged successfully - details. |
|
Risk: No findings The only change since the last review is a one-line cosmetic fix in a test file: wrapping Sentinel reviewed |
|
Hey @Gilbert09! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
🤖 CI report
|
| First copy | Second copy | Lines | Tokens |
|---|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/sources/adyen/source.py:18 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/source.py:18 |
17 | 143 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_budgets/source.py:140 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/source.py:135 |
26 | 96 |
✅ Duplication (TypeScript) — clean
New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
⚠️ Backend coverage — 99.0% of changed backend lines covered — 4 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ████████████████████ 99.0% (433 / 437)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/source.py |
92.9% | 88, 98 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/aws_cloudtrail.py |
98.8% | 64, 130 |
🤖 Agents: add a test only if an uncovered line exposes a realistic regression that existing tests miss. Otherwise explain why no new test is needed under "How did you test this code?". Gap list: the patch-coverage artifact on this run (gh run download 452598571599154 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
demo |
███████████░░░░░░░░░ 53.4% |
1,445 / 2,707 |
batch_exports |
████████████████░░░░ 81.3% |
21,575 / 26,552 |
cdp |
██████████████████░░ 88.3% |
4,559 / 5,164 |
mcp_analytics |
██████████████████░░ 89.2% |
5,038 / 5,651 |
product_tours |
██████████████████░░ 89.3% |
1,340 / 1,500 |
dashboards |
██████████████████░░ 89.6% |
6,924 / 7,727 |
notebooks |
██████████████████░░ 90.2% |
15,304 / 16,971 |
signals |
██████████████████░░ 90.4% |
60,158 / 66,529 |
cohorts |
██████████████████░░ 90.5% |
8,534 / 9,434 |
data_warehouse |
██████████████████░░ 90.6% |
14,375 / 15,860 |
streamlit_apps |
██████████████████░░ 90.8% |
2,684 / 2,956 |
managed_warehouse |
██████████████████░░ 91.0% |
10,252 / 11,263 |
data_modeling |
██████████████████░░ 91.2% |
10,562 / 11,584 |
tasks |
██████████████████░░ 91.3% |
79,542 / 87,115 |
exports |
██████████████████░░ 91.7% |
9,684 / 10,566 |
business_knowledge |
██████████████████░░ 92.0% |
8,472 / 9,208 |
engineering_analytics |
██████████████████░░ 92.2% |
11,497 / 12,475 |
today |
██████████████████░░ 92.3% |
999 / 1,082 |
early_access_features |
███████████████████░ 92.6% |
1,339 / 1,446 |
conversations |
███████████████████░ 92.6% |
29,225 / 31,559 |
stamphog |
███████████████████░ 92.8% |
8,109 / 8,742 |
canvas |
███████████████████░ 92.9% |
7,155 / 7,703 |
approvals |
███████████████████░ 93.0% |
3,974 / 4,271 |
mcp_registry |
███████████████████░ 93.1% |
1,670 / 1,794 |
notifications |
███████████████████░ 93.2% |
1,144 / 1,228 |
error_tracking |
███████████████████░ 93.3% |
16,389 / 17,573 |
surveys |
███████████████████░ 93.4% |
6,644 / 7,113 |
autoresearch |
███████████████████░ 93.6% |
8,837 / 9,442 |
slack_app |
███████████████████░ 93.7% |
14,611 / 15,600 |
context_layer |
███████████████████░ 93.8% |
3,415 / 3,639 |
web_analytics |
███████████████████░ 93.9% |
23,680 / 25,229 |
billing_alerts |
███████████████████░ 94.1% |
2,094 / 2,226 |
mcp_store |
███████████████████░ 94.3% |
8,959 / 9,501 |
wizard |
███████████████████░ 94.7% |
6,150 / 6,496 |
alerts |
███████████████████░ 94.7% |
9,320 / 9,844 |
ai_observability |
███████████████████░ 94.7% |
25,860 / 27,307 |
workflows |
███████████████████░ 94.7% |
15,187 / 16,034 |
reminders |
███████████████████░ 94.8% |
760 / 802 |
review_hog |
███████████████████░ 95.0% |
11,750 / 12,362 |
annotations |
███████████████████░ 95.1% |
817 / 859 |
endpoints |
███████████████████░ 95.1% |
9,234 / 9,706 |
customer_analytics |
███████████████████░ 95.2% |
26,116 / 27,428 |
marketing_analytics |
███████████████████░ 95.3% |
19,450 / 20,413 |
posthog_ai |
███████████████████░ 95.4% |
2,530 / 2,653 |
actions |
███████████████████░ 95.5% |
756 / 792 |
logs |
███████████████████░ 95.5% |
15,468 / 16,200 |
experiments |
███████████████████░ 95.5% |
33,000 / 34,548 |
data_catalog |
███████████████████░ 95.6% |
4,402 / 4,606 |
tracing |
███████████████████░ 95.6% |
3,536 / 3,699 |
replay_vision |
███████████████████░ 95.6% |
29,371 / 30,708 |
growth |
███████████████████░ 95.7% |
11,381 / 11,888 |
skills |
███████████████████░ 95.8% |
6,972 / 7,274 |
messaging |
███████████████████░ 95.9% |
3,834 / 3,999 |
product_analytics |
███████████████████░ 96.0% |
28,521 / 29,696 |
revenue_analytics |
███████████████████░ 96.4% |
1,889 / 1,959 |
user_interviews |
███████████████████░ 96.5% |
2,870 / 2,974 |
feature_flags |
███████████████████░ 96.6% |
27,119 / 28,060 |
access_control |
███████████████████░ 96.7% |
7,739 / 8,007 |
warehouse_sources |
███████████████████░ 97.3% |
471,293 / 484,348 |
data_quality |
████████████████████ 97.5% |
7,701 / 7,895 |
metrics |
████████████████████ 98.0% |
4,252 / 4,338 |
analytics_platform |
████████████████████ 98.3% |
2,784 / 2,833 |
pulse |
████████████████████ 98.5% |
2,046 / 2,078 |
live_debugger |
████████████████████ 99.2% |
626 / 631 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (8)📝 WalkthroughWalkthroughAdds AWS CloudTrail as an implemented source with four endpoints. The source sends signed API requests, validates credentials, supports incremental time windows and resumable pagination, and exposes endpoint schemas and descriptions. The change also adds source configuration and tests for requests, pagination, resume behavior, and errors. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to The CloudTrail source is usable, but its setup link points to a missing documentation route. Add the guide or remove the link before users rely on it. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The connector adds access to AWS credentials and audit data, but constrains requests to regional AWS endpoints and signs them using the supplied credentials. No concrete new attack path was established. Tenant-to-credential authorization and concurrent recovery isolation remain incompletely verified. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
products/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/source.py (1)
64-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the import to module level.
get_canonical_descriptionsimportsCANONICAL_DESCRIPTIONSinside the method.canonical_descriptions.pyonly defines static dicts, so a lazy import gives no benefit. The repository rule requires module-level imports.♻️ Proposed fix
def get_canonical_descriptions(self) -> CanonicalDescriptions: - from products.warehouse_sources.backend.temporal.data_imports.sources.aws_cloudtrail.canonical_descriptions import ( - CANONICAL_DESCRIPTIONS, - ) - return CANONICAL_DESCRIPTIONSAdd
from ...aws_cloudtrail.canonical_descriptions import CANONICAL_DESCRIPTIONSat the top of the file.As per coding guidelines: "Always place imports at the top of the file (module level), never inside functions or methods (local imports)".
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 1b5042a8-7c3c-4f30-9d96-b04053d8020b
📒 Files selected for processing (7)
products/warehouse_sources/backend/temporal/data_imports/sources/SOURCES.mdproducts/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/aws_cloudtrail.pyproducts/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/canonical_descriptions.pyproducts/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/settings.pyproducts/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/source.pyproducts/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/tests/test_aws_cloudtrail.pyproducts/warehouse_sources/backend/temporal/data_imports/sources/generated_configs/awscloudtrail.py
Limit details: You’ve used all 12 included reviews currently available.
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Approved.
This is a self-contained new warehouse source, authored by a STRONG-familiarity owning-team member, and it follows the existing aws_organizations pattern. It validates the region before building the endpoint URL, disables redirects, redacts credentials, and has thorough tests. Reviewer feedback was only a style nitpick.
- Author wrote 86% of the modified lines and has 102 merged PRs in these paths (familiarity STRONG).
- Style nit from CodeRabbit: the canonical descriptions import inside get_canonical_descriptions could move to module level. This is not blocking.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 576L, 5F substantive, 980L/7F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (980L, 7F, single-area, feat) |
| stamphog 2.3.1 | .stamphog/policy.yml @ e5de864 · reviewed head e5de864 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Add the aws-cloudtrail setup guide before linking to it. · source.py:133
products/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/source.py:133
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd the
aws-cloudtrailsetup guide before linking to it.The source-doc audit requires
docsUrlslugs to map to Markdown or MDX documentation files. Noaws-cloudtraildocumentation route exists in this checkout, so users can reach a missing setup guide. Add theaws-cloudtrailguide at the linked route, or remove this URL until the guide exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 39878644-76f9-4bbb-9d04-bc649189d889
📒 Files selected for processing (1)
products/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/tests/test_aws_cloudtrail.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 4 remain after this review.
|
All CI checks are green and there are no unresolved review threads. GitHub still reports the PR as blocked with the requested Team Warehouse Sources review outstanding; a reviewer from that team needs to approve it. 🦉 via talyn.dev |
|
/trunk merge |
Deploy status
|
Problem
Changes
eventsinsight_eventstrailsevent_data_storesaws_organizationssource.LookupEventsonly returns the last 90 days of management events.unreleasedSourceremoved.SOURCES.mdrow.How did you test this code?
Test rationale: the source is new, so no existing test covers its transport.
Release status
Automatic notifications
Docs update