Fix the analytics issue and update the README file #51
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes analytics functionality by correcting cost calculation formulas and replaces dummy data with actual database queries. The PR also adds a comprehensive README file to improve project documentation.
- Fixed cost calculation formulas in the analytics utils by removing incorrect scaling factors
- Updated analytics handlers to use actual database queries instead of returning hardcoded dummy data
- Added comprehensive README documentation covering architecture, API endpoints, and setup instructions
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| analytics/utils.go | Fixed cost calculation formulas by removing scaling factors and updating rate constants |
| analytics/models.go | Removed extensive dummy data variables that were previously used for testing |
| analytics/handlers.go | Updated handlers to use actual database queries and improved error handling for NULL values |
| README.md | Added comprehensive project documentation including setup, API endpoints, and architecture |
| // CalculateCosts calculates the costs associated with database usage based on read/write queries and CPU time. | ||
| func (d *DatabaseUsageStats) CalculateCosts() DatabaseUsageCost { | ||
| Cost := DatabaseUsageCost{ | ||
| ReadWriteCost: float64(d.ReadQueries)/1_000_000*1.00 + float64(d.WriteQueries)/1_000_000*1.50, | ||
| CPUCost: (d.TotalCPUTimeMs / 1000 / 3600) * 0.000463, | ||
| ReadWriteCost: float64(d.ReadQueries)*1.00 + float64(d.WriteQueries)*1.50, | ||
| CPUCost: d.TotalCPUTimeMs * 1.50, |
There was a problem hiding this comment.
The cost calculation uses magic numbers (1.00, 1.50) without explanation. Consider defining these as named constants with clear documentation about the pricing model.
| // CalculateCosts calculates the costs associated with database usage based on read/write queries and CPU time. | ||
| func (d *DatabaseUsageStats) CalculateCosts() DatabaseUsageCost { | ||
| Cost := DatabaseUsageCost{ | ||
| ReadWriteCost: float64(d.ReadQueries)/1_000_000*1.00 + float64(d.WriteQueries)/1_000_000*1.50, | ||
| CPUCost: (d.TotalCPUTimeMs / 1000 / 3600) * 0.000463, | ||
| ReadWriteCost: float64(d.ReadQueries)*1.00 + float64(d.WriteQueries)*1.50, | ||
| CPUCost: d.TotalCPUTimeMs * 1.50, |
There was a problem hiding this comment.
The CPU cost calculation uses a magic number (1.50) and appears to have incorrect units. CPU time in milliseconds multiplied by a rate should likely be converted to appropriate time units (seconds/hours) for meaningful cost calculation.
|
|
||
| _, apiErr := GetALLExecutionTimeStats(r.Context(), config.DB, projectOid) | ||
| stats, apiErr := GetALLExecutionTimeStats(r.Context(), config.DB, projectOid) | ||
| if len(stats) == 0 || (apiErr.Error() != nil && strings.Contains(apiErr.Error().Error(), "cannot scan NULL into")) { |
There was a problem hiding this comment.
The indentation uses spaces instead of tabs, which is inconsistent with Go formatting standards. Run 'go fmt' to fix formatting.
| if len(stats) == 0 || (apiErr.Error() != nil && strings.Contains(apiErr.Error().Error(), "cannot scan NULL into")) { | |
| if len(stats) == 0 || (apiErr.Error() != nil && strings.Contains(apiErr.Error().Error(), "cannot scan NULL into")) { |
| storage, apiErr := GetALLDatabaseStorage(r.Context(), config.DB, projectOid) | ||
| if apiErr.Error() != nil { | ||
| utils.ResponseHandler(w, r, apiErr) | ||
| if len(storage) == 0 || (apiErr.Error() != nil && strings.Contains(apiErr.Error().Error(), "cannot scan NULL into")) { |
There was a problem hiding this comment.
The error handling logic is duplicated across multiple handlers. Consider extracting this into a shared utility function to reduce code duplication.
| storage, apiErr := GetALLDatabaseStorage(r.Context(), config.DB, projectOid) | ||
| if apiErr.Error() != nil { | ||
| utils.ResponseHandler(w, r, apiErr) | ||
| if len(storage) == 0 || (apiErr.Error() != nil && strings.Contains(apiErr.Error().Error(), "cannot scan NULL into")) { |
There was a problem hiding this comment.
String matching on error messages ("cannot scan NULL into") is fragile and can break if the underlying library changes error messages. Consider using error type checking or defining custom error types for more robust error handling.
No description provided.