Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #14 add monitor view #152

Merged
merged 14 commits into from
Apr 4, 2024
35 changes: 20 additions & 15 deletions app/api/monitor/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import { NextResponse } from "next/server";
import { getServerSession } from "next-auth/next";
import authOptions, { getConnection } from "../auth/[...nextauth]/options";

const fileds = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix the typo in the variable name fileds.

-const fileds = [
+const fields = [

Correct spelling is crucial for code readability and maintainability.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const fileds = [
const fields = [

"used_memory",
"used_memory_rss"
]
// eslint-disable-next-line import/prefer-default-export
export async function GET() {

Expand All @@ -16,20 +20,21 @@ export async function GET() {
return NextResponse.json({ message: "Not authenticated" }, { status: 401 })
}

const info = await client.info("memory")

if(typeof info === 'string') {

const data = (info as string).split('\r\n').map((item) => {
const name = item.split(':')[0]
const num = item.split(':')[1]
return { name, series: num }
})

data.splice(0, 1)
return NextResponse.json(data, { status: 200 })
}

return NextResponse.json({message: "Failed to retrive info"}, { status: 500 })
const infoMemory = await client.connection.info("memory")
const infoGraph = await client.info()

const dataMemory = infoMemory.split('\r\n').map((item: string) => {
const name = item.split(':')[0]
const series = item.split(':')[1]
return { name, series }
}).filter((item: {name: string, series: string}) => fileds.find(filed => filed === item.name))
Comment on lines +26 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor the data processing logic to improve readability and efficiency.

-    const dataMemory = infoMemory.split('\r\n').map((item: string) => {
-        const name = item.split(':')[0]
-        const series = item.split(':')[1]
-        return { name, series }
-    }).filter((item: {name: string, series: string}) => fileds.find(filed => filed === item.name))
+    const dataMemory = infoMemory.split('\r\n')
+        .map(item => item.split(':'))
+        .filter(([name]) => fields.includes(name))
+        .map(([name, series]) => ({ name, series }));

This refactor simplifies the parsing and filtering logic by combining steps and using destructuring for clarity.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const dataMemory = infoMemory.split('\r\n').map((item: string) => {
const name = item.split(':')[0]
const series = item.split(':')[1]
return { name, series }
}).filter((item: {name: string, series: string}) => fileds.find(filed => filed === item.name))
const dataMemory = infoMemory.split('\r\n')
.map(item => item.split(':'))
.filter(([name]) => fields.includes(name))
.map(([name, series]) => ({ name, series }));

const dataGraph: {name: string, series: number}[] = []
for (let i = 0; i < infoGraph.length; i += 2) {
const name = (infoGraph[i] as string).substring(2)
const series = (infoGraph[i + 1] as string[]).length
dataGraph.push({name, series})
}
Comment on lines +32 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding error handling for the asynchronous operations.

+    try {
         const dataGraph: {name: string, series: number}[] = []
         for (let i = 0; i < infoGraph.length; i += 2) {
             const name = (infoGraph[i] as string).substring(2)
             const series = (infoGraph[i + 1] as string[]).length
             dataGraph.push({name, series})
         }
+    } catch (error) {
+        console.error('Error processing graph data:', error);
+        return NextResponse.json({ message: "Error processing data" }, { status: 500 })
+    }

Adding try-catch blocks around asynchronous operations can help catch and handle any unexpected errors, improving the robustness of your application.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
for (let i = 0; i < infoGraph.length; i += 2) {
const name = (infoGraph[i] as string).substring(2)
const series = (infoGraph[i + 1] as string[]).length
dataGraph.push({name, series})
}
try {
const dataGraph: {name: string, series: number}[] = []
for (let i = 0; i < infoGraph.length; i += 2) {
const name = (infoGraph[i] as string).substring(2)
const series = (infoGraph[i + 1] as string[]).length
dataGraph.push({name, series})
}
} catch (error) {
console.error('Error processing graph data:', error);
return NextResponse.json({ message: "Error processing data" }, { status: 500 })
}


return NextResponse.json({ memory: dataMemory, graph: dataGraph }, { status: 200 })

}
74 changes: 74 additions & 0 deletions app/monitor/MonitorView.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { ECharts } from "echarts";
import ReactEcharts, { EChartsInstance, EChartsOption } from "echarts-for-react"
import { useEffect, useRef, useState } from "react"

interface Props {
data: { name: string, series: string }[]
time: Date
}

export default function MonitorView({ data, time }: Props) {

const echartRef = useRef<EChartsInstance | null>(null)
const [timeArr] = useState<string[]>([])
const [chartReady, setChartReady] = useState<boolean>(false)
Comment on lines +12 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid initializing state with an empty array that's intended to be mutated directly.

-  const [timeArr] = useState<string[]>([])
+  const [timeArr, setTimeArr] = useState<string[]>([])

Use a state setter function (setTimeArr) to update the state to ensure React's state management and re-rendering mechanisms work as expected.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const echartRef = useRef<EChartsInstance | null>(null)
const [timeArr] = useState<string[]>([])
const [chartReady, setChartReady] = useState<boolean>(false)
const echartRef = useRef<EChartsInstance | null>(null)
const [timeArr, setTimeArr] = useState<string[]>([])
const [chartReady, setChartReady] = useState<boolean>(false)


useEffect(() => {
if (chartReady && echartRef.current) {
const myChart: ECharts = echartRef.current
data.forEach((item, index) => {
myChart.appendData({
seriesIndex: index,
data: [item.series]
})
})
timeArr.push(time.toLocaleTimeString().split(" ")[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

Update the state in an immutable way.

-      timeArr.push(time.toLocaleTimeString().split(" ")[0])
+      setTimeArr(prevTimeArr => [...prevTimeArr, time.toLocaleTimeString().split(" ")[0]])

This change ensures that the state update follows React's best practices for immutability, leading to more predictable state updates and re-renders.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
timeArr.push(time.toLocaleTimeString().split(" ")[0])
setTimeArr(prevTimeArr => [...prevTimeArr, time.toLocaleTimeString().split(" ")[0]])

myChart.setOption({
xAxis: {
type: "category",
data: timeArr
}
})
}
}, [data, time, timeArr, chartReady])

const options: EChartsOption = {
tooltip: {
trigger: 'axis',
axisPointer: {
type: 'cross',
},
},
legend: {
data: data.map(item => item.name)
},
xAxis: {
type: "category",
data: timeArr,
min: 0
},
yAxis: {
type: "value",
},
series: data.map(item => ({
name: item.name,
data: [],
type: "line",
smooth: true,
itemStyle: {
opacity: 0
Comment on lines +58 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider removing the opacity: 0 style if it's not required.

       itemStyle: {
-        opacity: 0
       }

If the intention is to make the items invisible, there might be more appropriate ways to achieve this, depending on the desired effect. If this style is not needed, removing it can simplify the code.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
itemStyle: {
opacity: 0
itemStyle: {

}
}))
}

return (
<ReactEcharts
className="w-full h-full"
option={options}
onChartReady={(e) => {
echartRef.current = e
setChartReady(true)
}}
/>
)
}
36 changes: 36 additions & 0 deletions app/monitor/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
"use client"

import useSWR from 'swr'
import React, { useState } from 'react'
import MonitorView from './MonitorView'

export default function Page() {

const [time, setTime] = useState<Date | null>(null)

const fetcher = (url: string) => fetch(url, {
method: 'GET',
headers: {
'Content-Type': 'application/json'
}
}).then((result) => {
if (result.status < 300) {
setTime(new Date())
return result.json()
}
return []
})
Comment on lines +11 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider handling HTTP status codes other than success more explicitly.

 }).then((result) => {
     if (result.status < 300) {
         setTime(new Date())
         return result.json()
     }
+    else if (result.status >= 400 && result.status < 500) {
+        console.error('Client error:', result.status)
+    } else if (result.status >= 500) {
+        console.error('Server error:', result.status)
+    }
     return []
 })

This ensures better error handling and logging for client and server errors.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const fetcher = (url: string) => fetch(url, {
method: 'GET',
headers: {
'Content-Type': 'application/json'
}
}).then((result) => {
if (result.status < 300) {
setTime(new Date())
return result.json()
}
return []
})
const fetcher = (url: string) => fetch(url, {
method: 'GET',
headers: {
'Content-Type': 'application/json'
}
}).then((result) => {
if (result.status < 300) {
setTime(new Date())
return result.json()
}
else if (result.status >= 400 && result.status < 500) {
console.error('Client error:', result.status)
} else if (result.status >= 500) {
console.error('Server error:', result.status)
}
return []
})


const { data } = useSWR(`/api/monitor/`, fetcher, { refreshInterval: 1000 })
return (
<div className='flex flex-col items-center w-full h-full'>
<h1 className='p-5 text-6xl'>Monitor</h1>
<div className='w-10/12 h-full'>
{(data?.memory && time) && <MonitorView data={data?.memory} time={time} />}
</div>
<div className='w-10/12 h-full'>
{(data?.graph && time) && <MonitorView data={data?.graph} time={time} />}
Comment on lines +29 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

Leverage conditional rendering more efficiently.

-                {(data?.memory && time) && <MonitorView data={data?.memory} time={time} />}
+                {data?.memory && <MonitorView data={data?.memory} time={time || new Date()} />}

This simplifies the condition by removing the redundant check for time since it's initialized as null and updated on successful data fetches. If time is critical for the MonitorView and should not be a fallback to the current date, consider a different approach to ensure time is set before rendering.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
{(data?.memory && time) && <MonitorView data={data?.memory} time={time} />}
</div>
<div className='w-10/12 h-full'>
{(data?.graph && time) && <MonitorView data={data?.graph} time={time} />}
{data?.memory && <MonitorView data={data?.memory} time={time || new Date()} />}
</div>
<div className='w-10/12 h-full'>
{(data?.graph && time) && <MonitorView data={data?.graph} time={time} />}

</div>
</div>
)
}
14 changes: 10 additions & 4 deletions app/providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import Navbar from "@/components/custom/navbar";
import { ResizableHandle, ResizablePanel, ResizablePanelGroup } from "@/components/ui/resizable";
import { Info, LogOut, Waypoints } from "lucide-react";
import { Activity, Info, LogOut, Waypoints } from "lucide-react";
import { SessionProvider, signOut } from "next-auth/react";
import { ThemeProvider } from 'next-themes'
import { useEffect, useRef, useState } from "react";
Expand All @@ -20,6 +20,12 @@ const LINKS = [
href: "/graph",
icon: (<Waypoints className="h-6 w-6" />),
},
{
name: "Monitor",
// href: "/api/monitor",
href: "/monitor",
icon: (<Activity className="h-6 w-6" />),
},
{
name: "Disconnect",
href: "",
Expand All @@ -32,12 +38,12 @@ export default function NextAuthProvider({ children }: { children: React.ReactNo

const { screenSize } = useScreenSize();
const isSmallScreen = screenSize === 'sm' || screenSize === 'xs'

const [isCollapsed, setCollapsed] = useState(isSmallScreen)
const navPanel = useRef<ImperativePanelHandle>(null)

useEffect(() => {
if (isSmallScreen){
if (isSmallScreen) {
setCollapsed(true)
if (navPanel.current) {
navPanel.current.collapse()
Expand Down Expand Up @@ -74,7 +80,7 @@ export default function NextAuthProvider({ children }: { children: React.ReactNo
<Navbar links={LINKS} collapsed={isCollapsed} onExpand={onExpand} />
</ResizablePanel>
<ResizableHandle withHandle />
<ResizablePanel defaultSize={100-panelSize}>{children}</ResizablePanel>
<ResizablePanel defaultSize={100 - panelSize}>{children}</ResizablePanel>
</ResizablePanelGroup>
</ThemeProvider>
</SessionProvider>
Expand Down
68 changes: 68 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
"cmdk": "^0.2.0",
"cytoscape": "^3.28.1",
"cytoscape-fcose": "^2.2.0",
"echarts": "^5.5.0",
"echarts-for-react": "^3.0.2",
"eslint-config-airbnb": "^19.0.4",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-import": "^2.29.1",
Expand All @@ -49,6 +51,7 @@
"react-hook-form": "^7.49.3",
"react-resizable-panels": "^1.0.9",
"save-dev": "^0.0.1-security",
"swr": "^2.2.5",
"tailwind-merge": "^2.2.0",
"tailwindcss": "^3.4.1",
"tailwindcss-animate": "^1.0.7",
Expand Down
Loading