Skip to content

feat: add support to slog in libs workspace #1928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 2, 2025

Conversation

aldoborrero
Copy link
Contributor

@aldoborrero aldoborrero commented Mar 29, 2025

Summary of Changes

This continues the work started on PR #1927 but for the libs workspace.

Summary by CodeRabbit

  • Chores
    • Upgraded to a structured logging system across the platform for enhanced error context and observability.
    • Refined error handling in various components to avoid abrupt terminations, providing clearer diagnostic feedback.
  • Tests
    • Adjusted error reporting in test suites to improve failure detection and ensure proper cleanup when issues arise.

Copy link
Contributor

coderabbitai bot commented Mar 29, 2025

Walkthrough

This pull request standardizes logging across the codebase by replacing the legacy logging package with log/slog. Multiple modules—including backend APIs, CI integrations, configuration handlers, execution routines, locking mechanisms, policy checks, scheduler components, and storage handlers—are updated with structured logging. In several cases, error handling is also adjusted to return or panic instead of terminating via fatal logs. No exported or public entities have been modified aside from the logging calls and import statements.

Changes

File(s) Change Summary
libs/backendapi/diggerapi.go Replaced log with log/slog; updated fatal errors to return errors and warnings to structured logs.
libs/ci/{bitbucket_service.go, github/github.go, gitlab/gitlab.go} Updated logging calls (info, error, debug) to add structured context for events and errors in CI processes.
libs/comment_utils/reporting/* Switched log statements to log/slog with added context for reporting, comment publishing, and updates.
libs/digger_config/* Transitioned from standard logging to structured logging; added debug logs for file handling and configuration operations.
libs/execution/* Updated logging in executor functions (e.g., command execution, plan retrieval) to log/slog; some test files now panic instead of using log.Fatal.
libs/license/license.go Replaced standard log calls with structured logging to improve error and info messages during license validation.
libs/locking/{aws, gcp, envprovider, locking}.go Converted all logging to log/slog with detailed context in lock acquisition, release, and error handling.
libs/policy/* Enhanced logging in policy checks using log/slog for errors, warnings, and debug info without altering core logic.
libs/scheduler/* Adopted structured logging across scheduler modules with minor method signature formatting adjustments.
libs/storage/* Replaced standard logging with log/slog for detailed logs on plan storage operations in AWS, GCP, and GitHub.
Test files (libs/digger_config/digger_config_test.go, libs/execution/{opentofu_test.go, test_utils.go, tf_test.go}) Changed error handling from log.Fatal to panic, altering the cleanup error behavior in tests.

Sequence Diagram(s)

Possibly related PRs

Poem

Oh, I'm hopping with glee, a code rabbit true,
Structured logs now sparkle—errors brightly in view.
From fatal to panic, warnings now dance,
Each line sings a rhythm in our logging advance.
With “slog” by our side, our code feels so light! 🐇✨
Cheers to cleaner logs and a future so bright!
Hop on, dear devs, let's code day and night!


📜 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 c63850a and 19f92bd.

⛔ Files ignored due to path filters (1)
  • backend/go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • backend/bootstrap/main.go (4 hunks)
  • backend/go.mod (4 hunks)
  • backend/models/setup.go (2 hunks)
  • backend/utils/log.go (1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
backend/bootstrap/main.go (1)
backend/utils/log.go (1)
  • NewSentrySlogWriter (16-18)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Build
  • GitHub Check: Build
🔇 Additional comments (14)
backend/utils/log.go (1)

1-38: Well-designed structured logging adapter for Sentry

This implementation creates a clean adapter between Sentry's log output and structured logging with slog. The approach properly implements the io.Writer interface to capture Sentry logs and redirect them to structured logging.

A few observations:

  • The code properly handles different line formats by attempting to extract only the message portion when possible
  • The implementation always returns success (nil error) which is appropriate for this use case
backend/models/setup.go (2)

7-7: Good choice of structured logging integration for GORM

The import of slog-gorm provides a clean integration between structured logging and the GORM ORM library.


24-24: Clean migration to structured logging for database operations

The replacement of the default GORM logger with slogGorm.New() properly integrates the database logging with the application's structured logging system. This maintains consistent logging patterns across the codebase.

backend/bootstrap/main.go (5)

17-17: Appropriate import for the new Sentry adapter

The import of the utils package is necessary to access the new SentrySlogWriter adapter.


19-19: Good addition of slog-gin integration

Adding the sloggin package enables structured logging for HTTP requests, which complements the overall transition to structured logging.


42-43: Control flow changed from log.Fatal to panic

The error handling approach has been changed from a log.Fatal (which would exit the program) to a combination of structured error logging followed by panic. While both approaches terminate execution, panic provides a stack trace and allows for potential recovery by upstream handlers.


113-113: Good integration of custom Sentry log adapter

Using the new SentrySlogWriter to process Sentry's debug messages ensures that all logs, including those from Sentry, are properly integrated into the structured logging system with appropriate grouping.


123-124: Proper middleware integration for HTTP request logging

Adding the sloggin middleware with appropriate group namespacing ensures that HTTP request logs are properly structured and categorized. The empty line after the middleware registration maintains good code readability.

backend/go.mod (6)

25-25: New Dependency Added: slog-gorm

The addition of github.com/orandin/slog-gorm v1.4.0 supports structured logging for GORM. Please ensure that the integration using this dependency is consistent with the new logging approach across the codebase.


28-28: New Dependency Added: slog-gin

Adding github.com/samber/slog-gin v1.15.0 brings structured logging support for Gin HTTP handlers. Verify that the updated logging implementation is properly integrated into the middleware and request handling layers.


112-112: Dependency Version Update: bytedance/sonic

The sonic library has been bumped to v1.11.9. Make sure to review the changelog for any breaking changes and confirm that existing functionality relying on this library remains compatible.


132-132: Dependency Version Update: mimetype

The version update to github.com/gabriel-vasile/mimetype v1.4.4 likely includes minor fixes or enhancements. Please verify that file type detection continues to behave as expected.


140-140: Dependency Version Update: validator

Upgrading github.com/go-playground/validator/v10 to v10.22.0 should bring performance improvements or bug fixes. Ensure that all validation routines are thoroughly tested under the new version.


209-209: Dependency Version Update: cpuid

The update to github.com/klauspost/cpuid/v2 v2.2.8 appears to provide improvements in hardware detection. Confirm that these changes do not affect any underlying assumptions in the code that uses cpuid functionalities.

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@aldoborrero aldoborrero marked this pull request as ready for review March 29, 2025 20:27
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

The changes replace the standard logging implementation with structured logging using slog across the libs workspace, enhancing visibility, error handling, and debugging context without altering core functionality.

• Updated multiple files (e.g., /libs/ci/bitbucket/bitbucket_service.go, /libs/ci/github/github.go) to log with key/value pairs and proper levels instead of raw log statements.
• Transitions fatal exits to error returns or panics in key functions (e.g., /libs/backendapi/diggerapi.go, /libs/digger_config/utils.go).
• Consistently applies structured logging in test files to improve diagnostics and traceability.

💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!

37 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 101 to 107
CommentId := sourceDetaiItem.CommentId
if err != nil {
log.Printf("Could not convert commentId to int64: %v", err)
slog.Error("could not convert commentId to int64",
"error", err,
"commentId", CommentId)
return fmt.Errorf("could not convert commentId to int64: %v", err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Redundant error check after CommentId assignment; err is not updated. Remove or update this block.

Suggested change
CommentId := sourceDetaiItem.CommentId
if err != nil {
log.Printf("Could not convert commentId to int64: %v", err)
slog.Error("could not convert commentId to int64",
"error", err,
"commentId", CommentId)
return fmt.Errorf("could not convert commentId to int64: %v", err)
}
```suggestion CommentId := sourceDetaiItem.CommentId

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🔭 Outside diff range comments (2)
libs/scheduler/models.go (1)

133-134: 💡 Verification agent

🧩 Analysis chain

Potential logic issue in IsApply method

The IsApply method is calling IsPlanJobSpecs instead of a hypothetical IsApplyJobSpecs function, which suggests this might not be correctly checking for apply operations.


🏁 Script executed:

#!/bin/bash
# Check if there is an IsApplyJobSpecs function in the codebase
rg -A 3 "func IsApplyJobSpecs" --type go
# See if IsPlanJobSpecs is used correctly in both IsPlan and IsApply methods
rg -A 5 "func IsPlanJobSpecs" --type go

Length of output: 649


Incorrect function call in IsApply method: use IsApplyJobSpecs instead of IsPlanJobSpecs

It appears that the IsApply method in libs/scheduler/models.go is currently returning the result of IsPlanJobSpecs(jobSpecs), which is likely a mistake. We have confirmed that the codebase contains a dedicated IsApplyJobSpecs function (in libs/scheduler/json_models.go) that is intended for apply operations. To align the behavior of the IsApply method with its intended functionality, please update the method to call IsApplyJobSpecs(jobSpecs) instead.

  • Location: libs/scheduler/models.go, Lines 133-134
  • Suggested Change: Replace
     return IsPlanJobSpecs(jobSpecs), nil
    with
     return IsApplyJobSpecs(jobSpecs), nil
libs/execution/opentofu_test.go (1)

33-34: ⚠️ Potential issue

Inconsistent error handling

These test functions still use log.Fatal(err) while the first test function uses panic(err). This inconsistency should be resolved to maintain a uniform approach across the codebase.

Update both occurrences to use panic:

if err != nil {
-	log.Fatal(err)
+	panic(err)
}

Also applies to: 50-51

🧹 Nitpick comments (6)
libs/digger_config/digger_config.go (2)

397-397: Simplify boolean comparison

Boolean comparison can be simplified for better readability.

-				if b.Terragrunt == true {
+				if b.Terragrunt {
🧰 Tools
🪛 golangci-lint (1.64.8)

397-397: S1002: should omit comparison to bool constant, can be simplified to b.Terragrunt

(gosimple)


519-519: Simplify boolean comparison

Another boolean comparison that can be simplified for better readability.

-	if generateProjects == true {
+	if generateProjects {
🧰 Tools
🪛 golangci-lint (1.64.8)

519-519: S1002: should omit comparison to bool constant, can be simplified to generateProjects

(gosimple)

libs/ci/github/github.go (2)

123-124: Logging message could be more specific.

While the structured logging is good, the error message in the return statement mentions "error getting pull request files" but the log is about "error getting issues."

-			slog.Error("error getting issues", "error", err)
-			return nil, fmt.Errorf("error getting pull request files: %v", err)
+			slog.Error("error getting issues", "error", err)
+			return nil, fmt.Errorf("error getting issues: %v", err)

222-223: Typo in error message.

There's a typo in the error message: "addd" should be "add".

-		return fmt.Errorf("could not addd reaction to comment: %v", err)
+		return fmt.Errorf("could not add reaction to comment: %v", err)
libs/locking/aws/envprovider/envprovider.go (1)

118-127: Minor improvement suggestion for variable assignment.

The sessionToken variable is now properly stored in a variable before use, which is a good practice. Consider adding error logging for session token retrieval to maintain consistency with the access key and secret key error handling.

 sessionToken := os.Getenv("AWS_SESSION_TOKEN")
 e.retrieved = true

 slog.Debug("AWS credentials successfully retrieved from environment")

+// Add conditional check if session token is required but missing
+// if sessionTokenRequired && sessionToken == "" {
+//   slog.Error("AWS session token not found in environment")
+// }
+
 return aws.Credentials{
libs/locking/gcp/gcp_lock.go (1)

65-66: Error handling inconsistency.

Unlike other error cases in this file, this error is returned without being logged first. Consider adding a structured error log before returning to maintain consistency.

if err != nil {
+	slog.Error("Failed to delete lock file", 
+		"resource", resource, 
+		"error", err)
	return false, fmt.Errorf("failed to delete lock file: %v", err)
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cfb4dc9 and 6f993e1.

📒 Files selected for processing (36)
  • libs/backendapi/diggerapi.go (15 hunks)
  • libs/ci/bitbucket/bitbucket_service.go (6 hunks)
  • libs/ci/github/github.go (26 hunks)
  • libs/ci/gitlab/gitlab.go (20 hunks)
  • libs/comment_utils/reporting/reporting.go (8 hunks)
  • libs/comment_utils/reporting/source_grouping.go (6 hunks)
  • libs/comment_utils/reporting/utils.go (2 hunks)
  • libs/comment_utils/summary/updater.go (2 hunks)
  • libs/digger_config/digger_config.go (25 hunks)
  • libs/digger_config/digger_config_test.go (4 hunks)
  • libs/digger_config/terragrunt/atlantis/generate.go (7 hunks)
  • libs/digger_config/utils.go (3 hunks)
  • libs/execution/execution.go (15 hunks)
  • libs/execution/opentofu.go (7 hunks)
  • libs/execution/opentofu_test.go (1 hunks)
  • libs/execution/pulumi.go (4 hunks)
  • libs/execution/terragrunt.go (3 hunks)
  • libs/execution/test_utils.go (6 hunks)
  • libs/execution/tf.go (5 hunks)
  • libs/execution/tf_test.go (3 hunks)
  • libs/license/license.go (3 hunks)
  • libs/locking/aws/dynamo_locking.go (10 hunks)
  • libs/locking/aws/envprovider/envprovider.go (4 hunks)
  • libs/locking/gcp/gcp_lock.go (5 hunks)
  • libs/locking/gcp/gcp_lock_test.go (8 hunks)
  • libs/locking/locking.go (16 hunks)
  • libs/policy/policy.go (14 hunks)
  • libs/policy/providers.go (2 hunks)
  • libs/scheduler/aws.go (9 hunks)
  • libs/scheduler/convert.go (4 hunks)
  • libs/scheduler/models.go (3 hunks)
  • libs/scheduler/serializers.go (2 hunks)
  • libs/spec/providers.go (10 hunks)
  • libs/storage/aws_plan_storage.go (6 hunks)
  • libs/storage/gcp_plan_storage.go (2 hunks)
  • libs/storage/plan_storage.go (9 hunks)
🧰 Additional context used
🧬 Code Definitions (15)
libs/execution/tf_test.go (1)
libs/execution/test_utils.go (1)
  • CreateTestTerraformProject (7-13)
libs/policy/providers.go (1)
libs/policy/policy.go (1)
  • NoOpPolicyChecker (31-32)
libs/execution/opentofu_test.go (1)
libs/execution/test_utils.go (1)
  • CreateTestTerraformProject (7-13)
libs/comment_utils/reporting/source_grouping.go (5)
libs/scheduler/serializers.go (1)
  • JobsToProjectMap (26-36)
libs/scheduler/models.go (2)
  • DiggerJobSucceeded (44-44)
  • SerializedJob (86-97)
libs/digger_config/digger_config.go (1)
  • ProjectToSourceMapping (822-825)
libs/scheduler/json_models.go (1)
  • JobJson (22-52)
libs/iac_utils/iac_utils.go (1)
  • IacPlanFootprint (29-31)
libs/comment_utils/summary/updater.go (2)
backend/utils/pr_comment.go (1)
  • GithubCommentMaxLength (161-161)
libs/scheduler/models.go (1)
  • SerializedJob (86-97)
libs/locking/locking.go (2)
libs/locking/aws/dynamo_locking.go (1)
  • DynamoDbLock (27-29)
libs/locking/gcp/gcp_lock.go (1)
  • GetGoogleStorageClient (99-111)
libs/comment_utils/reporting/utils.go (1)
libs/comment_utils/reporting/reporting.go (3)
  • CiReporter (13-18)
  • ReportStrategy (107-109)
  • CommentPerRunStrategy (111-114)
libs/ci/github/github.go (1)
libs/ci/generic/events.go (1)
  • FindAllProjectsDependantOnImpactedProjects (52-98)
libs/scheduler/serializers.go (1)
libs/scheduler/models.go (1)
  • SerializedJob (86-97)
libs/scheduler/convert.go (8)
libs/digger_config/config.go (2)
  • Project (43-60)
  • Workflow (62-67)
libs/scheduler/jobs.go (1)
  • Job (15-39)
libs/scheduler/aws.go (2)
  • Terragrunt (41-41)
  • CommandEnvProvider (363-363)
libs/execution/terragrunt.go (1)
  • Terragrunt (13-15)
libs/execution/opentofu.go (1)
  • OpenTofu (13-16)
libs/execution/pulumi.go (1)
  • Pulumi (13-16)
libs/digger_config/yaml.go (1)
  • AwsCognitoOidcConfig (129-134)
libs/ci/gitlab/gitlab.go (1)
  • CommandEnvProvider (483-483)
libs/locking/aws/dynamo_locking.go (3)
libs/locking/azure/storage_account.go (1)
  • TABLE_NAME (15-15)
libs/locking/core.go (1)
  • Lock (3-7)
libs/locking/locking.go (1)
  • GetLock (248-320)
libs/backendapi/diggerapi.go (1)
libs/iac_utils/iac_utils.go (1)
  • IacPlanFootprint (29-31)
libs/ci/gitlab/gitlab.go (4)
libs/digger_config/config.go (2)
  • Workflow (62-67)
  • Project (43-60)
libs/scheduler/jobs.go (1)
  • Job (15-39)
libs/digger_config/digger_config.go (1)
  • CollectTerraformEnvConfig (968-999)
libs/scheduler/aws.go (3)
  • StateEnvProvider (362-362)
  • CommandEnvProvider (363-363)
  • GetStateAndCommandProviders (361-384)
libs/spec/providers.go (11)
libs/locking/locking.go (1)
  • NoOpLock (36-37)
libs/locking/aws/envprovider/envprovider.go (1)
  • EnvProvider (39-42)
libs/locking/aws/dynamo_locking.go (1)
  • DynamoDbLock (27-29)
libs/locking/gcp/gcp_lock.go (2)
  • GetGoogleStorageClient (99-111)
  • GoogleStorageLock (13-17)
libs/spec/models.go (3)
  • ReporterSpec (7-11)
  • BackendSpec (22-27)
  • VcsSpec (29-37)
libs/comment_utils/reporting/reporting.go (1)
  • NewCiReporterLazy (44-51)
libs/ci/gitlab/gitlab.go (3)
  • Api (51-51)
  • ParseGitLabContext (61-74)
  • NewGitLabService (76-101)
libs/backendapi/diggerapi.go (2)
  • NoopApi (23-24)
  • NewBackendApi (337-350)
libs/ci/github/github.go (1)
  • GithubServiceProviderBasic (25-25)
libs/storage/storage.go (1)
  • PlanStorage (3-8)
libs/storage/plan_storage.go (1)
  • NewPlanStorage (336-401)
libs/scheduler/aws.go (5)
libs/scheduler/jobs.go (1)
  • Job (15-39)
libs/execution/terragrunt.go (1)
  • Terragrunt (13-15)
libs/ci/gitlab/gitlab.go (2)
  • StateEnvProvider (482-482)
  • CommandEnvProvider (483-483)
libs/digger_config/config.go (1)
  • Project (43-60)
libs/digger_config/yaml.go (1)
  • AwsCognitoOidcConfig (129-134)
🪛 golangci-lint (1.64.8)
libs/policy/policy.go

413-413: ineffectual assignment to err

(ineffassign)

libs/digger_config/digger_config.go

397-397: S1002: should omit comparison to bool constant, can be simplified to b.Terragrunt

(gosimple)


519-519: S1002: should omit comparison to bool constant, can be simplified to generateProjects

(gosimple)

libs/scheduler/aws.go

338-338: Error return value of (*encoding/json.Decoder).Decode is not checked

(errcheck)

🔇 Additional comments (266)
libs/execution/terragrunt.go (3)

7-7: Good migration to structured logging.

The import change from standard log to log/slog aligns with the PR objective to standardize logging across the codebase.


100-106: Excellent use of structured logging.

The implementation of structured logging with slog.Info significantly improves the log output by organizing command details in a logical group. This makes logs more readable and searchable, providing better context for debugging and monitoring.


129-134: Good addition of debug logging for error cases.

Adding debug logging for command execution errors provides valuable context for troubleshooting failed commands. The structured format with exit code and error details will make debugging easier.

libs/digger_config/utils.go (5)

4-8: Imports properly organized.

The change from standard logging to structured logging with slog is consistent with the PR objectives.


22-24: Error handling improved with structured logging.

The transition from log.Fatalf to slog.Error followed by panic is a good change. It maintains similar behavior while providing more structured error information before the panic occurs.


30-35: Code cleanup in loop variables.

Simplifying the loop variable declarations by using i := range instead of i, _ := range is a good cleanup that removes unused variables.


41-45: Error handling improved with structured logging.

The structured error logging provides much better context by including the file and pattern information along with the error details.


56-60: Error handling improved with structured logging.

Consistent with the earlier pattern, this structured logging provides better context for exclude pattern matching errors.

libs/execution/tf.go (5)

7-7: Good migration to structured logging.

The import change from standard log to log/slog aligns with the PR objective to standardize logging across the codebase.


37-40: Improved error handling with structured logging.

The structured error logging provides better context by including the workspace name along with the error details.


70-70: Helpful debug logging for workspace operations.

Adding debug logs for workspace selection and creation provides better visibility into the workspace management process, which will be useful for troubleshooting.

Also applies to: 76-77


109-115: Security-conscious structured logging.

Well-implemented structured logging with proper redaction of sensitive information through the RedactSecrets function. This is a secure approach to logging command execution.


130-134: Enhanced error logging with structured details.

The structured error logging for command execution failures provides comprehensive context, including command name, exit code, and error details.

libs/digger_config/terragrunt/atlantis/generate.go (7)

5-5: Good migration to structured logging.

The import change from logrus to log/slog aligns with the PR objective to standardize logging across the codebase.


681-682: Improved error handling with structured logging.

The structured error logging provides better context for directory walking errors.


693-694: Error handling with structured logging and proper panic.

The transition from fatal logging to structured error logging followed by panic maintains similar behavior while providing more detailed error information.


700-701: Structured logging for path resolution errors.

Good use of structured logging to provide context about the path resolution error.


799-814: Consistent logging for project updates and creation.

The transition to structured logging for project updates and creation maintains consistency with the rest of the codebase.


846-848: Enhanced project creation logging with structured details.

The structured logging for project creation now includes additional context about the project type and working directory, which provides better visibility into the project creation process.


902-902: Consistent warning logging.

The transition from log.Warn to slog.Warn maintains consistency with the structured logging approach throughout the codebase.

libs/execution/pulumi.go (5)

7-7: Well-structured logging implementation with slog

The switch from standard log package to structured log/slog is a good modernization that will provide more contextual information in logs.


67-67: Good addition of debug logging

Adding structured debug logging before stack selection provides valuable context for troubleshooting.


70-70: Improved error logging with structured data

Enhanced error logging with structured fields for stack name and error details makes debugging much easier.


100-106: Well-structured command execution logging

Using slog's group feature to organize command execution details makes logs more readable and provides comprehensive context about the Pulumi command being executed.


121-128: Comprehensive error logging for command failures

The structured error logging now includes all critical information (command, args, exit code, error, stdout, stderr) needed for debugging failed Pulumi commands.

libs/storage/aws_plan_storage.go (11)

9-9: Appropriate switch to structured logging

Replacing standard logging with structured logging improves log quality and consistency.


45-52: Enhanced AWS initialization logging

Adding structured error and debug logging for S3 bucket configuration provides better visibility into storage initialization.


56-57: Improved error context for client retrieval

Error logging now includes the actual error details, which helps with troubleshooting AWS client issues.


69-77: Better encryption configuration logging

Added debug logs with detailed encryption information improve visibility into the security configuration.


80-81: Enhanced error logging for invalid encryption types

Including the provided encryption type in the error log provides better context for configuration issues.


85-85: Clear initialization success logging

Adding a success log with bucket information confirms proper initialization of the AWS storage.


101-116: Comprehensive plan existence logging

Enhanced logging for plan existence checks with bucket and key information provides better observability.


137-146: Improved plan storage operation logging

Adding detailed error and success logs for plan storage operations with bucket and key information helps with auditing and troubleshooting.


156-191: Comprehensive plan retrieval logging

Enhanced error and success logs throughout the plan retrieval process provide a clear trail for debugging.


201-210: Better plan deletion logging

Detailed error and success logs for plan deletion with bucket and key information improve operation traceability.


217-224: Clear AWS client initialization logging

Adding debug logs for AWS configuration loading and client creation provides visibility into the client setup process.

libs/execution/opentofu.go (8)

7-7: Appropriate switch to structured logging

Replacing standard logging with structured logging improves log quality and consistency.


29-31: Enhanced workspace switching error logging

Adding structured error logging with workspace context in the Apply method provides better debugging information.


48-50: Consistent error logging for workspace switching

Maintaining the same structured logging pattern across similar error scenarios in the Plan method is good practice.


79-81: Uniform error handling across methods

Using consistent structured error logging in the Destroy method maintains code consistency.


97-97: Informative workspace selection logging

Adding debug logs when selecting existing workspaces improves operational visibility.


103-103: Helpful workspace creation logging

Adding debug logs when creating new workspaces provides better operational context.


136-142: Well-structured command execution logging

Using slog's group feature to organize command execution details makes logs more readable and provides comprehensive context about OpenTofu commands.


158-163: Detailed command failure logging

Enhanced error logging for command failures with command, arguments, exit code, and error details provides comprehensive troubleshooting information.

libs/license/license.go (7)

8-8: Appropriate switch to structured logging

Replacing standard logging with structured logging improves log quality and consistency.


29-30: Enhanced error logging for JSON marshalling

Improved error logging with structured data aids in troubleshooting license validation issues.


36-37: Better request creation error logging

Structured error logging during request creation provides clearer context for network-related issues.


46-46: Added request debugging information

Debug log with URL information helps track license validation requests.


49-50: Enhanced request sending error logging

Improved error logging for network issues when sending license validation requests.


54-55: Improved status code check and success logging

Using http.StatusOK instead of hardcoded 200 improves readability, and adding success logging helps track license validations.


60-66: Comprehensive error handling for failed validations

The improved error logging now captures response status code and body content, providing complete context for failed license validations.

libs/spec/providers.go (15)

7-11: Updated import packages for structured logging

The import statements have been updated to include log/slog package for structured logging, which is a good improvement over the standard log package.


45-46: Enhanced logging with context information

The logging statement now includes proper log level (debug) and context about the NoOp lock provider.


51-106: Improved error handling with structured logging

The AWS lock provider section now includes structured logging with appropriate log levels and contextual information, making debugging and tracing issues easier. Good use of informational logging for successful connections and error logging for failures.


100-117: Enhanced GCP lock provider logging

The GCP lock provider implementation now includes proper structured logging for the provider initialization and error handling.


124-127: Better error reporting for unknown lock providers

The error reporting for unknown lock providers has been improved with structured logging that includes the lock type and provider, making it easier to diagnose configuration issues.


133-139: Added context to reporter creation logs

Reporter creation now includes detailed debug logging with information about the reporter type, strategy, and other relevant context.


162-167: Appropriate log levels for reporter type selection

The reporter type selection now uses appropriate log levels (debug for operational details) with consistent structured format.


184-186: Warning logs for fallback scenarios

Good use of warning log level when falling back to NoOp reporter due to unknown reporter type, providing clear indication of unexpected configuration.


193-206: Enhanced backend API provider logging

The backend API provider now includes structured logging that clearly indicates which backend type is being used, with appropriate debug level information.


217-223: Improved PR service logging with context

The PR service initialization now includes debug logging with important context information like VCS type and repository details.


229-234: Proper error logging for missing environment variables

Appropriate error logging when GitHub token is missing, clearly indicating the issue to help with configuration troubleshooting.


268-272: Added structured logging for organization service

The organization service initialization now includes debug logging with relevant context information.


324-328: Enhanced policy provider logging

The policy provider implementation now includes structured logging with contextual information, which will help with diagnostics.


341-343: Improved error logging for unknown policy types

The error logging for unknown policy types has been enhanced to provide better visibility into configuration issues.


349-365: Enhanced plan storage provider logging

The plan storage provider now includes comprehensive logging across the entire function, with appropriate debug logs for initialization and error logs for failure scenarios.

libs/storage/gcp_plan_storage.go (5)

7-8: Added structured logging package

Added the log/slog package to enable structured logging throughout the GCP plan storage implementation.


21-47: Enhanced PlanExists method with structured logging

The PlanExists method now includes comprehensive logging that tracks the operation from start to finish, with appropriate debug logs for success cases and error logs for failure scenarios. The contextual information included in logs (bucket, path, size) will be valuable for troubleshooting.


51-73: Improved StorePlanFile method with detailed logging

StorePlanFile method now includes structured logging with bucket details, file paths, and content size information, making it easier to track plan storage operations and diagnose issues.


77-122: Enhanced RetrievePlan method with proper logging

RetrievePlan method now includes comprehensive logging across the entire operation flow, with proper error logging and success confirmation logs. The contextual information will help with troubleshooting.


126-145: Improved DeleteStoredPlan method with informative logging

The DeleteStoredPlan method now includes debug logging that provides clear information about the operation and its outcomes.

libs/digger_config/digger_config.go (15)

6-7: Updated import for structured logging

The import statement has been updated to include log/slog package for structured logging, which is a good improvement over the standard log package.


37-45: Enhanced file reading error handling with structured logging

The ReadDiggerYmlFileContents function now includes better debug and error logging that clearly indicates when fallback to alternative file formats occurs.


52-77: Improved CheckOrCreateDiggerFile function with appropriate log levels

The CheckOrCreateDiggerFile function now includes debug logs for existing files and info logs for file creation, with proper error logging for failures.


83-101: Enhanced GetFilesWithExtension error handling and result logging

The GetFilesWithExtension function now includes proper error logging and debug information about found files, which will help with troubleshooting file discovery issues.


104-138: Improved FileSystemTopLevelTerraformDirWalker.GetDirs logging

The GetDirs method now includes comprehensive logging that tracks directory scanning, with debug information about found directories and skipping logic.


419-426: Improved error logging in terragrunt project generation

The error logging for terragrunt configuration hydration issues now includes more context (block name) to help identify which block caused the problem.


440-444: Enhanced terraform project generation logging

The terraform project generation logic now includes informative logs about block processing with relevant context, making it easier to track the generation process.


475-478: Added summary logging for project generation

Good addition of summary logging that reports the total number of projects generated, which is helpful for monitoring the overall generation process.


577-583: Improved IAC type validation error logging

The validation error for multiple IAC types now includes comprehensive logging that clearly shows which types are conflicting, making it easier to correct configuration issues.


619-622: Enhanced ValidateDiggerConfig logging

The config validation now includes informative logging with project and workflow counts, making it easier to understand the scale of the configuration being validated.


671-675: Improved terragrunt hydration with structured logging

The terragrunt hydration function now includes context-rich logging that helps track the hydration process with key configuration details.


712-716: Added detailed debug logging for terragrunt parsing

Good addition of debug logging with context about the terragrunt configuration being parsed, which will be valuable for troubleshooting configuration issues.


756-759: Added summary logging for terragrunt projects discovery

Good addition of summary logging that reports the total number of terragrunt projects found, which helps track the overall configuration process.


791-793: Added completion log for terragrunt hydration

Good addition of completion logging that confirms the successful hydration of terragrunt projects with a count of total projects.


827-875: Enhanced GetModifiedProjects with structured logging

The GetModifiedProjects method now includes comprehensive logging that tracks project discovery and details about source changes, which is valuable for understanding dependency relationships.

libs/storage/plan_storage.go (16)

9-10: Updated import for structured logging

The import statement has been updated to include log/slog package for structured logging, which is a good improvement over the standard log package.


28-35: Enhanced StorePlanFile method with detailed context logging

The StorePlanFile method now includes comprehensive debug logging with detailed context (owner, repo, artifact name, path, size), making it easier to track plan storage operations.


52-58: Added GitHub artifact creation logging

Added debug logging for artifact creation with error logging for failures, providing better visibility into the GitHub artifact creation process.


75-84: Improved file upload logging

Added logging for file uploads to GitHub artifacts with proper error handling, making it easier to diagnose upload issues.


96-105: Enhanced artifact finalization logging

Added logging for the artifact finalization process with appropriate error handling, providing better visibility into the complete artifact creation workflow.


107-112: Added success confirmation logging

Good addition of success logging that confirms the successful storage of the plan file with relevant details, which helps track the completion of operations.


119-144: Improved HTTP request error logging in doRequest

Enhanced error logging in the doRequest function with detailed context about the request method, URL, and response information, which will help diagnose API integration issues.


149-169: Enhanced RetrievePlan method with comprehensive logging

The RetrievePlan method now includes detailed logging throughout the retrieval process, with appropriate debug and error logs that include relevant context.


175-180: Improved zip extraction error logging

Added detailed error logging for zip extraction failures, including the zip file path and output path, which will help diagnose file processing issues.


182-187: Added success confirmation for plan retrieval

Good addition of success logging that confirms the successful retrieval of the plan with relevant details.


191-226: Enhanced PlanExists method with detailed logging

The PlanExists method now includes comprehensive logging that tracks the artifact existence check, with appropriate debug logs for both existence and non-existence cases.


231-234: Added informative logging for DeleteStoredPlan

Added informative debug logging explaining GitHub artifacts behavior, which helps users understand system limitations.


238-290: Enhanced DownloadLatestPlans with structured logging

The DownloadLatestPlans method now includes comprehensive logging throughout the download process, with appropriate debug, info, and error logs that include relevant context.


293-308: Improved artifact download logging

Added detailed logging for artifact downloads using wget, with appropriate debug logs for the command execution and error logs for failures.


323-332: Enhanced latest artifact search logging

Added useful debug logging for artifact search results, clearly indicating whether a matching artifact was found.


340-397: Improved NewPlanStorage with structured logging

The NewPlanStorage function now includes comprehensive logging across all storage providers, with appropriate info and debug logs that provide context about the selected storage mechanism.

libs/locking/gcp/gcp_lock_test.go (4)

4-12: Well-structured import reorganization

The import section is now well-organized with the newly added log/slog package for structured logging.


38-41: Good error handling conversion pattern

Replacing log.Fatalf with slog.Error followed by t.Fatalf maintains the test's failure behavior while enhancing logging with structured context.


47-48: Improved test observability

Adding structured logging with context about the test operation improves debugging and understanding of test execution flow.


100-104: Excellent use of structured logging

Multi-line structured logging with multiple fields (fileName, bucketName, transactionId) provides comprehensive context for debugging.

libs/scheduler/models.go (3)

5-5: Updated import for structured logging

Successfully replaced standard logging with structured logging package.


115-119: Enhanced error logging with contextual information

The error logging has been improved by adding structured context (batchId, prNumber, error), making debugging much more effective.


127-131: Consistent error logging pattern

Applied the same structured logging pattern as in the IsPlan method, maintaining consistency.

libs/execution/tf_test.go (3)

3-8: Clean import reorganization

Imports are now better organized with log removed and the assert package properly positioned.


15-16: Changed error handling approach

Replaced log.Fatal(err) with panic(err), which maintains test failure behavior while aligning with the codebase's new error handling approach.


32-33: Consistent error handling across test functions

Error handling has been consistently updated across all test functions, ensuring uniform behavior.

Also applies to: 49-50

libs/policy/providers.go (3)

4-4: LGTM: Updated import for structured logging.

The import change from log to log/slog supports structured logging, which is a good improvement over the standard logging package.


14-14: LGTM: Replaced basic logging with structured warning.

Good conversion from log.Println to slog.Warn, maintaining the same message but using more appropriate log level semantics.


17-19: Added informative logging with context.

This new log statement provides valuable context about the policy checker initialization, including hostname and organization name parameters.

libs/digger_config/digger_config_test.go (3)

11-11: Added errors package for improved error handling.

The errors package provides better error formatting capabilities compared to the standard library errors.


786-786: Changed error handling from fatal logs to panic.

Converting from log.Fatal() to panic() in test utilities allows deferred cleanup operations to run, which is important in tests to prevent resource leakage.

Also applies to: 795-795, 802-802, 807-807, 813-813


832-832: Improved error message formatting.

Using errors.Errorf() provides more context in the error message by including the file name.

libs/execution/test_utils.go (1)

10-10: Standardized error handling in test utilities.

Changed all instances of log.Fatal(err) to panic(err) in test utility functions. This change is beneficial for testing as it allows deferred cleanup functions to execute before the program terminates, preventing resource leaks in tests.

Also applies to: 18-19, 30-31, 37-38, 49-50, 56-57, 80-81, 87-88, 125-126, 132-133, 144-145

libs/locking/locking.go (5)

4-11: LGTM: Updated imports for structured logging.

The imports have been correctly updated to use log/slog instead of the standard log package, enabling structured logging capabilities.


53-53: Improved lock acquisition logging with contextual information.

The lock acquisition process now uses structured logging with additional context like lock IDs and project information, making debugging lock-related issues much easier.

Also applies to: 66-67, 92-95


103-104: Enhanced error logging for comment publishing failures.

Error logging for comment publishing failures now includes the error details in a structured format, improving observability.

Also applies to: 108-109, 117-118, 122-123, 194-195, 199-200


143-146: Added context-rich logging for lock operations.

Lock operations (release, force unlock) now have improved logging with contextual information, making it easier to track lock lifecycle.

Also applies to: 164-165, 182-182, 206-207, 221-222


255-257: Standardized provider selection logging.

Lock provider selection and configuration now uses consistent structured logging across all providers (NoOp, AWS, GCP, Azure), with additional context where appropriate.

Also applies to: 260-261, 278-279, 290-293, 298-299, 303-304, 315-316

libs/scheduler/convert.go (6)

4-6: Good transition to structured logging.

The code is transitioning from standard Go logging to the structured logging package log/slog, which will provide better organized log messages with richer context.


13-15: Improved logging with structured context.

This structured log provides clear information about the conversion process, capturing the command being executed and the number of projects being processed.


20-23: Enhanced error logging.

The error logging now includes structured context about the workflow and project, making it easier to diagnose issues when they occur.


36-46: Code formatting improvement.

The code formatting has been cleaned up with proper spacing and variable initialization.


48-58: Excellent addition of detailed debug logging.

This detailed debug log provides rich context about the project being processed, including configuration flags and AWS role information. This will be very helpful for troubleshooting.


85-86: Helpful completion logging.

Adding a log statement to indicate successful conversion with the job count provides a clear indication that the process completed successfully.

libs/comment_utils/reporting/source_grouping.go (12)

3-7: Good transition to structured logging.

Replacing standard logging with the log/slog package for structured logging will improve log organization and searchability.


36-37: Enhanced error logging with context.

Error log now includes the specific location that wasn't found, making debugging easier.


42-44: Improved error context.

The error log now includes the error details when failing to convert jobs to a map.


52-56: Better error reporting for unmarshaling failures.

The structured logging now includes the project name and error details, which will help identify which project had issues during footprint unmarshaling.


74-78: Helpful error context for similarity checks.

The error log now includes the specific location and error details for similarity check failures.


83-87: Informative log for comment generation.

This log provides useful context about the comment being generated, including location, project count, and similarity status.


91-95: Useful debug logging for skipped projects.

The debug log now captures which projects are being skipped due to unsuccessful jobs, along with their status. This will help with troubleshooting issues.


103-107: Improved error logging for comment ID conversion.

The error log now includes the comment ID and error details when conversion fails.


109-113: Clear logging for comment updates.

This log clearly indicates when a comment is being updated with plan details, including the comment ID, PR number, and location.


114-121: Enhanced error handling for comment editing.

This structured error log provides detailed context when comment editing fails, including the comment ID, PR number, and error details.


128-130: Informative debug log for mapping conversion.

This log helps track the conversion of impacted sources to group mapping, including the project count.


148-151: Useful grouping metrics in debug log.

This debug log provides metrics about the source grouping operation, showing both original count and group count after operation.

libs/comment_utils/summary/updater.go (7)

3-6: Good transition to structured logging.

The code is transitioning from standard Go logging to the structured logging package log/slog, which will provide better organized log messages with richer context.


22-25: Enhanced error logging for jobspec retrieval.

Adding structured logging with the error and job count provides better context when jobspec retrieval fails.


30-36: Informative log for comment updates.

This log provides clear context about the comment update operation, including PR number, comment ID, job count, and job type.


44-54: Improved message formatting.

The formatting of the message has been cleaned up for better readability.


59-68: Better warning logs for message trimming.

These logs provide detailed information about the message trimming operation, including the original length, maximum length, and the new length after trimming.


72-80: Comprehensive logging for comment update results.

The code now logs both success and failure scenarios with appropriate context, providing better visibility into the result of the update operation.


89-93: Useful debug logging for noop updater.

Adding a debug log for the NoopCommentUpdater indicates when it's called, making it clear when this implementation is used instead of the standard updater.

libs/scheduler/serializers.go (5)

4-6: Good transition to structured logging.

The code is transitioning from standard Go logging to the structured logging package log/slog, which will provide better organized log messages with richer context.


14-18: Enhanced error logging for job unmarshaling.

The structured error log now includes critical context information (project name, job ID, and error details) that will help identify which job failed to unmarshal and why.


22-23: Helpful debug log for successful operations.

Adding a debug log to indicate successful unmarshaling provides confirmation that the operation completed, along with the count of processed job specs.


30-33: Detailed job mapping debug logs.

These debug logs provide visibility into each job being added to the project map, including the project name and job ID for better traceability.


34-35: Clear summary log for map creation.

This log provides a useful summary of the project map creation operation, showing the total number of projects in the map.

libs/ci/github/github.go (21)

3-19: Import reorganization and slog adoption looks good.

The imports have been properly updated to include the log/slog package and the imports have been reorganized into logical groupings with proper spacing.


71-72: Well-structured error logging with context.

Good conversion from traditional logging to structured logging. Including the prNumber as context will make debugging and tracing issues much easier.


96-97: Appropriate error context for commit operations.

The structured logging now includes the commit ID, which is valuable context for debugging issues with specific commits.


448-450: Efficient error propagation with context.

Good use of structured logging with workflow and project name context, which will help with debugging configuration issues.


476-481: Well-formatted structured logging for PR processing.

The structured logging includes relevant PR information and project details, which will be valuable for monitoring the process flow.


507-512: Consistent structured logging pattern.

The logging pattern is consistent with other parts of the file, maintaining good readability and useful context.


539-542: Streamlined logging for PR closed events.

The logging is concise and includes just the necessary information for tracing PR closure events.


576-580: Complete context for draft PR conversions.

Good inclusion of the allowDraftPRs configuration value in the logs, which will be helpful for understanding behavior differences.


617-621: Clear structured logging for PR event processing.

The log message clearly identifies the event type, PR number, and action being performed.


630-633: Useful project impact logging.

Including the count of impacted projects provides good visibility into the scope of changes for each PR.


655-658: Helpful debug logging for requested projects.

The debug log provides clear visibility into which specific project was requested in the comment.


667-671: Improved error logging with detailed context.

The error log now provides specific details about why the operation failed, including the requested project name.


673-675: Appropriate logging level for unhandled event.

Using debug level for a non-error condition that's simply not handled is the right approach.


688-692: Clear event processing indication in logs.

The structured log provides clear information about the event being processed, including the PR number and action.


700-704: Informative impact assessment logging.

Good log message that clearly indicates the count of directly impacted projects.


706-720: Detailed dependency resolution logging.

The logs provide excellent visibility into the dependency resolution process, showing both the original count and total count after resolving dependencies.


734-738: Clear push event processing logs.

The log includes all necessary information to track push events, including commit ID, owner, and repo.


751-754: Concise impact assessment for push events.

The log effectively communicates the number of impacted projects from a push event.


762-765: Helpful debug logging for comment matching.

Including the pattern and comment ID in debug logs will help with troubleshooting comment-related issues.


772-776: Good debug confirmation for help commands.

The logging helps trace when help commands are detected in the system.


780-784: Consistent debug logging pattern.

The logging for show-projects follows the same pattern as help commands, maintaining consistency.

libs/backendapi/diggerapi.go (18)

3-21: Clean import reorganization with slog.

The imports have been properly updated to include the log/slog package and other imports have been reorganized with appropriate spacing.


55-57: Good transition from fatal to error+return pattern.

Converting from log.Fatalf to slog.Error followed by returning an error is a significant improvement to error handling, as it allows callers to handle errors gracefully rather than terminating the program.


67-69: Consistent error logging pattern.

The error logging follows the same pattern as in other places, maintaining consistency throughout the codebase.


96-98: Well-structured error with context.

The error logging includes the original error as context, which helps with debugging.


112-114: Consistent error handling approach.

The pattern of logging the error and then returning a formatted error is consistently applied.


142-144: Proper error propagation in API reporting.

The structured logging and error return pattern is well-implemented for the cloud URL parsing error.


149-152: Appropriate warning level for non-critical issues.

Using warning level for a nil summary is appropriate since it's not necessarily an error condition but is worth noting.


158-159: Error logging without error return.

In this case, the error is logged but not returned, which is appropriate since the function can continue without the footprint.


175-177: Consistent error handling in JSON marshaling.

The error handling follows the established pattern for the rest of the file.


221-223: Clear file operation error handling.

Good error message that clearly indicates what operation failed.


235-237: Informative error for form file creation.

The error message clearly explains what failed during the file upload process.


242-244: Descriptive error for file content operations.

The error message is specific about what operation failed.


252-254: Precise error logging for request creation.

The logging includes the error details for debugging HTTP request issues.


264-266: Comprehensive error handling for HTTP requests.

The structured logging and error return correctly handle HTTP request failures.


272-274: Consistent error pattern for response handling.

The error handling maintains the same pattern as other HTTP operations.


297-299: Simple error logging for URL creation.

Appropriate error logging for a basic operation like URL creation.


304-306: Consistent error handling for request creation.

The error pattern is maintained throughout the HTTP operations.


340-341: Informative warning for backend mode.

The warning clearly indicates that some features won't be available when running in backendless mode.

libs/execution/execution.go (23)

3-20: Clean import reorganization with slog.

The imports have been properly updated to include the log/slog package and other dependencies have been organized with appropriate spacing.


41-42: Simple informational logging for lock operations.

The log provides useful information about the lock state, which is helpful for debugging locking issues.


55-56: Consistent lock result logging.

The logging follows the same pattern as the previous lock operation, maintaining consistency.


68-69: Uniform lock status logging.

This maintains the same logging pattern as the other locking operations.


180-181: Informative log for plan retrieval.

The log clearly indicates what operation is being performed and its context.


223-224: Clear step execution logging.

The log provides precise information about which step is currently running.


276-277: Appropriate error logging for comment publication.

The log correctly captures errors that occur during comment publishing.


285-288: Structured logging with rich context.

The log includes the command being run and the project identifier, which is valuable for tracking execution progress.


302-303: Consistent error logging pattern.

The error logging follows the same pattern as in other error-handling sections.


307-308: Uniform error logging approach.

The error handling is consistent with the rest of the error-logging patterns.


360-361: Appropriate warning level for non-critical issues.

Using warning level for a failure to get summary from apply output is appropriate, as the operation can continue.


369-372: Detailed command execution logging.

The logs provide clear information about the command being executed and its context.


386-387: Error logging for comment publishing.

The log correctly captures errors during comment publishing.


391-392: Consistent error logging pattern.

The error logging follows the established pattern throughout the file.


406-407: Uniform error handling for comments.

The error logging maintains consistency with other comment-related error handling.


414-415: Consistent error pattern for comment publishing.

The error logging follows the same pattern as other comment operations.


419-420: Uniform error handling approach.

The error logging approach is consistent with other similar operations.


438-439: Appropriate error logging for comment publishing.

The error logging correctly captures comment publishing errors.


441-442: Good use of debug level for non-issues.

Using debug level for noting an empty output file is appropriate as it's not an error condition.


445-446: Proper error logging for file operations.

The error log includes the file path, which is helpful for debugging file removal issues.


448-449: Appropriate debug logging for missing files.

Using debug level for noting a missing file is correct since it's an expected condition in some cases.


549-553: Rich structured logging for command failures.

The log includes the executor name, exit code, and error details, providing comprehensive information for debugging.


554-557: Fallback error logging with context.

Good fallback error logging that still includes the available information even when the executor name can't be determined.

libs/comment_utils/reporting/reporting.go (9)

3-11: Clean import reorganization with slog.

The imports have been properly updated to include the log/slog package and other dependencies have been reorganized with appropriate spacing.


61-62: Informative suppression log.

The log clearly indicates that the reporter has been suppressed, which helps with understanding the system's behavior.


69-70: Structured error logging for reporting failures.

The error logging includes the error details as structured data, which helps with debugging reporting issues.


91-92: Simple informational logging for reports.

The log clearly shows the content of the report being processed.


119-120: Contextualized error logging for comments.

The error log includes the PR number, which is useful for tracking down comment retrieval issues.


156-157: Detailed error logging for comment publishing.

The log includes the PR number, providing context for comment publishing errors.


179-180: Rich error logging for comment editing.

The log includes both the comment ID and PR number, which are essential for debugging comment editing issues.


192-193: Consistent error logging pattern.

The error logging follows the same pattern as in other comment-related operations.


204-210: Enhanced return values with structured logging.

The method now correctly returns the comment ID and URL upon successful publication, and includes structured error logging.

libs/locking/aws/envprovider/envprovider.go (6)

7-7: Good migration to structured logging.

The change from standard log package to structured log/slog is appropriate and in line with modern Go practices for enhancing observability.


53-54: Good addition of debug logging.

The added debug log provides valuable context for troubleshooting when retrieving keys from a role.


68-70: Excellent structured logging implementation.

Good work using structured fields instead of string interpolation. The log now clearly separates the roleArn and sessionName values, making it easier to filter and search logs.


74-77: Proper error logging with context.

Good implementation of structured error logging that includes the role being assumed and the detailed error, which will be valuable for troubleshooting authentication issues.


84-87: Helpful success log with credential expiration.

This is a valuable addition that logs when a role is successfully assumed and includes the expiration time, which can help diagnose issues with short-lived credentials.


104-106: Comprehensive error context.

Good implementation showing which environment variables were searched for the access key, providing clear debugging information.

libs/comment_utils/reporting/utils.go (6)

5-7: Good import organization.

Appropriate replacement of standard log package with structured slog and maintaining clean import organization.


22-25: Well-structured entry log.

Good initial log with meaningful context data that helps track the PR processing flow from start to finish.


33-36: Helpful intermediate progress logging.

Good debug log that provides visibility into the iteration process, showing exactly which location is being processed and how many projects are associated with it.


45-49: Comprehensive error context.

The error log now includes the specific location and PR number, which significantly improves debuggability when issues occur.


52-56: Useful success confirmation log.

Including the comment ID in the success log is helpful for tracking the lifecycle of comments across the system.


60-63: Good summary log.

The final summary log provides a clear indication of completion and summarizes the work done, which is valuable for monitoring and auditing.

libs/locking/gcp/gcp_lock.go (6)

6-7: Good migration to structured logging.

Appropriate replacement of standard log package with structured slog for improved observability.


37-41: Enhanced error logging.

The structured error log includes important context (bucket and file name) that will simplify troubleshooting of GCP storage operations.


43-47: Valuable debug log for lock acquisition.

This log entry clearly documents successful lock operations and includes all relevant context, which is essential for distributed systems where lock contention can be difficult to debug.


68-71: Good lock release logging.

Adding the transaction ID to the release log provides important context for audit trails and debugging lock-related issues.


89-93: Comprehensive error context.

The structured error log provides detailed information about parsing failures, including the raw lock ID that caused the problem.


104-107:

✅ Verification successful

Consider error handling approach.

While the comment explains why panic is used (to maintain the original behavior of log.Fatalf), this is still an abrupt termination approach. Consider refactoring to return an error instead of panicking.

if err != nil {
	slog.Error("Failed to create Google Storage client", "error", err)
-	// Since the original used log.Fatalf which exits, we'll maintain that behavior
-	panic(fmt.Sprintf("Failed to create Google Storage client: %v", err))
+	return nil, fmt.Errorf("failed to create Google Storage client: %v", err)
}

Are there other places in the codebase that might catch a panic, or is this function called in a context where returning an error would be preferred to maintain application stability?


🏁 Script executed:

#!/bin/bash
# Look for functions that call GetGoogleStorageClient and how they handle errors
rg -A 5 "GetGoogleStorageClient\(\)" --type go

Length of output: 4816


Subject: Confirmed: Current Panic Behavior is Consistent with Codebase Usage

After reviewing how GetGoogleStorageClient() is utilized across the repository (e.g., in libs/spec/providers.go, libs/storage/plan_storage.go, and several test files in libs/locking/gcp/gcp_lock_test.go), it’s clear that the function is expected to either return a valid client or immediately terminate the application (via panic) in the event of a failure. This behavior mimics the original log.Fatalf call and is relied upon by its callers, which do not include additional panic recovery or error checks.

  • The function’s signature and usage throughout the codebase indicate that a failure to create the client is treated as unrecoverable.
  • Changing its error handling to return an error would necessitate significant refactoring of many call sites.
  • While returning an error may be desirable for more graceful error propagation, the current design intentionally ensures that execution is halted when a critical dependency (the Google Storage client) fails to initialize.

We can revisit this design in a future refactor if a more resilient error-handling strategy is needed, but for now, the abrupt termination via panic is consistent with the overall usage and expectations.

libs/ci/bitbucket/bitbucket_service.go (10)

5-7: Good migration to structured logging.

The change from standard log to structured logging with slog is appropriate and follows best practices.


47-52: Well-structured context log.

Excellent inclusion of repository details, event type, and PR ID in the structured log, which will significantly improve traceability.


63-66: Good event processing log.

This log clearly marks the beginning of event processing and includes all necessary context to track the request through the system.


82-85: Helpful PR comment processing log.

This debug log provides visibility into the comment handling flow, including the exact command being processed, which will aid in troubleshooting comment-related issues.


106-109: Good logging of event conversion.

The log clearly indicates the start of the event-to-commands conversion process and provides context about the number of impacted projects.


115-118: Enhanced error logging.

The error log now includes the specific workflow and project name, which will simplify debugging of workflow configuration issues.


150-154: Valuable job creation log.

This debug log provides essential visibility into the job creation process, associating each job with its source PR and event type.


164-167: Improved logging for project selection.

This log clearly indicates when a specific project is selected from multiple impacted projects, helping to trace down the effects of commands.


190-193: Good fallback logging.

This debug log properly documents when the default workflow is used as a fallback, which helps understand the system's behavior when configurations are incomplete.


232-236: Comprehensive job creation logging.

The debug log captures all relevant details about jobs created from comments, including the project, command, and workspace, which is excellent for audit trails and debugging.

libs/ci/gitlab/gitlab.go (5)

68-72: Well-structured logging implementation!

The addition of structured logging with key-value pairs provides much better context compared to the previous string interpolation approach. This will make logs more searchable and easier to parse.


111-114: Good enhancement with structured logging.

Adding structured logging for the GitLab event processing provides better visibility into what events are being handled, making troubleshooting easier.


444-448: Great use of debug logging for traceability.

Using debug logging for MR update events helps with understanding the flow of operations when troubleshooting issues.


499-502: Good context provided in logs.

Including the project name and event type in the logs helps correlate actions across the system.


580-584: Well-documented job creation in logs.

The detailed debug logging provides excellent context for understanding what commands are being created from user comments.

libs/locking/aws/dynamo_locking.go (3)

64-67: Good debugging context for DynamoDB table status.

Adding structured debug logging with table status and retry count improves observability when troubleshooting table creation issues.


148-151: Well-structured logging for lock operations.

The debug logging for lock acquisition attempts with both resource and transaction ID provides excellent context for troubleshooting lock-related issues.


203-207: Good operational confirmation in logs.

The success confirmation logging with timeout information helps track lock lifecycle and potential expiration issues.

libs/policy/policy.go (2)

526-529: Well-structured debug logging for drift policy.

The addition of structured debug logging with organization, repository, and project provides good context for understanding drift policy application.


535-542: Thorough error logging for drift policy failures.

The detailed error logging for drift policy issues helps with troubleshooting complex policy evaluation problems.

libs/scheduler/aws.go (3)

144-154: Method receiver syntax corrected.

Fixed method receiver syntax from func(job *Job) to func (job *Job), which ensures proper method resolution.


393-397: Good structured logging for Cognito authentication.

Adding debug logging with identifiable fields like idpName and poolId greatly improves the traceability of Cognito token operations.


443-459: Well-structured logging for identity operations.

The sequence of debug logs provides clear insight into the Cognito identity workflow, making it easier to diagnose authentication issues.

@motatoes
Copy link
Contributor

motatoes commented Mar 30, 2025

Hey @aldoborrero ! Thanks for this !! 🫡 🫡 🫡

please take a look at the comments from the bots that are unresolved, I resolved the irrelevant ones.

Also looks like the CI tests are failing with some NPE error, seems due to that return statement in MultiCommentStrategyReporter. I haven't checked but if you are not able to chase it down quickly feel free to revert that return line and we can take a look at it seperately!

Thanks!!

@aldoborrero
Copy link
Contributor Author

Hey @aldoborrero ! Thanks for this !! 🫡 🫡 🫡

please take a look at the comments from the bots that are unresolved, I resolved the irrelevant ones.

Also looks like the CI tests are failing with some NPE error, seems due to that return statement in MultiCommentStrategyReporter. I haven't checked but if you are not able to chase it down quickly feel free to revert that return line and we can take a look at it seperately!

Thanks!!

You're welcome! I think the issue is caused mainly by the mocked value providing a nil *ci.Comment?

I added a fix to the proper `MockPRManager' and it should pass now.

@aldoborrero
Copy link
Contributor Author

aldoborrero commented Mar 31, 2025

@motatoes I also added the conversion to slog for gorm and sentry in this PR (as it made sense).

I have more plans to structure and initialize the backend with better configurations and code, but this PR is good enough for now, we can discuss those improvements in a separate issue.

@motatoes motatoes merged commit 2e301b4 into diggerhq:develop Apr 2, 2025
8 checks passed
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