feat: implement buildMonitoringResources function and add tests for resource extraction#3761
feat: implement buildMonitoringResources function and add tests for resource extraction#3761Darkace01 wants to merge 2 commits intoDokploy:canaryfrom
Conversation
…esource extraction
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (1)
|
There was a problem hiding this comment.
Pull request overview
Implements a utility to derive “monitoring resources” (apps/compose/databases) from the project.all API response and wires it into the monitoring dashboard to let users scope free monitoring to a selected project/resource (issue #2316).
Changes:
- Added
buildMonitoringResources()to flatten projects/environments/services into a single resource list. - Updated
/dashboard/monitoring(free path) to include Project + Resource selectors and passappName/appTypeintoContainerFreeMonitoring. - Added Vitest coverage for the new resource extraction utility.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| apps/dokploy/utils/monitoring-resources.ts | New resource-extraction helper used by the monitoring UI. |
| apps/dokploy/pages/dashboard/monitoring.tsx | Adds project/resource selection UI and feeds selected resource into free monitoring view. |
| apps/dokploy/test/monitoring-resources.test.ts | Unit tests for resource extraction behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| compose: Array<{ | ||
| composeId: string; | ||
| appName?: string; | ||
| name: string; | ||
| composeType?: string; | ||
| }>; |
There was a problem hiding this comment.
composeType is typed as a generic string and then cast to MonitoringResource["appType"] later, which defeats type-safety and can allow invalid values to flow into the monitoring UI/WebSocket query params. Type composeType as the real union ("docker-compose" | "stack") and derive appType without a cast (or validate against the allowed set).
| export function buildMonitoringResources( | ||
| projects?: ProjectShape[] | null, | ||
| ): MonitoringResource[] { | ||
| if (!projects) return []; |
There was a problem hiding this comment.
buildMonitoringResources defines local ProjectShape / ProjectEnvironment interfaces for the api.project.all response. This is prone to drifting from the actual tRPC output shape. Consider typing the argument as RouterOutputs["project"]["all"] | null | undefined (type-only import) so this utility stays aligned with the API contract.
|
|
||
| export default buildMonitoringResources; |
There was a problem hiding this comment.
This is the only file under apps/dokploy/utils using a default export; the other utils use named exports. Consider dropping the default export and importing buildMonitoringResources by name for consistency across the utils layer.
| export default buildMonitoringResources; |
|
|
||
| const { data: monitoring, isLoading } = api.user.getMetricsToken.useQuery(); | ||
| const { data: projects } = api.project.all.useQuery(undefined, { | ||
| refetchOnWindowFocus: false, |
There was a problem hiding this comment.
api.project.all returns a fairly heavy graph (projects + environments + services). This query currently runs even when toggleMonitoring is true (paid monitoring path), where the data isn't used. Consider adding enabled: !toggleMonitoring (or moving the query into the free-monitoring branch) to avoid unnecessary network/load.
| refetchOnWindowFocus: false, | |
| refetchOnWindowFocus: false, | |
| enabled: !toggleMonitoring, |
|
|
||
| expect(appResource?.label).toBe("App A"); |
There was a problem hiding this comment.
The last assertion duplicates an earlier check (appResource?.label is already asserted). Consider removing the redundant expectation to keep the test focused.
| expect(appResource?.label).toBe("App A"); |
- Only fetch projects if paid monitoring is not active.
1 participant
What is this PR about?
feat: implement buildMonitoringResources function and add tests for resource extraction
Checklist
Before submitting this PR, please make sure that:
canarybranch.Issues related (if applicable)
closes #2316
Screenshots (if applicable)
Greptile Summary
This PR adds resource filtering functionality to the monitoring page, allowing users to select specific projects and resources (applications, compose services, databases) to monitor. The implementation includes a new utility function
buildMonitoringResourcesthat extracts all monitorable resources from projects and their environments, comprehensive tests, and UI updates with dropdown selectors.Major changes:
buildMonitoringResourcesextracts applications, compose services, and all database types (postgres, redis, mysql, mongo, mariadb) into a flat list of monitoring resourcesIssues found:
appTypeinContainerFreeMonitoringcomponent will prevent reconnection when resource type changesConfidence Score: 3/5
apps/dokploy/components/dashboard/monitoring/free/container/show-free-container-monitoring.tsx- the WebSocket dependency issue must be fixed before mergingLast reviewed commit: 321d29e
(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!