Add uptime column to the dashboard #170
Conversation
WalkthroughThe changes introduce a new column titled "Up%" in the table of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant IndexPage as Index.cshtml
participant IndexModel as Index.cshtml.cs
participant Database
User->>IndexPage: Request Page
IndexPage->>IndexModel: OnGetAsync()
IndexModel->>Database: Fetch Orcanode Events
Database-->>IndexModel: Return Events
IndexModel->>IndexModel: Filter Events
IndexModel->>IndexModel: GetUptimePercentage()
IndexModel-->>IndexPage: Render Page with Uptime
IndexPage-->>User: Display Page with Uptime Column
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
OrcanodeMonitor/Pages/Index.cshtml.cs (2)
96-96: Add XML documentation for the method.Add documentation to describe the method's purpose, parameters, return value, and behavior for edge cases.
Add this documentation:
/// <summary> /// Calculates the uptime percentage for a node over the past 7 days based on hydrophone stream events. /// </summary> /// <param name="node">The node to calculate uptime for.</param> /// <returns> /// The percentage of time (0-100) the node was up over the past week. /// Returns 0 if no events exist or if the events list is null. /// </returns>
Line range hint
146-158: Add error handling and consider separating concerns.The method has several areas for improvement:
- Missing error handling for database operations
- Combines node and event fetching which could be separated
- Could benefit from explaining the filtering criteria in comments
Consider refactoring like this:
public async Task OnGetAsync() { + try + { // Fetch nodes for display. + // Filter out nodes that are absent or unauthorized, excluding dev environment var nodes = await _databaseContext.Orcanodes.ToListAsync(); _nodes = nodes.Where(n => ((n.DataplicityConnectionStatus != OrcanodeOnlineStatus.Absent) || (n.OrcasoundStatus != OrcanodeOnlineStatus.Absent) || (n.S3StreamStatus != OrcanodeOnlineStatus.Absent && n.S3StreamStatus != OrcanodeOnlineStatus.Unauthorized)) && (n.OrcasoundHost != "dev.orcasound.net")) .OrderBy(n => n.DisplayName) .ToList(); // Fetch events for uptime computation. + // Only consider hydrophone stream events as they indicate node availability var events = await _databaseContext.OrcanodeEvents.ToListAsync(); _events = events.Where(e => e.Type == "hydrophone stream").ToList(); + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to fetch nodes or events"); + // Consider setting an error message to display to the user + _nodes = new List<Orcanode>(); + _events = new List<OrcanodeEvent>(); + } }Consider extracting the fetching logic into separate methods:
private async Task FetchNodesAsync() { var nodes = await _databaseContext.Orcanodes.ToListAsync(); _nodes = nodes.Where(/* ... */).ToList(); } private async Task FetchEventsAsync() { var events = await _databaseContext.OrcanodeEvents.ToListAsync(); _events = events.Where(e => e.Type == "hydrophone stream").ToList(); }OrcanodeMonitor/Pages/Index.cshtml (1)
21-21: Consider enhancing the uptime column header and documentation.The "Up%" column would benefit from:
- A tooltip or hover text explaining the calculation period (e.g., "7-day uptime percentage")
- An entry in the legend section explaining how uptime is calculated
- <th>Up%</th> + <th title="Node uptime percentage over the past 7 days">Up%</th>Also, consider adding to the legend section:
+ <li> + <b>Up%</b>: Percentage of time the node was operational over the past 7 days. + </li>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- OrcanodeMonitor/Pages/Index.cshtml (2 hunks)
- OrcanodeMonitor/Pages/Index.cshtml.cs (4 hunks)
🧰 Additional context used
🔇 Additional comments (3)
OrcanodeMonitor/Pages/Index.cshtml.cs (2)
17-17: LGTM: Field addition follows conventions.The private field
_eventsis appropriately scoped and follows C# naming conventions.
96-132: Verify event types and values in the database.Let's verify the assumptions about event types and values used in the uptime calculation.
Also applies to: 156-158
✅ Verification successful
Let me try a different approach to verify the event values and types by searching for the OrcanodeEvent class definition and its usages.
This should help us understand:
- The structure of OrcanodeEvent class
- How event values are assigned in the codebase
- Related monitoring/status code that might show the event value patterns
Let me try one more search strategy focusing on the OrcanodeEvent class and its usage in monitoring:
This should help us understand:
- The OrcanodeEvent class structure
- How "up" and "down" values are used
- The complete context of the monitoring implementation
Based on the search results, I can now provide a final verification of the uptime calculation. The code shows:
- The uptime calculation correctly uses events of type "hydrophone stream" (filtered in
Index.cshtml.csline:_events = events.Where(e => e.Type == "hydrophone stream").ToList();)- The event values are consistently set as "up" or status string (from
Orcanode.cs:return (status == OrcanodeOnlineStatus.Online) ? "up" : S3StreamStatus.ToString().ToUpper();)- The uptime percentage calculation logic correctly:
- Processes events chronologically
- Handles the "up" and non-"up" states appropriately
- Accounts for gaps between events
- Considers the last known state up to the current time
The uptime calculation implementation is correct and consistent with the codebase. The code properly tracks the hydrophone stream status through events and calculates the percentage of time it was "up" over the past week.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check event types and values in the database # Note: This assumes the code is in a .NET project with Entity Framework # Test 1: Check unique event types echo "Unique event types in OrcanodeEvents:" rg -g "*.cs" -A 5 "Type\s*=\s*\".*\".*OrcanodeEvent" # Test 2: Check event value assignments echo "Event value assignments:" rg -g "*.cs" -A 5 "Value\s*=\s*\".*\".*OrcanodeEvent"Length of output: 250
Script:
#!/bin/bash # Find OrcanodeEvent class definition and usages echo "=== OrcanodeEvent class definition ===" ast-grep --pattern 'class OrcanodeEvent { $$$ }' echo -e "\n=== OrcanodeEvent value assignments ===" rg -g "*.cs" "\.Value\s*=\s*" -B 2 -A 2 echo -e "\n=== Checking monitor/status related files ===" fd -e cs -e cshtml monitor status -I | xargs catLength of output: 1896
Script:
#!/bin/bash # Search for OrcanodeEvent class and monitoring logic echo "=== OrcanodeEvent class and properties ===" rg -g "*.cs" "class OrcanodeEvent" -B 2 -A 10 echo -e "\n=== Monitoring status checks ===" rg -g "*.cs" -B 3 -A 3 "\"up\"|\"down\"|OrcanodeEvent" echo -e "\n=== Files in OrcanodeMonitor directory ===" fd . OrcanodeMonitor -t f -x echo "=== {} ===" \; cat "{}"Length of output: 30644
OrcanodeMonitor/Pages/Index.cshtml (1)
74-76: 🛠️ Refactor suggestionEnhance the uptime percentage display with formatting and visual cues.
Consider improving the uptime display with:
- Consistent styling with other status columns
- Formatted percentage values
- Visual indicators for different uptime ranges
- <td> - @Model.GetUptimePercentage(item)% - </td> + <td style="background-color: @Model.GetUptimeBackgroundColor(item)"> + @String.Format("{0:0.0}%", Model.GetUptimePercentage(item)) + </td>This assumes you'll add a
GetUptimeBackgroundColormethod to the model that returns appropriate colors based on uptime ranges (e.g., red for <90%, yellow for 90-98%, green for >98%).Let's verify if the GetUptimePercentage method handles edge cases:
Signed-off-by: Dave Thaler <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
OrcanodeMonitor/Pages/Index.cshtml.cs (1)
Line range hint
152-164: Optimize database queries for better performance.The current implementation loads all events into memory before filtering. Consider these improvements:
- Filter events in the database query
- Add error handling for database operations
- Consider limiting the time range of events fetched
Here's a suggested implementation:
public async Task OnGetAsync() { // Fetch nodes for display. - var nodes = await _databaseContext.Orcanodes.ToListAsync(); + try + { + var nodes = await _databaseContext.Orcanodes.ToListAsync(); + _nodes = nodes.Where(n => ((n.DataplicityConnectionStatus != OrcanodeOnlineStatus.Absent) || + (n.OrcasoundStatus != OrcanodeOnlineStatus.Absent) || + (n.S3StreamStatus != OrcanodeOnlineStatus.Absent && + n.S3StreamStatus != OrcanodeOnlineStatus.Unauthorized)) && + (n.OrcasoundHost != "dev.orcasound.net")) + .OrderBy(n => n.DisplayName) + .ToList(); + // Fetch events for uptime computation, filtering at the database level + var cutoffDate = DateTime.UtcNow - _uptimeEvaluationPeriod; + _events = await _databaseContext.OrcanodeEvents + .Where(e => e.Type == "hydrophone stream" && e.DateTimeUtc >= cutoffDate) + .ToListAsync(); + } + catch (Exception ex) + { + _logger.LogError(ex, "Error fetching dashboard data"); + throw; + } - _nodes = nodes.Where(n => ((n.DataplicityConnectionStatus != OrcanodeOnlineStatus.Absent) || - (n.OrcasoundStatus != OrcanodeOnlineStatus.Absent) || - (n.S3StreamStatus != OrcanodeOnlineStatus.Absent && - n.S3StreamStatus != OrcanodeOnlineStatus.Unauthorized)) && - (n.OrcasoundHost != "dev.orcasound.net")) - .OrderBy(n => n.DisplayName) - .ToList(); - // Fetch events for uptime computation. - var events = await _databaseContext.OrcanodeEvents.ToListAsync(); - _events = events.Where(e => e.Type == "hydrophone stream").ToList(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- OrcanodeMonitor/Pages/Index.cshtml (2 hunks)
- OrcanodeMonitor/Pages/Index.cshtml.cs (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- OrcanodeMonitor/Pages/Index.cshtml
🧰 Additional context used
🔇 Additional comments (1)
OrcanodeMonitor/Pages/Index.cshtml.cs (1)
17-17: LGTM! Field declarations are well-structured.The new fields are properly scoped and initialized with appropriate values.
Also applies to: 22-22
Signed-off-by: Dave Thaler <[email protected]>
Signed-off-by: Dave Thaler <[email protected]>
Signed-off-by: Dave Thaler <[email protected]>
Add uptime column to the dashboard where percentage is computed from "hydrophone stream" events over the past week
Summary by CodeRabbit
New Features
Bug Fixes