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

[Dashboard] Update Serve System UI #36787

Merged
merged 5 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 33 additions & 10 deletions dashboard/client/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,16 @@ import {
ServeApplicationDetailPage,
} from "./pages/serve/ServeApplicationDetailPage";
import { ServeApplicationsListPage } from "./pages/serve/ServeApplicationsListPage";
import { ServeLayout } from "./pages/serve/ServeLayout";
import { ServeLayout, ServeSideTabLayout } from "./pages/serve/ServeLayout";
import { ServeReplicaDetailPage } from "./pages/serve/ServeReplicaDetailPage";
import {
ServeControllerDetailPage,
ServeHttpProxyDetailPage,
} from "./pages/serve/ServeSystemActorDetailPage";
import {
ServeSystemDetailLayout,
ServeSystemDetailPage,
} from "./pages/serve/ServeSystemDetailPage";
import { TaskPage } from "./pages/task/TaskPage";
import { getNodeList } from "./service/node";
import { lightTheme } from "./theme";
Expand Down Expand Up @@ -225,15 +229,34 @@ const App = () => {
</Route>
<Route element={<Metrics />} path="metrics" />
<Route element={<ServeLayout />} path="serve">
<Route element={<ServeApplicationsListPage />} path="" />
<Route
element={<ServeControllerDetailPage />}
path="controller"
/>
<Route
element={<ServeHttpProxyDetailPage />}
path="httpProxies/:httpProxyId"
/>
<Route element={<ServeSideTabLayout />} path="">
<Route
element={
<SideTabPage tabId="system">
<ServeSystemDetailPage />
</SideTabPage>
}
path="system"
/>
<Route
element={
<SideTabPage tabId="applications">
<ServeApplicationsListPage />
</SideTabPage>
}
path=""
/>
</Route>
<Route element={<ServeSystemDetailLayout />} path="system">
<Route
element={<ServeControllerDetailPage />}
path="controller"
/>
<Route
element={<ServeHttpProxyDetailPage />}
path="httpProxies/:httpProxyId"
/>
</Route>
<Route
element={<ServeApplicationDetailLayout />}
path="applications/:applicationName"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,16 @@ const MetadataList: React.FC<{
export const MetadataSection = ({
header,
metadataList,
footer,
}: {
header?: string;
metadataList: Metadata[];
footer?: JSX.Element;
}) => {
return (
<Section title={header} marginTop={1} marginBottom={4}>
<MetadataList metadataList={metadataList} />
<Box marginTop={1}>{footer}</Box>
</Section>
);
};
17 changes: 7 additions & 10 deletions dashboard/client/src/components/StatusChip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const colorMap = {
[ServeApplicationStatus.RUNNING]: green,
[ServeApplicationStatus.DEPLOY_FAILED]: red,
[ServeApplicationStatus.DELETING]: orange,
[ServeApplicationStatus.UNHEALTHY]: red,
},
serveDeployment: {
[ServeDeploymentStatus.UPDATING]: orange,
Expand Down Expand Up @@ -105,7 +106,6 @@ const useStyles = makeStyles((theme) =>
border: "solid 1px",
borderRadius: 4,
fontSize: 12,
margin: 2,
display: "inline-flex",
alignItems: "center",
},
Expand All @@ -115,17 +115,14 @@ const useStyles = makeStyles((theme) =>
}),
);

export const StatusChip = ({
type,
status,
suffix,
icon,
}: {
type: string;
export type StatusChipProps = {
type: keyof typeof colorMap;
status: string | ActorEnum | ReactNode;
suffix?: string;
suffix?: ReactNode;
icon?: ReactNode;
}) => {
};

export const StatusChip = ({ type, status, suffix, icon }: StatusChipProps) => {
const classes = useStyles();
let color: Color | string = blueGrey;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import { ServeDeploymentRow } from "./ServeDeploymentRow";

const useStyles = makeStyles((theme) =>
createStyles({
root: {
padding: theme.spacing(3),
},
table: {
tableLayout: "fixed",
},
Expand Down Expand Up @@ -77,7 +80,7 @@ export const ServeApplicationDetailPage = () => {

const appName = application.name ? application.name : "-";
return (
<div>
<div className={classes.root}>
<MetadataSection
metadataList={[
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const mockGetActor = jest.mocked(getActor);

describe("ServeApplicationsListPage", () => {
it("renders list", async () => {
expect.assertions(14);
expect.assertions(5);

// Mock ServeController actor fetch
mockGetActor.mockResolvedValue({
Expand Down Expand Up @@ -79,29 +79,15 @@ describe("ServeApplicationsListPage", () => {
} as any);

render(<ServeApplicationsListPage />, { wrapper: TEST_APP_WRAPPER });

await screen.findByText("System");
expect(screen.getByText("System")).toBeVisible();
expect(screen.getByText("1.2.3.4")).toBeVisible();
expect(screen.getByText("8000")).toBeVisible();

// HTTP Proxy row
expect(screen.getByText("HTTPProxyActor:node:12345")).toBeVisible();
expect(screen.getByText("STARTING")).toBeVisible();

// Serve Controller row
expect(screen.getByText("Serve Controller")).toBeVisible();
expect(screen.getByText("HEALTHY")).toBeVisible();
await screen.findByText("Application status");

// First row
expect(screen.getByText("home")).toBeVisible();
expect(screen.getByText("/")).toBeVisible();
expect(screen.getByText("RUNNING")).toBeVisible();

// Second row
expect(screen.getByText("second-app")).toBeVisible();
expect(screen.getByText("/second-app")).toBeVisible();
expect(screen.getByText("DEPLOYING")).toBeVisible();

expect(screen.getByText("Metrics")).toBeVisible();
});
Expand Down
75 changes: 64 additions & 11 deletions dashboard/client/src/pages/serve/ServeApplicationsListPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,25 @@ import {
import { Alert, Autocomplete, Pagination } from "@material-ui/lab";
import React, { ReactElement } from "react";
import { CollapsibleSection } from "../../common/CollapsibleSection";
import {
MultiTabLogViewer,
MultiTabLogViewerTabDetails,
} from "../../common/MultiTabLogViewer";
import { Section } from "../../common/Section";
import Loading from "../../components/Loading";
import { HelpInfo } from "../../components/Tooltip";
import { ServeSystemActor } from "../../type/serve";
import { useFetchActor } from "../actor/hook/useActorDetail";
import { useServeApplications } from "./hook/useServeApplications";
import { ServeApplicationRow } from "./ServeApplicationRow";
import { ServeMetricsSection } from "./ServeMetricsSection";
import { ServeSystemDetails } from "./ServeSystemDetails";
import { ServeSystemPreview } from "./ServeSystemDetails";

const useStyles = makeStyles((theme) =>
createStyles({
root: {
padding: theme.spacing(3),
},
table: {
tableLayout: "fixed",
},
Expand All @@ -37,7 +47,7 @@ const useStyles = makeStyles((theme) =>
applicationsSection: {
marginTop: theme.spacing(4),
},
metricsSection: {
section: {
marginTop: theme.spacing(4),
},
}),
Expand All @@ -59,13 +69,11 @@ export const ServeApplicationsListPage = () => {
const {
serveDetails,
filteredServeApplications,
httpProxies,
error,
allServeApplications,
page,
setPage,
httpProxiesPage,
setHttpProxiesPage,
httpProxies,
changeFilter,
} = useServeApplications();

Expand All @@ -78,18 +86,17 @@ export const ServeApplicationsListPage = () => {
}

return (
<div>
<div className={classes.root}>
{serveDetails.http_options === undefined ? (
<Alert className={classes.serveInstanceWarning} severity="warning">
Serve not started. Please deploy a serve application first.
</Alert>
) : (
<React.Fragment>
<ServeSystemDetails
serveDetails={serveDetails}
<ServeSystemPreview
Copy link
Member

Choose a reason for hiding this comment

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

Screenshot 2023-06-23 at 7 46 06 PM

Shouldn't ServeSystemPreview be shown in ServeSystemDetailPage instead? Along with the logs panel (title="Logs")

We can also remove having Serve applications be a collapsible section, and make the title of the page Applications instead (similar to System tab)

Copy link
Contributor

@scottsun94 scottsun94 Jun 27, 2023

Choose a reason for hiding this comment

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

We can also remove having Serve applications be a collapsible section, and make the title of the page Applications instead (similar to System tab)

Serve applications -> Applications makes sense.

Let's keep the collapsable behavior for now since it doesn't hurt? I don't know if people might want to collapse it when the table is long and they just want to focus on logs and metrics.

allApplications={allServeApplications}
httpProxies={httpProxies}
page={httpProxiesPage}
setPage={setHttpProxiesPage}
serveDetails={serveDetails}
/>
<CollapsibleSection
title="Serve applications"
Expand Down Expand Up @@ -194,7 +201,53 @@ export const ServeApplicationsListPage = () => {
</CollapsibleSection>
</React.Fragment>
)}
<ServeMetricsSection className={classes.metricsSection} />
<CollapsibleSection
title="Logs"
startExpanded
className={classes.section}
>
<Section noTopPadding>
<ServeControllerLogs controller={serveDetails.controller_info} />
</Section>
</CollapsibleSection>
<ServeMetricsSection className={classes.section} />
</div>
);
};

type ServeControllerLogsProps = {
controller: ServeSystemActor;
};

const ServeControllerLogs = ({
controller: { actor_id, log_file_path },
}: ServeControllerLogsProps) => {
const { data: fetchedActor } = useFetchActor(actor_id);

if (!fetchedActor || !log_file_path) {
return <Loading loading={true} />;
}

const tabs: MultiTabLogViewerTabDetails[] = [
{
title: "Controller logs",
nodeId: fetchedActor.address.rayletId,
filename: log_file_path.startsWith("/")
? log_file_path.substring(1)
: log_file_path,
},
{
title: "Other logs",
contents:
"Replica logs contain the application logs emitted by each Serve Replica.\n" +
"To view replica logs, please click into a Serve application from " +
"the table above to enter the Application details page.\nThen, click " +
"into a Serve Replica in the Deployments table.\n\n" +
"HTTP Proxy logs contains HTTP access logs for each HTTP Proxy.\n" +
"To view HTTP Proxy logs, click into a HTTP Proxy from the Serve System " +
"Details page.\nThis page can be accessed via the left tab menu or by " +
'clicking "View system status and configuration" link above.',
},
];
return <MultiTabLogViewer tabs={tabs} />;
};
21 changes: 20 additions & 1 deletion dashboard/client/src/pages/serve/ServeLayout.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { createStyles, makeStyles } from "@material-ui/core";
import React from "react";
import { RiInformationLine, RiTableLine } from "react-icons/ri";
import { Outlet } from "react-router-dom";
import { MainNavPageInfo } from "../layout/mainNavContext";
import { SideTabLayout, SideTabRouteLink } from "../layout/SideTabLayout";

const useStyles = makeStyles((theme) =>
createStyles({
root: {
padding: theme.spacing(3),
width: "100%",
minHeight: 800,
background: "white",
Expand All @@ -30,3 +31,21 @@ export const ServeLayout = () => {
</div>
);
};

export const ServeSideTabLayout = () => {
return (
<SideTabLayout>
<SideTabRouteLink
tabId="system"
title="System"
Icon={RiInformationLine}
/>
<SideTabRouteLink
to=""
tabId="applications"
title="Applications"
Icon={RiTableLine}
/>
</SideTabLayout>
);
};
5 changes: 4 additions & 1 deletion dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import { ServeReplicaMetricsSection } from "./ServeDeploymentMetricsSection";

const useStyles = makeStyles((theme) =>
createStyles({
root: {
padding: theme.spacing(3),
},
section: {
marginTop: theme.spacing(4),
},
Expand Down Expand Up @@ -61,7 +64,7 @@ export const ServeReplicaDetailPage = () => {
start_time_s,
} = replica;
return (
<div>
<div className={classes.root}>
<MainNavPageInfo
pageInfo={{
id: "serveReplicaDetail",
Expand Down
Loading