From 3f39b4b601a67a8de48b2abc96c381cee8095e5a Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Wed, 21 Oct 2020 19:11:32 +0200 Subject: [PATCH] Loki: Visually distinguish error logs for LogQL2 (#28359) * Loki: Add errored logs and update UI * Update messaging * Add icon and tooltip for errored logs * Update name of variable for more semantic meaning * Add tests * Update test * Refactor, remove unnecessary state * Update packages/grafana-data/src/types/logs.ts * Update packages/grafana-ui/src/components/Logs/LogDetails.tsx Co-authored-by: Giordano Ricci Co-authored-by: Giordano Ricci --- packages/grafana-data/src/types/logs.ts | 2 + packages/grafana-data/src/utils/logs.test.ts | 13 ++++ packages/grafana-data/src/utils/logs.ts | 13 ++++ .../src/components/Logs/LogDetails.test.tsx | 6 ++ .../src/components/Logs/LogDetails.tsx | 6 +- .../grafana-ui/src/components/Logs/LogRow.tsx | 30 +++++--- .../src/components/Logs/getLogRowStyles.ts | 7 +- public/app/core/logs_model.test.ts | 75 +++++++++++++++++++ public/app/core/logs_model.ts | 11 +++ public/app/features/explore/Logs.tsx | 2 + public/app/features/explore/MetaInfoText.tsx | 4 + .../datasource/loki/result_transformer.ts | 4 + 12 files changed, 162 insertions(+), 11 deletions(-) diff --git a/packages/grafana-data/src/types/logs.ts b/packages/grafana-data/src/types/logs.ts index bf3a6fdb958..737cba83ce9 100644 --- a/packages/grafana-data/src/types/logs.ts +++ b/packages/grafana-data/src/types/logs.ts @@ -27,10 +27,12 @@ export enum LogLevel { unknown = 'unknown', } +// Used for meta information such as common labels or returned log rows in logs view in Explore export enum LogsMetaKind { Number, String, LabelsMap, + Error, } export enum LogsSortOrder { diff --git a/packages/grafana-data/src/utils/logs.test.ts b/packages/grafana-data/src/utils/logs.test.ts index b92877b6db1..b712f38c914 100644 --- a/packages/grafana-data/src/utils/logs.test.ts +++ b/packages/grafana-data/src/utils/logs.test.ts @@ -9,6 +9,7 @@ import { calculateStats, getLogLevelFromKey, sortLogsResult, + checkLogsError, } from './logs'; describe('getLoglevel()', () => { @@ -352,3 +353,15 @@ describe('sortLogsResult', () => { }); }); }); + +describe('checkLogsError()', () => { + const log = ({ + labels: { + __error__: 'Error Message', + foo: 'boo', + }, + } as any) as LogRowModel; + test('should return correct error if error is present', () => { + expect(checkLogsError(log)).toStrictEqual({ hasError: true, errorMessage: 'Error Message' }); + }); +}); diff --git a/packages/grafana-data/src/utils/logs.ts b/packages/grafana-data/src/utils/logs.ts index 2a9e48c62a0..10b9af98342 100644 --- a/packages/grafana-data/src/utils/logs.ts +++ b/packages/grafana-data/src/utils/logs.ts @@ -210,3 +210,16 @@ export const sortLogsResult = (logsResult: LogsModel | null, sortOrder: LogsSort export const sortLogRows = (logRows: LogRowModel[], sortOrder: LogsSortOrder) => sortOrder === LogsSortOrder.Ascending ? logRows.sort(sortInAscendingOrder) : logRows.sort(sortInDescendingOrder); + +// Currently supports only error condition in Loki logs +export const checkLogsError = (logRow: LogRowModel): { hasError: boolean; errorMessage?: string } => { + if (logRow.labels.__error__) { + return { + hasError: true, + errorMessage: logRow.labels.__error__, + }; + } + return { + hasError: false, + }; +}; diff --git a/packages/grafana-ui/src/components/Logs/LogDetails.test.tsx b/packages/grafana-ui/src/components/Logs/LogDetails.test.tsx index ebd5c394114..767654554a1 100644 --- a/packages/grafana-ui/src/components/Logs/LogDetails.test.tsx +++ b/packages/grafana-ui/src/components/Logs/LogDetails.test.tsx @@ -45,6 +45,12 @@ describe('LogDetails', () => { expect(wrapper.text().includes('key1label1key2label2')).toBe(true); }); }); + describe('when log row has error', () => { + it('should not render log level border', () => { + const wrapper = setup({ hasError: true }, undefined); + expect(wrapper.find({ 'aria-label': 'Log level' }).html()).not.toContain('logs-row__level'); + }); + }); describe('when row entry has parsable fields', () => { it('should render heading ', () => { const wrapper = setup(undefined, { entry: 'test=successful' }); diff --git a/packages/grafana-ui/src/components/Logs/LogDetails.tsx b/packages/grafana-ui/src/components/Logs/LogDetails.tsx index 71541ab0453..000bd7193f0 100644 --- a/packages/grafana-ui/src/components/Logs/LogDetails.tsx +++ b/packages/grafana-ui/src/components/Logs/LogDetails.tsx @@ -28,6 +28,7 @@ export interface Props extends Themeable { showDuplicates: boolean; getRows: () => LogRowModel[]; className?: string; + hasError?: boolean; onMouseEnter?: () => void; onMouseLeave?: () => void; onClickFilterLabel?: (key: string, value: string) => void; @@ -70,6 +71,7 @@ class UnThemedLogDetails extends PureComponent { const { row, theme, + hasError, onClickFilterOutLabel, onClickFilterLabel, getRows, @@ -88,6 +90,8 @@ class UnThemedLogDetails extends PureComponent { const labelsAvailable = Object.keys(labels).length > 0; const fields = getAllFields(row, getFieldLinks); const parsedFieldsAvailable = fields && fields.length > 0; + // If logs with error, we are not showing the level color + const levelClassName = cx(!hasError && [style.logsRowLevel, styles.logsRowLevelDetails]); return ( { onMouseLeave={onMouseLeave} > {showDuplicates && } - +
diff --git a/packages/grafana-ui/src/components/Logs/LogRow.tsx b/packages/grafana-ui/src/components/Logs/LogRow.tsx index 0f5ccdf8918..691793895c2 100644 --- a/packages/grafana-ui/src/components/Logs/LogRow.tsx +++ b/packages/grafana-ui/src/components/Logs/LogRow.tsx @@ -8,8 +8,10 @@ import { DataQueryResponse, GrafanaTheme, dateTimeFormat, + checkLogsError, } from '@grafana/data'; import { Icon } from '../Icon/Icon'; +import { Tooltip } from '../Tooltip/Tooltip'; import { cx, css } from 'emotion'; import { @@ -72,6 +74,10 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => { label: hoverBackground; background-color: ${bgColor}; `, + errorLogRow: css` + label: erroredLogRow; + color: ${theme.colors.textWeak}; + `, }; }); /** @@ -160,22 +166,27 @@ class UnThemedLogRow extends PureComponent { const { showDetails, showContext, hasHoverBackground } = this.state; const style = getLogRowStyles(theme, row.logLevel); const styles = getStyles(theme); - const hoverBackground = cx(style.logsRow, { [styles.hoverBackground]: hasHoverBackground }); + const { errorMessage, hasError } = checkLogsError(row); + const logRowBackground = cx(style.logsRow, { + [styles.hoverBackground]: hasHoverBackground, + [styles.errorLogRow]: hasError, + }); return ( <> - + {showDuplicates && ( )} - {!allowDetails && ( {this.state.showDetails && ( { onClickHideParsedField={onClickHideParsedField} getRows={getRows} row={row} + hasError={hasError} showParsedFields={showParsedFields} /> )} diff --git a/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts b/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts index 18e05ef7c02..a4a27427d7a 100644 --- a/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts +++ b/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts @@ -9,6 +9,7 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo let logColor = selectThemeVariant({ light: theme.palette.gray5, dark: theme.palette.gray2 }, theme.type); const borderColor = selectThemeVariant({ light: theme.palette.gray5, dark: theme.palette.gray2 }, theme.type); const bgColor = selectThemeVariant({ light: theme.palette.gray5, dark: theme.palette.dark4 }, theme.type); + const hoverBgColor = selectThemeVariant({ light: theme.palette.gray7, dark: theme.palette.dark2 }, theme.type); const context = css` label: context; visibility: hidden; @@ -92,7 +93,7 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo } &:hover { - background: ${theme.colors.bodyBg}; + background: ${hoverBgColor}; } `, logsRowDuplicates: css` @@ -116,6 +117,10 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo background-color: ${logColor}; } `, + logIconError: css` + color: ${theme.palette.red}; + margin-left: -5px; + `, logsRowToggleDetails: css` label: logs-row-toggle-details__level; position: relative; diff --git a/public/app/core/logs_model.test.ts b/public/app/core/logs_model.test.ts index 407c117b04d..7a6256b0f37 100644 --- a/public/app/core/logs_model.test.ts +++ b/public/app/core/logs_model.test.ts @@ -252,6 +252,81 @@ describe('dataFrameToLogsModel', () => { }); }); + it('given one series with error should return expected logs model', () => { + const series: DataFrame[] = [ + new MutableDataFrame({ + fields: [ + { + name: 'time', + type: FieldType.time, + values: ['2019-04-26T09:28:11.352440161Z', '2019-04-26T14:42:50.991981292Z'], + }, + { + name: 'message', + type: FieldType.string, + values: [ + 't=2019-04-26T11:05:28+0200 lvl=info msg="Initializing DatasourceCacheService" logger=server', + 't=2019-04-26T16:42:50+0200 lvl=eror msg="new token…t unhashed token=56d9fdc5c8b7400bd51b060eea8ca9d7', + ], + labels: { + filename: '/var/log/grafana/grafana.log', + job: 'grafana', + __error__: 'Failed while parsing', + }, + }, + { + name: 'id', + type: FieldType.string, + values: ['foo', 'bar'], + }, + ], + meta: { + limit: 1000, + custom: { + error: 'Error when parsing some of the logs', + }, + }, + }), + ]; + const logsModel = dataFrameToLogsModel(series, 1, 'utc'); + expect(logsModel.hasUniqueLabels).toBeFalsy(); + expect(logsModel.rows).toHaveLength(2); + expect(logsModel.rows).toMatchObject([ + { + entry: 't=2019-04-26T11:05:28+0200 lvl=info msg="Initializing DatasourceCacheService" logger=server', + labels: { filename: '/var/log/grafana/grafana.log', job: 'grafana', __error__: 'Failed while parsing' }, + logLevel: 'info', + uniqueLabels: {}, + uid: 'foo', + }, + { + entry: 't=2019-04-26T16:42:50+0200 lvl=eror msg="new token…t unhashed token=56d9fdc5c8b7400bd51b060eea8ca9d7', + labels: { filename: '/var/log/grafana/grafana.log', job: 'grafana', __error__: 'Failed while parsing' }, + logLevel: 'error', + uniqueLabels: {}, + uid: 'bar', + }, + ]); + + expect(logsModel.series).toHaveLength(2); + expect(logsModel.meta).toHaveLength(3); + expect(logsModel.meta![0]).toMatchObject({ + label: 'Common labels', + value: series[0].fields[1].labels, + kind: LogsMetaKind.LabelsMap, + }); + expect(logsModel.meta![1]).toMatchObject({ + label: 'Limit', + value: `1000 (2 returned)`, + kind: LogsMetaKind.String, + }); + expect(logsModel.meta![2]).toMatchObject({ + label: '', + value: 'Error when parsing some of the logs', + kind: LogsMetaKind.Error, + }); + }); + it('given one series without labels should return expected logs model', () => { const series: DataFrame[] = [ new MutableDataFrame({ diff --git a/public/app/core/logs_model.ts b/public/app/core/logs_model.ts index 2f16f4941f6..66d91e422db 100644 --- a/public/app/core/logs_model.ts +++ b/public/app/core/logs_model.ts @@ -419,11 +419,22 @@ export function logSeriesToLogsModel(logSeries: DataFrame[]): LogsModel | undefi // Hack to print loki stats in Explore. Should be using proper stats display via drawer in Explore (rework in 7.1) let totalBytes = 0; const queriesVisited: { [refId: string]: boolean } = {}; + // To add just 1 error message + let errorMetaAdded = false; for (const series of logSeries) { const totalBytesKey = series.meta?.custom?.lokiQueryStatKey; const { refId } = series; // Stats are per query, keeping track by refId + if (!errorMetaAdded && series.meta?.custom?.error) { + meta.push({ + label: '', + value: series.meta?.custom.error, + kind: LogsMetaKind.Error, + }); + errorMetaAdded = true; + } + if (refId && !queriesVisited[refId]) { if (totalBytesKey && series.meta?.stats) { const byteStat = series.meta.stats.find(stat => stat.displayName === totalBytesKey); diff --git a/public/app/features/explore/Logs.tsx b/public/app/features/explore/Logs.tsx index 9394e56783f..9396fe9a185 100644 --- a/public/app/features/explore/Logs.tsx +++ b/public/app/features/explore/Logs.tsx @@ -40,6 +40,8 @@ function renderMetaItem(value: any, kind: LogsMetaKind) { ); + } else if (kind === LogsMetaKind.Error) { + return {value}; } return value; } diff --git a/public/app/features/explore/MetaInfoText.tsx b/public/app/features/explore/MetaInfoText.tsx index 34c8dcb9e8d..3dad0ac9645 100644 --- a/public/app/features/explore/MetaInfoText.tsx +++ b/public/app/features/explore/MetaInfoText.tsx @@ -15,6 +15,10 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => ({ margin-right: ${theme.spacing.d}; display: flex; align-items: baseline; + + .logs-meta-item__error { + color: ${theme.palette.red}; + } `, metaLabel: css` margin-right: calc(${theme.spacing.d} / 2); diff --git a/public/app/plugins/datasource/loki/result_transformer.ts b/public/app/plugins/datasource/loki/result_transformer.ts index b16ea1e02c3..5ec9afdd71c 100644 --- a/public/app/plugins/datasource/loki/result_transformer.ts +++ b/public/app/plugins/datasource/loki/result_transformer.ts @@ -325,6 +325,10 @@ export function lokiStreamsToDataframes( const dataFrame = lokiStreamResultToDataFrame(stream, reverse); enhanceDataFrame(dataFrame, config); + if (meta.custom && dataFrame.fields.some(f => f.labels && Object.keys(f.labels).some(l => l === '__error__'))) { + meta.custom.error = 'Error when parsing some of the logs'; + } + return { ...dataFrame, refId: target.refId,
{row.duplicates && row.duplicates > 0 ? `${row.duplicates + 1}x` : null} + + {hasError && ( + + + + )} + @@ -207,7 +218,7 @@ class UnThemedLogRow extends PureComponent {