diff --git a/ui/cypress/e2e/dashboardsView.test.ts b/ui/cypress/e2e/dashboardsView.test.ts index 07a0483015..072466abfe 100644 --- a/ui/cypress/e2e/dashboardsView.test.ts +++ b/ui/cypress/e2e/dashboardsView.test.ts @@ -296,6 +296,117 @@ describe('Dashboard', () => { }) }) + /*\ + built to approximate an instance with docker metrics, + operating with the variables: + + depbuck: + from(bucket: v.buckets) + |> range(start: v.timeRangeStart, stop: v.timeRangeStop) + |> filter(fn: (r) => r["_measurement"] == "docker_container_cpu") + |> keep(columns: ["container_name"]) + |> rename(columns: {"container_name": "_value"}) + |> last() + |> group() + + buckets: + buckets() + |> filter(fn: (r) => r.name !~ /^_/) + |> rename(columns: {name: "_value"}) + |> keep(columns: ["_value"]) + + and a dashboard built of : + cell one: + from(bucket: v.buckets) + |> range(start: v.timeRangeStart, stop: v.timeRangeStop) + |> filter(fn: (r) => r["_measurement"] == "docker_container_cpu") + |> filter(fn: (r) => r["_field"] == "usage_percent") + + cell two: + from(bucket: v.buckets) + |> range(start: v.timeRangeStart, stop: v.timeRangeStop) + |> filter(fn: (r) => r["_measurement"] == "docker_container_cpu") + |> filter(fn: (r) => r["_field"] == "usage_percent") + |> filter(fn: (r) => r["container_name"] == v.depbuck) + + with only 4 api queries being sent to fulfill it all + + \*/ + it('can load dependent queries without much fuss', () => { + cy.get('@org').then(({id: orgID}: Organization) => { + cy.createDashboard(orgID).then(({body: dashboard}) => { + const now = Date.now() + cy.writeData([ + `test,container_name=cool dopeness=12 ${now - 1000}000000`, + `test,container_name=beans dopeness=18 ${now - 1200}000000`, + `test,container_name=cool dopeness=14 ${now - 1400}000000`, + `test,container_name=beans dopeness=10 ${now - 1600}000000`, + ]) + cy.createCSVVariable(orgID, 'static', ['beans', 'defbuck']) + cy.createQueryVariable( + orgID, + 'dependent', + `from(bucket: v.static) + |> range(start: v.timeRangeStart, stop: v.timeRangeStop) + |> filter(fn: (r) => r["_measurement"] == "test") + |> keep(columns: ["container_name"]) + |> rename(columns: {"container_name": "_value"}) + |> last() + |> group()` + ) + + cy.fixture('routes').then(({orgs}) => { + cy.visit(`${orgs}/${orgID}/dashboards/${dashboard.id}`) + }) + }) + }) + + cy.getByTestID('add-cell--button').click() + cy.getByTestID('switch-to-script-editor').should('be.visible') + cy.getByTestID('switch-to-script-editor').click() + cy.getByTestID('toolbar-tab').click() + + cy + .getByTestID('flux-editor') + .should('be.visible') + .click() + .focused().type(`from(bucket: v.static) +|> range(start: v.timeRangeStart, stop: v.timeRangeStop) +|> filter(fn: (r) => r["_measurement"] == "test") +|> filter(fn: (r) => r["_field"] == "dopeness") +|> filter(fn: (r) => r["container_name"] == v.dependent)`) + cy.getByTestID('save-cell--button').click() + + // the default bucket selection should have no results + cy.getByTestID('variable-dropdown') + .eq(0) + .should('contain', 'beans') + + // and cause the rest to exist in loading states + cy.getByTestID('variable-dropdown') + .eq(1) + .should('contain', 'Loading') + + cy.getByTestID('cell--view-empty') + + // But selecting a nonempty bucket should load some data + cy.getByTestID('variable-dropdown--button') + .eq(0) + .click() + cy.get(`#defbuck`).click() + + // default select the first result + cy.getByTestID('variable-dropdown') + .eq(1) + .should('contain', 'beans') + + // and also load the second result + cy.getByTestID('variable-dropdown--button') + .eq(1) + .click() + cy.get(`#cool`).click() + }) + it('can create a view through the API', () => { cy.get('@org').then(({id: orgID}: Organization) => { cy.createDashWithViewAndVar(orgID).then(() => { diff --git a/ui/cypress/support/commands.ts b/ui/cypress/support/commands.ts index 1d774638b3..cde6102006 100644 --- a/ui/cypress/support/commands.ts +++ b/ui/cypress/support/commands.ts @@ -173,13 +173,14 @@ export const createTask = ( export const createQueryVariable = ( orgID?: string, - name: string = 'Little Variable' + name: string = 'Little Variable', + query?: string ): Cypress.Chainable => { const argumentsObj = { type: 'query', values: { language: 'flux', - query: `filter(fn: (r) => r._field == "cpu")`, + query: query || `filter(fn: (r) => r._field == "cpu")`, }, } @@ -196,11 +197,12 @@ export const createQueryVariable = ( export const createCSVVariable = ( orgID?: string, - name: string = 'CSVVariable' + name: string = 'CSVVariable', + csv?: string[] ): Cypress.Chainable => { const argumentsObj = { type: 'constant', - values: ['c1', 'c2', 'c3', 'c4'], + values: csv || ['c1', 'c2', 'c3', 'c4'], } return cy.request({ diff --git a/ui/src/shared/components/RefreshingView.tsx b/ui/src/shared/components/RefreshingView.tsx index fe01082dfd..6d62fd3a42 100644 --- a/ui/src/shared/components/RefreshingView.tsx +++ b/ui/src/shared/components/RefreshingView.tsx @@ -11,11 +11,8 @@ import ViewSwitcher from 'src/shared/components/ViewSwitcher' // Utils import {GlobalAutoRefresher} from 'src/utils/AutoRefresher' import {getTimeRange} from 'src/dashboards/selectors' -import {getRangeVariable} from 'src/variables/utils/getTimeRangeVars' -import {getVariables, asAssignment} from 'src/variables/selectors' import {checkResultsLength} from 'src/shared/utils/vis' import {getActiveTimeRange} from 'src/timeMachine/selectors/index' -import {TIME_RANGE_START, TIME_RANGE_STOP} from 'src/variables/constants' // Types import { @@ -23,7 +20,6 @@ import { TimeZone, AppState, DashboardQuery, - VariableAssignment, QueryViewProperties, Theme, } from 'src/types' @@ -38,7 +34,6 @@ interface StateProps { timeRange: TimeRange ranges: TimeRange | null timeZone: TimeZone - variableAssignments: VariableAssignment[] } interface State { @@ -68,14 +63,7 @@ class RefreshingView extends PureComponent { } public render() { - const { - ranges, - properties, - manualRefresh, - timeZone, - variableAssignments, - theme, - } = this.props + const {ranges, properties, manualRefresh, timeZone, theme} = this.props const {submitToken} = this.state return ( @@ -83,7 +71,6 @@ class RefreshingView extends PureComponent { submitToken={submitToken} queries={this.queries} key={manualRefresh} - variables={variableAssignments} > {({ giraffeResult, @@ -152,18 +139,6 @@ class RefreshingView extends PureComponent { const mstp = (state: AppState, ownProps: OwnProps): StateProps => { const timeRange = getTimeRange(state) - - // NOTE: cannot use getAllVariables here because the TimeSeries - // component appends it automatically. That should be fixed - const vars = getVariables(state) - const variableAssignments = [ - ...vars, - getRangeVariable(TIME_RANGE_START, timeRange), - getRangeVariable(TIME_RANGE_STOP, timeRange), - ] - .map(v => asAssignment(v)) - .filter(v => !!v) - const ranges = getActiveTimeRange(timeRange, ownProps.properties.queries) const {timeZone, theme} = state.app.persisted @@ -171,7 +146,6 @@ const mstp = (state: AppState, ownProps: OwnProps): StateProps => { timeRange, ranges, timeZone, - variableAssignments, theme, } } diff --git a/ui/src/shared/components/TimeSeries.tsx b/ui/src/shared/components/TimeSeries.tsx index c2d5c19abb..caf1d7bd44 100644 --- a/ui/src/shared/components/TimeSeries.tsx +++ b/ui/src/shared/components/TimeSeries.tsx @@ -19,6 +19,10 @@ import { import {runStatusesQuery} from 'src/alerting/utils/statusEvents' // Utils +import {getTimeRange} from 'src/dashboards/selectors' +import {getVariables, asAssignment} from 'src/variables/selectors' +import {getRangeVariable} from 'src/variables/utils/getTimeRangeVars' +import {isInQuery} from 'src/variables/utils/hydrateVars' import {getWindowVars} from 'src/variables/utils/getWindowVars' import {buildVarsOption} from 'src/variables/utils/buildVarsOption' import 'intersection-observer' @@ -27,6 +31,7 @@ import {getOrgIDFromBuckets} from 'src/timeMachine/actions/queries' // Constants import {rateLimitReached, resultTooLarge} from 'src/shared/copy/notifications' +import {TIME_RANGE_START, TIME_RANGE_STOP} from 'src/variables/constants' // Actions import {notify as notifyAction} from 'src/shared/actions/notifications' @@ -39,6 +44,7 @@ import { Bucket, ResourceType, DashboardQuery, + Variable, VariableAssignment, AppState, CancelBox, @@ -57,6 +63,7 @@ interface QueriesState { interface StateProps { queryLink: string buckets: Bucket[] + variables: Variable[] } interface OwnProps { @@ -187,14 +194,23 @@ class TimeSeries extends Component { // Cancel any existing queries this.pendingResults.forEach(({cancel}) => cancel()) + const usedVars = variables.filter(v => v.arguments.type !== 'system') + const waitList = usedVars.filter(v => v.status !== RemoteDataState.Done) + // If a variable is loading, and a cell requires it, leave the cell to never resolve, + // keeping it in a loading state until the variable is resolved + if (usedVars.length && waitList.length) { + await new Promise(() => {}) + } + + const vars = variables.map(v => asAssignment(v)) // Issue new queries this.pendingResults = queries.map(({text}) => { const orgID = getOrgIDFromBuckets(text, buckets) || this.props.params.orgID - const windowVars = getWindowVars(text, variables) - const extern = buildVarsOption([...variables, ...windowVars]) + const windowVars = getWindowVars(text, vars) + const extern = buildVarsOption([...vars, ...windowVars]) return runQuery(orgID, text, extern) }) @@ -204,7 +220,7 @@ class TimeSeries extends Component { let statuses = [] as StatusRow[][] if (check) { - const extern = buildVarsOption(variables) + const extern = buildVarsOption(vars) this.pendingCheckStatuses = runStatusesQuery( this.props.params.orgID, check.id, @@ -291,12 +307,27 @@ class TimeSeries extends Component { } } -const mstp = (state: AppState): StateProps => { - const {links} = state +const mstp = (state: AppState, props: OwnProps): StateProps => { + const timeRange = getTimeRange(state) + + // NOTE: cannot use getAllVariables here because the TimeSeries + // component appends it automatically. That should be fixed + // NOTE: limit the variables returned to those that are used, + // as this prevents resending when other queries get sent + const queries = props.queries.map(q => q.text).filter(t => !!t.trim()) + const vars = getVariables(state).filter(v => + queries.some(t => isInQuery(t, v)) + ) + const variables = [ + ...vars, + getRangeVariable(TIME_RANGE_START, timeRange), + getRangeVariable(TIME_RANGE_STOP, timeRange), + ] return { - queryLink: links.query.self, + queryLink: state.links.query.self, buckets: getAll(state, ResourceType.Buckets), + variables, } } diff --git a/ui/src/variables/actions/thunks.ts b/ui/src/variables/actions/thunks.ts index 74384ef6a8..06d829471e 100644 --- a/ui/src/variables/actions/thunks.ts +++ b/ui/src/variables/actions/thunks.ts @@ -118,6 +118,7 @@ export const getVariables = () => async ( } } +// TODO: make this context aware export const hydrateVariables = (skipCache?: boolean) => async ( dispatch: Dispatch, getState: GetState @@ -125,37 +126,25 @@ export const hydrateVariables = (skipCache?: boolean) => async ( const state = getState() const org = getOrg(state) const vars = getVariablesFromState(state) - const vals = await hydrateVars(vars, getAllVariablesFromState(state), { + const hydration = hydrateVars(vars, getAllVariablesFromState(state), { orgID: org.id, url: state.links.query.self, skipCache, - }).promise - - const lookup = vals.reduce((prev, curr) => { - prev[curr.id] = curr - return prev - }, {}) - - const updated = vars.map(vari => { - if (!lookup.hasOwnProperty(vari.id)) { - return vari - } - - return lookup[vari.id] }) - - await dispatch( - setVariables(RemoteDataState.Done, { - result: updated.map(v => v.id), - entities: { - variables: updated.reduce((prev, curr) => { - prev[curr.id] = curr - - return prev - }, {}), - }, - }) - ) + hydration.on('status', (variable, status) => { + if (status === RemoteDataState.Loading) { + dispatch(setVariable(variable.id, status)) + return + } + if (status === RemoteDataState.Done) { + const _variable = normalize( + variable, + variableSchema + ) + dispatch(setVariable(variable.id, RemoteDataState.Done, _variable)) + } + }) + await hydration.promise } export const getVariable = (id: string) => async ( diff --git a/ui/src/variables/components/VariableDropdown.tsx b/ui/src/variables/components/VariableDropdown.tsx index 95bc54e1c4..7abcdbfb2f 100644 --- a/ui/src/variables/components/VariableDropdown.tsx +++ b/ui/src/variables/components/VariableDropdown.tsx @@ -16,11 +16,12 @@ import {selectValue} from 'src/variables/actions/thunks' import {getVariable, normalizeValues} from 'src/variables/selectors' // Types -import {AppState} from 'src/types' +import {AppState, RemoteDataState} from 'src/types' interface StateProps { values: string[] selectedValue: string + status: RemoteDataState } interface DispatchProps { @@ -56,7 +57,7 @@ class VariableDropdown extends PureComponent { testID="variable-dropdown--button" status={dropdownStatus} > - {selectedValue || 'No Values'} + {this.selectedText} )} menu={onCollapse => ( @@ -94,15 +95,31 @@ class VariableDropdown extends PureComponent { onSelect() } } + + private get selectedText() { + const {selectedValue, status} = this.props + if (status === RemoteDataState.Loading) { + return 'Loading' + } + + if (selectedValue) { + return selectedValue + } + + return 'No Values' + } } const mstp = (state: AppState, props: OwnProps): StateProps => { const {variableID} = props - const variable = getVariable(state, variableID) + const selected = + variable.selected && variable.selected.length ? variable.selected[0] : null + return { + status: variable.status, values: normalizeValues(variable), - selectedValue: variable.selected[0], + selectedValue: selected, } } diff --git a/ui/src/variables/selectors/index.tsx b/ui/src/variables/selectors/index.tsx index 8b8385afbf..500ba8089a 100644 --- a/ui/src/variables/selectors/index.tsx +++ b/ui/src/variables/selectors/index.tsx @@ -165,12 +165,16 @@ export const getVariable = (state: AppState, variableID: string): Variable => { // Now validate that the selected value makes sense for // the current situation const vals = normalizeValues(vari) - if (vari.selected && !vals.includes(vari.selected[0])) { + vari = {...vari} + if ( + !vari.selected || + (vari.selected && vari.selected.length && !vals.includes(vari.selected[0])) + ) { vari.selected = [] } - if ((!vari.selected || !vari.selected.length) && vals.length) { - vari.selected = [vals[0]] + if (!vari.selected.length && vals.length) { + vari.selected.push(vals[0]) } return vari diff --git a/ui/src/variables/utils/hydrateVars.test.ts b/ui/src/variables/utils/hydrateVars.test.ts index 168aa80bb6..50b058fc41 100644 --- a/ui/src/variables/utils/hydrateVars.test.ts +++ b/ui/src/variables/utils/hydrateVars.test.ts @@ -237,7 +237,6 @@ describe('hydrate vars', () => { // Basic test for now, we would need an icky mock to assert that the // appropriate substitution is actually taking place - expect( actual.filter(v => v.id === 'a')[0].arguments.values.results ).toEqual(['aVal']) diff --git a/ui/src/variables/utils/hydrateVars.ts b/ui/src/variables/utils/hydrateVars.ts index 9d57226900..30f91f8f13 100644 --- a/ui/src/variables/utils/hydrateVars.ts +++ b/ui/src/variables/utils/hydrateVars.ts @@ -32,17 +32,25 @@ interface HydrateVarsOptions { skipCache?: boolean } +export interface EventedCancelBox extends CancelBox { + on?: any +} + export const createVariableGraph = ( allVariables: Variable[] ): VariableNode[] => { const nodesByID: {[variableID: string]: VariableNode} = allVariables.reduce( (prev, curr) => { + let status = RemoteDataState.Done + if (curr.arguments.type === 'query') { + status = RemoteDataState.NotStarted + } prev[curr.id] = { variable: curr, values: null, parents: [], children: [], - status: RemoteDataState.NotStarted, + status, cancel: () => {}, } return prev @@ -179,9 +187,11 @@ export const collectDescendants = ( This assumes that every descendant of this node has already been hydrated. */ +// TODO: figure out how to type the `on` function const hydrateVarsHelper = async ( node: VariableNode, - options: HydrateVarsOptions + options: HydrateVarsOptions, + on?: any ): Promise => { const variableType = node.variable.arguments.type @@ -202,6 +212,28 @@ const hydrateVarsHelper = async ( } } + if (variableType === 'system') { + return { + valueType: 'string', + values: node.variable.arguments.values, + selected: node.variable.selected, + } + } + + if (node.status !== RemoteDataState.Loading) { + node.status = RemoteDataState.Loading + on.fire('status', node.variable, node.status) + + collectAncestors(node) + .filter(parent => parent.variable.arguments.type === 'query') + .forEach(parent => { + if (parent.status !== RemoteDataState.Loading) { + parent.status = RemoteDataState.Loading + on.fire('status', parent.variable, parent.status) + } + }) + } + const descendants = collectDescendants(node) const assignments = descendants .map(node => asAssignment(node.variable)) @@ -225,6 +257,9 @@ const hydrateVarsHelper = async ( const values = await request.promise + // NOTE: do not fire `done` event here, as the value + // has not been properly hydrated yet + node.status = RemoteDataState.Done return values } @@ -233,17 +268,13 @@ const hydrateVarsHelper = async ( resolved (successfully or not). */ const readyToResolve = (parent: VariableNode): boolean => - parent.status === RemoteDataState.NotStarted && parent.children.every(child => child.status === RemoteDataState.Done) /* Find all `NotStarted` nodes in the graph that have no children. */ const findLeaves = (graph: VariableNode[]): VariableNode[] => - graph.filter( - node => - node.children.length === 0 && node.status === RemoteDataState.NotStarted - ) + graph.filter(node => node.children.length === 0) /* Given a node, attempt to find a cycle that the node is a part of. If no cycle @@ -359,8 +390,11 @@ export const hydrateVars = ( variables: Variable[], allVariables: Variable[], options: HydrateVarsOptions -): CancelBox => { - const graph = findSubgraph(createVariableGraph(allVariables), variables) +): EventedCancelBox => { + const graph = findSubgraph( + createVariableGraph(allVariables), + variables + ).filter(n => n.variable.arguments.type !== 'system') invalidateCycles(graph) let isCancelled = false @@ -370,11 +404,9 @@ export const hydrateVars = ( return } - node.status === RemoteDataState.Loading - try { // TODO: terminate the concept of node.values at the fetcher and just use variables - node.values = await hydrateVarsHelper(node, options) + node.values = await hydrateVarsHelper(node, options, on) if (node.variable.arguments.type === 'query') { node.variable.arguments.values.results = node.values.values as string[] @@ -407,7 +439,7 @@ export const hydrateVars = ( node.variable.selected = node.values.selected } - node.status = RemoteDataState.Done + on.fire('status', node.variable, node.status) return Promise.all(node.parents.filter(readyToResolve).map(resolve)) } catch (e) { @@ -430,9 +462,37 @@ export const hydrateVars = ( deferred.reject(new CancellationError()) } - Promise.all(findLeaves(graph).map(resolve)).then(() => { - deferred.resolve(extractResult(graph)) - }) + const on = (function() { + const callbacks = {} + const ret = (evt, cb) => { + if (!callbacks.hasOwnProperty(evt)) { + callbacks[evt] = [] + } - return {promise: deferred.promise, cancel} + callbacks[evt].push(cb) + } + + ret.fire = (evt, ...args) => { + if (!callbacks.hasOwnProperty(evt)) { + return + } + + callbacks[evt].forEach(cb => cb.apply(cb, args)) + } + + return ret + })() + + // NOTE: wrapping in a resolve disconnects the following findLeaves + // from the main execution thread, allowing external services to + // register listeners for the loading state changes + Promise.resolve() + .then(() => { + return Promise.all(findLeaves(graph).map(resolve)) + }) + .then(() => { + deferred.resolve(extractResult(graph)) + }) + + return {promise: deferred.promise, cancel, on} }