-
Notifications
You must be signed in to change notification settings - Fork 952
chore(site): replace agent log service #9814
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
Changes from all commits
a39b51e
16479f3
0223105
12e7bce
0ca16dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,17 @@ | ||
import Popover from "@mui/material/Popover"; | ||
import { makeStyles, useTheme } from "@mui/styles"; | ||
import Skeleton from "@mui/material/Skeleton"; | ||
import { useMachine } from "@xstate/react"; | ||
import * as API from "api/api"; | ||
import CodeOutlined from "@mui/icons-material/CodeOutlined"; | ||
import { | ||
CloseDropdown, | ||
OpenDropdown, | ||
} from "components/DropdownArrows/DropdownArrows"; | ||
import { LogLine, logLineHeight } from "components/WorkspaceBuildLogs/Logs"; | ||
import { | ||
Line, | ||
LogLine, | ||
logLineHeight, | ||
} from "components/WorkspaceBuildLogs/Logs"; | ||
import { PortForwardButton } from "./PortForwardButton"; | ||
import { VSCodeDesktopButton } from "components/Resources/VSCodeDesktopButton/VSCodeDesktopButton"; | ||
import { | ||
|
@@ -25,15 +29,11 @@ import AutoSizer from "react-virtualized-auto-sizer"; | |
import { FixedSizeList as List, ListOnScrollProps } from "react-window"; | ||
import { colors } from "theme/colors"; | ||
import { combineClasses } from "utils/combineClasses"; | ||
import { | ||
LineWithID, | ||
workspaceAgentLogsMachine, | ||
} from "xServices/workspaceAgentLogs/workspaceAgentLogsXService"; | ||
import { | ||
Workspace, | ||
WorkspaceAgent, | ||
WorkspaceAgentMetadata, | ||
} from "../../api/typesGenerated"; | ||
} from "api/typesGenerated"; | ||
import { AppLink } from "./AppLink/AppLink"; | ||
import { SSHButton } from "./SSHButton/SSHButton"; | ||
import { Stack } from "../Stack/Stack"; | ||
|
@@ -44,6 +44,14 @@ import { AgentVersion } from "./AgentVersion"; | |
import { AgentStatus } from "./AgentStatus"; | ||
import Collapse from "@mui/material/Collapse"; | ||
import { useProxy } from "contexts/ProxyContext"; | ||
import { displayError } from "components/GlobalSnackbar/utils"; | ||
|
||
// Logs are stored as the Line interface to make rendering | ||
// much more efficient. Instead of mapping objects each time, we're | ||
// able to just pass the array of logs to the component. | ||
export interface LineWithID extends Line { | ||
id: number; | ||
} | ||
|
||
export interface AgentRowProps { | ||
agent: WorkspaceAgent; | ||
|
@@ -68,24 +76,11 @@ export const AgentRow: FC<AgentRowProps> = ({ | |
hideVSCodeDesktopButton, | ||
serverVersion, | ||
onUpdateAgent, | ||
storybookLogs, | ||
storybookAgentMetadata, | ||
sshPrefix, | ||
storybookLogs, | ||
}) => { | ||
const styles = useStyles(); | ||
const [logsMachine, sendLogsEvent] = useMachine(workspaceAgentLogsMachine, { | ||
context: { agentID: agent.id }, | ||
services: process.env.STORYBOOK | ||
? { | ||
getLogs: async () => { | ||
return storybookLogs || []; | ||
}, | ||
streamLogs: () => async () => { | ||
// noop | ||
}, | ||
} | ||
: undefined, | ||
}); | ||
const theme = useTheme(); | ||
const startupScriptAnchorRef = useRef<HTMLButtonElement>(null); | ||
const [startupScriptOpen, setStartupScriptOpen] = useState(false); | ||
|
@@ -94,36 +89,20 @@ export const AgentRow: FC<AgentRowProps> = ({ | |
showApps && | ||
((agent.status === "connected" && hasAppsToDisplay) || | ||
agent.status === "connecting"); | ||
const hasStartupFeatures = | ||
Boolean(agent.logs_length) || Boolean(logsMachine.context.logs?.length); | ||
const hasStartupFeatures = Boolean(agent.logs_length); | ||
const { proxy } = useProxy(); | ||
|
||
const [showLogs, setShowLogs] = useState( | ||
["starting", "start_timeout"].includes(agent.lifecycle_state) && | ||
hasStartupFeatures, | ||
); | ||
useEffect(() => { | ||
setShowLogs(agent.lifecycle_state !== "ready" && hasStartupFeatures); | ||
}, [agent.lifecycle_state, hasStartupFeatures]); | ||
// External applications can provide startup logs for an agent during it's spawn. | ||
// These could be Kubernetes logs, or other logs that are useful to the user. | ||
// For this reason, we want to fetch these logs when the agent is starting. | ||
useEffect(() => { | ||
if (agent.lifecycle_state === "starting") { | ||
sendLogsEvent("FETCH_LOGS"); | ||
} | ||
}, [sendLogsEvent, agent.lifecycle_state]); | ||
useEffect(() => { | ||
// We only want to fetch logs when they are actually shown, | ||
// otherwise we can make a lot of requests that aren't necessary. | ||
if (showLogs && logsMachine.can("FETCH_LOGS")) { | ||
sendLogsEvent("FETCH_LOGS"); | ||
} | ||
}, [logsMachine, sendLogsEvent, showLogs]); | ||
const agentLogs = useAgentLogs(agent.id, { | ||
enabled: showLogs, | ||
initialData: process.env.STORYBOOK ? storybookLogs || [] : undefined, | ||
}); | ||
const logListRef = useRef<List>(null); | ||
const logListDivRef = useRef<HTMLDivElement>(null); | ||
const startupLogs = useMemo(() => { | ||
const allLogs = logsMachine.context.logs || []; | ||
const allLogs = agentLogs || []; | ||
|
||
const logs = [...allLogs]; | ||
if (agent.logs_overflowed) { | ||
|
@@ -135,8 +114,13 @@ export const AgentRow: FC<AgentRowProps> = ({ | |
}); | ||
} | ||
return logs; | ||
}, [logsMachine.context.logs, agent.logs_overflowed]); | ||
}, [agentLogs, agent.logs_overflowed]); | ||
const [bottomOfLogs, setBottomOfLogs] = useState(true); | ||
|
||
useEffect(() => { | ||
setShowLogs(agent.lifecycle_state !== "ready" && hasStartupFeatures); | ||
}, [agent.lifecycle_state, hasStartupFeatures]); | ||
|
||
// This is a layout effect to remove flicker when we're scrolling to the bottom. | ||
useLayoutEffect(() => { | ||
// If we're currently watching the bottom, we always want to stay at the bottom. | ||
|
@@ -396,6 +380,51 @@ export const AgentRow: FC<AgentRowProps> = ({ | |
); | ||
}; | ||
|
||
const useAgentLogs = ( | ||
agentId: string, | ||
{ enabled, initialData }: { enabled: boolean; initialData?: LineWithID[] }, | ||
) => { | ||
const [logs, setLogs] = useState<LineWithID[] | undefined>(initialData); | ||
const socket = useRef<WebSocket | null>(null); | ||
|
||
useEffect(() => { | ||
if (!enabled) { | ||
socket.current?.close(); | ||
return; | ||
} | ||
|
||
socket.current = API.watchWorkspaceAgentLogs(agentId, { | ||
// Get all logs | ||
after: 0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not be using We intentionally have an endpoint to fetch all logs, and then we should stream afterward, just like before. This hopefully would reduce lagging/flickering on the UI as logs stream in from the WS, because they'd be delivered in a payload first. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm.. I found having this way faster and better since we don't have to do two operations. It is also easy to understand. In this case, there is no lagging or flickering since when using after 0, all the "past" logs are being sent at once and not one by one. |
||
onMessage: (logs) => { | ||
setLogs((previousLogs) => { | ||
const newLogs: LineWithID[] = logs.map((log) => ({ | ||
id: log.id, | ||
level: log.level || "info", | ||
output: log.output, | ||
time: log.created_at, | ||
})); | ||
|
||
if (!previousLogs) { | ||
return newLogs; | ||
} | ||
|
||
return [...previousLogs, ...newLogs]; | ||
}); | ||
}, | ||
onError: () => { | ||
displayError("Error on getting agent logs"); | ||
}, | ||
}); | ||
|
||
return () => { | ||
socket.current?.close(); | ||
}; | ||
}, [agentId, enabled]); | ||
|
||
return logs; | ||
}; | ||
|
||
const useStyles = makeStyles((theme) => ({ | ||
agentRow: { | ||
backgroundColor: theme.palette.background.paperLight, | ||
|
Uh oh!
There was an error while loading. Please reload this page.