Added support for pulling data from moesif #25
Conversation
WalkthroughThis PR introduces dual data-source support to the dashboard, enabling runtime selection between OpenSearch and Moesif Analytics via configuration flags. A new MoesifService class provides data retrieval and analytics capabilities, while the dashboard controller routes requests conditionally based on the selected source. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as DashboardController
participant Config as Environment
participant OpenSearch as OpenSearchService
participant Moesif as MoesifService
Client->>Controller: Request data (e.g., getMessages)
Controller->>Config: Read USE_OPENSEARCH flag
alt OpenSearch enabled (USE_OPENSEARCH=true)
Controller->>OpenSearch: Call openSearchService method
OpenSearch-->>Controller: Return data
else Moesif (default)
Controller->>Moesif: Call moesifService method
Moesif-->>Controller: Return data
end
Controller-->>Client: Return response
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dashboard/swift_dashboard/server/controllers/dashboardController.ts (1)
433-444: Inconsistent defaulttimeFiltervalue may skew error stats when param is omittedHere:
const timeFilter = (query.timeFilter as 'Daily' | 'Weekly' | 'Monthly' | 'All') || 'all';
'all'(lowercase) does not match the union ('All') and does not align withMoesifService.getErrorStatistics, which switches on'Daily'/'Weekly'and treats any other value as “monthly”. So whentimeFilteris absent you effectively get monthly stats, not a true “all” period.Suggest aligning the default with the actual semantics, e.g.:
const timeFilter = (query.timeFilter as 'Daily' | 'Weekly' | 'Monthly' | 'All') || 'All';and then implementing explicit
'All'handling in both services if needed.
🧹 Nitpick comments (4)
dashboard/swift_dashboard/server/.env.example (1)
6-17: Env values are functionally fine; consider quoting to satisfy dotenv‑linterThe new variables (USE_OPENSEARCH, MOESIF_*) are consistent with how they’re read in code (string comparison /
parseInt), so they’ll work as written. If you want a clean run ofdotenv-linter, you can optionally:
- Quote non-string values, e.g.:
USE_OPENSEARCH="false" MOESIF_TIMEOUT="30000" MOESIF_MAX_RETRIES="3" MOESIF_RETRY_DELAY="1000" MOESIF_DEBUG="false" MOESIF_API_KEY="YOUR_MOESIF_API_KEY_HERE"- Reorder keys as the linter suggests.
Purely cosmetic; no runtime impact.
dashboard/README.md (2)
29-42: Add language identifiers to fenced blocks to satisfy markdownlint
markdownlintis complaining about code fences without a language at the architecture diagram and the two “Access Dashboard” URL snippets.You can fix MD040 by tagging them as
text(or another suitable language), for example:```text ┌─────────────────────┐ ...http://localhost:5601/app/swiftDashboardAlso applies to: 128-133, 411-416
289-304: Minor URL typo in plugin download instructionsThe plain URL line includes a trailing
):https://github.com/wso2/reference-implementation-cbpr/releases?q=Dashboard+Plugin)This can confuse copy‑pasting. Suggest removing the extra
):https://github.com/wso2/reference-implementation-cbpr/releases?q=Dashboard+Plugindashboard/swift_dashboard/server/services/moesifService.ts (1)
28-33: Caching fields are unused andinvalidateCachedoesn’t clear logsYou maintain
cachedMessages,cachedLogs,lastCacheTime, andcacheTTL, and update them ingetLogData/getAllMessages, but no method ever reads from the cache or checks TTL.invalidateCacheonly clearscachedMessagesandlastCacheTime, notcachedLogs.Right now this is just dead state and a misleading cache invalidation API. Consider either:
- Implementing read‑side caching with TTL checks (and clearing both messages and logs), or
- Removing the cache fields entirely until you need them.
Example minimal alignment if you keep caching:
public invalidateCache() { - this.cachedMessages = null; - this.lastCacheTime = 0; + this.cachedMessages = null; + this.cachedLogs = null; + this.lastCacheTime = 0; }and then wire
cached*+cacheTTLinto the read paths.Also applies to: 371-407, 626-641, 805-808
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignore(1 hunks)dashboard/README.md(3 hunks)dashboard/swift_dashboard/server/.env.example(1 hunks)dashboard/swift_dashboard/server/controllers/dashboardController.ts(15 hunks)dashboard/swift_dashboard/server/services/moesifService.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
dashboard/swift_dashboard/server/controllers/dashboardController.ts (3)
dashboard/swift_dashboard/server/controllers/index.ts (1)
DashboardController(19-19)dashboard/swift_dashboard/server/types.ts (1)
PluginContext(46-53)dashboard/swift_dashboard/server/services/index.ts (1)
OpenSearchService(19-19)
dashboard/swift_dashboard/server/services/moesifService.ts (1)
dashboard/swift_dashboard/server/types.ts (3)
PluginContext(46-53)MessageDocument(56-72)LogDocument(74-79)
🪛 dotenv-linter (4.0.0)
dashboard/swift_dashboard/server/.env.example
[warning] 6-6: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 12-12: [UnorderedKey] The MOESIF_LOG_ACTION_NAME key should go before the MOESIF_SEARCH_PATH key
(UnorderedKey)
[warning] 13-13: [UnorderedKey] The MOESIF_TIMEOUT key should go before the MOESIF_TRANSLATION_ACTION_NAME key
(UnorderedKey)
[warning] 13-13: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 14-14: [UnorderedKey] The MOESIF_MAX_RETRIES key should go before the MOESIF_SEARCH_PATH key
(UnorderedKey)
[warning] 14-14: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 15-15: [UnorderedKey] The MOESIF_RETRY_DELAY key should go before the MOESIF_SEARCH_PATH key
(UnorderedKey)
[warning] 15-15: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 16-16: [UnorderedKey] The MOESIF_DEBUG key should go before the MOESIF_LOG_ACTION_NAME key
(UnorderedKey)
[warning] 16-16: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 17-17: [UnorderedKey] The MOESIF_API_KEY key should go before the MOESIF_BASE_URL key
(UnorderedKey)
[warning] 17-17: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🪛 markdownlint-cli2 (0.18.1)
dashboard/README.md
29-29: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
128-128: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
411-411: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
.gitignore (1)
26-30: LGTM!Adding
.DS_Storeand.idea/to the ignore list follows best practices—preventing macOS system artifacts and JetBrains IDE configuration files from being committed to the repository. The entries are properly formatted and consistent with the existing patterns.dashboard/swift_dashboard/server/controllers/dashboardController.ts (1)
19-41: Data‑source toggle wiring looks consistentThe controller cleanly introduces
useOpenSearchand routes all downstream calls either toOpenSearchServiceorMoesifServicebased onenv.USE_OPENSEARCH === 'true'. OpenSearch is only constructed when needed, Moesif is always available as the default, and the log messages correctly reflect the chosen backend. No functional issues here.
| public async getMonthlyMessages( | ||
| direction?: string | ||
| ): Promise<MessageDocument[]> { | ||
| // Get current month date range | ||
| const today = new Date(); | ||
|
|
||
| // Start of month | ||
| const startOfMonth = new Date(today.getFullYear(), today.getMonth(), 2); // Changed from 2 to 1 to include the 1st day | ||
| startOfMonth.setHours(0, 0, 0, 0); | ||
|
|
||
| // End of month | ||
| const endOfMonth = new Date(today.getFullYear(), today.getMonth() + 1, 0); | ||
| endOfMonth.setHours(23, 59, 59, 999); | ||
|
|
||
| // Format dates for query | ||
| const startDateStr = startOfMonth.toISOString().split('T')[0]; | ||
| const endDateStr = endOfMonth.toISOString().split('T')[0]; | ||
|
|
||
| try { |
There was a problem hiding this comment.
Monthly range starts from the 2nd, not the 1st (off‑by‑one bug)
getMonthlyMessages currently does:
const startOfMonth = new Date(today.getFullYear(), today.getMonth(), 2); // Changed from 2 to 1 to include the 1st dayThis actually excludes the 1st of the month from the query range. Since several higher‑level methods (monthly charts, recent messages with monthly period, etc.) depend on this, monthly stats will miss one day.
Fix the off‑by‑one and align the comment:
- const startOfMonth = new Date(today.getFullYear(), today.getMonth(), 2); // Changed from 2 to 1 to include the 1st day
+ const startOfMonth = new Date(today.getFullYear(), today.getMonth(), 1); // Start from the 1st day of the month🤖 Prompt for AI Agents
In dashboard/swift_dashboard/server/services/moesifService.ts around lines 905
to 923, the startOfMonth constructor currently uses day 2 which excludes the 1st
of the month; change the Date instantiation to use day 1 (new
Date(today.getFullYear(), today.getMonth(), 1)) so the range includes the first
day, keep the setHours(0,0,0,0) call, and update the inline comment to reflect
that the start of month is the 1st.
There was a problem hiding this comment.
This is intentional and is the same for open search as well
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check OpenSearch service for monthly message implementation
fd -e ts openSearchService.ts --exec cat {}
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find any files related to OpenSearch service
fd -e ts -e js . dashboard/swift_dashboard/server/services/
# Also search for monthly message implementations
rg -n "getMonthlyMessages" --type ts -A 20
Length of output: 27497
@aka4rKO, thank you for the clarification! I've verified that both opensearchService.ts (Line 921) and moesifService.ts (Line 912) use day 2, so this is indeed consistent across both data sources.
However, I'd suggest updating the inline comment in both files since it's currently misleading. The comment says:
// Changed from 2 to 1 to include the 1st dayBut the code still uses 2. Consider updating it to reflect the actual intention, for example:
// Start from the 2nd day of the month (intentional - consistent with data requirements)This would prevent future confusion for developers reviewing the code.
✏️ Learnings added
Learnt from: aka4rKO
Repo: wso2/reference-implementation-cbpr PR: 25
File: dashboard/swift_dashboard/server/services/moesifService.ts:905-923
Timestamp: 2025-11-24T11:41:41.214Z
Learning: In the dashboard services (both MoesifService and OpenSearchService), the getMonthlyMessages() method intentionally starts from day 2 of the month (new Date(year, month, 2)) rather than day 1. This is a deliberate design decision consistent across both data source implementations.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
1 participant
Purpose
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.