Ft: Refactor codebase for health logging and different routing stragety - #73
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive refactoring of the PWS codebase to add health logging capabilities and implement different routing strategies. The main goal is to level up the API with better monitoring, improved worker management, and more robust configuration.
- Introduces a new health monitoring system with dedicated workers and middleware
- Refactors worker management from global singleton pattern to dependency injection pattern
- Adds circuit breaker patterns for database operations to improve resilience
Reviewed Changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/server/workers/manager_test.go | New comprehensive test suite for the worker manager with concurrency and benchmark tests |
| apps/server/workers/manager.go | New worker manager implementation with dependency injection and coordinated lifecycle management |
| apps/server/workers/helper.go | Dead letter queue implementation for handling failed audit operations |
| apps/server/workers/health_worker.go | Health monitoring worker for tracking API route metrics and service health |
| apps/server/workers/cleanup_worker.go | Dedicated cleanup worker with scheduled maintenance operations |
| apps/server/workers/audit_worker.go | Refactored audit worker with improved error handling and circuit breaker patterns |
| apps/server/types/config.go | Extended configuration types with health monitoring and Google OAuth support |
| apps/server/types/app.go | Updated health response structure and added health log type definitions |
| apps/server/tests/health_logs_test.go | Test coverage for health logging functionality |
| apps/server/tests/health_endpoints_test.go | Comprehensive tests for health monitoring endpoints with panic protection |
| apps/server/services/database.go | Enhanced database service with circuit breaker protection |
| apps/server/main.go | Updated application initialization to use new worker manager pattern |
| apps/server/lib/constants.go | Added health logs table constant |
| apps/server/lib/circuit_breaker.go | New circuit breaker implementation for database resilience |
| apps/server/database/migrations/create_health_logs.sql | Database schema for health logging storage |
| apps/server/config/logger.go | Added audit warning capability for improved logging granularity |
| apps/server/config/domains.go | New domain-specific configuration management for better separation of concerns |
| apps/server/config/config.go | Refactored configuration to support domain-based organization |
| apps/server/api/routes/workers.go | New worker monitoring endpoints for health status and metrics |
| apps/server/api/routes/manager.go | Route manager for coordinating different route groups |
| apps/server/api/routes/health.go | Health check endpoints with admin middleware protection |
| apps/server/api/routes/files.go | Refactored file routes to use new route manager pattern |
| apps/server/api/routes/auth.go | Updated authentication routes for new router architecture |
| apps/server/api/router.go | Enhanced router with health middleware and route discovery |
| apps/server/api/middleware/health.go | New middleware for automatic health metric collection |
| apps/server/api/middleware/auth.go | Added admin middleware for protected endpoints |
| apps/server/api/internal/workers.go | Worker health monitoring API handlers |
| apps/server/api/internal/files/upload.go | Updated file upload handlers for new router structure |
| apps/server/api/internal/files/retrieve.go | Updated file retrieval handlers |
| apps/server/api/internal/files/manager.go | File routes manager implementation |
| apps/server/api/internal/auth/google_oauth.go | Moved Google OAuth handlers to auth package |
| apps/server/api/internal/auth/auth.go | Refactored authentication handlers with improved structure |
| apps/server/api/internal/app.go | Updated application handlers with simplified health responses |
| apps/server/.env.example | Updated environment configuration with new health monitoring settings |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+180
to
+200
| wg.Go(func() { | ||
| if err := wm.auditWorker.Stop(ctx); err != nil { | ||
| errChan <- fmt.Errorf("audit worker stop error: %w", err) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| if wm.healthWorker != nil { | ||
| wg.Go(func() { | ||
| if err := wm.healthWorker.Stop(ctx); err != nil { | ||
| errChan <- fmt.Errorf("health worker stop error: %w", err) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| if wm.cleanupWorker != nil { | ||
| wg.Go(func() { | ||
| if err := wm.cleanupWorker.Stop(ctx); err != nil { | ||
| errChan <- fmt.Errorf("cleanup worker stop error: %w", err) | ||
| } | ||
| }) |
There was a problem hiding this comment.
The method wg.Go does not exist on sync.WaitGroup. Use wg.Add(1) followed by go func() { defer wg.Done(); ... }() pattern instead.
Suggested change
| wg.Go(func() { | |
| if err := wm.auditWorker.Stop(ctx); err != nil { | |
| errChan <- fmt.Errorf("audit worker stop error: %w", err) | |
| } | |
| }) | |
| } | |
| if wm.healthWorker != nil { | |
| wg.Go(func() { | |
| if err := wm.healthWorker.Stop(ctx); err != nil { | |
| errChan <- fmt.Errorf("health worker stop error: %w", err) | |
| } | |
| }) | |
| } | |
| if wm.cleanupWorker != nil { | |
| wg.Go(func() { | |
| if err := wm.cleanupWorker.Stop(ctx); err != nil { | |
| errChan <- fmt.Errorf("cleanup worker stop error: %w", err) | |
| } | |
| }) | |
| wg.Add(1) | |
| go func() { | |
| defer wg.Done() | |
| if err := wm.auditWorker.Stop(ctx); err != nil { | |
| errChan <- fmt.Errorf("audit worker stop error: %w", err) | |
| } | |
| }() | |
| } | |
| if wm.healthWorker != nil { | |
| wg.Add(1) | |
| go func() { | |
| defer wg.Done() | |
| if err := wm.healthWorker.Stop(ctx); err != nil { | |
| errChan <- fmt.Errorf("health worker stop error: %w", err) | |
| } | |
| }() | |
| } | |
| if wm.cleanupWorker != nil { | |
| wg.Add(1) | |
| go func() { | |
| defer wg.Done() | |
| if err := wm.cleanupWorker.Stop(ctx); err != nil { | |
| errChan <- fmt.Errorf("cleanup worker stop error: %w", err) | |
| } | |
| }() |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
IMPORTANT: Use this template for all PRs to ensure consistency.
You can remove sections that do not apply to your change.
Description & Motivation
Introduces a bunch of new features, leveling up the api
Related Issues
Relates #28
(Only include “Closes” for issues you intend this PR to fully resolve.)
Scope of Change
(Mark or list the relevant ones)