Skip to content

Add Cloudwatch logging#85

Merged
codycooperross merged 1 commit intomainfrom
logging
Mar 19, 2026
Merged

Add Cloudwatch logging#85
codycooperross merged 1 commit intomainfrom
logging

Conversation

@codycooperross
Copy link
Contributor

@codycooperross codycooperross commented Mar 19, 2026

Purpose

closes: Add github issue that originated this PR

Approach

Open Questions and Pre-Merge TODOs

Learning

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to change)

Reviewer, please remember our guidelines:

  • Be humble in the language and feedback you give, ask don't tell.
  • Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
  • Offer suggestions on how to improve code e.g. simplification or expanding clarity.
  • Ensure you give reasons for the changes you are proposing.

Summary by CodeRabbit

  • Chores
    • Improved internal logging infrastructure by updating event reindex task logging mechanism to include additional timestamp details for better monitoring and debugging purposes.

@coderabbitai
Copy link

coderabbitai bot commented Mar 19, 2026

📝 Walkthrough

Walkthrough

A logging output statement in a Rake task is refactored from a direct puts call to Rails.logger.info, now capturing start_date and end_date alongside the count parameter. The underlying task logic remains unchanged.

Changes

Cohort / File(s) Summary
Rake Task Logging
lib/tasks/event.rake
Replaced puts output with Rails.logger.info call, adding start_date and end_date parameters to the completion message for the reindex-touched DOIs operation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Add Cloudwatch logging' is overly broad and vague. The actual change is replacing a puts call with Rails.logger.info in a Rake task, not adding Cloudwatch logging infrastructure. Use a more specific title that accurately describes the change, such as 'Replace puts with Rails logger in event reindex Rake task' or 'Log DOI reindex completion to Rails logger'.
✅ 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 changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch logging
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

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

Copy link

@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.

🧹 Nitpick comments (1)
lib/tasks/event.rake (1)

52-52: Good migration to Rails.logger.info.

This change correctly routes the log output through Rails' logging infrastructure, enabling CloudWatch integration.

However, the first task (import_crossref_event_dois) still uses puts on lines 9-11, 18, 24, and 35. For consistent CloudWatch logging across all tasks in this file, consider updating those as well.

,

♻️ Suggested changes for consistency
-    puts("Import date: #{date}")
-    puts("Start date: #{start_date}")
-    puts("End date: #{end_date}")
+    Rails.logger.info("Import date: #{date}")
+    Rails.logger.info("Start date: #{start_date}")
+    Rails.logger.info("End date: #{end_date}")
-    puts("Number of events: #{events.count}")
+    Rails.logger.info("Number of events: #{events.count}")
-      puts("Processing batch: #{batch_count}")
+      Rails.logger.info("Processing batch: #{batch_count}")
-    puts("Rake task has completed!")
+    Rails.logger.info("Rake task has completed!")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/tasks/event.rake` at line 52, The task import_crossref_event_dois still
uses puts for console output; replace each puts call in that rake task with
Rails.logger.info (preserving the original message text and interpolation) so
all logging in lib/tasks/event.rake goes through Rails.logger (this applies to
the puts invocations inside the import_crossref_event_dois task where messages
are printed at the start, progress, and completion points).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@lib/tasks/event.rake`:
- Line 52: The task import_crossref_event_dois still uses puts for console
output; replace each puts call in that rake task with Rails.logger.info
(preserving the original message text and interpolation) so all logging in
lib/tasks/event.rake goes through Rails.logger (this applies to the puts
invocations inside the import_crossref_event_dois task where messages are
printed at the start, progress, and completion points).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f80f17d1-69b1-4d65-9ead-7dadd0c25d26

📥 Commits

Reviewing files that changed from the base of the PR and between 846a1b2 and 9051029.

📒 Files selected for processing (1)
  • lib/tasks/event.rake

@codycooperross codycooperross merged commit 14434ea into main Mar 19, 2026
4 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