feat: add Amazon Data Firehose + S3 telemetry mirror - #25
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds an optional, fail-open AWS Data Firehose mirror for accepted inference telemetry. It provisions Firehose, S3, IAM, CloudWatch, and ECR resources with Terraform, adds runtime configuration and metrics, documents the path, and adds Go and Terraform CI validation. ChangesAWS telemetry mirror
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant logHandler
participant Warehouse
participant firehoseDispatcher
participant AmazonDataFirehose
participant AmazonS3
Client->>logHandler: Submit inference log
logHandler->>Warehouse: Persist accepted entry
Warehouse-->>logHandler: Write succeeds
logHandler->>firehoseDispatcher: Enqueue telemetry
firehoseDispatcher->>AmazonDataFirehose: PutRecord with NDJSON
AmazonDataFirehose->>AmazonS3: Deliver GZIP-compressed object
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74b689636c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| resource "aws_ecr_repository" "sentinel_repo" { | ||
| name = "${var.project_name}-repo" | ||
| image_tag_mutability = "IMMUTABLE" |
There was a problem hiding this comment.
Avoid making the chart's reused latest tag immutable
With the default project_name, this is the same sentinelai-repo that the checked Helm deployment references as sentinelai-repo:latest (helm/sentinel/templates/deployment.yaml:17). ECR will reject overwriting that tag after the first push, so subsequent releases using the chart's fixed image reference cannot be published. Use versioned tags or digests in the chart, or keep the repository mutable while latest is reused.
Useful? React with 👍 / 👎.
| FIREHOSE_ENABLED: ${FIREHOSE_ENABLED:-false} | ||
| FIREHOSE_DELIVERY_STREAM: ${FIREHOSE_DELIVERY_STREAM:-sentinelai-telemetry} | ||
| FIREHOSE_QUEUE_SIZE: ${FIREHOSE_QUEUE_SIZE:-1000} | ||
| AWS_REGION: ${AWS_REGION:-us-east-1} |
There was a problem hiding this comment.
Pass AWS credentials into the Compose service
When the mirror is enabled through the documented local docker compose up flow, this environment block forwards the feature settings and region but none of the AWS credential/profile variables, and it does not mount the host's shared AWS configuration. Consequently, credentials available through the host's normal SDK chain are invisible inside the container and every PutRecord fails while /log still returns accepted. Provide a secure credential or workload-role forwarding mechanism, or document the required Compose override.
Useful? React with 👍 / 👎.
Summary
terraform/main,TFwith valid Terraform filesValidation
PutRecordpayloadsgo mod tidyand fails on module-file drift before runninggo test ./...FIREHOSE_ENABLED=falseterraform validateHonest scope
This PR adds a best-effort observability mirror, not a transactional dual-write guarantee. PostgreSQL remains the default local persistence path. The Firehose queue is bounded and in-memory; producer errors/drops are surfaced through
ingestion_firehose_records_totaland logs. No Firehose/S3 throughput benchmark or deployed-production claim is added by this PR.Summary by CodeRabbit
New Features
Documentation
Tests & Validation