Skip to content

Conversation

Br0wnHammer
Copy link
Member

@Br0wnHammer Br0wnHammer commented Aug 18, 2025

Describe your changes

This PR improves our uptime monitor pipeline. Currently it was taking a whole lot of time which ultimately affected the UI.

Please ensure all items are checked off before requesting a review. "Checked off" means you need to add an "x" character between brackets so they turn into checkmarks.

  • (Do not skip this or your PR will be closed) I deployed the application locally.
  • (Do not skip this or your PR will be closed) I have performed a self-review and testing of my code.
  • I have included the issue # in the PR.
  • I have added i18n support to visible strings (instead of <div>Add</div>, use):
const { t } = useTranslation();
<div>{t('add')}</div>
  • I have not included any files that are not related to my pull request, including package-lock and package-json if dependencies have not changed
  • I didn't use any hardcoded values (otherwise it will not scale, and will make it difficult to maintain consistency across the application).
  • I made sure font sizes, color choices etc are all referenced from the theme. I don't have any hardcoded dimensions.
  • My PR is granular and targeted to one specific feature.
  • I ran npm run format in server and client directories, which automatically formats your code.
  • I took a screenshot or a video and attached to this PR if there is a UI change.

Before
Screenshot 2025-08-19 023333

After
Screenshot 2025-08-19 032315

Summary by CodeRabbit

  • New Features

    • Per-period buckets now include per-bucket up/down breakdowns and grouped averages, with consolidated overall totals; preserves existing API.
  • Bug Fixes

    • More accurate uptime percentage calculations with explicit zero-check handling.
    • Consistent average response-time reporting across period and overall views.
  • Refactor

    • Streamlined aggregation into a single bucketed pipeline using creation-date grouping for simpler, more reliable results.

@Br0wnHammer Br0wnHammer added this to the 3.0 milestone Aug 18, 2025
@Br0wnHammer Br0wnHammer self-assigned this Aug 18, 2025
Copy link

coderabbitai bot commented Aug 18, 2025

Walkthrough

Switches uptime-details aggregation to use createdAt for date filtering and bucketing; replaces multi-facet per-window pipeline with a single bucketed aggregation that produces per-bucket totals and averages, overall totals, and mapped up/down breakdowns; monitor lookup and public signatures unchanged.

Changes

Cohort / File(s) Summary of changes
Uptime details aggregation refactor
server/src/db/mongo/modules/monitorModuleQueries.js
Primary date filter and bucketing switched from updatedAt to createdAt. Replaced facet-based per-window pipelines with a single pipeline: date-string grouping for per-bucket totals (totalChecks, upChecks, downChecks) and response-time sums/averages; final $group collects buckets and accumulates totals; $project computes uptimePercentage, avgResponseTime/groupedAvgResponseTime, groupedChecks, groupedUpChecks, and groupedDownChecks. Final sort changed to _id: 1. Monitor lookup and exported signatures remain the same.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Server
  participant MongoDB

  Client->>Server: Request uptime details
  Server->>MongoDB: Aggregation pipeline\n($match by createdAt -> $lookup(monitor) -> $dateToString grouping -> per-bucket $group -> overall $group -> $project)
  Note right of MongoDB: Computes buckets array, per-bucket totals/averages,\naccumulates overall totals, computes uptimePercentage and avgResponseTime
  MongoDB-->>Server: Aggregated result (buckets, totals, computed fields)
  Server-->>Client: Response with uptimePercentage, groupedAvgResponseTime, groupedChecks/up/down
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Joke: A Canadian and an American walked into a code review — the Canadian politely fixed the bug and apologised for breaking the build, while the American congratulated them for being so modest.

Pre-merge checks (1 passed, 1 warning, 1 inconclusive)

❌ Failed Checks (1 warning, 1 inconclusive)
Check Name Status Explanation Resolution
Description Check ⚠️ Warning The description is missing the required “Fixes #” section and has an incomplete checklist, as the issue reference is not provided and the screenshot checkbox remains unchecked despite images being included. Add a “Fixes #” line referencing the associated issue and update the checklist to mark all relevant items, including confirming the screenshot/video attachment.
Title Check ❓ Inconclusive The title “Feat: Uptime Pipeline” is technically related but too generic to clearly summarize the core optimization changes made to the uptime monitor aggregation pipeline. Rename the title to explicitly reflect the main change, for example “Optimize uptime monitor aggregation pipeline performance” to convey the specific improvement.
✅ Passed Checks (1 passed)
Check Name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  - Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.
  - Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/be/uptime-monitor-pipeline

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.

Actionable comments posted: 3

🔭 Outside diff range comments (1)
server/src/db/mongo/modules/monitorModuleQueries.js (1)

113-121: groupedUptimePercentage is projected from a structure that no longer exists.

$arrayElemAt: ["$uptimePercentage.percentage", 0] assumes uptimePercentage is an array of objects with a percentage field. In this refactor, uptimePercentage is a scalar computed in the prior $project. This will error at runtime.

Apply this diff:

-				groupedUptimePercentage: { $arrayElemAt: ["$uptimePercentage.percentage", 0] },
+				groupedUptimePercentage: "$uptimePercentage",
🧹 Nitpick comments (3)
server/src/db/mongo/modules/monitorModuleQueries.js (3)

12-15: Remove unnecessary pre-group sort; add explicit sort by bucket after grouping for stable ordering.

Sorting before a $group is wasted work here and doesn’t guarantee output order. Sort buckets after the first $group to ensure groupedChecks are chronologically ordered.

Apply this diff:

-		{
-			$sort: {
-				updatedAt: 1,
-			},
-		},
@@
 		},
+		{
+			$sort: { "_id.bucket": 1 }
+		},
 		{
 			$group: {

Also applies to: 31-33


49-66: Naming nit: totalChecks inside groupedUpChecks/Down actually represents only up/down counts.

Using totalChecks to mean “up checks” or “down checks” is ambiguous and can confuse consumers.

Consider renaming to count for clarity:

-							totalChecks: "$$b.upChecks",
+							count: "$$b.upChecks",
@@
-							totalChecks: "$$b.downChecks",
+							count: "$$b.downChecks",

If the UI relies on totalChecks, skip this and we can add a TODO for later.

Also applies to: 66-82


6-9: Performance reminder: ensure supporting index exists.

For checks collection, this pipeline benefits from a compound index on { monitorId: 1, updatedAt: 1 } to accelerate $match and avoid in-memory sorts.

If you want, I can prep the index migration snippet and a quick readme note.

📜 Review details

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

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9bd2e33 and 30f4df6.

📒 Files selected for processing (1)
  • server/src/db/mongo/modules/monitorModuleQueries.js (2 hunks)
🔇 Additional comments (1)
server/src/db/mongo/modules/monitorModuleQueries.js (1)

48-48: Potential shape mismatch: groupedChecks still contains _id: { bucket: <string> }.

Previously, _id might have been the bucket string itself. If the UI expects _id to be the date string, this change can break charts.

Would you prefer to flatten _id now? Example adjustment (optional):

-				groupedChecks: "$buckets",
+				groupedChecks: {
+					$map: {
+						input: "$buckets",
+						as: "b",
+						in: {
+							_id: "$$b._id.bucket",
+							totalChecks: "$$b.totalChecks",
+							upChecks: "$$b.upChecks",
+							downChecks: "$$b.downChecks",
+							avgResponseTime: "$$b.avgResponseTime"
+						}
+					}
+				},

Copy link

@llamapreview llamapreview bot left a comment

Choose a reason for hiding this comment

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

Auto Pull Request Review from LlamaPReview

1. Overview

1.1 Core Changes

  • Primary purpose and scope: Optimize uptime monitoring pipeline to reduce processing time and improve UI responsiveness
  • Key components modified: MongoDB aggregation pipeline in monitorModuleQueries.js
  • Cross-component impacts: Affects uptime metrics calculation and dashboard visualization
  • Business value alignment: Improves user experience by reducing UI lag for real-time monitoring

1.2 Technical Architecture

  • System design modifications: Replaces multi-pass $facet pipeline with optimized single-pass aggregation
  • Component interaction changes: Reduces database load for monitor checks processing
  • Integration points impact: Preserves output structure for frontend compatibility
  • Dependency changes and implications: No new dependencies introduced

2. Critical Findings

2.1 Must Fix (P0🔴)

Issue: Incorrect response time calculation (averaging bucket averages)

  • Analysis Confidence: High
  • Impact: Produces mathematically incorrect metrics that misrepresent system performance
  • Resolution: Calculate response time sums per bucket and compute overall average correctly

Issue: Missing response time segmentation for up/down checks

  • Analysis Confidence: High
  • Impact: Uses same response time for successful/failed checks, distorting metrics
  • Resolution: Track separate response time sums for up/down statuses

Issue: Changed bucketing field from createdAt to updatedAt

  • Analysis Confidence: Medium
  • Impact: Potential data misalignment if checks are updated after creation
  • Resolution: Revert to createdAt for bucketing consistency

2.2 Should Fix (P1🟡)

Issue: Missing bucket sorting

  • Analysis Confidence: High
  • Impact: Time series data may appear out-of-order in UI visualizations
  • Suggested Solution: Add explicit sort stage after bucket grouping

Issue: Potential memory overflow with large datasets

  • Analysis Confidence: Medium
  • Impact: Pipeline could OOM when processing large time ranges
  • Suggested Solution: Implement bucket limit safeguard

2.3 Consider (P2🟢)

Area: Testing coverage

  • Analysis Confidence: High
  • Improvement Opportunity: Prevent regressions with pipeline validation tests

Area: Pipeline documentation

  • Analysis Confidence: High
  • Improvement Opportunity: Improve maintainability with inline comments

2.4 Summary of Action Items

  • Immediate (P0): Fix response calculations, revert to createdAt, add segmentation
  • High Priority (P1): Add bucket sorting, implement memory safeguards
  • Follow-up (P2): Add tests and documentation

3. Technical Analysis

3.1 Code Logic Analysis

📁 server/src/db/mongo/modules/monitorModuleQueries.js - getMonitorChecksPipeline

  • Submitted PR Code:
  {
    $group: {
      _id: {
        bucket: {
          $dateToString: {
            format: dateString,
            date: "$updatedAt",  // Uses updatedAt instead of createdAt
          },
        },
      },
      avgResponseTime: { $avg: "$responseTime" },
    }
  },
  {
    $group: {
      _id: null,
      avgResponseTime: { $avg: "$avgResponseTime" }, // Incorrect average
    }
  }
  • Analysis:
    • Current logic averages per-bucket averages instead of calculating overall average
    • Uses updatedAt instead of createdAt for bucketing
    • Missing segmentation between up/down check response times
    • No sorting of time buckets
    • Risk of memory overflow with $push of all buckets
  • LlamaPReview Suggested Improvements:
  // First group stage (per bucket)
  {
    $group: {
      _id: {
        bucket: {
          $dateToString: {
            format: dateString,
            date: "$createdAt",  // Revert to createdAt
          },
        },
      },
      upResponseSum: { 
        $sum: { 
          $cond: [{ $eq: ["$status", true] }, "$responseTime", 0] 
        } 
      },
      downResponseSum: { 
        $sum: { 
          $cond: [{ $eq: ["$status", false] }, "$responseTime", 0] 
        } 
      },
      // ... other metrics
    }
  },
  { $sort: { "_id.bucket": 1 } },  // Add sorting
  { $limit: 1000 },  // Add safeguard
  
  // Second group stage (overall)
  {
    $group: {
      _id: null,
      avgResponseTime: { $avg: "$responseTime" }, // Correct overall average
      // ... other aggregates
    }
  }
  • Improvement rationale:
    • Technical benefits: Fixes mathematical errors, ensures chronological ordering
    • Business value: Provides accurate monitoring metrics for decision-making
    • Risk assessment: Prevents OOM errors and maintains data consistency

3.2 Key Quality Aspects

  • System scalability considerations: Added bucket limit prevents memory issues
  • Performance bottlenecks and optimizations: Reduced pipeline stages improve efficiency
  • Testing strategy and coverage: Critical need for aggregation pipeline tests
  • Documentation needs: Requires inline comments explaining pipeline logic

4. Overall Evaluation

  • Technical assessment: Good architectural approach but contains critical calculation flaws
  • Business impact: Essential optimization for UI responsiveness but must ensure metric accuracy
  • Risk evaluation: High - current implementation produces misleading metrics
  • Notable positive aspects: Effective pipeline simplification, preserves output structure
  • Implementation quality: Needs improvement for mathematical correctness and edge cases
  • Final recommendation: Request Changes - Must fix critical P0 issues before merging

💡 LlamaPReview Community
Have feedback on this AI Code review tool? Join our GitHub Discussions to share your thoughts and help shape the future of LlamaPReview.

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.

Actionable comments posted: 2

♻️ Duplicate comments (2)
server/src/db/mongo/modules/monitorModuleQueries.js (2)

73-77: Standardize uptimePercentage scale and zero-check semantics

You emit uptimePercentage as a 0–1 ratio (good), but for totalChecks == 0 you return 0. Downstream code sometimes multiplies by 100 and sometimes appends “%” to raw values. This creates “0.85% vs 85%” bugs and “no data” looking like 0%.

Pick one:

  • Always 0–1 ratio; when totalChecks == 0, return null (no data) or 1 (optimistic).
  • Or always 0–100 percent; ensure all consumers expect that.

If you choose ratio + null on no data, adjust as:

-        uptimePercentage: {
-          $cond: [{ $eq: ["$totalChecks", 0] }, 0, { $divide: ["$upChecks", "$totalChecks"] }],
-        },
+        uptimePercentage: {
+          $cond: [{ $eq: ["$totalChecks", 0] }, null, { $divide: ["$upChecks", "$totalChecks"] }],
+        },

And confirm MonitorsTable and Host components convert consistently before rendering.

Want me to scan client usage and open a small PR to normalize formatting?

Also applies to: 157-163


19-22: Add explicit timezone to $dateToString and thread a tz parameter

Buckets will default to UTC without timezone, which often doesn’t match UI expectations. Thread a tz (e.g., from user/team settings) and pass it to $dateToString.

-const buildUptimeDetailsPipeline = (monitorId, dates, dateString) => {
+const buildUptimeDetailsPipeline = (monitorId, dates, dateString, tz) => {
               $dateToString: {
                 format: dateString,
-                date: "$updatedAt",
+                date: "$updatedAt",
+                timezone: tz,
               },

Run this to locate callers to update their signatures to include timezone:

#!/bin/bash
rg -n -C2 '\bbuildUptimeDetailsPipeline\s*\('

I can provide a small shim that falls back to a default tz while you wire through settings.

Also applies to: 3-3

🧹 Nitpick comments (2)
server/src/db/mongo/modules/monitorModuleQueries.js (2)

93-109: Naming nit: “totalChecks” inside groupedUpChecks is ambiguous

The objects in groupedUpChecks use totalChecks to mean “number of up checks in this bucket.” Consider renaming that field to count to avoid confusion with the bucket’s totalChecks field.

This is a schema change; only do it if consumers are ready.

I can grep usages and stage a follow-up if you want to adopt clearer naming.


12-15: Optional: drop the pre-group sort for better performance

You sort by updatedAt before a $group that only uses $sum/$avg; that sort is unnecessary and adds cost. You already sort bucket keys after grouping.

Consider removing this sort unless a downstream accumulator depends on order.

📜 Review details

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

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 30f4df6 and c913b66.

📒 Files selected for processing (1)
  • server/src/db/mongo/modules/monitorModuleQueries.js (2 hunks)

@gorkem-bwl gorkem-bwl modified the milestones: 3.0, 3.2 Aug 26, 2025
@Br0wnHammer
Copy link
Member Author

@ajhollid I have updated the query logic. Please review when possible.

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.

Actionable comments posted: 0

♻️ Duplicate comments (3)
server/src/db/mongo/modules/monitorModuleQueries.js (3)

12-18: Add explicit timezone to date buckets

$dateToString defaults to UTC, which can shift daily/hourly buckets vs. the UI. Pass a timezone.

-const buildUptimeDetailsPipeline = (monitorId, dates, dateString) => {
+const buildUptimeDetailsPipeline = (monitorId, dates, dateString, tz = "UTC") => {
   return [
     {
       $group: {
         _id: {
           $dateToString: {
             format: dateString,
-            date: "$createdAt",
+            date: "$createdAt",
+            timezone: tz,
           },
         },

41-66: Invalid self-reference to $buckets inside $group; compute weighted sum instead

You can’t reference $buckets while defining it; and the current logic averages weighted terms by bucket count (wrong). Aggregate a streaming weighted sum, then divide later.

         $group: {
           _id: null,
           buckets: { $push: "$$ROOT" },
           totalChecks: { $sum: "$totalChecks" },
           upChecks: { $sum: "$upChecks" },
           downChecks: { $sum: "$downChecks" },
-          overallAvgResponseTime: {
-            $avg: {
-              $map: {
-                input: { $range: [0, { $size: { $ifNull: ["$buckets", []] } }] },
-                as: "i",
-                in: {
-                  $let: {
-                    vars: { bucket: { $arrayElemAt: ["$buckets", "$$i"] } },
-                    in: { $multiply: ["$$bucket.avgResponseTime", "$$bucket.totalChecks"] },
-                  },
-                },
-              },
-            },
-          },
+          sumWeightedResponseTime: { $sum: { $multiply: ["$avgResponseTime", "$totalChecks"] } },
         },

68-86: Guard divide-by-zero and use the weighted sum for groupedAvgResponseTime

$divide by 0 can error. Also no need to $reduce over buckets after the fix above.

   $project: {
     _id: 0,
     uptimePercentage: {
       $cond: [{ $eq: ["$totalChecks", 0] }, 0, { $divide: ["$upChecks", "$totalChecks"] }],
     },
-    groupedAvgResponseTime: {
-      $divide: [
-        {
-          $reduce: {
-            input: "$buckets",
-            initialValue: 0,
-            in: { $add: ["$$value", { $multiply: ["$$this.avgResponseTime", "$$this.totalChecks"] }] },
-          },
-        },
-        "$totalChecks",
-      ],
-    },
+    groupedAvgResponseTime: {
+      $cond: [
+        { $eq: ["$totalChecks", 0] },
+        0,
+        { $divide: ["$sumWeightedResponseTime", "$totalChecks"] }
+      ]
+    },
     groupedChecks: "$buckets",
🧹 Nitpick comments (1)
server/src/db/mongo/modules/monitorModuleQueries.js (1)

8-11: Confirm intended time field (createdAt) and add index

You switched matching to createdAt. If that’s the intended “time of record,” great—just confirm UI/analytics expect this (not updatedAt). Also ensure a compound index exists on checks: { monitorId: 1, createdAt: 1 } to keep this fast over large ranges. Americans argue about BBQ; Canadians quietly add the index and smile.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c913b66 and dffa559.

📒 Files selected for processing (1)
  • server/src/db/mongo/modules/monitorModuleQueries.js (2 hunks)
🔇 Additional comments (6)
server/src/db/mongo/modules/monitorModuleQueries.js (6)

19-33: Per-bucket stats look good

The conditional avgs ignore nulls as expected; filters later handle zero-count buckets.


36-38: Sort by bucket id is fine

String sort is OK if format is zero-padded ISO-like.


70-72: Standardize uptimePercentage scale and empty-state semantics

You emit a 0–1 ratio and 0 when no data. Confirm the client expects a ratio (and not 0–100) and whether no-data should be 1, 0, or null. Canadians would pick one polite convention and stick to it.


87-104: Up buckets mapping is correct

Filtered to positive upChecks; fields are minimal and useful.


105-121: Down buckets mapping is correct

Mirrors the up path; consistent.


152-159: Final projection fields OK

Names are consistent with earlier projection; just ensure consumers use groupedUptimePercentage (ratio) consistently.

@bluewave-labs bluewave-labs deleted a comment from coderabbitai bot Aug 28, 2025
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.

Actionable comments posted: 0

♻️ Duplicate comments (3)
server/src/db/mongo/modules/monitorModuleQueries.js (3)

8-17: Consistent timestamp field — good correction

Matching/sorting/bucketing all on createdAt is now aligned. Nice.

Also applies to: 36-38


14-17: Add explicit timezone to $dateToString (avoid silent UTC shifts)

Without timezone, buckets default to UTC and can appear “off by one” day/hour for users. Pass tz and default it. Canadians politely adapt; Americans open a ticket.

-const buildUptimeDetailsPipeline = (monitorId, dates, dateString) => {
+const buildUptimeDetailsPipeline = (monitorId, dates, dateString, tz) => {
@@
           $dateToString: {
             format: dateString,
-            date: "$createdAt",
+            date: "$createdAt",
+            timezone: tz ?? "UTC",
           },

Run to find and update callers to pass a timezone (team/user setting or config):

#!/bin/bash
rg -n 'buildUptimeDetailsPipeline\(' -C2

12-32: Fix “average of averages” — compute weighted average instead

groupedAvgResponseTime currently averages per-bucket averages, biasing results when bucket sizes differ. Carry response-time sums and divide once.

@@
         $group: {
           _id: {
             $dateToString: {
               format: dateString,
               date: "$createdAt",
             },
           },
           totalChecks: { $sum: 1 },
           upChecks: { $sum: { $cond: [{ $eq: ["$status", true] }, 1, 0] } },
           downChecks: { $sum: { $cond: [{ $eq: ["$status", false] }, 1, 0] } },
           avgResponseTime: { $avg: "$responseTime" },
+          sumResponseTime: { $sum: "$responseTime" },
           avgUpResponseTime: {
             $avg: {
               $cond: [{ $eq: ["$status", true] }, "$responseTime", null],
             },
           },
           avgDownResponseTime: {
             $avg: {
               $cond: [{ $eq: ["$status", false] }, "$responseTime", null],
             },
           },
         },
@@
         $group: {
           _id: null,
           buckets: { $push: "$$ROOT" },
           totalChecks: { $sum: "$totalChecks" },
           upChecks: { $sum: "$upChecks" },
           downChecks: { $sum: "$downChecks" },
+          totalResponseTime: { $sum: "$sumResponseTime" },
         },
@@
         $project: {
           _id: 0,
           uptimePercentage: {
             $cond: [{ $eq: ["$totalChecks", 0] }, 0, { $divide: ["$upChecks", "$totalChecks"] }],
           },
-          groupedAvgResponseTime: {
-            $avg: {
-              $map: {
-                input: "$buckets",
-                as: "b",
-                in: "$$b.avgResponseTime",
-              },
-            },
-          },
+          groupedAvgResponseTime: {
+            $cond: [
+              { $eq: ["$totalChecks", 0] },
+              0,
+              { $divide: ["$totalResponseTime", "$totalChecks"] }
+            ]
+          },

Also applies to: 41-47, 50-63

🧹 Nitpick comments (4)
server/src/db/mongo/modules/monitorModuleQueries.js (4)

8-9: Ensure a supporting index for the new filter

To keep this fast under load, ensure an index like: checks(monitorId:1, createdAt:1).


102-126: Simplify $lookup: avoid $toObjectId and $expr when a direct match suffices

Slightly cleaner and marginally faster.

-      let: { monitor_id: { $toObjectId: monitorId } },
-      pipeline: [
-        { $match: { $expr: { $eq: ["$_id", "$$monitor_id"] } } },
+      pipeline: [
+        { $match: { _id: new ObjectId(monitorId) } },
         { $project: { _id: 1, teamId: 1, name: 1, status: 1, interval: 1, type: 1, url: 1, isActive: 1, notifications: 1 } },
       ],

66-81: Name reflects content: use avgUpResponseTime key

The field holds an “up-only” average; naming it avgResponseTime can confuse consumers.

-            in: {
-              _id: "$$b._id",
-              totalChecks: "$$b.upChecks",
-              avgResponseTime: "$$b.avgUpResponseTime",
-            },
+            in: {
+              _id: "$$b._id",
+              totalChecks: "$$b.upChecks",
+              avgUpResponseTime: "$$b.avgUpResponseTime",
+            },

82-98: Same here: use avgDownResponseTime

Keeps API self-descriptive. Americans shout “type safety,” Canadians nod approvingly.

-            in: {
-              _id: "$$b._id",
-              totalChecks: "$$b.downChecks",
-              avgResponseTime: "$$b.avgDownResponseTime",
-            },
+            in: {
+              _id: "$$b._id",
+              totalChecks: "$$b.downChecks",
+              avgDownResponseTime: "$$b.avgDownResponseTime",
+            },
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between dffa559 and 0f5c637.

📒 Files selected for processing (1)
  • server/src/db/mongo/modules/monitorModuleQueries.js (2 hunks)

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.

Actionable comments posted: 0

♻️ Duplicate comments (1)
server/src/db/mongo/modules/monitorModuleQueries.js (1)

8-18: Add explicit timezone to bucketing (or switch to dateTrunc).

$dateToString defaults to UTC; pass a timezone to avoid UI bucket shifts. This was raised before; still applies. Canadians quietly accept UTC; Americans will demand EST.

Apply within this block:

-          $dateToString: {
-            format: dateString,
-            date: "$createdAt",
-          },
+          $dateToString: {
+            format: dateString,
+            date: "$createdAt",
+            timezone: tz
+          },

Outside this block, update the signature to accept tz (and thread it from callers):

// before
const buildUptimeDetailsPipeline = (monitorId, dates, dateString) => {
// after
const buildUptimeDetailsPipeline = (monitorId, dates, dateString, tz = "UTC") => {

Optional: prefer $dateTrunc (keeps a Date type, faster) and only stringify at the very end.

🧹 Nitpick comments (4)
server/src/db/mongo/modules/monitorModuleQueries.js (4)

41-51: Avoid pushing $$ROOT; push a lean shape to cut memory/CPU.

You only need a handful of fields from each bucket; pushing the whole doc increases group memory.

-        buckets: { $push: "$$ROOT" },
+        buckets: {
+          $push: {
+            _id: "$_id",
+            totalChecks: "$totalChecks",
+            upChecks: "$upChecks",
+            downChecks: "$downChecks",
+            avgResponseTime: "$avgResponseTime",
+            avgUpResponseTime: "$avgUpResponseTime",
+            avgDownResponseTime: "$avgDownResponseTime"
+          }
+        },

62-78: Preserve empty buckets in groupedUpChecks.

Filtering out buckets with 0 up checks yields ragged series; many charts expect fixed bucket counts. Map all buckets and default to 0.

-        input: {
-          $filter: {
-            input: "$buckets",
-            as: "b",
-            cond: { $gt: ["$$b.upChecks", 0] },
-          },
-        },
+        input: "$buckets",
...
-        in: {
-          _id: "$$b._id",
-          totalChecks: "$$b.upChecks",
-          avgResponseTime: { $ifNull: ["$$b.avgUpResponseTime", 0] },
-        },
+        in: {
+          _id: "$$b._id",
+          totalChecks: { $ifNull: ["$$b.upChecks", 0] },
+          avgResponseTime: { $ifNull: ["$$b.avgUpResponseTime", 0] },
+        },

79-95: Do the same for groupedDownChecks for symmetry.

Mirror the “include all buckets” approach for down checks.

-        input: {
-          $filter: {
-            input: "$buckets",
-            as: "b",
-            cond: { $gt: ["$$b.downChecks", 0] },
-          },
-        },
+        input: "$buckets",
...
-        in: {
-          _id: "$$b._id",
-          totalChecks: "$$b.downChecks",
-          avgResponseTime: { $ifNull: ["$$b.avgDownResponseTime", 0] },
-        },
+        in: {
+          _id: "$$b._id",
+          totalChecks: { $ifNull: ["$$b.downChecks", 0] },
+          avgResponseTime: { $ifNull: ["$$b.avgDownResponseTime", 0] },
+        },

8-11: Validate inputs and indexes for performance and safety.

  • Ensure monitorId is a valid ObjectId; otherwise new ObjectId(monitorId) throws.
  • Confirm dates.start/end are Date objects (not strings).
  • Add/verify compound index on checks: { monitorId: 1, createdAt: 1 }. It’s the hot path for this $match. Canadians love polite indexes; Americans love fast ones.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0f5c637 and a825c81.

📒 Files selected for processing (1)
  • server/src/db/mongo/modules/monitorModuleQueries.js (2 hunks)
🔇 Additional comments (3)
server/src/db/mongo/modules/monitorModuleQueries.js (3)

12-32: Per-bucket aggregations look correct.

Counts, up/down splits, and conditional averages are sound; nulls are ignored as expected.


53-61: Confirm uptime units (ratio vs percent).

uptimePercentage is 0..1. If the UI expects 0..100, multiply by 100 here or rename to uptimeRatio.


127-133: Final projection mapping reads cleanly.

Aliases are clear and match earlier computations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants