Skip to content

Revert "CAB-4856: Apply master-branch kno2-discharge-summary Terraform and check for drift"#39

Merged
irsanchez merged 1 commit intopp_masterfrom
revert-38-CAB-4856-logvars
Oct 3, 2025
Merged

Revert "CAB-4856: Apply master-branch kno2-discharge-summary Terraform and check for drift"#39
irsanchez merged 1 commit intopp_masterfrom
revert-38-CAB-4856-logvars

Conversation

@irsanchez
Copy link
Copy Markdown

@irsanchez irsanchez commented Oct 3, 2025

Reverts #38

Summary by CodeRabbit

  • Refactor
    • Simplified logging setup by always provisioning the log group and using direct references, reducing configuration complexity.
  • Chores
    • Removed the obsolete option to toggle log group management.
  • Impact
    • More consistent and reliable logging is enabled by default; no user action required.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 3, 2025

Walkthrough

The change removes conditional log group management: the CloudWatch log group is now always created, and the subscription filter directly references it. The variable manage_log_group is deleted. No other resources or logic were modified.

Changes

Cohort / File(s) Summary of Changes
Log group and subscription filter updates
lambda_function/lambda_function.tf
Removed count-based conditional on aws_cloudwatch_log_group.lambda_log_group so it is always created. Simplified aws_cloudwatch_log_subscription_filter to reference aws_cloudwatch_log_group.lambda_log_group.name directly, removing conditional/local fallback. No changes to aws_lambda_function.
Variable removal
lambda_function/variables.tf
Deleted public variable manage_log_group (boolean, default true). No other variables changed.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Dev as Developer
    participant TF as Terraform
    participant AWS as AWS (CloudWatch/Lambda)
    participant CW as CloudWatch Logs
    participant Dest as Subscription Destination

    Dev->>TF: plan/apply
    TF->>AWS: Create aws_cloudwatch_log_group (unconditional)
    TF->>AWS: Create aws_cloudwatch_log_subscription_filter<br/>log_group_name = log_group.name
    Note right of AWS: No conditional logic on manage_log_group

    AWS->>CW: Lambda writes logs to log group
    CW->>Dest: Forward logs via subscription filter
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • smallesh90
  • dhayespp

Poem

Thump-thump go my paws in gleeful loop,
Logs now sprout without a hoop!
Filters sip the stream just right,
No toggles needed, clean and light.
I nibble carrots, configs neat—
Cloudy trails where lambdas tweet. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title claims to revert “CAB-4856: Apply master-branch kno2-discharge-summary Terraform and check for drift,” but the actual changes simply remove the manage_log_group variable and always create the CloudWatch log group. The title is misleading because it does not summarize the main modifications made in this revert. Please update the title to accurately describe the reverted changes, for example “Revert conditional CloudWatch log group creation and removal of manage_log_group variable.”
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch revert-38-CAB-4856-logvars

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8f7391 and b08558d.

📒 Files selected for processing (2)
  • lambda_function/lambda_function.tf (1 hunks)
  • lambda_function/variables.tf (0 hunks)
💤 Files with no reviewable changes (1)
  • lambda_function/variables.tf

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

@irsanchez irsanchez merged commit 3c76bdc into pp_master Oct 3, 2025
1 check passed
@irsanchez irsanchez deleted the revert-38-CAB-4856-logvars branch October 3, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants