feat: add aws cloud watch logs multi cluster support#110
Open
chalindukodikara wants to merge 5 commits into
Open
Conversation
Signed-off-by: Chalindu Kodikara <chalindumkodikara@gmail.com>
Signed-off-by: Chalindu Kodikara <chalindumkodikara@gmail.com>
Signed-off-by: Chalindu Kodikara <chalindumkodikara@gmail.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR consolidates multi-cluster logging by replacing per-cluster naming with a centralized log group naming scheme. ClusterName is removed from configuration, replaced by direct LogGroupName specification. Log group paths are now derived from global.logGroupPrefix (defaulting to ChangesMulti-Cluster Log Group Consolidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
observability-logs-cloudwatch/README.md (1)
143-163:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDocument that IAM policy ARNs must be updated if
global.logGroupPrefixis customized.The IAM policy examples hardcode the log group resource as
arn:aws:logs:<region>:<account-id>:log-group:/aws/containerinsights/application:*, which assumes the defaultglobal.logGroupPrefixvalue of/aws/containerinsights.If a user customizes
global.logGroupPrefixto a different value, these IAM policy resource ARNs must also be updated to match<custom-prefix>/application, otherwise all adapter operations will fail with permission errors.📝 Suggested clarification
Add a note after line 143:
Replace: - `<region>` with your AWS region. - `<account-id>` with your AWS account ID. - `/aws/containerinsights/application` with `<your-custom-prefix>/application` if you customize `global.logGroupPrefix`.Apply the same note to the Setup Job IAM policy section.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@observability-logs-cloudwatch/README.md` around lines 143 - 163, The README's IAM policy examples assume the default global.logGroupPrefix (/aws/containerinsights) so update the documentation to warn that if users customize global.logGroupPrefix they must also update the IAM policy resource ARNs to match <your-custom-prefix>/application; add a short note (referencing global.logGroupPrefix and the IAM policy examples shown around the existing "LogsScoped" statement) that tells readers to replace <region>, <account-id>, and replace /aws/containerinsights/application with <your-custom-prefix>/application when customized, and add the same note to the "Setup Job IAM policy" section.
🧹 Nitpick comments (1)
observability-logs-cloudwatch/README.md (1)
443-450: ⚡ Quick winAdd security guidance for handling static AWS credentials.
The export commands show AWS credentials in plaintext without security warnings. Users copying these examples may inadvertently persist credentials in shell history or commit them to version control.
🔒 Suggested security note
Add a warning before the export commands:
> **Security note:** Avoid hardcoding credentials in scripts or committing them to version control. Use a secure credential manager or read from a secrets vault. These exports will be stored in your shell history unless `HISTCONTROL=ignorespace` is set and commands are prefixed with a space.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@observability-logs-cloudwatch/README.md` around lines 443 - 450, Add a security note before the export block that warns users not to hardcode or expose AWS credentials (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_REGION, WEBHOOK_SHARED_SECRET) in scripts or version control and recommend using a credential manager, secrets vault, or environment-specific secret injection; mention shell-history risk (HISTCONTROL=ignorespace and prefixing with a space) and advise alternatives like AWS CLI credential files, IAM roles, or secrets managers to securely provide these values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@observability-logs-cloudwatch/internal/config.go`:
- Around line 78-84: The current atoi-based check accepts signed or
non-canonical forms like "+9098" or "00080"; update validation to first ensure
serverPort is a canonical decimal integer (no leading + or leading zeros) e.g.
by matching serverPort against a regex such as ^[1-9][0-9]{0,4}$ (which also
prevents "0" and limits length), then convert with strconv.Atoi (or
strconv.ParseInt) and keep the existing range check on port; ensure the change
is applied where serverPort is parsed (the strconv.Atoi call and subsequent port
range check) so invalid non-canonical strings are rejected.
In `@observability-logs-cloudwatch/README.md`:
- Line 230: The README currently uses invalid shell syntax `export
AWS_REGION=<your-aws-region>`; update these export lines (the AWS_REGION export
and the other export commands referenced around the README) to use valid shell
syntax by either providing a concrete example (e.g., use a real region string)
or a quoted placeholder (e.g., export AWS_REGION="your-aws-region"), and apply
the same change to the similar export lines mentioned (lines near 445, 448, 449)
so angle brackets are removed and the shell will not treat them as redirection.
---
Outside diff comments:
In `@observability-logs-cloudwatch/README.md`:
- Around line 143-163: The README's IAM policy examples assume the default
global.logGroupPrefix (/aws/containerinsights) so update the documentation to
warn that if users customize global.logGroupPrefix they must also update the IAM
policy resource ARNs to match <your-custom-prefix>/application; add a short note
(referencing global.logGroupPrefix and the IAM policy examples shown around the
existing "LogsScoped" statement) that tells readers to replace <region>,
<account-id>, and replace /aws/containerinsights/application with
<your-custom-prefix>/application when customized, and add the same note to the
"Setup Job IAM policy" section.
---
Nitpick comments:
In `@observability-logs-cloudwatch/README.md`:
- Around line 443-450: Add a security note before the export block that warns
users not to hardcode or expose AWS credentials (AWS_ACCESS_KEY_ID,
AWS_SECRET_ACCESS_KEY, AWS_REGION, WEBHOOK_SHARED_SECRET) in scripts or version
control and recommend using a credential manager, secrets vault, or
environment-specific secret injection; mention shell-history risk
(HISTCONTROL=ignorespace and prefixing with a space) and advise alternatives
like AWS CLI credential files, IAM roles, or secrets managers to securely
provide these values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f70cebde-5c72-46a0-9a35-1d46ff6fb5b2
⛔ Files ignored due to path filters (8)
observability-logs-cloudwatch/images/1-rules-creation.pngis excluded by!**/*.pngobservability-logs-cloudwatch/images/2-rules-creation-config.pngis excluded by!**/*.pngobservability-logs-cloudwatch/images/3-rules-creation-add-api-destination.pngis excluded by!**/*.pngobservability-logs-cloudwatch/images/4-create-api-destination.pngis excluded by!**/*.pngobservability-logs-cloudwatch/images/5-create-api-destination-add-connection.pngis excluded by!**/*.pngobservability-logs-cloudwatch/images/6-create-api-destination-add-api-key.pngis excluded by!**/*.pngobservability-logs-cloudwatch/images/7-rules-creation-select-api-destination.pngis excluded by!**/*.pngobservability-logs-cloudwatch/images/alarm-triggered.pngis excluded by!**/*.png
📒 Files selected for processing (19)
observability-logs-cloudwatch/README.mdobservability-logs-cloudwatch/helm/templates/_helpers.tplobservability-logs-cloudwatch/helm/templates/adapter/configmap.yamlobservability-logs-cloudwatch/helm/templates/aws-credentials/secret.yamlobservability-logs-cloudwatch/helm/templates/cloudwatch-agent/post-install-hook.yamlobservability-logs-cloudwatch/helm/templates/cloudwatch-setup/job.yamlobservability-logs-cloudwatch/helm/templates/validate.yamlobservability-logs-cloudwatch/helm/values.yamlobservability-logs-cloudwatch/init/setup-cloudwatch.shobservability-logs-cloudwatch/internal/cloudwatch/alerts.goobservability-logs-cloudwatch/internal/cloudwatch/alerts_crud.goobservability-logs-cloudwatch/internal/cloudwatch/alerts_crud_test.goobservability-logs-cloudwatch/internal/cloudwatch/alerts_test.goobservability-logs-cloudwatch/internal/cloudwatch/client.goobservability-logs-cloudwatch/internal/cloudwatch/client_test.goobservability-logs-cloudwatch/internal/cloudwatch/webhook.goobservability-logs-cloudwatch/internal/config.goobservability-logs-cloudwatch/internal/config_test.goobservability-logs-cloudwatch/main.go
💤 Files with no reviewable changes (2)
- observability-logs-cloudwatch/helm/templates/cloudwatch-setup/job.yaml
- observability-logs-cloudwatch/helm/templates/aws-credentials/secret.yaml
Signed-off-by: Chalindu Kodikara <chalindumkodikara@gmail.com>
Signed-off-by: Chalindu Kodikara <chalindumkodikara@gmail.com>
akila-i
approved these changes
May 16, 2026
JanakaSandaruwan
approved these changes
May 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
$subject
Approach
Related Issues
Checklist
Remarks
Summary by CodeRabbit
New Features
Documentation
Bug Fixes