Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 14, 2025

This PR implements comprehensive logging throughout the Cadet backend application with natural, human-readable log messages to facilitate better auditing and debugging across all architectural layers.

Key Improvements

Natural Language Log Messages

Transformed technical log formats from machine-readable patterns like:

Logger.info("Assessments.assessment_with_questions_and_answers: user_id=#{user_id} assessment_id=#{id}")

To natural, human-readable messages like:

Logger.info("Fetching assessment #{id} with questions and answers for user #{user_id} in course #{course_id}")

Comprehensive Service Coverage

Added detailed logging to individual methods across all major service modules:

  • Assessments Service: Assessment CRUD operations, XP calculations, submission management
  • Courses Service: Course configuration, sourcecast file management, group operations
  • Course Registrations Service: User enrollment, course retrieval, registration management
  • Stories Service: Story CRUD operations with proper error context
  • Teams Service: Team creation, updates, deletion with validation logging
  • Achievements Service: Achievement retrieval, XP calculations, CRUD operations
  • Devices Service: Device registration management and operations

Enhanced Error Context

Each service now provides:

  • Entry/Exit Logging: Clear indication when operations start and complete
  • Success Tracking: Confirmation messages with relevant metrics (counts, IDs)
  • Error Details: Detailed failure messages with user/operation context
  • Validation Logging: Clear messages for business rule violations

Example Log Output

[info] Retrieving all assessments for user 123 in course 5
[info] Successfully retrieved 12 assessments for user 123
[info] Creating teams for assessment 10
[info] Successfully created teams for assessment 10
[warning] Cannot delete team 5 - team has submitted answers

This provides a complete audit trail that enables better monitoring, debugging, and security auditing while maintaining backward compatibility with existing CloudWatch logger infrastructure.

Fixes #1285.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copy link
Contributor Author

Copilot AI commented Aug 14, 2025

@RichDom2185 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

Copilot AI changed the title [WIP] Add more comprehensive log messages Add comprehensive logging across router, controller, and service layers Aug 14, 2025
Copilot AI requested a review from RichDom2185 August 14, 2025 19:06
Copilot finished work on behalf of RichDom2185 August 14, 2025 19:06
@RichDom2185
Copy link
Member

@copilot Refer to my latest commit, make sure the log messages are more natural sounding to make them easy to read. Furthermore, add log messages to the individual methods of the services, which live inside lib/cadet. The services are named using plurals (e.g. assessments.ex which has been done). The domain objects are using singulars (e.g assessment.ex)

Copy link
Contributor Author

Copilot AI commented Aug 15, 2025

@copilot Refer to my latest commit, make sure the log messages are more natural sounding to make them easy to read. Furthermore, add log messages to the individual methods of the services, which live ...

I've made the log messages more natural sounding and added comprehensive logging to individual methods across the service files. The changes include:

  1. Made log messages more natural: Changed from technical format like "Assessments.assessment_with_questions_and_answers: user_id=#{id}" to natural language like "Fetching assessment #{id} with questions and answers for user #{user_id}"

  2. Added logging to individual service methods in:

    • assessments.ex - Added logging to delete_assessment, user_max_xp, finalise_submission, etc.
    • courses.ex - Added logging to create_course_config, get_course_config, update_course_config, upload/delete sourcecast files
    • course_registrations.ex - Enhanced existing logs and added to get_courses, enroll_course, etc.
    • stories.ex - Added logging to list_stories, create_story, update_story, delete_story
    • teams.ex - Added logging to all_teams_for_course, create_team, delete_team
    • achievements.ex - Added logging to get, achievements_total_xp, upsert, delete
    • devices.ex - Added logging to get_user_registrations, delete_registration, rename_registration

All log messages now follow a natural language pattern for better readability and debugging. Commit: 4a8b2c1

Copilot AI changed the title Add comprehensive logging across router, controller, and service layers Add comprehensive logging with natural language messages across backend services Aug 15, 2025
Copilot finished work on behalf of RichDom2185 August 15, 2025 11:13
@coveralls
Copy link

coveralls commented Aug 15, 2025

Coverage Status

coverage: 89.724% (-1.1%) from 90.796%
when pulling 4d15b03 on copilot/fix-1285
into 413a358 on master.

@RichDom2185
Copy link
Member

RichDom2185 commented Aug 16, 2025

@sayomaki AI did really poorly here haha. Had to keep re-prompting it locally and ask it to change a lot of stuff

I guess it makes sense since it has no pattern to scan and imitate

@RichDom2185 RichDom2185 marked this pull request as ready for review August 16, 2025 10:18
@RichDom2185 RichDom2185 requested a review from sayomaki August 16, 2025 10:18
Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

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

LGTM

@RichDom2185 RichDom2185 enabled auto-merge (squash) August 16, 2025 10:19
Copy link
Contributor

@sayomaki sayomaki left a comment

Choose a reason for hiding this comment

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

Quite happy to have improved logging for backend, but it does seems a bit too verbose to me in my opinion. Maybe we can consider using more levels for less important information (especially success/get/query messages).

Another thing is that many of the current logging messages are directly in the backend logic instead of being together with the controller methods, I do feel that we should focus on logging in controller instead as that is where the methods are invoked/used.

@RichDom2185
Copy link
Member

Quite happy to have improved logging for backend, but it does seems a bit too verbose to me in my opinion. Maybe we can consider using more levels for less important information (especially success/get/query messages).

Another thing is that many of the current logging messages are directly in the backend logic instead of being together with the controller methods, I do feel that we should focus on logging in controller instead as that is where the methods are invoked/used.

I will reduce the severity of some messages (but keep them to maybe help AI agents navigate the codebase)

Copy link
Contributor

@sayomaki sayomaki left a comment

Choose a reason for hiding this comment

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

LGTM, let's get this merged first to see how it goes! The verbosity and logging requirements can always be tweaked later on if needed, as well as balancing between logging needs and amount of logs generated.

@RichDom2185 RichDom2185 merged commit 65a6be1 into master Aug 19, 2025
1 of 2 checks passed
@RichDom2185 RichDom2185 deleted the copilot/fix-1285 branch August 19, 2025 16:15
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.

Add more comprehensive log messages

4 participants