From d8f1b4250106381a4874ac8a2bcafa641e886d47 Mon Sep 17 00:00:00 2001 From: Brandon Farmer Date: Mon, 11 Feb 2019 15:48:22 -0800 Subject: [PATCH] Be less aggressive on windowInterval - backport logic from v2 line --- CHANGELOG.md | 1 + .../dashboards/components/ReactCodeMirror.tsx | 3 +-- ui/src/flux/helpers/templates.ts | 8 +++---- ui/src/shared/apis/query.ts | 15 ++++-------- .../components/time_series/TimeSeries.tsx | 15 +++--------- ui/src/shared/constants/index.ts | 2 +- ui/src/shared/parsing/flux/durations.ts | 2 +- ui/src/shared/utils/downloadTimeseriesCSV.ts | 13 ++-------- ui/src/tempVars/utils/replace.ts | 24 +++++-------------- ui/test/shared/parsing/flux/durations.ts | 2 +- ui/test/tempVars/utils/replace.test.ts | 22 +++++++---------- 11 files changed, 33 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 535a4cf24..553d337e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ 1. [#5068](https://github.com/influxdata/chronograf/pull/5068): Escape injected meta query values 1. [#5073](https://github.com/influxdata/chronograf/pull/5073): Fix out of range decimal places 1. [#5076](https://github.com/influxdata/chronograf/pull/5076): Stop raw yaxis format from getting updated to 10 +1. [#5077](https://github.com/influxdata/chronograf/pull/5077): Correct autoInterval calculations ## v1.7.7 [2018-01-16] diff --git a/ui/src/dashboards/components/ReactCodeMirror.tsx b/ui/src/dashboards/components/ReactCodeMirror.tsx index f54c6d1cb..cdd6188da 100644 --- a/ui/src/dashboards/components/ReactCodeMirror.tsx +++ b/ui/src/dashboards/components/ReactCodeMirror.tsx @@ -12,7 +12,6 @@ import replaceTemplates, {replaceInterval} from 'src/tempVars/utils/replace' import {duration} from 'src/shared/apis/query' import {applyMasks, insertTempVar, unMask} from 'src/tempVars/constants' -import {DEFAULT_X_PIXELS} from 'src/shared/constants' import {Template, QueryConfig} from 'src/types' import {EditorChange, EditorConfiguration, Position} from 'codemirror' @@ -334,7 +333,7 @@ class ReactCodeMirror extends PureComponent { const query = replaceTemplates(value, templates) const durationMs = await duration(query, config.source) - return replaceInterval(':interval:', DEFAULT_X_PIXELS, durationMs) + return replaceInterval(':interval:', durationMs) } private getTemplateTokens() { diff --git a/ui/src/flux/helpers/templates.ts b/ui/src/flux/helpers/templates.ts index 5ef7256b0..17226dc37 100644 --- a/ui/src/flux/helpers/templates.ts +++ b/ui/src/flux/helpers/templates.ts @@ -13,8 +13,7 @@ const INTERVAL_REGEX = /autoInterval/g export const renderTemplatesInScript = async ( script: string, timeRange: TimeRange, - astLink: string, - xPixels: number + astLink: string ): Promise => { let dashboardTime: string let upperDashboardTime: string @@ -27,7 +26,7 @@ export const renderTemplatesInScript = async ( upperDashboardTime = new Date().toISOString() } - let rendered = `${DASHBOARD_TIME} = ${dashboardTime}\n${UPPER_DASHBOARD_TIME} = ${upperDashboardTime}\n\n${script}` + let rendered = `\n${DASHBOARD_TIME} = ${dashboardTime}\n${UPPER_DASHBOARD_TIME} = ${upperDashboardTime}\n\n${script}` if (!script.match(INTERVAL_REGEX)) { return rendered @@ -38,10 +37,11 @@ export const renderTemplatesInScript = async ( try { duration = await getMinDuration(astLink, rendered) } catch (error) { + console.error(error) duration = DEFAULT_DURATION_MS } - const interval = computeInterval(duration, xPixels) + const interval = computeInterval(duration) rendered = `${INTERVAL} = ${interval}ms\n${rendered}` diff --git a/ui/src/shared/apis/query.ts b/ui/src/shared/apis/query.ts index 5e66e0818..7434ef262 100644 --- a/ui/src/shared/apis/query.ts +++ b/ui/src/shared/apis/query.ts @@ -25,7 +25,6 @@ export function executeQueries( source: Source, queries: Query[], templates: Template[], - resolution?: number, uuid?: string ): Promise { return new Promise(resolve => { @@ -34,7 +33,7 @@ export function executeQueries( let counter = queries.length for (let i = 0; i < queries.length; i++) { - executeQuery(source, queries[i], templates, resolution, uuid) + executeQuery(source, queries[i], templates, uuid) .then(result => (results[i] = {value: result, error: null})) .catch(result => (results[i] = {value: null, error: result})) .then(() => { @@ -52,10 +51,9 @@ export const executeQuery = async ( source: Source, query: Query, templates: Template[], - resolution: number, uuid?: string ): Promise => { - const text = await replace(query.text, source, templates, resolution) + const text = await replace(query.text, source, templates) const {data} = await proxy({ source: source.links.proxy, @@ -71,8 +69,7 @@ export const executeQuery = async ( const replace = async ( query: string, source: Source, - templates: Template[], - resolution: number + templates: Template[] ): Promise => { const templateReplacedQuery = replaceTemplates(query, templates) @@ -81,11 +78,7 @@ const replace = async ( } const durationMs = await duration(templateReplacedQuery, source) - const replacedQuery = replaceInterval( - templateReplacedQuery, - resolution, - durationMs - ) + const replacedQuery = replaceInterval(templateReplacedQuery, durationMs) return replacedQuery } diff --git a/ui/src/shared/components/time_series/TimeSeries.tsx b/ui/src/shared/components/time_series/TimeSeries.tsx index fad66414f..4814a1945 100644 --- a/ui/src/shared/components/time_series/TimeSeries.tsx +++ b/ui/src/shared/components/time_series/TimeSeries.tsx @@ -265,22 +265,14 @@ class TimeSeries extends PureComponent { } private executeTemplatedFluxQuery = async (latestUUID: string) => { - const { - queries, - onNotify, - source, - timeRange, - fluxASTLink, - xPixels, - } = this.props + const {queries, onNotify, source, timeRange, fluxASTLink} = this.props const script: string = _.get(queries, '0.text', '') const renderedScript = await renderTemplatesInScript( script, timeRange, - fluxASTLink, - xPixels + fluxASTLink ) const results = await this.executeFluxQuery( @@ -299,7 +291,7 @@ class TimeSeries extends PureComponent { private executeInfluxQLWithStatus = async ( latestUUID: string ): Promise => { - const {source, templates, editQueryStatus, queries, xPixels} = this.props + const {source, templates, editQueryStatus, queries} = this.props for (const query of queries) { editQueryStatus(query.id, {loading: true}) @@ -309,7 +301,6 @@ class TimeSeries extends PureComponent { source, queries, templates, - xPixels, latestUUID ) diff --git a/ui/src/shared/constants/index.ts b/ui/src/shared/constants/index.ts index 353304a4c..baa443feb 100644 --- a/ui/src/shared/constants/index.ts +++ b/ui/src/shared/constants/index.ts @@ -8,7 +8,7 @@ export const GIT_SHA = process.env.GIT_SHA export const DEFAULT_DURATION_MS = 1000 export const DEFAULT_X_PIXELS = 700 -export const PIXELS_PER_POINT = 1 +export const PIXELS_PER_POINT = 10 export const NO_CELL = 'none' diff --git a/ui/src/shared/parsing/flux/durations.ts b/ui/src/shared/parsing/flux/durations.ts index afe54c51b..980a853a0 100644 --- a/ui/src/shared/parsing/flux/durations.ts +++ b/ui/src/shared/parsing/flux/durations.ts @@ -197,7 +197,7 @@ function durationDuration(durationLiteral: DurationLiteral): number { function resolveDeclaration(ast: any, name: string): RangeCallPropertyValue { const isDeclarator = node => { return ( - get(node, 'type') === 'VariableDeclarator' && + get(node, 'type') === 'VariableAssignment' && get(node, 'id.name') === name ) } diff --git a/ui/src/shared/utils/downloadTimeseriesCSV.ts b/ui/src/shared/utils/downloadTimeseriesCSV.ts index 4429f91b1..29868432d 100644 --- a/ui/src/shared/utils/downloadTimeseriesCSV.ts +++ b/ui/src/shared/utils/downloadTimeseriesCSV.ts @@ -8,9 +8,6 @@ import {executeQuery as executeInfluxQLQuery} from 'src/shared/apis/query' import {executeQuery as executeFluxQuery} from 'src/shared/apis/flux/query' import {renderTemplatesInScript} from 'src/flux/helpers/templates' -// Constants -import {DEFAULT_X_PIXELS} from 'src/shared/constants' - // Types import {Query, Template, Source, TimeRange} from 'src/types' import {TimeSeriesResponse} from 'src/types/series' @@ -21,12 +18,7 @@ export const downloadInfluxQLCSV = async ( ): Promise => { const responses = await Promise.all( queries.map(query => - executeInfluxQLQuery( - query.queryConfig.source, - query, - templates, - DEFAULT_X_PIXELS - ) + executeInfluxQLQuery(query.queryConfig.source, query, templates) ) ) @@ -44,8 +36,7 @@ export const downloadFluxCSV = async ( const renderedScript = await renderTemplatesInScript( script, timeRange, - fluxASTLink, - DEFAULT_X_PIXELS + fluxASTLink ) const {csv, didTruncate, rowCount} = await executeFluxQuery( diff --git a/ui/src/tempVars/utils/replace.ts b/ui/src/tempVars/utils/replace.ts index e466440d3..f9232034d 100644 --- a/ui/src/tempVars/utils/replace.ts +++ b/ui/src/tempVars/utils/replace.ts @@ -6,31 +6,19 @@ import { TemplateValueType, TemplateValue, } from 'src/types/tempVars' -import {TEMP_VAR_INTERVAL, PIXELS_PER_POINT} from 'src/shared/constants' +import {TEMP_VAR_INTERVAL} from 'src/shared/constants' +const DESIRED_POINTS_PER_GRAPH = 360 -export const computeInterval = ( - durationMs: number, - xPixels: number -): number => { - const customPxPerPoint = +window.localStorage.PIXELS_PER_POINT - const pxPerPoint = !isNaN(customPxPerPoint) - ? customPxPerPoint - : PIXELS_PER_POINT - const msPerPoint = Math.floor(durationMs * pxPerPoint / xPixels) - - return msPerPoint +export const computeInterval = (durationMs: number): number => { + return Math.round(durationMs / DESIRED_POINTS_PER_GRAPH) } -export const replaceInterval = ( - query: string, - xPixels: number, - durationMs: number -) => { +export const replaceInterval = (query: string, durationMs: number) => { if (!query.includes(TEMP_VAR_INTERVAL)) { return query } - const interval = computeInterval(durationMs, xPixels) + const interval = computeInterval(durationMs) const renderedQuery = replaceAll(query, TEMP_VAR_INTERVAL, `${interval}ms`) return renderedQuery diff --git a/ui/test/shared/parsing/flux/durations.ts b/ui/test/shared/parsing/flux/durations.ts index cf0a0b1d0..b5f2e5418 100644 --- a/ui/test/shared/parsing/flux/durations.ts +++ b/ui/test/shared/parsing/flux/durations.ts @@ -161,7 +161,7 @@ const AST_2 = { }, declarations: [ { - type: 'VariableDeclarator', + type: 'VariableAssignment', id: { type: 'Identifier', location: { diff --git a/ui/test/tempVars/utils/replace.test.ts b/ui/test/tempVars/utils/replace.test.ts index abe4c8da9..40c510ceb 100644 --- a/ui/test/tempVars/utils/replace.test.ts +++ b/ui/test/tempVars/utils/replace.test.ts @@ -288,21 +288,19 @@ describe('templates.utils.replace', () => { describe('replaceInterval', () => { it('can replace :interval:', () => { const query = `SELECT mean(usage_idle) from "cpu" where time > now() - 4320h group by time(:interval:)` - const expected = `SELECT mean(usage_idle) from "cpu" where time > now() - 4320h group by time(46702702ms)` - const pixels = 333 + const expected = `SELECT mean(usage_idle) from "cpu" where time > now() - 4320h group by time(43200000ms)` const durationMs = 15551999999 - const actual = replaceInterval(query, pixels, durationMs) + const actual = replaceInterval(query, durationMs) expect(actual).toBe(expected) }) it('can replace multiple intervals', () => { const query = `SELECT NON_NEGATIVE_DERIVATIVE(mean(usage_idle), :interval:) from "cpu" where time > now() - 4320h group by time(:interval:)` - const expected = `SELECT NON_NEGATIVE_DERIVATIVE(mean(usage_idle), 46702702ms) from "cpu" where time > now() - 4320h group by time(46702702ms)` + const expected = `SELECT NON_NEGATIVE_DERIVATIVE(mean(usage_idle), 43200000ms) from "cpu" where time > now() - 4320h group by time(43200000ms)` - const pixels = 333 const durationMs = 15551999999 - const actual = replaceInterval(query, pixels, durationMs) + const actual = replaceInterval(query, durationMs) expect(actual).toBe(expected) }) @@ -333,12 +331,11 @@ describe('templates.utils.replace', () => { }, ] - const pixels = 333 const durationMs = 86399999 const query = `SELECT mean(usage_idle) from "cpu" WHERE time > :dashboardTime: group by time(:interval:)` let actual = templateReplace(query, vars) - actual = replaceInterval(actual, pixels, durationMs) - const expected = `SELECT mean(usage_idle) from "cpu" WHERE time > now() - 24h group by time(259459ms)` + actual = replaceInterval(actual, durationMs) + const expected = `SELECT mean(usage_idle) from "cpu" WHERE time > now() - 24h group by time(240000ms)` expect(actual).toBe(expected) }) @@ -368,12 +365,11 @@ describe('templates.utils.replace', () => { }, ] - const pixels = 38 const durationMs = 3599999 const query = `SELECT mean(usage_idle) from "cpu" WHERE time > :dashboardTime: group by time(:interval:)` let actual = templateReplace(query, vars) - actual = replaceInterval(actual, pixels, durationMs) - const expected = `SELECT mean(usage_idle) from "cpu" WHERE time > now() - 1h group by time(94736ms)` + actual = replaceInterval(actual, durationMs) + const expected = `SELECT mean(usage_idle) from "cpu" WHERE time > now() - 1h group by time(10000ms)` expect(actual).toBe(expected) }) @@ -382,7 +378,7 @@ describe('templates.utils.replace', () => { describe('with no :interval: present', () => { it('returns the query', () => { const expected = `SELECT mean(usage_idle) FROM "cpu" WHERE time > :dashboardTime: GROUP BY time(20ms)` - const actual = replaceInterval(expected, 10, 20000) + const actual = replaceInterval(expected, 20000) expect(actual).toBe(expected) })