From 57abf39f25d8d82843e9dd106540d48a9079ffaa Mon Sep 17 00:00:00 2001 From: David Date: Fri, 29 May 2020 15:39:13 +0200 Subject: [PATCH] Logs: Use result range instead of timepicker range for log histogram (#25027) * Logs: Use result range instead of timepicker range for log histogram If a logs datasource does not send histogram data for the requested time range, the logs model computes a timeseries based on the log row counts, bucketed by an automcatically calculated time interval. Even when this histogram time series did not span the whole requested time range it was still rendered in the graph across the whole range, leaving an empty area at its start. Users find this confusing and are lead to believe their log data is missing. This change fixes this by anchoring the start of the timeseries on the first log row's timestamp from the result, and adds this smaller range as `visibleRange` to the logs model and passes it through to the logs component that optionally takes it into account to not render the empty area. The interval (bucket size) is also adjusted to account for a potentially finer resolution on the shorter visible time interval. The bucketsize multiplier was also changed from 10 to 20 to account for the space between the chart's bars. * Aligned visible range with buckets * Extract bucket size calculation and add test --- packages/grafana-data/src/types/logs.ts | 2 + public/app/core/logs_model.test.ts | 38 +++++++++++- public/app/core/logs_model.ts | 61 ++++++++++++++++--- public/app/features/explore/Logs.tsx | 4 +- public/app/features/explore/LogsContainer.tsx | 4 ++ .../features/explore/utils/ResultProcessor.ts | 2 +- 6 files changed, 98 insertions(+), 13 deletions(-) diff --git a/packages/grafana-data/src/types/logs.ts b/packages/grafana-data/src/types/logs.ts index dd34a8ec4e9..381d0db1765 100644 --- a/packages/grafana-data/src/types/logs.ts +++ b/packages/grafana-data/src/types/logs.ts @@ -1,6 +1,7 @@ import { Labels } from './data'; import { GraphSeriesXY } from './graph'; import { DataFrame } from './dataFrame'; +import { AbsoluteTimeRange } from './time'; /** * Mapping of log level abbreviation to canonical log level. @@ -74,6 +75,7 @@ export interface LogsModel { meta?: LogsMetaItem[]; rows: LogRowModel[]; series?: GraphSeriesXY[]; + visibleRange?: AbsoluteTimeRange; } export interface LogSearchMatch { diff --git a/public/app/core/logs_model.test.ts b/public/app/core/logs_model.test.ts index b7ba513fad9..8f0840a7972 100644 --- a/public/app/core/logs_model.test.ts +++ b/public/app/core/logs_model.test.ts @@ -8,7 +8,7 @@ import { MutableDataFrame, toDataFrame, } from '@grafana/data'; -import { dataFrameToLogsModel, dedupLogRows } from './logs_model'; +import { dataFrameToLogsModel, dedupLogRows, getSeriesProperties } from './logs_model'; describe('dedupLogRows()', () => { test('should return rows as is when dedup is set to none', () => { @@ -595,3 +595,39 @@ describe('dataFrameToLogsModel', () => { ]); }); }); + +describe('getSeriesProperties()', () => { + it('sets a minimum bucket size', () => { + const result = getSeriesProperties([], 2, undefined, 3, 123); + expect(result.bucketSize).toBe(123); + }); + + it('does not adjust the bucketSize if there is no range', () => { + const result = getSeriesProperties([], 30, undefined, 70); + expect(result.bucketSize).toBe(2100); + }); + + it('does not adjust the bucketSize if the logs row times match the given range', () => { + const rows: LogRowModel[] = [ + { entry: 'foo', timeEpochMs: 10 }, + { entry: 'bar', timeEpochMs: 20 }, + ] as any; + const range = { from: 10, to: 20 }; + const result = getSeriesProperties(rows, 1, range, 2, 1); + expect(result.bucketSize).toBe(2); + expect(result.visibleRange).toMatchObject(range); + }); + + it('clamps the range and adjusts the bucketSize if the logs row times do not completely cover the given range', () => { + const rows: LogRowModel[] = [ + { entry: 'foo', timeEpochMs: 10 }, + { entry: 'bar', timeEpochMs: 20 }, + ] as any; + const range = { from: 0, to: 30 }; + const result = getSeriesProperties(rows, 3, range, 2, 1); + // Bucketsize 6 gets shortened to 4 because of new visible range is 20ms vs original range being 30ms + expect(result.bucketSize).toBe(4); + // From time is also aligned to bucketSize (divisible by 4) + expect(result.visibleRange).toMatchObject({ from: 8, to: 30 }); + }); +}); diff --git a/public/app/core/logs_model.ts b/public/app/core/logs_model.ts index 3204b4648ec..971bf779874 100644 --- a/public/app/core/logs_model.ts +++ b/public/app/core/logs_model.ts @@ -27,6 +27,7 @@ import { getDisplayProcessor, textUtil, dateTime, + AbsoluteTimeRange, } from '@grafana/data'; import { getThemeColor } from 'app/core/utils/colors'; @@ -90,18 +91,14 @@ export function filterLogLevels(logRows: LogRowModel[], hiddenLogLevels: Set 0) { + const sortedRows = logsModel.rows.sort(sortInAscendingOrder); + const { visibleRange, bucketSize } = getSeriesProperties(sortedRows, intervalMs, absoluteRange); + logsModel.visibleRange = visibleRange; + logsModel.series = makeSeriesForLogs(sortedRows, bucketSize, timeZone); + } else { + logsModel.series = []; + } } else { // We got metrics in the dataFrame so process those logsModel.series = getGraphSeriesModel( @@ -234,6 +238,43 @@ export function dataFrameToLogsModel( }; } +/** + * Returns a clamped time range and interval based on the visible logs and the given range. + * + * @param sortedRows Log rows from the query response + * @param intervalMs Dynamnic data interval based on available pixel width + * @param absoluteRange Requested time range + * @param pxPerBar Default: 20, buckets will be rendered as bars, assuming 10px per histogram bar plus some free space around it + */ +export function getSeriesProperties( + sortedRows: LogRowModel[], + intervalMs: number, + absoluteRange: AbsoluteTimeRange, + pxPerBar = 20, + minimumBucketSize = 1000 +) { + let visibleRange = absoluteRange; + let resolutionIntervalMs = intervalMs; + let bucketSize = Math.max(resolutionIntervalMs * pxPerBar, minimumBucketSize); + // Clamp time range to visible logs otherwise big parts of the graph might look empty + if (absoluteRange) { + const earliest = sortedRows[0].timeEpochMs; + const latest = absoluteRange.to; + const visibleRangeMs = latest - earliest; + if (visibleRangeMs > 0) { + // Adjust interval bucket size for potentially shorter visible range + const clampingFactor = visibleRangeMs / (absoluteRange.to - absoluteRange.from); + resolutionIntervalMs *= clampingFactor; + // Minimum bucketsize of 1s for nicer graphing + bucketSize = Math.max(Math.ceil(resolutionIntervalMs * pxPerBar), minimumBucketSize); + // makeSeriesForLogs() aligns dataspoints with time buckets, so we do the same here to not cut off data + const adjustedEarliest = Math.floor(earliest / bucketSize) * bucketSize; + visibleRange = { from: adjustedEarliest, to: latest }; + } + } + return { bucketSize, visibleRange }; +} + function separateLogsAndMetrics(dataFrames: DataFrame[]) { const metricSeries: DataFrame[] = []; const logSeries: DataFrame[] = []; diff --git a/public/app/features/explore/Logs.tsx b/public/app/features/explore/Logs.tsx index 90b125b8172..3c00fd16189 100644 --- a/public/app/features/explore/Logs.tsx +++ b/public/app/features/explore/Logs.tsx @@ -45,6 +45,7 @@ interface Props { logsMeta?: LogsMetaItem[]; logsSeries?: GraphSeriesXY[]; dedupedRows?: LogRowModel[]; + visibleRange?: AbsoluteTimeRange; width: number; highlighterExpressions?: string[]; @@ -144,6 +145,7 @@ export class Logs extends PureComponent { logRows, logsMeta, logsSeries, + visibleRange, highlighterExpressions, loading = false, onClickFilterLabel, @@ -190,7 +192,7 @@ export class Logs extends PureComponent { width={width} onHiddenSeriesChanged={this.onToggleLogLevel} loading={loading} - absoluteRange={absoluteRange} + absoluteRange={visibleRange || absoluteRange} isStacked={true} showPanel={false} showingGraph={true} diff --git a/public/app/features/explore/LogsContainer.tsx b/public/app/features/explore/LogsContainer.tsx index 37bc1b7c6cf..abcb42d631f 100644 --- a/public/app/features/explore/LogsContainer.tsx +++ b/public/app/features/explore/LogsContainer.tsx @@ -40,6 +40,7 @@ interface LogsContainerProps { logsMeta?: LogsMetaItem[]; logsSeries?: GraphSeriesXY[]; dedupedRows?: LogRowModel[]; + visibleRange?: AbsoluteTimeRange; onClickFilterLabel?: (key: string, value: string) => void; onClickFilterOutLabel?: (key: string, value: string) => void; @@ -107,6 +108,7 @@ export class LogsContainer extends PureComponent { onStopScanning, absoluteRange, timeZone, + visibleRange, scanning, range, width, @@ -150,6 +152,7 @@ export class LogsContainer extends PureComponent { onDedupStrategyChange={this.handleDedupStrategyChange} onToggleLogLevel={this.handleToggleLogLevel} absoluteRange={absoluteRange} + visibleRange={visibleRange} timeZone={timeZone} scanning={scanning} scanRange={range.raw} @@ -190,6 +193,7 @@ function mapStateToProps(state: StoreState, { exploreId }: { exploreId: string } logRows: logsResult && logsResult.rows, logsMeta: logsResult && logsResult.meta, logsSeries: logsResult && logsResult.series, + visibleRange: logsResult && logsResult.visibleRange, scanning, timeZone, dedupStrategy, diff --git a/public/app/features/explore/utils/ResultProcessor.ts b/public/app/features/explore/utils/ResultProcessor.ts index 091495bb4d2..20082f83674 100644 --- a/public/app/features/explore/utils/ResultProcessor.ts +++ b/public/app/features/explore/utils/ResultProcessor.ts @@ -117,7 +117,7 @@ export class ResultProcessor { return null; } - const newResults = dataFrameToLogsModel(this.dataFrames, this.intervalMs, this.timeZone); + const newResults = dataFrameToLogsModel(this.dataFrames, this.intervalMs, this.timeZone, this.state.absoluteRange); const sortOrder = refreshIntervalToSortOrder(this.state.refreshInterval); const sortedNewResults = sortLogsResult(newResults, sortOrder); const rows = sortedNewResults.rows;