Skip to content

feat(experimentation): enable Firehose CloudWatch error logging - #8068

Merged
gagantrivedi merged 1 commit into
mainfrom
feat/firehose-cloudwatch-error-logging
Jul 22, 2026
Merged

feat(experimentation): enable Firehose CloudWatch error logging#8068
gagantrivedi merged 1 commit into
mainfrom
feat/firehose-cloudwatch-error-logging

Conversation

@gagantrivedi

Copy link
Copy Markdown
Member

Thanks for submitting a PR! Please check the boxes below:

  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

Enables Amazon Data Firehose CloudWatch error logging on the per-organisation delivery streams, as AWS recommends, so delivery failures are diagnosable.

  • Creates a per-organisation CloudWatch log group (/aws/kinesisfirehose/events-ingestion-org-{id}) and DestinationDelivery log stream when provisioning, and removes them on teardown.
  • Sets CloudWatchLoggingOptions on the Firehose delivery stream's S3 destination.

Depends on matching Pulumi IAM changes: the Firehose delivery role needs logs:PutLogEvents, and the task-processor role needs logs:CreateLogGroup/CreateLogStream/DeleteLogGroup on /aws/kinesisfirehose/events-ingestion-org-* (separate Pulumi PR).

How did you test this code?

Unit tests (moto): assert the log group + DestinationDelivery stream are created, CloudWatchLoggingOptions is set on the delivery stream, and both are removed on teardown. experimentation.ingestion_infra_service remains at 100% coverage.

Create a per-organisation CloudWatch log group and stream and enable
CloudWatchLoggingOptions on the delivery stream so Firehose delivery
failures are logged; remove them on teardown.
@gagantrivedi
gagantrivedi requested review from a team as code owners July 22, 2026 10:16
@gagantrivedi
gagantrivedi requested review from emyller and removed request for a team July 22, 2026 10:16
@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Jul 22, 2026 10:16am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
flagsmith-frontend-preview Ignored Ignored Jul 22, 2026 10:16am
flagsmith-frontend-staging Ignored Ignored Jul 22, 2026 10:16am

Request Review

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Firehose ingestion infrastructure now provisions a per-organisation CloudWatch Logs group and DestinationDelivery stream, enables Firehose destination logging, and removes the log group during deprovisioning. Unit tests cover configuration, resource creation, failure handling, and cleanup using mocked CloudWatch Logs. Observability documentation updates the recorded source line numbers for three ingestion infrastructure events.

Estimated code review effort: 2 (Simple) | ~15 minutes


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added api Issue related to the REST API docs Documentation updates feature New feature or request and removed docs Documentation updates labels Jul 22, 2026
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-e2e:pr-8068 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-api-test:pr-8068 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-8068 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-api:pr-8068 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-private-cloud:pr-8068 Finished ✅ Results
ghcr.io/flagsmith/flagsmith:pr-8068 Finished ✅ Results

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: cef87322-9fcd-4d22-a42a-213775e9c26d

📥 Commits

Reviewing files that changed from the base of the PR and between 6bf7a23 and fc91ecb.

📒 Files selected for processing (3)
  • api/experimentation/ingestion_infra_service.py
  • api/tests/unit/experimentation/test_ingestion_infra_service.py
  • docs/docs/deployment-self-hosting/observability/_events-catalogue.md

Comment thread api/experimentation/ingestion_infra_service.py
Comment thread api/experimentation/ingestion_infra_service.py
Comment thread api/tests/unit/experimentation/test_ingestion_infra_service.py
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18618 (attempt 2)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  2 passed

Details

stats  2 tests across 2 suites
duration  37.6 seconds
commit  fc91ecb
info  🔄 Run: #18618 (attempt 2)

🗂️ Previous results
❌ private-cloud · depot-ubuntu-latest-arm-16 — run #18618 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

failed  1 failed
passed  1 passed

Details

stats  2 tests across 2 suites
duration  60 seconds
commit  fc91ecb
info  📦 Artifacts: View test results and HTML report
🔄 Run: #18618 (attempt 1)

Failed tests

firefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise

✅ private-cloud · depot-ubuntu-latest-16 — run #18618 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  55.7 seconds
commit  fc91ecb
info  🔄 Run: #18618 (attempt 1)

✅ oss · depot-ubuntu-latest-16 — run #18618 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  43.2 seconds
commit  fc91ecb
info  🔄 Run: #18618 (attempt 1)

✅ oss · depot-ubuntu-latest-arm-16 — run #18618 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  2 passed

Details

stats  2 tests across 2 suites
duration  49.2 seconds
commit  fc91ecb
info  🔄 Run: #18618 (attempt 1)

@github-actions

Copy link
Copy Markdown
Contributor

Visual Regression

19 screenshots compared. See report for details.
View full report

@gagantrivedi
gagantrivedi requested review from Zaimwa9 and removed request for emyller July 22, 2026 11:26
@gagantrivedi
gagantrivedi merged commit 506b838 into main Jul 22, 2026
48 of 49 checks passed
@gagantrivedi
gagantrivedi deleted the feat/firehose-cloudwatch-error-logging branch July 22, 2026 12:00
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.66%. Comparing base (02a92d7) to head (fc91ecb).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8068   +/-   ##
=======================================
  Coverage   98.66%   98.66%           
=======================================
  Files        1517     1517           
  Lines       60318    60363   +45     
=======================================
+ Hits        59512    59557   +45     
  Misses        806      806           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Issue related to the REST API feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants