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

new-log-viewer: Add log query support in StateContextProvider. #80

Merged
merged 29 commits into from
Oct 19, 2024
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
49bd9a4
query log structure demo (won't work)
Henry8192 Sep 23, 2024
5b48fd3
clean up queryLog
Henry8192 Sep 27, 2024
7a777ee
various changes
Henry8192 Sep 30, 2024
fdd761a
finish draft of query log
Henry8192 Oct 1, 2024
5e4571a
queryResults should useRef
Henry8192 Oct 1, 2024
6be07e9
add test query icon to menubar
Henry8192 Oct 2, 2024
908ee7a
fix query log button, query logs now appear properly in console
Henry8192 Oct 2, 2024
da20db4
remove console logs, rename handleChunkResult to chunkResultsHandler …
Henry8192 Oct 6, 2024
5bf7df1
remove console logs in debugging
Henry8192 Oct 6, 2024
9a3cca9
address suggested changes in pr
Henry8192 Oct 7, 2024
8aa0c14
add parameters for queryLogs()
Henry8192 Oct 7, 2024
38fe8e1
address part of the suggestions in code review
Henry8192 Oct 9, 2024
fb14c88
minor fixes
Henry8192 Oct 9, 2024
47c2b82
Merge branch 'main' into log-query
Henry8192 Oct 10, 2024
1286879
address suggestions from review after merge
Henry8192 Oct 10, 2024
ea6b1ee
Merge branch 'main' into log-query
Henry8192 Oct 15, 2024
7ed8baf
use Map instead of Object for query results
Henry8192 Oct 15, 2024
46fe49f
fix queryResults not showing issue
Henry8192 Oct 15, 2024
9706bd0
provide queryResults in StateContextProvider.tsx
Henry8192 Oct 15, 2024
8e91e83
fix lint errors
Henry8192 Oct 15, 2024
8f60afc
fix lint docstring
Henry8192 Oct 15, 2024
3de7f80
Apply suggestions from code review
Henry8192 Oct 16, 2024
3c6c53d
Apply suggestions from code review
Henry8192 Oct 16, 2024
e04f169
address rest of the comments
Henry8192 Oct 16, 2024
e3913d5
fix lint comment warning
Henry8192 Oct 16, 2024
b732607
Update new-log-viewer/src/utils/config.ts
Henry8192 Oct 17, 2024
45e5e61
Apply suggestions from code review
Henry8192 Oct 19, 2024
142275e
apply suggestions
Henry8192 Oct 19, 2024
24e86c8
Apply suggestions from code review
Henry8192 Oct 19, 2024
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
41 changes: 39 additions & 2 deletions new-log-viewer/src/contexts/StateContextProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
EVENT_POSITION_ON_PAGE,
FileSrcType,
MainWorkerRespMessage,
QueryResults,
WORKER_REQ_CODE,
WORKER_RESP_CODE,
WorkerReq,
Expand Down Expand Up @@ -55,11 +56,13 @@ interface StateContextType {
numPages: number,
onDiskFileSizeInBytes: number,
pageNum: number,
queryResults: QueryResults,

exportLogs: () => void,
loadFile: (fileSrc: FileSrcType, cursor: CursorType) => void,
loadPageByAction: (navAction: NavigationAction) => void,
setLogLevelFilter: (newLogLevelFilter: LogLevelFilter) => void,
startQuery: (queryString: string, isRegex: boolean, isCaseSensitive: boolean) => void,
}
const StateContext = createContext<StateContextType>({} as StateContextType);

Expand All @@ -75,11 +78,13 @@ const STATE_DEFAULT: Readonly<StateContextType> = Object.freeze({
numPages: 0,
onDiskFileSizeInBytes: 0,
pageNum: 0,
queryResults: new Map(),

exportLogs: () => null,
loadFile: () => null,
loadPageByAction: () => null,
setLogLevelFilter: () => null,
startQuery: () => null,
});

interface StateContextProviderProps {
Expand Down Expand Up @@ -226,17 +231,18 @@ const StateContextProvider = ({children}: StateContextProviderProps) => {
const {filePath, logEventNum} = useContext(UrlContext);

// States
const [exportProgress, setExportProgress] =
useState<Nullable<number>>(STATE_DEFAULT.exportProgress);
const [fileName, setFileName] = useState<string>(STATE_DEFAULT.fileName);
const [logData, setLogData] = useState<string>(STATE_DEFAULT.logData);
const [numEvents, setNumEvents] = useState<number>(STATE_DEFAULT.numEvents);
const [numPages, setNumPages] = useState<number>(STATE_DEFAULT.numPages);
const [onDiskFileSizeInBytes, setOnDiskFileSizeInBytes] =
useState(STATE_DEFAULT.onDiskFileSizeInBytes);
const [pageNum, setPageNum] = useState<number>(STATE_DEFAULT.pageNum);
const [queryResults, setQueryResults] = useState<QueryResults>(STATE_DEFAULT.queryResults);
const beginLineNumToLogEventNumRef =
useRef<BeginLineNumToLogEventNumMap>(STATE_DEFAULT.beginLineNumToLogEventNum);
const [exportProgress, setExportProgress] =
useState<Nullable<number>>(STATE_DEFAULT.exportProgress);

// Refs
const logEventNumRef = useRef(logEventNum);
Expand Down Expand Up @@ -276,12 +282,41 @@ const StateContextProvider = ({children}: StateContextProviderProps) => {
});
break;
}
case WORKER_RESP_CODE.QUERY_RESULT:
setQueryResults((v) => {
args.results.forEach((resultsPerPage, queryPageNum) => {
if (false === v.has(queryPageNum)) {
v.set(queryPageNum, []);
}
v.get(queryPageNum)?.push(...resultsPerPage);
});

return v;
});
break;
Comment on lines +285 to +296
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider refactoring the QUERY_RESULT case for clarity and robustness

The implementation of the QUERY_RESULT case is functional, but there are opportunities for improvement:

  1. Use a more descriptive variable name instead of v for better readability.
  2. Consider simplifying the state update logic using Map methods.
  3. Add error handling for potential type mismatches or unexpected data structures.

Here's a suggested refactor:

case WORKER_RESP_CODE.QUERY_RESULT:
    setQueryResults((prevResults) => {
        const newResults = new Map(prevResults);
        args.results.forEach((resultsPerPage, queryPageNum) => {
            if (typeof queryPageNum !== 'number' || !Array.isArray(resultsPerPage)) {
                console.error(`Invalid data structure for page ${queryPageNum}`);
                return;
            }
            const existingResults = newResults.get(queryPageNum) || [];
            newResults.set(queryPageNum, [...existingResults, ...resultsPerPage]);
        });
        return newResults;
    });
    break;

This refactored version improves readability, simplifies the logic, and adds basic error checking.

default:
console.error(`Unexpected ev.data: ${JSON.stringify(ev.data)}`);
break;
}
}, []);

const startQuery = useCallback((
queryString: string,
isRegex: boolean,
isCaseSensitive: boolean
) => {
if (null === mainWorkerRef.current) {
junhaoliao marked this conversation as resolved.
Show resolved Hide resolved
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
console.error("Unexpected null mainWorkerRef.current");

return;
}
workerPostReq(mainWorkerRef.current, WORKER_REQ_CODE.START_QUERY, {
queryString: queryString,
isRegex: isRegex,
isCaseSensitive: isCaseSensitive,
});
}, []);

const exportLogs = useCallback(() => {
if (null === mainWorkerRef.current) {
console.error("Unexpected null mainWorkerRef.current");
Expand Down Expand Up @@ -437,11 +472,13 @@ const StateContextProvider = ({children}: StateContextProviderProps) => {
numPages: numPages,
onDiskFileSizeInBytes: onDiskFileSizeInBytes,
pageNum: pageNum,
queryResults: queryResults,

exportLogs: exportLogs,
loadFile: loadFile,
loadPageByAction: loadPageByAction,
setLogLevelFilter: setLogLevelFilter,
startQuery: startQuery,
}}
>
{children}
Expand Down
1 change: 1 addition & 0 deletions new-log-viewer/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ root.render(
<App/>
</StrictMode>
);
export {QUERY_CHUNK_SIZE} from "./utils/config";
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
129 changes: 115 additions & 14 deletions new-log-viewer/src/services/LogFileManager/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint max-lines: ["error", 400] */
import {
Decoder,
DecoderOptionsType,
Expand All @@ -11,11 +12,16 @@ import {
CursorType,
EMPTY_PAGE_RESP,
FileSrcType,
QueryResults,
WORKER_RESP_CODE,
WorkerResp,
} from "../../typings/worker";
import {EXPORT_LOGS_CHUNK_SIZE} from "../../utils/config";
import {
EXPORT_LOGS_CHUNK_SIZE,
QUERY_CHUNK_SIZE,
} from "../../utils/config";
import {getChunkNum} from "../../utils/math";
import {defer} from "../../utils/time";
import {formatSizeInBytes} from "../../utils/units";
import ClpIrDecoder from "../decoders/ClpIrDecoder";
import JsonlDecoder from "../decoders/JsonlDecoder";
Expand All @@ -31,35 +37,43 @@ import {
* Class to manage the retrieval and decoding of a given log file.
*/
class LogFileManager {
readonly #fileName: string;

readonly #numEvents: number = 0;

readonly #pageSize: number;

readonly #fileName: string;
#queryId: number = 0;
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved

readonly #onDiskFileSizeInBytes: number;

#decoder: Decoder;
readonly #onQueryResults: (queryResults: QueryResults) => void;

#numEvents: number = 0;
#decoder: Decoder;

/**
* Private constructor for LogFileManager. This is not intended to be invoked publicly.
* Instead, use LogFileManager.create() to create a new instance of the class.
*
* @param decoder
* @param fileName
* @param onDiskFileSizeInBytes
* @param pageSize Page size for setting up pagination.
* @param params
* @param params.decoder
* @param params.fileName
* @param params.onDiskFileSizeInBytes
* @param params.pageSize Page size for setting up pagination.
* @param params.onQueryResults
*/
constructor (
constructor ({decoder, fileName, onDiskFileSizeInBytes, pageSize, onQueryResults}: {
decoder: Decoder,
fileName: string,
onDiskFileSizeInBytes: number,
pageSize: number,
) {
onQueryResults: (queryResults: QueryResults) => void,
}) {
this.#decoder = decoder;
this.#fileName = fileName;
this.#onDiskFileSizeInBytes = onDiskFileSizeInBytes;
this.#pageSize = pageSize;
this.#decoder = decoder;
this.#onDiskFileSizeInBytes = onDiskFileSizeInBytes;
this.#onQueryResults = onQueryResults;

// Build index for the entire file.
const buildResult = decoder.build();
Expand Down Expand Up @@ -90,17 +104,26 @@ class LogFileManager {
* File object.
* @param pageSize Page size for setting up pagination.
* @param decoderOptions Initial decoder options.
* @param onQueryResults
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
* @return A Promise that resolves to the created LogFileManager instance.
*/
static async create (
fileSrc: FileSrcType,
pageSize: number,
decoderOptions: DecoderOptionsType
decoderOptions: DecoderOptionsType,
onQueryResults: (queryResults: QueryResults) => void,
): Promise<LogFileManager> {
const {fileName, fileData} = await loadFile(fileSrc);
const decoder = await LogFileManager.#initDecoder(fileName, fileData, decoderOptions);

return new LogFileManager(decoder, fileName, fileData.length, pageSize);
return new LogFileManager({
decoder: decoder,
fileName: fileName,
onDiskFileSizeInBytes: fileData.length,
pageSize: pageSize,

onQueryResults: onQueryResults,
});
}

/**
Expand Down Expand Up @@ -254,6 +277,84 @@ class LogFileManager {
};
}

/**
* Creates a RegExp object based on the given query string and options,
* and starts querying the first log chunk.
*
* @param queryString
* @param isRegex
* @param isCaseSensitive
*/
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
startQuery (queryString: string, isRegex: boolean, isCaseSensitive: boolean): void {
this.#queryId++;
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved

// If the query string is empty, or there are no logs, return
if ("" === queryString || 0 === this.#numEvents) {
return;
}

// Construct query RegExp
const regexPattern = isRegex ?
queryString :
queryString.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const regexFlags = isCaseSensitive ?
"" :
"i";
const queryRegex = new RegExp(regexPattern, regexFlags);

this.#queryChunkAndScheduleNext(this.#queryId, 0, queryRegex);
}

/**
* Queries a chunk of log events, sends the results,
* and schedules the next chunk query if more log events remain.
*
* @param queryId
* @param chunkBeginIdx
* @param queryRegex
*/
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
#queryChunkAndScheduleNext (
queryId: number,
chunkBeginIdx: number,
queryRegex: RegExp
): void {
if (queryId !== this.#queryId) {
// Current task no longer corresponds to the latest query in the LogFileManager.
return;
}
const chunkEndIdx = Math.min(chunkBeginIdx + QUERY_CHUNK_SIZE, this.#numEvents);
const results: QueryResults = new Map();
const decodedEvents = this.#decoder.decodeRange(
chunkBeginIdx,
chunkEndIdx,
null !== this.#decoder.getFilteredLogEventMap()
);

decodedEvents?.forEach(([message, , , logEventNum]) => {
const matchResult = message.match(queryRegex);
if (null !== matchResult && "number" === typeof matchResult.index) {
const pageNum = Math.ceil(logEventNum / this.#pageSize);
if (false === results.has(pageNum)) {
results.set(pageNum, []);
}
results.get(pageNum)?.push({
logEventNum: logEventNum,
message: message,
matchRange: [matchResult.index,
(matchResult.index + matchResult[0].length)],
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
});
}
});

this.#onQueryResults(results);

if (chunkEndIdx < this.#numEvents) {
defer(() => {
this.#queryChunkAndScheduleNext(queryId, chunkEndIdx, queryRegex);
});
}
}

/**
* Gets the data that corresponds to the cursor.
*
Expand Down
31 changes: 30 additions & 1 deletion new-log-viewer/src/services/MainWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import dayjsUtc from "dayjs/plugin/utc";
import {LOG_LEVEL} from "../typings/logs";
import {
MainWorkerReqMessage,
QueryResults,
WORKER_REQ_CODE,
WORKER_RESP_CODE,
WorkerResp,
Expand Down Expand Up @@ -36,6 +37,16 @@ const postResp = <T extends WORKER_RESP_CODE>(
postMessage({code, args});
};


/**
* Post a response for a chunk of query results.
*
* @param queryResults
*/
const onQueryResults = (queryResults: QueryResults) => {
postResp(WORKER_RESP_CODE.QUERY_RESULT, {results: queryResults});
};

// eslint-disable-next-line no-warning-comments
// TODO: Break this function up into smaller functions.
// eslint-disable-next-line max-lines-per-function,max-statements
Expand Down Expand Up @@ -63,7 +74,8 @@ onmessage = async (ev: MessageEvent<MainWorkerReqMessage>) => {
LOG_FILE_MANAGER = await LogFileManager.create(
args.fileSrc,
args.pageSize,
args.decoderOptions
args.decoderOptions,
onQueryResults
);

postResp(WORKER_RESP_CODE.LOG_FILE_INFO, {
Expand Down Expand Up @@ -97,6 +109,23 @@ onmessage = async (ev: MessageEvent<MainWorkerReqMessage>) => {
LOG_FILE_MANAGER.loadPage(args.cursor)
);
break;
case WORKER_REQ_CODE.START_QUERY:
if (null === LOG_FILE_MANAGER) {
throw new Error("Log file manager hasn't been initialized");
}
if (
"string" !== typeof args.queryString ||
"boolean" !== typeof args.isRegex ||
"boolean" !== typeof args.isCaseSensitive
) {
throw new Error("Invalid arguments for QUERY_LOG");
}
LOG_FILE_MANAGER.startQuery(
args.queryString,
args.isRegex,
args.isCaseSensitive
);
break;
default:
console.error(`Unexpected ev.data: ${JSON.stringify(ev.data)}`);
break;
Expand Down
Loading
Loading