From 2134d097e9f49f31ca525b207fee5486aaf2b3e4 Mon Sep 17 00:00:00 2001 From: Guangqing Tang <40620519+gt2345@users.noreply.github.com> Date: Wed, 13 Nov 2024 15:43:40 -0600 Subject: [PATCH] revert: log search UI and regex search (#10203) (cherry picked from commit 6feaddf4fb1b9d81d73a406d827d9133b579a275) --- docs/release-notes/log-search-improvement.rst | 9 - master/internal/api/filters.go | 2 - master/internal/api_tasks.go | 18 +- master/internal/api_tasks_intg_test.go | 17 - master/internal/api_trials.go | 1 - master/internal/db/postgres_filters.go | 2 - .../internal/db/postgres_tasks_intg_test.go | 25 - master/internal/elastic/elastic_task_logs.go | 10 +- .../pages/TrialDetails/LogViewer.module.scss | 42 -- .../src/pages/TrialDetails/LogViewer.tsx | 472 ------------------ .../TrialDetails/TrialDetailsLogs.module.scss | 41 -- .../pages/TrialDetails/TrialDetailsLogs.tsx | 363 +------------- webui/react/src/services/decoder.ts | 1 - webui/react/src/types.ts | 1 - 14 files changed, 26 insertions(+), 978 deletions(-) delete mode 100644 docs/release-notes/log-search-improvement.rst delete mode 100644 webui/react/src/pages/TrialDetails/LogViewer.module.scss delete mode 100644 webui/react/src/pages/TrialDetails/LogViewer.tsx diff --git a/docs/release-notes/log-search-improvement.rst b/docs/release-notes/log-search-improvement.rst deleted file mode 100644 index 5ce8d46fff1..00000000000 --- a/docs/release-notes/log-search-improvement.rst +++ /dev/null @@ -1,9 +0,0 @@ -:orphan: - -**Improvements** - -- Logs: In the WebUI, add a tab for specifically for displaying log search results. Clicking on any - search result will take users directly to the relevant position in the log, allowing them to - easily view logs both before and after the matched entry. Additionally, add support for - regex-based searches, providing more flexible log filtering. For more details, refer to - :ref:`WebUI `. diff --git a/master/internal/api/filters.go b/master/internal/api/filters.go index 782b0b76793..e829204ec5f 100644 --- a/master/internal/api/filters.go +++ b/master/internal/api/filters.go @@ -14,8 +14,6 @@ const ( FilterOperationLessThanEqual // FilterOperationStringContainment checks if the field contains a value as a substring. FilterOperationStringContainment - // FilterOperationRegexContainment checks if the field contains the regex. - FilterOperationRegexContainment ) // Filter is a general representation for a filter provided to an API. diff --git a/master/internal/api_tasks.go b/master/internal/api_tasks.go index 009d5bcc8c8..d8f83f7bfd5 100644 --- a/master/internal/api_tasks.go +++ b/master/internal/api_tasks.go @@ -729,19 +729,11 @@ func constructTaskLogsFilters(req *apiv1.TaskLogsRequest) ([]api.Filter, error) } if req.SearchText != "" { - if req.EnableRegex { - filters = append(filters, api.Filter{ - Field: "log", - Operation: api.FilterOperationRegexContainment, - Values: req.SearchText, - }) - } else { - filters = append(filters, api.Filter{ - Field: "log", - Operation: api.FilterOperationStringContainment, - Values: req.SearchText, - }) - } + filters = append(filters, api.Filter{ + Field: "log", + Operation: api.FilterOperationStringContainment, + Values: req.SearchText, + }) } return filters, nil } diff --git a/master/internal/api_tasks_intg_test.go b/master/internal/api_tasks_intg_test.go index 157d95edf4d..90f2718ddb9 100644 --- a/master/internal/api_tasks_intg_test.go +++ b/master/internal/api_tasks_intg_test.go @@ -103,23 +103,6 @@ func TestPostTaskLogs(t *testing.T) { require.Equal(t, e.Source, a.Source) require.Equal(t, e.Stdtype, a.Stdtype) } - - // Test log filtering by regex - stream = &mockStream[*apiv1.TaskLogsResponse]{ctx: ctx} - err = api.TaskLogs(&apiv1.TaskLogsRequest{ - TaskId: string(task.TaskID), - SearchText: "^lo.{4}xt", - }, stream) - require.NoError(t, err) - require.Empty(t, stream.getData()) - - err = api.TaskLogs(&apiv1.TaskLogsRequest{ - TaskId: string(task.TaskID), - SearchText: "^lo.{4}xt", - EnableRegex: true, - }, stream) - require.NoError(t, err) - require.Len(t, stream.getData(), 1) } func mockNotebookWithWorkspaceID( diff --git a/master/internal/api_trials.go b/master/internal/api_trials.go index ad1ca9ca10d..fb73c48951c 100644 --- a/master/internal/api_trials.go +++ b/master/internal/api_trials.go @@ -203,7 +203,6 @@ func (a *apiServer) TrialLogs( TimestampAfter: req.TimestampAfter, OrderBy: req.OrderBy, SearchText: req.SearchText, - EnableRegex: req.EnableRegex, }, res) err := processBatches(res, func(b api.Batch) error { return b.ForEach(func(i interface{}) error { diff --git a/master/internal/db/postgres_filters.go b/master/internal/db/postgres_filters.go index 7effcfe9a1e..3a6cdd2e060 100644 --- a/master/internal/db/postgres_filters.go +++ b/master/internal/db/postgres_filters.go @@ -75,8 +75,6 @@ func filterToSQL( return fmt.Sprintf("AND encode(%s::bytea, 'escape') ILIKE ('%%%%' || $%d || '%%%%')", field, paramID) - case api.FilterOperationRegexContainment: - return fmt.Sprintf("AND encode(%s::bytea, 'escape') ~ $%d", field, paramID) default: panic(fmt.Sprintf("cannot convert operation %d to SQL", f.Operation)) } diff --git a/master/internal/db/postgres_tasks_intg_test.go b/master/internal/db/postgres_tasks_intg_test.go index fb69cb5bb69..99691d5312f 100644 --- a/master/internal/db/postgres_tasks_intg_test.go +++ b/master/internal/db/postgres_tasks_intg_test.go @@ -733,31 +733,6 @@ func TestTaskLogsFlow(t *testing.T) { require.NoError(t, err) require.Len(t, logs, 2) - // Filter by search text. - logs, _, err = db.TaskLogs(t1In.TaskID, 5, []api.Filter{{ - Field: "log", - Operation: api.FilterOperationStringContainment, - Values: []string{"this"}, - }}, apiv1.OrderBy_ORDER_BY_UNSPECIFIED, nil) - require.NoError(t, err) - require.Len(t, logs, 2) - - logs, _, err = db.TaskLogs(t1In.TaskID, 5, []api.Filter{{ - Field: "log", - Operation: api.FilterOperationStringContainment, - Values: []string{"^th.s"}, - }}, apiv1.OrderBy_ORDER_BY_UNSPECIFIED, nil) - require.NoError(t, err) - require.Empty(t, logs) - - logs, _, err = db.TaskLogs(t1In.TaskID, 5, []api.Filter{{ - Field: "log", - Operation: api.FilterOperationRegexContainment, - Values: []string{"^th.s"}, - }}, apiv1.OrderBy_ORDER_BY_UNSPECIFIED, nil) - require.NoError(t, err) - require.Len(t, logs, 2) - // Test DeleteTaskLogs. err = db.DeleteTaskLogs([]model.TaskID{t2In.TaskID}) require.NoError(t, err) diff --git a/master/internal/elastic/elastic_task_logs.go b/master/internal/elastic/elastic_task_logs.go index 4f8956faed6..f62efc19515 100644 --- a/master/internal/elastic/elastic_task_logs.go +++ b/master/internal/elastic/elastic_task_logs.go @@ -463,15 +463,7 @@ func filtersToElastic(fs []api.Filter) []jsonObj { }, }, }) - case api.FilterOperationRegexContainment: - terms = append(terms, - jsonObj{ - "regexp": jsonObj{ - f.Field: jsonObj{ - "value": fmt.Sprintf("%s", f.Values), - }, - }, - }) + default: panic(fmt.Sprintf("unsupported filter operation: %d", f.Operation)) } diff --git a/webui/react/src/pages/TrialDetails/LogViewer.module.scss b/webui/react/src/pages/TrialDetails/LogViewer.module.scss deleted file mode 100644 index e7cf24c719c..00000000000 --- a/webui/react/src/pages/TrialDetails/LogViewer.module.scss +++ /dev/null @@ -1,42 +0,0 @@ -.options { - border-bottom: var(--theme-stroke-width) solid var(--theme-stage-border); - margin-bottom: var(--spacing-md); - padding-block: var(--spacing-md); -} -.base { - background-color: var(--theme-stage); - color: var(--theme-stage-on); - position: relative; - - .logs { - display: flex; - flex-direction: column; - font-family: var(--theme-font-family-code); - font-size: 12px; - height: 100%; - min-height: 150px; - overflow-y: auto; - padding: var(--spacing-md); - - & > div { - flex: 1; - } - } - .buttons { - bottom: var(--spacing-xl-3); - display: flex; - flex-direction: column; - gap: var(--spacing-lg); - position: absolute; - right: var(--spacing-xl-3); - - button { - background-color: var(--theme-float); - border-color: var(--theme-float-border); - box-shadow: var(--theme-elevation); - color: var(--theme-float-on); - transition: opacity 0.2s linear; - user-select: none; - } - } -} diff --git a/webui/react/src/pages/TrialDetails/LogViewer.tsx b/webui/react/src/pages/TrialDetails/LogViewer.tsx deleted file mode 100644 index 9df295b5279..00000000000 --- a/webui/react/src/pages/TrialDetails/LogViewer.tsx +++ /dev/null @@ -1,472 +0,0 @@ -import Button from 'hew/Button'; -import Icon from 'hew/Icon'; -import { clone, dateTimeStringSorter, formatDatetime, numericSorter } from 'hew/internal/functions'; -import { readLogStream } from 'hew/internal/services'; -import { FetchArgs, Log, LogLevel, RecordKey } from 'hew/internal/types'; -import LogViewerEntry, { DATETIME_FORMAT } from 'hew/LogViewer/LogViewerEntry'; -import Message from 'hew/Message'; -import Section from 'hew/Section'; -import Spinner from 'hew/Spinner'; -import { ErrorHandler } from 'hew/utils/error'; -import { ValueOf } from 'hew/utils/types'; -import React, { useCallback, useEffect, useId, useLayoutEffect, useRef, useState } from 'react'; -import { ListItem, Virtuoso, VirtuosoHandle } from 'react-virtuoso'; -import { throttle } from 'throttle-debounce'; - -import css from './LogViewer.module.scss'; - -export interface Props { - decoder: (data: T) => Log; - - height?: number | 'stretch'; - initialLogs?: T[]; - - onFetch?: (config: FetchConfig, type: FetchType) => FetchArgs; - onError: ErrorHandler; - serverAddress: (path: string) => string; - sortKey?: keyof Log; - selectedLog?: Log; - logs: ViewerLog[]; - setLogs: React.Dispatch>; - logsRef: React.RefObject; - local: React.MutableRefObject<{ - idSet: Set; - isScrollReady: boolean; - }>; - scrollToIndex: number; -} - -export interface ViewerLog extends Log { - formattedTime: string; -} - -export interface FetchConfig { - canceler: AbortController; - fetchDirection: FetchDirection; - limit: number; - offset?: number; - offsetLog?: Log; -} - -export const FetchType = { - Initial: 'Initial', - Newer: 'Newer', - Older: 'Older', - Stream: 'Stream', -} as const; - -export type FetchType = ValueOf; - -export const FetchDirection = { - Newer: 'Newer', - Older: 'Older', -} as const; - -export type FetchDirection = ValueOf; - -export const ARIA_LABEL_ENABLE_TAILING = 'Enable Tailing'; -export const ARIA_LABEL_SCROLL_TO_OLDEST = 'Scroll to Oldest'; - -export const PAGE_LIMIT = 100; -const THROTTLE_TIME = 50; - -export const formatLogEntry = (log: Log): ViewerLog => { - const formattedTime = log.time ? formatDatetime(log.time, { format: DATETIME_FORMAT }) : ''; - return { ...log, formattedTime }; -}; - -const logSorter = - (key: keyof Log) => - (a: Log, b: Log): number => { - const aValue = a[key]; - const bValue = b[key]; - if (key === 'id') return numericSorter(aValue as number, bValue as number); - if (key === 'time') return dateTimeStringSorter(aValue as string, bValue as string); - return 0; - }; - -// This is an arbitrarily large number used as an index. See https://virtuoso.dev/prepend-items/ -const START_INDEX = 2_000_000_000; - -function LogViewer({ - decoder, - height = 'stretch', - initialLogs, - onFetch, - onError, - serverAddress, - sortKey = 'time', - selectedLog, - logs, - setLogs, - logsRef, - local, - scrollToIndex, -}: Props): JSX.Element { - const componentId = useId(); - - const virtuosoRef = useRef(null); - const [isFetching, setIsFetching] = useState(false); - - const [canceler] = useState(new AbortController()); - const [fetchDirection, setFetchDirection] = useState(FetchDirection.Older); - const [isTailing, setIsTailing] = useState(true); - const [showButtons, setShowButtons] = useState(false); - - const [firstItemIndex, setFirstItemIndex] = useState(START_INDEX); - const [scrolledForSearch, setScrolledForSearch] = useState(true); - - const processLogs = useCallback( - (newLogs: Log[]) => { - return newLogs - .filter((log) => { - const isDuplicate = local.current.idSet.has(log.id); - const isTqdm = log.message.includes('\r'); - local.current.idSet.add(log.id); - return !isDuplicate && !isTqdm; - }) - .map((log) => formatLogEntry(log)) - .sort(logSorter(sortKey)); - }, - [sortKey, local], - ); - - const addLogs = useCallback( - (newLogs: ViewerLog[] = [], prepend = false): void => { - if (newLogs.length === 0) return; - setLogs((prevLogs) => (prepend ? newLogs.concat(prevLogs) : prevLogs.concat(newLogs))); - if (prepend) setFirstItemIndex((prev) => prev - newLogs.length); - }, - [setLogs], - ); - - useEffect(() => { - if (scrollToIndex < 0) return; - virtuosoRef.current?.scrollToIndex({ index: scrollToIndex }); - }, [scrollToIndex]); - - useEffect(() => { - setScrolledForSearch(false); - }, [selectedLog]); - - useEffect(() => { - if (scrolledForSearch || !selectedLog || !local.current.isScrollReady) return; - setTimeout(() => { - const index = logs.findIndex((l) => l.id === selectedLog.id); - if (index > -1) { - virtuosoRef.current?.scrollToIndex({ - behavior: 'smooth', - index: index, - }); - setScrolledForSearch(true); - } - }, 250); - }, [scrolledForSearch, selectedLog, logs, local]); - - const fetchLogs = useCallback( - async (config: Partial, type: FetchType): Promise => { - if (!onFetch) return []; - - const buffer: Log[] = []; - - setIsFetching(true); - - await readLogStream( - serverAddress, - onFetch({ limit: PAGE_LIMIT, ...config } as FetchConfig, type), - onError, - - (event: T) => { - const logEntry = decoder(event); - fetchDirection === FetchDirection.Older - ? buffer.unshift(logEntry) - : buffer.push(logEntry); - }, - ); - - setIsFetching(false); - - return processLogs(buffer); - }, - [decoder, fetchDirection, onFetch, onError, processLogs, serverAddress], - ); - - const handleFetchMoreLogs = useCallback( - async (positionReached: 'start' | 'end') => { - // Scroll may occur before the initial logs have rendered. - if (!local.current.isScrollReady) return; - - // Still busy with a previous fetch, prevent another fetch. - if (isFetching) return; - - // Detect when user scrolls to the "edge" and requires more logs to load. - const shouldFetchNewLogs = positionReached === 'end'; - const shouldFetchOldLogs = positionReached === 'start'; - if (shouldFetchNewLogs || shouldFetchOldLogs) { - const newLogs = await fetchLogs( - { - canceler, - fetchDirection: shouldFetchNewLogs ? FetchType.Newer : FetchType.Older, - offsetLog: shouldFetchNewLogs ? logs[logs.length - 1] : logs[0], - }, - shouldFetchNewLogs ? FetchType.Newer : FetchType.Older, - ); - - const prevLogs = clone(logs); - - addLogs(newLogs, shouldFetchOldLogs); - - if (newLogs.length > 0 && shouldFetchNewLogs) { - setTimeout(() => { - virtuosoRef.current?.scrollToIndex({ - align: 'end', - index: prevLogs.length - 1, - }); - }, 100); - } - return newLogs; - } - return; - }, - [addLogs, canceler, fetchLogs, isFetching, logs, local], - ); - - const handleScrollToOldest = useCallback(() => { - setIsTailing(false); - - if (fetchDirection === FetchDirection.Newer) { - virtuosoRef.current?.scrollToIndex({ index: firstItemIndex }); - } else { - local.current = { - idSet: new Set([]), - isScrollReady: false as boolean, - }; - - setLogs([]); - setFetchDirection(FetchDirection.Newer); - setFirstItemIndex(0); - } - }, [fetchDirection, firstItemIndex, setLogs, local]); - - const handleEnableTailing = useCallback(() => { - setIsTailing(true); - - if (fetchDirection === FetchDirection.Older) { - virtuosoRef.current?.scrollToIndex({ index: 'LAST' }); - } else { - local.current = { - idSet: new Set([]), - isScrollReady: false as boolean, - }; - - setLogs([]); - setFetchDirection(FetchDirection.Older); - setFirstItemIndex(START_INDEX); - } - }, [fetchDirection, setLogs, local]); - - // Re-fetch logs when fetch callback changes. - useEffect(() => { - local.current = { - idSet: new Set([]), - isScrollReady: false as boolean, - }; - - setLogs([]); - setIsTailing(true); - setFetchDirection(FetchDirection.Older); - setFirstItemIndex(START_INDEX); - }, [onFetch, setLogs, local]); - - // Add initial logs if applicable. - useEffect(() => { - if (!initialLogs) return; - addLogs(processLogs(initialLogs.map((log) => decoder(log)))); - }, [addLogs, decoder, initialLogs, processLogs]); - - // Initial fetch on mount or when fetch direction changes. - useEffect(() => { - fetchLogs({ canceler, fetchDirection }, FetchType.Initial).then((logs) => { - addLogs(logs, false); - - // Slight delay on scrolling to the end for the log viewer to render and resolve everything. - setTimeout(() => { - local.current.isScrollReady = true; - }, 200); - }); - }, [addLogs, canceler, fetchDirection, fetchLogs, local]); - - // Enable streaming for loading latest entries. - useEffect(() => { - const canceler = new AbortController(); - let buffer: Log[] = []; - - const processBuffer = () => { - const logs = processLogs(buffer); - buffer = []; - - addLogs(logs); - }; - const throttledProcessBuffer = throttle(THROTTLE_TIME, processBuffer); - - if (fetchDirection === FetchDirection.Older && onFetch) { - readLogStream( - serverAddress, - onFetch({ canceler, fetchDirection, limit: PAGE_LIMIT }, FetchType.Stream), - onError, - (event: T) => { - buffer.push(decoder(event)); - throttledProcessBuffer(); - }, - ); - } - - return () => { - canceler.abort(); - throttledProcessBuffer.cancel(); - }; - }, [addLogs, decoder, fetchDirection, onError, serverAddress, onFetch, processLogs]); - - // Abort all outstanding API calls if log viewer unmounts. - useEffect(() => { - return () => { - canceler.abort(); - }; - }, [canceler]); - - const handleItemsRendered = useCallback( - (renderedLogs: ListItem[]) => { - setShowButtons(renderedLogs.length < logs.length); - }, - [logs.length], - ); - - const handleReachedBottom = useCallback( - async (atBottom: boolean) => { - // May trigger before the initial logs have rendered. - if (isFetching || !local.current.isScrollReady) return; - - if (atBottom && !isTailing) { - const newLogs = await handleFetchMoreLogs('end'); - if (newLogs?.length === 0) setIsTailing(true); - } else if (!atBottom) setIsTailing(false); - }, - [handleFetchMoreLogs, isFetching, isTailing, local], - ); - - const handleReachedTop = useCallback( - (atTop: boolean) => { - if (atTop) handleFetchMoreLogs('start'); - }, - [handleFetchMoreLogs], - ); - - /* - * This overwrites the copy to clipboard event handler for the purpose of modifying the user - * selected content. By default when copying content from a collection of HTML elements, each - * element content will have a newline appended in the clipboard content. This handler will - * detect which lines within the copied content to be the timestamp content and strip out the - * newline from that field. - */ - useLayoutEffect(() => { - if (!logsRef.current) return; - - const target = logsRef.current; - const handleCopy = (e: ClipboardEvent): void => { - const clipboardFormat = 'text/plain'; - const levelValues = Object.values(LogLevel).join('|'); - const levelRegex = new RegExp(`<\\[(${levelValues})\\]>\n`, 'gim'); - const selection = (window.getSelection()?.toString() || '').replace(levelRegex, '<$1> '); - const lines = selection?.split('\n'); - - if (lines?.length <= 1) { - e.clipboardData?.setData(clipboardFormat, selection); - } else { - const oddOrEven = lines - .map((line) => /^\[/.test(line) || /\]$/.test(line)) - .reduce( - (acc, isTimestamp, index) => { - if (isTimestamp) acc[index % 2 === 0 ? 'even' : 'odd']++; - return acc; - }, - { even: 0, odd: 0 }, - ); - const isEven = oddOrEven.even > oddOrEven.odd; - const content = lines.reduce((acc, line, index) => { - const skipNewline = (isEven && index % 2 === 0) || (!isEven && index % 2 === 1); - return acc + line + (skipNewline ? ' ' : '\n'); - }, ''); - e.clipboardData?.setData(clipboardFormat, content); - } - e.preventDefault(); - }; - - target.addEventListener('copy', handleCopy); - - return (): void => target?.removeEventListener('copy', handleCopy); - }, [logsRef]); - - return ( -
- -
-
- {logs.length > 0 ? ( - ( - - )} - itemsRendered={handleItemsRendered} - key={(logs.length === 0 ? 'Loading' : fetchDirection) + componentId} - ref={virtuosoRef} - /> - ) : ( - - )} -
-
-
-
-
-
- ); -} - -export default LogViewer; diff --git a/webui/react/src/pages/TrialDetails/TrialDetailsLogs.module.scss b/webui/react/src/pages/TrialDetails/TrialDetailsLogs.module.scss index 2e944055136..802d7c03ee2 100644 --- a/webui/react/src/pages/TrialDetails/TrialDetailsLogs.module.scss +++ b/webui/react/src/pages/TrialDetails/TrialDetailsLogs.module.scss @@ -1,44 +1,3 @@ .base { height: 100%; - - .container { - display: flex; - } - .header { - border-bottom: var(--theme-stroke-width) solid var(--theme-stage-border); - display: flex; - flex-wrap: wrap; - justify-content: space-between; - margin-bottom: var(--spacing-md); - padding-block: var(--spacing-md); - padding-right: var(--spacing-md); - - .filters { - display: flex; - flex-wrap: wrap; - gap: var(--spacing-md); - } - } - .panes { - height: calc(100% - 64px); - } - .search { - .logContainer { - height: calc(100% - 22px); - overflow-y: auto; - - .log { - cursor: pointer; - font-family: var(--theme-font-family-code); - font-size: 12px; - - &:hover { - color: var(--theme-ix-on-active); - } - .key { - background-color: var(--theme-ix-active); - } - } - } - } } diff --git a/webui/react/src/pages/TrialDetails/TrialDetailsLogs.tsx b/webui/react/src/pages/TrialDetails/TrialDetailsLogs.tsx index fd25fe43405..f159fce255e 100644 --- a/webui/react/src/pages/TrialDetails/TrialDetailsLogs.tsx +++ b/webui/react/src/pages/TrialDetails/TrialDetailsLogs.tsx @@ -1,23 +1,10 @@ -import Button from 'hew/Button'; -import Checkbox from 'hew/Checkbox'; -import ClipboardButton from 'hew/ClipboardButton'; import CodeSample from 'hew/CodeSample'; -import Icon from 'hew/Icon'; -import Input from 'hew/Input'; -import { RecordKey } from 'hew/internal/types'; -import LogViewerEntry, { MAX_DATETIME_LENGTH } from 'hew/LogViewer/LogViewerEntry'; +import LogViewer, { FetchConfig, FetchDirection, FetchType } from 'hew/LogViewer/LogViewer'; import LogViewerSelect, { Filters } from 'hew/LogViewer/LogViewerSelect'; import { Settings, settingsConfigForTrial } from 'hew/LogViewer/LogViewerSelect.settings'; -import Message from 'hew/Message'; -import Row from 'hew/Row'; import Spinner from 'hew/Spinner'; -import SplitPane, { Pane } from 'hew/SplitPane'; import useConfirm from 'hew/useConfirm'; -import _ from 'lodash'; import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import screenfull from 'screenfull'; -import { sprintf } from 'sprintf-js'; -import { debounce } from 'throttle-debounce'; import useUI from 'components/ThemeProvider'; import useFeature from 'hooks/useFeature'; @@ -27,22 +14,11 @@ import { serverAddress } from 'routes/utils'; import { detApi } from 'services/apiConfig'; import { mapV1LogsResponse } from 'services/decoder'; import { readStream } from 'services/utils'; -import { ExperimentBase, Log, TrialDetails, TrialLog } from 'types'; +import { ExperimentBase, TrialDetails } from 'types'; import { downloadTrialLogs } from 'utils/browser'; -import { ansiToHtml } from 'utils/dom'; import handleError, { ErrorType } from 'utils/error'; import mergeAbortControllers from 'utils/mergeAbortControllers'; -import { dateTimeStringSorter } from 'utils/sort'; -import { pluralizer } from 'utils/string'; -import LogViewer, { - FetchConfig, - FetchDirection, - FetchType, - formatLogEntry, - PAGE_LIMIT, - ViewerLog, -} from './LogViewer'; import css from './TrialDetailsLogs.module.scss'; export interface Props { @@ -52,30 +28,13 @@ export interface Props { type OrderBy = 'ORDER_BY_UNSPECIFIED' | 'ORDER_BY_ASC' | 'ORDER_BY_DESC'; -const INITIAL_SEARCH_WIDTH = 420; - const TrialDetailsLogs: React.FC = ({ experiment, trial }: Props) => { const { ui } = useUI(); const [filterOptions, setFilterOptions] = useState({}); - const [logs, setLogs] = useState([]); - const [searchOn, setSearchOn] = useState(false); - const [logViewerOn, setLogViewerOn] = useState(true); - const [searchInput, setSearchInput] = useState(undefined); - const [searchResults, setSearchResults] = useState([]); - const [selectedLog, setSelectedLog] = useState(); - const [searchWidth, setSearchWidth] = useState(INITIAL_SEARCH_WIDTH); - const [scrollToIndex, setScrollToIndex] = useState(-1); const confirm = useConfirm(); const canceler = useRef(new AbortController()); - const container = useRef(null); - const logsRef = useRef(null); const f_flat_runs = useFeature().isOn('flat_runs'); - const local = useRef({ - idSet: new Set([]), - isScrollReady: false as boolean, - }); - const trialSettingsConfig = useMemo(() => settingsConfigForTrial(trial?.id || -1), [trial?.id]); const { resetSettings, settings, updateSettings } = useSettings(trialSettingsConfig); @@ -83,7 +42,6 @@ const TrialDetailsLogs: React.FC = ({ experiment, trial }: Props) => { () => ({ agentIds: settings.agentId, containerIds: settings.containerId, - enableRegex: settings.enableRegex, levels: settings.level, rankIds: settings.rankId, searchText: settings.searchText, @@ -91,34 +49,28 @@ const TrialDetailsLogs: React.FC = ({ experiment, trial }: Props) => { [settings], ); - useEffect(() => { - settings.searchText?.length && setSearchOn(true); - }, [settings.searchText]); - const handleFilterChange = useCallback( (filters: Filters) => { // request should have already been canceled when resetSettings updated // the settings hash if (Object.keys(filters).length === 0) return; + canceler.current.abort(); const newCanceler = new AbortController(); canceler.current = newCanceler; + updateSettings({ agentId: filters.agentIds, containerId: filters.containerIds, level: filters.levels, rankId: filters.rankIds, + searchText: filters.searchText, }); }, [updateSettings], ); - const handleFilterReset = useCallback(() => { - resetSettings(); - setSearchResults([]); - setSearchInput(''); - setSelectedLog(undefined); - }, [resetSettings]); + const handleFilterReset = useCallback(() => resetSettings(), [resetSettings]); const handleDownloadConfirm = useCallback(async () => { if (!trial?.id) return; @@ -159,7 +111,7 @@ const TrialDetailsLogs: React.FC = ({ experiment, trial }: Props) => { }, [confirm, experiment.id, f_flat_runs, handleDownloadConfirm, trial?.id]); const handleFetch = useCallback( - (config: FetchConfig, type: FetchType, searchText?: string, enableRegex?: boolean) => { + (config: FetchConfig, type: FetchType) => { const { signal } = mergeAbortControllers(config.canceler, canceler.current); const options = { @@ -199,12 +151,12 @@ const TrialDetailsLogs: React.FC = ({ experiment, trial }: Props) => { decode(optional(DateString), options.timestampBefore), decode(optional(DateString), options.timestampAfter), options.orderBy as OrderBy, - searchText, - enableRegex, + settings.searchText, + false, { signal }, ); }, - [settings.agentId, settings.containerId, settings.rankId, settings.level, trial?.id], + [settings, trial?.id], ); useEffect(() => { @@ -230,299 +182,24 @@ const TrialDetailsLogs: React.FC = ({ experiment, trial }: Props) => { const logFilters = ( ); - const debouncedChangeSearch = useMemo( - () => - debounce( - 500, - (s: string) => { - updateSettings({ searchText: s }); - }, - { atBegin: false }, - ), - [updateSettings], - ); - - useEffect(() => { - return () => { - debouncedChangeSearch.cancel(); - // kinda gross but we want this to run only on unmount - setSearchInput((s) => { - updateSettings({ searchText: s }); - return s; - }); - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [debouncedChangeSearch]); - - const onSearchChange = useCallback( - (e: React.ChangeEvent) => { - setSearchInput(e.target.value); - debouncedChangeSearch(e.target.value); - if (!e.target.value) { - setSearchResults([]); - setSelectedLog(undefined); - } - }, - [debouncedChangeSearch], - ); - - const formattedSearchResults = useMemo(() => { - const key = settings.searchText; - - if (!key) return []; - const formatted: ViewerLog[] = []; - _.uniqBy(searchResults, (l) => l.id).forEach((l) => { - const content = l.log; - if (!content) return; - if (settings.enableRegex) { - try { - new RegExp(key); - } catch { - return; - } - } - - const logEntry = formatLogEntry(l); - - const i = settings.enableRegex - ? content.match(`${key}`)?.index - : content.toLowerCase().indexOf(key.toLowerCase()); - if (_.isUndefined(i) || i < 0) return; - const keyLen = settings.enableRegex ? content.match(`${key}`)?.[0].length || 0 : key.length; - const j = i + keyLen; - formatted.push({ - ...logEntry, - message: `${ansiToHtml(content.slice(0, i))}${ansiToHtml(content.slice(i, j))}${ansiToHtml(content.slice(j))}`, - }); - }); - return formatted.sort((a, b) => dateTimeStringSorter(a.time as string, b.time as string)); - }, [searchResults, settings.searchText, settings.enableRegex]); - - useEffect(() => { - if (settings.searchText) { - setSearchResults([]); - setSelectedLog(undefined); - readStream( - handleFetch( - { - canceler: canceler.current, - fetchDirection: FetchDirection.Newer, - limit: 0, - }, - FetchType.Initial, - settings.searchText, - settings.enableRegex, - ), - (log) => setSearchResults((prev) => [...prev, mapV1LogsResponse(log)]), - ); - } - }, [settings.searchText, handleFetch, settings.enableRegex, canceler]); - - const processLogs = useCallback((newLogs: Log[]) => { - return newLogs - .filter((log) => { - const isDuplicate = local.current.idSet.has(log.id); - const isTqdm = log.message.includes('\r'); - local.current.idSet.add(log.id); - return !isDuplicate && !isTqdm; - }) - .map((log) => formatLogEntry(log)) - .sort((a, b) => dateTimeStringSorter(a['time'] as string, b['time'] as string)); - }, []); - - const onSelectLog = useCallback( - async (logEntry: ViewerLog) => { - setSelectedLog(logEntry); - setLogViewerOn(true); - const index = logs.findIndex((l) => l.id === logEntry.id); - if (index > -1) { - // Selected log is already fetched. Just need to scroll to the place. - return; - } - local.current = { - idSet: new Set([]), - isScrollReady: true, - }; - const bufferBefore: TrialLog[] = []; - const bufferAfter: TrialLog[] = []; - - await readStream( - handleFetch( - { - canceler: canceler.current, - fetchDirection: FetchDirection.Older, - limit: PAGE_LIMIT, - offsetLog: logEntry, - }, - FetchType.Older, - ), - (log) => bufferBefore.push(mapV1LogsResponse(log)), - ); - await readStream( - handleFetch( - { - canceler: canceler.current, - fetchDirection: FetchDirection.Newer, - limit: PAGE_LIMIT, - offsetLog: logEntry, - }, - FetchType.Newer, - ), - (log) => bufferAfter.push(mapV1LogsResponse(log)), - ); - setLogs(processLogs([...bufferBefore, ...bufferAfter])); - }, - [handleFetch, logs, processLogs], - ); - - const renderSearch = useCallback(() => { - const height = container.current?.getBoundingClientRect().height || 0; - return ( -
- updateSettings({ enableRegex: e.target.checked })}> - Regex - -
- {formattedSearchResults.length > 0 ? ( - formattedSearchResults.map((logEntry) => ( -
{ - onSelectLog(logEntry); - }}> - -
- )) - ) : ( - - )} -
-
- ); - }, [ - settings.enableRegex, - formattedSearchResults, - container, - selectedLog, - updateSettings, - onSelectLog, - ]); - - const formatClipboardHeader = (log: Log): string => { - const logEntry = formatLogEntry(log); - const format = `%${MAX_DATETIME_LENGTH - 1}s `; - const level = `<${logEntry.level || ''}>`; - return sprintf(`%-9s ${format}`, level, logEntry.formattedTime); - }; - - const clipboardCopiedMessage = useMemo(() => { - const linesLabel = pluralizer(logs.length, 'entry', 'entries'); - return `Copied ${logs.length} ${linesLabel}!`; - }, [logs]); - - const getClipboardContent = useCallback(() => { - return logs.map((log) => `${formatClipboardHeader(log)}${log.message || ''}`).join('\n'); - }, [logs]); - - const handleFullScreen = useCallback(() => { - if (logsRef.current && screenfull.isEnabled) screenfull.toggle(); - }, []); - - const onSwitchSearch = useCallback(() => { - setSearchOn((prev) => !prev); - // open log pane when closing the search pane - searchOn && setLogViewerOn(true); - // sometime the selected log of the log pane would offset when closing the search pane, since the width of the log pane changes - searchOn && selectedLog && setScrollToIndex(logs.findIndex((l) => l.id === selectedLog.id)); - }, [searchOn, selectedLog, logs]); - - const rightButtons = ( - - - - - {logFilters} - - {rightButtons} - -
- - } - onChange={(w) => setSearchWidth(w)} - /> -
+ ); diff --git a/webui/react/src/services/decoder.ts b/webui/react/src/services/decoder.ts index 70d7417c5de..39d3f694e88 100644 --- a/webui/react/src/services/decoder.ts +++ b/webui/react/src/services/decoder.ts @@ -829,7 +829,6 @@ export const mapV1LogsResponse =