chore: Adding debug log for assume already applied

[yhakbar]

Description

Adds some simple debug logs to track that units are being included in the queue despite dependents being excluded.

TODOs

Read the Gruntwork contribution guidelines.

• I authored this code entirely myself
• I am submitting code based on open source software (e.g. MIT, MPL-2.0, Apache)]
• I am adding or upgrading a dependency or adapted code and confirm it has a compatible open source license
• Update the docs.
• Run the relevant tests successfully, including pre-commit checks.
• Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added / Removed / Updated [X].

Migration Guide

Summary by CodeRabbit

Release Notes

• Improvements
  • Enhanced logging for queue dependency resolution to improve debugging visibility.
  • Improved handling of dependencies in terminal states when error tolerance is enabled.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
terragrunt-docs	Ready	Preview	Comment	Dec 12, 2025 6:14pm

[coderabbitai]

📝 Walkthrough

Walkthrough

The change introduces a logger parameter to the GetReadyWithDependencies method and its helper areDependenciesReadyUnsafe to enable debug logging during dependency resolution. The signature change is propagated across test and call sites. The helper now logs when dependencies are unavailable and allows terminal-status dependencies when IgnoreDependencyErrors is enabled.

Changes

Cohort / File(s)	Summary
Queue method signatures internal/queue/queue.go	Updated GetReadyWithDependencies() to accept log.Logger parameter; updated areDependenciesReadyUnsafe() to accept logger parameter. Added debug logging for missing dependencies and terminal-status handling when IgnoreDependencyErrors is set. Added log package import.
Test updates internal/queue/queue_test.go	Refactored all test calls to GetReadyWithDependencies to pass logger instances using logger.CreateLogger(). Updated imports to include test helper logger package.
Call site update internal/runner/runnerpool/controller.go	Updated GetReadyWithDependencies call to pass logger parameter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Changes are largely consistent parameter propagation across multiple files
• Logic addition in dependency handling (terminal status check with IgnoreDependencyErrors) warrants verification
• Review focus: ensure logger is correctly threaded through all call paths and logging output is appropriate

Possibly related PRs

• fix: Ensure queue entries are still marked as ready if they have dependencies/dependents that are excluded #5194: Modifies Queue.areDependenciesReadyUnsafe and dependency-readiness logic, including IgnoreDependencyErrors behavior—likely related to the same feature work.

Suggested reviewers

• ThisGuyCodes
• denis256

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding debug logging to track units being included despite excluded dependents.
Description check	✅ Passed	The description provides a clear explanation of changes and follows the template structure, though several TODO items remain unchecked and release notes are not populated.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore/adding-debug-log-for-assume-already-applied

---

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

internal/queue/queue.go (2)

301-338: GetReadyWithDependencies(logger) preserves behavior; consider a small split to address complexity

The new logger parameter is threaded cleanly: dependency-ordering logic is unchanged, and the method still holds only an RLock while scanning entries, so concurrency guarantees remain intact. The IgnoreDependencyOrder fast path continues to return all StatusReady entries as before.

SonarCloud’s cognitive-complexity warning here is marginal (just over the threshold). If you want to quiet it without changing behavior, you could split the two branches into helpers that assume the lock is already held, e.g. getReadyIgnoringOrderUnsafe() and getReadyRespectingOrderUnsafe(l log.Logger), and have GetReadyWithDependencies just handle locking and dispatch.

---

344-383: Dependency readiness logic with logging looks sound; complexity could be trimmed

The updated areDependenciesReadyUnsafe correctly:

• Treats dependencies missing from the queue as “ready” while adding useful debug logs, with an extra, more specific message when unitsMap indicates Execution.AssumeAlreadyApplied.
• Honors IgnoreDependencyErrors by allowing scheduling when dependency statuses are terminal (isTerminal), aligning with the destroy/ignore-error tests and with areDependentsReadyUnsafe.
• Accesses unitsMap and entries only under the caller-held RLock, which matches how SetUnitsMap writes under a full Lock, so map access stays data-race free.

Functionally this looks solid. SonarCloud’s cognitive-complexity warning here is mostly due to the nested nil/map checks and the IgnoreDependencyErrors branch. If desired, you could extract the “dependency not in queue” block (including the logging and unitsMap handling) into a helper like depsMissingButAssumedReady(l, dep component.Component) bool to make this function flatter while keeping behavior identical.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 480ef76 and d3f5055.

📒 Files selected for processing (3)

• internal/queue/queue.go (4 hunks)
• internal/queue/queue_test.go (33 hunks)
• internal/runner/runnerpool/controller.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

⚙️ CodeRabbit configuration file

Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.

Files:

• internal/runner/runnerpool/controller.go
• internal/queue/queue.go
• internal/queue/queue_test.go

🧠 Learnings (1)

📚 Learning: 2025-02-10T13:36:19.542Z

Learnt from: levkohimins
Repo: gruntwork-io/terragrunt PR: 3723
File: cli/commands/stack/action.go:160-160
Timestamp: 2025-02-10T13:36:19.542Z
Learning: The project uses a custom error package `github.com/gruntwork-io/terragrunt/internal/errors` which provides similar functionality to `fmt.Errorf` but includes stack traces. Prefer using this package's error functions (e.g., `errors.Errorf`, `errors.New`) over the standard library's error handling.

Applied to files:

• internal/queue/queue_test.go

🧬 Code graph analysis (2)

internal/queue/queue.go (1)

pkg/log/log.go (1)

• Debugf (72-74)

internal/queue/queue_test.go (2)

test/helpers/logger/logger.go (1)

• CreateLogger (9-14)

internal/queue/queue.go (1)

• NewQueue (220-298)

🪛 GitHub Check: SonarCloud Code Analysis

internal/queue/queue.go

[failure] 301-301: Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=gruntwork-io_terragrunt&issues=AZsKDSOK_zp3qJc-iF7j&open=AZsKDSOK_zp3qJc-iF7j&pullRequest=5198

---

[failure] 344-344: Refactor this method to reduce its Cognitive Complexity from 23 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=gruntwork-io_terragrunt&issues=AZsKDSOK_zp3qJc-iF7k&open=AZsKDSOK_zp3qJc-iF7k&pullRequest=5198

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Pull Request has non-contributor approval

🔇 Additional comments (3)

internal/runner/runnerpool/controller.go (1)

109-112: Logger propagation into the queue looks correct

Passing l into dr.q.GetReadyWithDependencies(l) keeps dependency-resolution logs on the same logger as the rest of the controller and does not change locking or control flow. No changes requested.

internal/queue/queue_test.go (2)

3-11: Using the test helper logger to satisfy the new signature is appropriate

Importing test/helpers/logger and relying on logger.CreateLogger() keeps tests decoupled from production logging configuration while satisfying GetReadyWithDependencies(logger)’s contract. Looks good.

---

254-1012: Tests correctly exercise the new GetReadyWithDependencies(logger) contract

All updated tests now pass a log.Logger instance into GetReadyWithDependencies, either via a shared l := logger.CreateLogger() or inline calls, and the assertions still verify the intended queue behavior (including IgnoreDependencyErrors + destroy flows). The changes are consistent and do not introduce new edge cases.