Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion components/log-viewer-webui/client/settings.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"MongoDbSearchResultsMetadataCollectionName": "results-metadata"
"MongoDbSearchResultsMetadataCollectionName": "results-metadata",
"ClpStorageEngine": "clp"
}
Original file line number Diff line number Diff line change
@@ -1,40 +1,52 @@
import {Link} from "react-router";

import {LinkOutlined} from "@ant-design/icons";
import {
Tooltip,
Typography,
} from "antd";

import {STREAM_TYPE} from "../utils";
import styles from "./index.module.css";


const {Link} = Typography;

// eslint-disable-next-line no-warning-comments
// TODO: Fix link to connect to package log viewer when log viewer setup finished. Also pass
// proper args to package log viewer.
const LOG_VIEWER_URL = "https://y-scope.github.io/yscope-log-viewer/";

interface LogViewerLinkProps {
filePath: string;
logEventIdx: number;
streamId: string;
}

/**
* Render a link to the log viewer with open file.
*
* @param props
* @param props.filePath
* @param props.logEventIdx
* @param props.streamId
* @return
*/
const LogViewerLink = ({filePath}: LogViewerLinkProps) => (
const LogViewerLink = ({
filePath,
logEventIdx,
streamId,
}: LogViewerLinkProps) => (
<Tooltip title={"Open file"}>
<Link
href={LOG_VIEWER_URL}
target={"_blank"}
type={"secondary"}
>
<LinkOutlined className={styles["linkIcon"] || ""}/>
{filePath}
</Link>
<Typography.Link>
<Link
className={styles["linkIcon"] || ""}
target={"_blank"}
to={{
pathname: "/streamFile",
search:
`?type=${encodeURIComponent(STREAM_TYPE)}` +
`&streamId=${encodeURIComponent(streamId)}` +
`&logEventIdx=${encodeURIComponent(logEventIdx)}`,
}}
>
<LinkOutlined/>
{filePath}
</Link>
</Typography.Link>
</Tooltip>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,28 @@ import "highlight.js/styles/intellij-light.css";
const {Text} = Typography;

interface MessageProps {
message: string;
filePath: string;
message: string;
logEventIdx: number;
streamId: string;
}

/**
* Renders a message with syntax highlighting and a file path link.
*
* @param props
* @param props.message
* @param props.filePath
* @param props.logEventIdx
* @param props.message
* @param props.streamId
* @return
*/
const Message = ({message, filePath}: MessageProps) => {
const Message = ({
message,
filePath,
streamId,
logEventIdx,
}: MessageProps) => {
return (
<>
{/* Parent `Text` component allows syntax highlighter to inherit AntD fonts. */}
Expand All @@ -37,7 +46,10 @@ const Message = ({message, filePath}: MessageProps) => {
{message}
</SyntaxHighlighter>
</Text>
<LogViewerLink filePath={filePath}/>
<LogViewerLink
filePath={filePath}
logEventIdx={logEventIdx}
streamId={streamId}/>
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,21 @@ import dayjs from "dayjs";

import {DATETIME_FORMAT_TEMPLATE} from "../../../../typings/datetime";
import Message from "./Message";
import {getStreamId} from "./utils";


/**
* Structure of search results data displayed in the table.
*/
interface SearchResult {
_id: string;
timestamp: number;
message: string;
archive_id: string;
Comment thread
davemarco marked this conversation as resolved.
filePath: string;
orig_file_path: string;
orig_file_id: string;
log_event_ix: number;
message: string;
orig_file_id: string;
orig_file_path: string;
timestamp: number;
}

/**
Expand All @@ -36,7 +38,9 @@ const searchResultsTableColumns: NonNullable<TableProps<SearchResult>["columns"]
render: (_, record) => (
<Message
filePath={record.orig_file_path}
message={record.message}/>
logEventIdx={record.log_event_ix}
message={record.message}
streamId={getStreamId(record)}/>
),
title: "Message",
width: 85,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import settings from "../../../../../settings.json";
import type {SearchResult} from "./typings";


const SETTINGS_STORAGE_ENGINE = settings.ClpStorageEngine;

/**
* Stream type based on the storage engine (i.e. clp vs. clp-s).
*/
const STREAM_TYPE = "clp" === SETTINGS_STORAGE_ENGINE ?
"ir" :
"json";

/**
* Returns the stream id based on the storage engine.
*
* @param result The search result object.
* @return The stream id string, either from orig_file_id or archive_id.
*/
const getStreamId = (result: SearchResult): string => {
return "clp" === SETTINGS_STORAGE_ENGINE ?
result.orig_file_id :
result.archive_id;
};


export {
getStreamId,
STREAM_TYPE,
};
5 changes: 5 additions & 0 deletions components/log-viewer-webui/client/src/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
import MainLayout from "./components/Layout/MainLayout";
import IngestPage from "./pages/IngestPage";
import SearchPage from "./pages/SearchPage";
import QueryStatus from "./ui/QueryStatus";


const router = createBrowserRouter([
Expand All @@ -23,6 +24,10 @@ const router = createBrowserRouter([
{path: "search", Component: SearchPage},
],
},
{
path: "/streamFile",
Component: QueryStatus,
},
]);


Expand Down
5 changes: 5 additions & 0 deletions components/log-viewer-webui/server/src/routes/static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ const routes: FastifyPluginAsync = async (fastify) => {
prefix: "/",
root: clientDir,
decorateReply: false,
wildcard: false,
});

fastify.get("/streamFile", (_, reply) => {
reply.sendFile("index.html", clientDir);
});
Comment on lines +55 to 57

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

to get the other React-Router defined routes in the SPA working, i believe we also need to let Fastify serve those routes with the index.html

@coderabbitai please create an issue to track this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

@junhaoliao I'll create an issue to track the SPA routing problem you've identified.


🏁 Script executed:

#!/bin/bash
# Create a GitHub issue to track the SPA routing problem
gh issue create \
  --title "SPA routing: Fastify needs to serve index.html for all React Router routes" \
  --body "## Problem

Currently, only the \`/streamFile\` route is configured to serve \`index.html\` in the Fastify server. Other React Router defined routes in the SPA will return 404 errors when accessed directly via URL or when users refresh the page.

## Current Implementation

In \`components/log-viewer-webui/server/src/routes/static.ts\`, we have:
- Static file serving with \`wildcard: false\`
- Specific route handler for \`/streamFile\` that serves \`index.html\`

## Expected Behavior

All React Router defined routes should be served with \`index.html\` so that the React application can handle client-side routing properly.

## Related

- PR: https://github.com/y-scope/clp/pull/950
- Comment: https://github.com/y-scope/clp/pull/950#discussion_r2148823183
- Requested by: @junhaoliao" \
  --assignee junhaoliao

Length of output: 952


@junhaoliao I've successfully created issue #1011 to track the SPA routing problem: #1011

The issue documents the current implementation where only /streamFile serves index.html, and explains that other React Router routes will need similar handling to prevent 404 errors when accessed directly or on page refresh. The issue has been assigned to you as requested.

🐰📝
Taking notes for future improvements


🧬 Code Graph Analysis Results

Relevant snippet from components/log-viewer-webui/server/src/routes/static.ts (lines 1-56)

import path from "node:path";
import process from "node:process";
import {fileURLToPath} from "node:url";

import {fastifyStatic} from "@fastify/static";
import {FastifyPluginAsync} from "fastify";

import settings from "../../settings.json" with {type: "json"};


/**
 * Creates static files serving routes.
 *
 * @param fastify
 */
const routes: FastifyPluginAsync = async (fastify) => {
    const filename = fileURLToPath(import.meta.url);
    const dirname = path.dirname(filename);
    const rootDirname = path.resolve(dirname, "../..");

    let streamFilesDir = settings.StreamFilesDir;
    if (false === path.isAbsolute(streamFilesDir)) {
        streamFilesDir = path.resolve(rootDirname, streamFilesDir);
    }
    await fastify.register(fastifyStatic, {
        prefix: "/streams",
        root: streamFilesDir,
    });

    let logViewerDir = settings.LogViewerDir;
    if (false === path.isAbsolute(logViewerDir)) {
        logViewerDir = path.resolve(rootDirname, logViewerDir);
    }
    await fastify.register(fastifyStatic, {
        prefix: "/log-viewer",
        root: logViewerDir,
        decorateReply: false,
    });

    if ("production" === process.env.NODE_ENV) {
        // In the development environment, we expect the client to use a separate webserver that
        // supports live reloading.
        let clientDir = settings.ClientDir;
        if (false === path.isAbsolute(clientDir)) {
            clientDir = path.resolve(rootDirname, settings.ClientDir);
        }

        await fastify.register(fastifyStatic, {
            prefix: "/",
            root: clientDir,
            decorateReply: false,
            wildcard: false,
        });

        fastify.get("/streamFile", (_, reply) => {
            reply.sendFile("index.html", clientDir);
        });
    }
};

export default routes;

Summary of possibly relevant snippet from components/webui/server/main.js (lines 30-33)

const {
        CLP_DB_USER,
        CLP_DB_PASS,
    } = process.env;
  • This snippet extracts environment variables related to database credentials from process.env.
  • It is a small part of the server main file, likely related to configuration.
  • No direct relation to static file serving or React Router routes handling.

No other snippets provided appear relevant to the static file serving or SPA route handling discussed in the user comment.

}
};
Expand Down