From 5aef9ffb194e7eafe119143c4c1b36f99abab1be Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Wed, 9 Feb 2022 12:01:47 +0100 Subject: [PATCH] logs-volume: remove custom timeout (#45041) (#45085) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * logs-volume: remove custom timeout * make error message a little better (cherry picked from commit 2c0030b1b4020b6b0f1d669707c254e54e83d2a1) Co-authored-by: Gábor Farkas --- public/app/core/logs_model.ts | 74 ++++++++----------- .../features/explore/LogsVolumePanel.test.tsx | 18 ++++- .../app/features/explore/LogsVolumePanel.tsx | 39 ++++++++-- .../app/plugins/datasource/loki/datasource.ts | 7 -- 4 files changed, 78 insertions(+), 60 deletions(-) diff --git a/public/app/core/logs_model.ts b/public/app/core/logs_model.ts index e3e22b60cd3..d2f66114231 100644 --- a/public/app/core/logs_model.ts +++ b/public/app/core/logs_model.ts @@ -39,7 +39,7 @@ import { } from '@grafana/data'; import { getThemeColor } from 'app/core/utils/colors'; import { SIPrefix } from '@grafana/data/src/valueFormats/symbolFormatters'; -import { Observable, throwError, timeout } from 'rxjs'; +import { Observable } from 'rxjs'; export const LIMIT_LABEL = 'Line limit'; export const COMMON_LABELS = 'Common labels'; @@ -617,10 +617,7 @@ function aggregateFields(dataFrames: DataFrame[], config: FieldConfig): DataFram return aggregatedDataFrame; } -const LOGS_VOLUME_QUERY_DEFAULT_TIMEOUT = 60000; - type LogsVolumeQueryOptions = { - timeout?: number; extractLevel: (dataFrame: DataFrame) => LogLevel; targets: T[]; range: TimeRange; @@ -651,45 +648,36 @@ export function queryLogsVolume( data: [], }); - const subscription = (datasource.query(logsVolumeRequest) as Observable) - .pipe( - timeout({ - each: options.timeout || LOGS_VOLUME_QUERY_DEFAULT_TIMEOUT, - with: () => throwError(new Error('Request timed-out. Please make your query more specific and try again.')), - }) - ) - .subscribe({ - complete: () => { - const aggregatedLogsVolume = aggregateRawLogsVolume(rawLogsVolume, options.extractLevel); - if (aggregatedLogsVolume[0]) { - aggregatedLogsVolume[0].meta = { - custom: { - targets: options.targets, - absoluteRange: { from: options.range.from.valueOf(), to: options.range.to.valueOf() }, - }, - }; - } - observer.next({ - state: LoadingState.Done, - error: undefined, - data: aggregatedLogsVolume, - }); - observer.complete(); - }, - next: (dataQueryResponse: DataQueryResponse) => { - rawLogsVolume = rawLogsVolume.concat(dataQueryResponse.data.map(toDataFrame)); - }, - error: (error) => { - const errorMessage = error.data?.message || error.statusText || error.message; - console.error('Log volume query failed with error: ', errorMessage); - observer.next({ - state: LoadingState.Error, - error: error, - data: [], - }); - observer.error(error); - }, - }); + const subscription = (datasource.query(logsVolumeRequest) as Observable).subscribe({ + complete: () => { + const aggregatedLogsVolume = aggregateRawLogsVolume(rawLogsVolume, options.extractLevel); + if (aggregatedLogsVolume[0]) { + aggregatedLogsVolume[0].meta = { + custom: { + targets: options.targets, + absoluteRange: { from: options.range.from.valueOf(), to: options.range.to.valueOf() }, + }, + }; + } + observer.next({ + state: LoadingState.Done, + error: undefined, + data: aggregatedLogsVolume, + }); + observer.complete(); + }, + next: (dataQueryResponse: DataQueryResponse) => { + rawLogsVolume = rawLogsVolume.concat(dataQueryResponse.data.map(toDataFrame)); + }, + error: (error) => { + observer.next({ + state: LoadingState.Error, + error: error, + data: [], + }); + observer.error(error); + }, + }); return () => { subscription?.unsubscribe(); }; diff --git a/public/app/features/explore/LogsVolumePanel.test.tsx b/public/app/features/explore/LogsVolumePanel.test.tsx index de591e10095..251097f2221 100644 --- a/public/app/features/explore/LogsVolumePanel.test.tsx +++ b/public/app/features/explore/LogsVolumePanel.test.tsx @@ -40,11 +40,23 @@ describe('LogsVolumePanel', () => { expect(screen.getByText('ExploreGraph')).toBeInTheDocument(); }); - it('shows warning message without details', () => { + it('shows short warning message', () => { renderPanel({ state: LoadingState.Error, error: { data: { message: 'Test error message' } }, data: [] }); expect(screen.getByText('Failed to load log volume for this query')).toBeInTheDocument(); - expect(screen.getByText('Please check console logs for more details.')).toBeInTheDocument(); - expect(screen.queryByText('Test error message')).not.toBeInTheDocument(); + expect(screen.getByText('Test error message')).toBeInTheDocument(); + }); + + it('shows long warning message', () => { + // we make a long message + const messagePart = 'One two three four five six seven eight nine ten.'; + const message = messagePart + ' ' + messagePart + ' ' + messagePart; + + renderPanel({ state: LoadingState.Error, error: { data: { message } }, data: [] }); + expect(screen.getByText('Failed to load log volume for this query')).toBeInTheDocument(); + expect(screen.queryByText(message)).not.toBeInTheDocument(); + const button = screen.getByText('Show details'); + button.click(); + expect(screen.getByText(message)).toBeInTheDocument(); }); it('does not show the panel when there is no volume data', () => { diff --git a/public/app/features/explore/LogsVolumePanel.tsx b/public/app/features/explore/LogsVolumePanel.tsx index ca19e023d72..806e0c7dbca 100644 --- a/public/app/features/explore/LogsVolumePanel.tsx +++ b/public/app/features/explore/LogsVolumePanel.tsx @@ -1,7 +1,7 @@ -import { AbsoluteTimeRange, DataQueryResponse, LoadingState, SplitOpen, TimeZone } from '@grafana/data'; +import { AbsoluteTimeRange, DataQueryError, DataQueryResponse, LoadingState, SplitOpen, TimeZone } from '@grafana/data'; import { Alert, Button, Collapse, InlineField, TooltipDisplayMode, useStyles2, useTheme2 } from '@grafana/ui'; import { ExploreGraph } from './ExploreGraph'; -import React from 'react'; +import React, { useState } from 'react'; import { css } from '@emotion/css'; type Props = { @@ -14,6 +14,35 @@ type Props = { onLoadLogsVolume: () => void; }; +const SHORT_ERROR_MESSAGE_LIMIT = 100; + +function ErrorAlert(props: { error: DataQueryError }) { + const [isOpen, setIsOpen] = useState(false); + // generic get-error-message-logic, taken from + // /public/app/features/explore/ErrorContainer.tsx + const message = props.error.message || props.error.data?.message || ''; + + const showButton = !isOpen && message.length > SHORT_ERROR_MESSAGE_LIMIT; + + return ( + + {showButton ? ( + + ) : ( + message + )} + + ); +} + export function LogsVolumePanel(props: Props) { const { width, logsVolumeData, absoluteRange, timeZone, splitOpen, onUpdateTimeRange, onLoadLogsVolume } = props; const theme = useTheme2(); @@ -26,11 +55,7 @@ export function LogsVolumePanel(props: Props) { if (!logsVolumeData) { return null; } else if (logsVolumeData?.error) { - return ( - - Please check console logs for more details. - - ); + return ; } else if (logsVolumeData?.state === LoadingState.Loading) { LogsVolumePanelContent = Log volume is loading...; } else if (logsVolumeData?.data) { diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index cc9aebcfeda..eebcb3c9ec1 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -70,12 +70,6 @@ export const DEFAULT_MAX_LINES = 1000; export const LOKI_ENDPOINT = '/loki/api/v1'; const NS_IN_MS = 1000000; -/** - * Loki's logs volume query may be expensive as it requires counting all logs in the selected range. If such query - * takes too much time it may need be made more specific to limit number of logs processed under the hood. - */ -const LOGS_VOLUME_TIMEOUT = 10000; - const RANGE_QUERY_ENDPOINT = `${LOKI_ENDPOINT}/query_range`; const INSTANT_QUERY_ENDPOINT = `${LOKI_ENDPOINT}/query`; @@ -150,7 +144,6 @@ export class LokiDatasource }); return queryLogsVolume(this, logsVolumeRequest, { - timeout: LOGS_VOLUME_TIMEOUT, extractLevel, range: request.range, targets: request.targets,