From b731540939a3f8d8a2411cb7a6b6b1239c882eaf Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Fri, 3 Mar 2023 10:19:48 +0100 Subject: [PATCH] Log Components: Optimize style computing and re-renders in Row, Details, and Context (#63728) * Log Details: grab parsed styles from parent * Log Details Row: better memoization of style parsing * Log Context: less re-renders * Log Row Styles: memoize * Log Row: use class method instead of anonymous function --- .../logs/components/LogDetails.test.tsx | 6 +- .../features/logs/components/LogDetails.tsx | 39 ++++--------- .../logs/components/LogDetailsRow.tsx | 54 ++++++------------ .../app/features/logs/components/LogRow.tsx | 22 +++++--- .../logs/components/LogRowContext.tsx | 5 +- .../logs/components/LogRowContextProvider.tsx | 56 ++++++++++++------- .../logs/components/getLogRowStyles.ts | 20 ++++++- 7 files changed, 102 insertions(+), 100 deletions(-) diff --git a/public/app/features/logs/components/LogDetails.test.tsx b/public/app/features/logs/components/LogDetails.test.tsx index 1d7e5806299..4ac7009a90d 100644 --- a/public/app/features/logs/components/LogDetails.test.tsx +++ b/public/app/features/logs/components/LogDetails.test.tsx @@ -5,8 +5,11 @@ import { Field, LogLevel, LogRowModel, MutableDataFrame, createTheme } from '@gr import { LogDetails, Props } from './LogDetails'; import { createLogRow } from './__mocks__/logRow'; +import { getLogRowStyles } from './getLogRowStyles'; const setup = (propOverrides?: Partial, rowOverrides?: Partial) => { + const theme = createTheme(); + const styles = getLogRowStyles(theme); const props: Props = { displayedFields: [], showDuplicates: false, @@ -17,7 +20,8 @@ const setup = (propOverrides?: Partial, rowOverrides?: Partial {}, onClickShowField: () => {}, onClickHideField: () => {}, - theme: createTheme(), + theme, + styles, ...(propOverrides || {}), }; diff --git a/public/app/features/logs/components/LogDetails.tsx b/public/app/features/logs/components/LogDetails.tsx index a4d43df0ac6..73f4a075500 100644 --- a/public/app/features/logs/components/LogDetails.tsx +++ b/public/app/features/logs/components/LogDetails.tsx @@ -1,13 +1,13 @@ -import { css, cx } from '@emotion/css'; +import { cx } from '@emotion/css'; import React, { PureComponent } from 'react'; -import { CoreApp, DataFrame, Field, GrafanaTheme2, LinkModel, LogRowModel } from '@grafana/data'; +import { CoreApp, DataFrame, Field, LinkModel, LogRowModel } from '@grafana/data'; import { Themeable2, withTheme2 } from '@grafana/ui'; import { calculateLogsLabelStats, calculateStats } from '../utils'; import { LogDetailsRow } from './LogDetailsRow'; -import { getLogLevelStyles, getLogRowStyles } from './getLogRowStyles'; +import { getLogLevelStyles, LogRowStyles } from './getLogRowStyles'; import { getAllFields } from './logParser'; export interface Props extends Themeable2 { @@ -18,6 +18,7 @@ export interface Props extends Themeable2 { className?: string; hasError?: boolean; app?: CoreApp; + styles: LogRowStyles; onClickFilterLabel?: (key: string, value: string) => void; onClickFilterOutLabel?: (key: string, value: string) => void; @@ -27,25 +28,6 @@ export interface Props extends Themeable2 { onClickHideField?: (key: string) => void; } -const getStyles = (theme: GrafanaTheme2) => { - return { - logsRowLevelDetails: css` - label: logs-row__level_details; - &::after { - top: -3px; - } - `, - logDetails: css` - label: logDetailsDefaultCursor; - cursor: default; - - &:hover { - background-color: ${theme.colors.background.primary}; - } - `, - }; -}; - class UnThemedLogDetails extends PureComponent { render() { const { @@ -63,10 +45,9 @@ class UnThemedLogDetails extends PureComponent { displayedFields, getFieldLinks, wrapLogMessage, + styles, } = this.props; - const rowStyles = getLogRowStyles(theme); const levelStyles = getLogLevelStyles(theme, row.logLevel); - const styles = getStyles(theme); const labels = row.labels ? row.labels : {}; const labelsAvailable = Object.keys(labels).length > 0; const fieldsAndLinks = getAllFields(row, getFieldLinks); @@ -78,19 +59,19 @@ class UnThemedLogDetails extends PureComponent { // If logs with error, we are not showing the level color const levelClassName = hasError ? '' - : `${levelStyles.logsRowLevelColor} ${rowStyles.logsRowLevel} ${styles.logsRowLevelDetails}`; + : `${levelStyles.logsRowLevelColor} ${styles.logsRowLevel} ${styles.logsRowLevelDetails}`; return ( {showDuplicates && } -
- +
+
{(labelsAvailable || fieldsAvailable) && ( - @@ -139,7 +120,7 @@ class UnThemedLogDetails extends PureComponent { {linksAvailable && ( - diff --git a/public/app/features/logs/components/LogDetailsRow.tsx b/public/app/features/logs/components/LogDetailsRow.tsx index 20af14fc806..924f9ca049d 100644 --- a/public/app/features/logs/components/LogDetailsRow.tsx +++ b/public/app/features/logs/components/LogDetailsRow.tsx @@ -34,41 +34,7 @@ interface State { fieldStats: LogLabelStatsModel[] | null; } -const getStyles = memoizeOne((theme: GrafanaTheme2, activeButton: boolean) => { - // those styles come from ToolbarButton. Unfortunately this is needed because we can not control the variant of the menu-button in a ToolbarButtonRow. - const defaultOld = css` - color: ${theme.colors.text.secondary}; - background-color: ${theme.colors.background.primary}; - - &:hover { - color: ${theme.colors.text.primary}; - background: ${theme.colors.background.secondary}; - } - `; - - const defaultTopNav = css` - color: ${theme.colors.text.secondary}; - background-color: transparent; - border-color: transparent; - - &:hover { - color: ${theme.colors.text.primary}; - background: ${theme.colors.background.secondary}; - } - `; - - const active = css` - color: ${theme.v1.palette.orangeDark}; - border-color: ${theme.v1.palette.orangeDark}; - background-color: transparent; - - &:hover { - color: ${theme.colors.text.primary}; - background: ${theme.colors.emphasize(theme.colors.background.canvas, 0.03)}; - } - `; - - const defaultToolbarButtonStyle = theme.flags.topnav ? defaultTopNav : defaultOld; +const getStyles = memoizeOne((theme: GrafanaTheme2) => { return { noHoverBackground: css` label: noHoverBackground; @@ -130,8 +96,17 @@ const getStyles = memoizeOne((theme: GrafanaTheme2, activeButton: boolean) => { } } } + `, + toolbarButtonRowActive: css` & div:last-child > button:not(.stats-button) { - ${activeButton ? active : defaultToolbarButtonStyle}; + color: ${theme.v1.palette.orangeDark}; + border-color: ${theme.v1.palette.orangeDark}; + background-color: transparent; + + &:hover { + color: ${theme.colors.text.primary}; + background: ${theme.colors.emphasize(theme.colors.background.canvas, 0.03)}; + } } `, logDetailsStats: css` @@ -268,7 +243,7 @@ class UnThemedLogDetailsRow extends PureComponent { } = this.props; const { showFieldsStats, fieldStats, fieldCount } = this.state; const activeButton = displayedFields?.includes(parsedKey) || showFieldsStats; - const styles = getStyles(theme, activeButton); + const styles = getStyles(theme); const style = getLogRowStyles(theme); const hasFilteringFunctionality = onClickFilterLabel && onClickFilterOutLabel; @@ -289,7 +264,10 @@ class UnThemedLogDetailsRow extends PureComponent { <> { - onLogRowHover && onLogRowHover(row); - }} - onMouseLeave={() => { - onLogRowHover && onLogRowHover(undefined); - }} + onMouseEnter={this.onMouseEnter} + onMouseLeave={this.onMouseLeave} > {showDuplicates && (
+ Fields
+ Links
- + {hasFilteringFunctionality && ( { }); } + onMouseEnter = () => { + if (this.props.onLogRowHover) { + this.props.onLogRowHover(this.props.row); + } + }; + + onMouseLeave = () => { + if (this.props.onLogRowHover) { + this.props.onLogRowHover(undefined); + } + }; + renderLogRow( context?: LogRowContextRows, errors?: LogRowContextQueryErrors, @@ -148,7 +160,6 @@ class UnThemedLogRow extends PureComponent { theme, getFieldLinks, forceEscape, - onLogRowHover, app, scrollElement, styles, @@ -171,12 +182,8 @@ class UnThemedLogRow extends PureComponent {
@@ -246,6 +253,7 @@ class UnThemedLogRow extends PureComponent { hasError={hasError} displayedFields={displayedFields} app={app} + styles={styles} /> )} diff --git a/public/app/features/logs/components/LogRowContext.tsx b/public/app/features/logs/components/LogRowContext.tsx index 9c3e76283fa..ad562a56c2b 100644 --- a/public/app/features/logs/components/LogRowContext.tsx +++ b/public/app/features/logs/components/LogRowContext.tsx @@ -1,5 +1,5 @@ import { css, cx } from '@emotion/css'; -import React, { useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; import usePrevious from 'react-use/lib/usePrevious'; import { @@ -376,9 +376,10 @@ export const LogRowContext: React.FunctionComponent = ({ const { afterContext, beforeContext, title, top, actions, width } = useStyles2((theme) => getLogRowContextStyles(theme, wrapLogMessage) ); + const handleOutsideClick = useCallback(() => onOutsideClick('close_outside_click'), [onOutsideClick]); return ( - onOutsideClick('close_outside_click')}> + {/* e.stopPropagation is necessary so the log details doesn't open when clicked on log line in context * and/or when context log line is being highlighted */} diff --git a/public/app/features/logs/components/LogRowContextProvider.tsx b/public/app/features/logs/components/LogRowContextProvider.tsx index e79d9476903..15cdb025201 100644 --- a/public/app/features/logs/components/LogRowContextProvider.tsx +++ b/public/app/features/logs/components/LogRowContextProvider.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import useAsync from 'react-use/lib/useAsync'; import { @@ -211,31 +211,45 @@ export const LogRowContextProvider: React.FunctionComponent { + setLimit(limit + 10); + + const { datasourceType, uid: logRowUid } = row; + reportInteraction('grafana_explore_logs_log_context_load_more_clicked', { + datasourceType, + logRowUid, + newLimit: limit + 10, + }); + }, [limit, row]); + + const runContextQuery = useCallback(async () => { + const results = await getRowContexts(getRowContext, row, limit, logsSortOrder); + results.doNotCheckForMore = true; + setResults(results); + }, [getRowContext, limit, logsSortOrder, row]); + + const resultData = useMemo( + () => ({ before: result ? result.data[0] : [], after: result ? result.data[1] : [], - }, - errors: { + }), + [result] + ); + + const errorsData = useMemo( + () => ({ before: result ? result.errors[0] : undefined, after: result ? result.errors[1] : undefined, - }, - hasMoreContextRows, - updateLimit: () => { - setLimit(limit + 10); + }), + [result] + ); - const { datasourceType, uid: logRowUid } = row; - reportInteraction('grafana_explore_logs_log_context_load_more_clicked', { - datasourceType, - logRowUid, - newLimit: limit + 10, - }); - }, - runContextQuery: async () => { - const results = await getRowContexts(getRowContext, row, limit, logsSortOrder); - results.doNotCheckForMore = true; - setResults(results); - }, + return children({ + result: resultData, + errors: errorsData, + hasMoreContextRows, + updateLimit, + runContextQuery, limit, logsSortOrder, }); diff --git a/public/app/features/logs/components/getLogRowStyles.ts b/public/app/features/logs/components/getLogRowStyles.ts index 526e19d8997..9f5686b29e3 100644 --- a/public/app/features/logs/components/getLogRowStyles.ts +++ b/public/app/features/logs/components/getLogRowStyles.ts @@ -1,4 +1,5 @@ import { css } from '@emotion/css'; +import memoizeOne from 'memoize-one'; import tinycolor from 'tinycolor2'; import { GrafanaTheme2, LogLevel } from '@grafana/data'; @@ -39,7 +40,7 @@ export const getLogLevelStyles = (theme: GrafanaTheme2, logLevel?: LogLevel) => }; }; -export const getLogRowStyles = (theme: GrafanaTheme2) => { +export const getLogRowStyles = memoizeOne((theme: GrafanaTheme2) => { const hoverBgColor = styleMixins.hoverColor(theme.colors.background.secondary, theme); const contextOutlineColor = tinycolor(theme.components.dashboard.background).setAlpha(0.7).toRgbString(); return { @@ -263,7 +264,22 @@ export const getLogRowStyles = (theme: GrafanaTheme2) => { padding: 0; user-select: text; `, + // Log details + logsRowLevelDetails: css` + label: logs-row__level_details; + &::after { + top: -3px; + } + `, + logDetails: css` + label: logDetailsDefaultCursor; + cursor: default; + + &:hover { + background-color: ${theme.colors.background.primary}; + } + `, }; -}; +}); export type LogRowStyles = ReturnType;