fix: load variables one at a time (#17875)

pull/17925/head^2
Alex Boatwright 2020-04-30 13:59:38 -07:00 committed by GitHub
parent c7e97f1625
commit 22899aee25
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 276 additions and 89 deletions

View File

@ -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(() => {

View File

@ -173,13 +173,14 @@ export const createTask = (
export const createQueryVariable = (
orgID?: string,
name: string = 'Little Variable'
name: string = 'Little Variable',
query?: string
): Cypress.Chainable<Cypress.Response> => {
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<Cypress.Response> => {
const argumentsObj = {
type: 'constant',
values: ['c1', 'c2', 'c3', 'c4'],
values: csv || ['c1', 'c2', 'c3', 'c4'],
}
return cy.request({

View File

@ -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<Props, State> {
}
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<Props, State> {
submitToken={submitToken}
queries={this.queries}
key={manualRefresh}
variables={variableAssignments}
>
{({
giraffeResult,
@ -152,18 +139,6 @@ class RefreshingView extends PureComponent<Props, State> {
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,
}
}

View File

@ -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<Props & WithRouterProps, State> {
// 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<Props & WithRouterProps, State> {
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<Props & WithRouterProps, State> {
}
}
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<Bucket>(state, ResourceType.Buckets),
variables,
}
}

View File

@ -118,6 +118,7 @@ export const getVariables = () => async (
}
}
// TODO: make this context aware
export const hydrateVariables = (skipCache?: boolean) => async (
dispatch: Dispatch<Action>,
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
})
hydration.on('status', (variable, status) => {
if (status === RemoteDataState.Loading) {
dispatch(setVariable(variable.id, status))
return
}
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
}, {}),
},
})
if (status === RemoteDataState.Done) {
const _variable = normalize<Variable, VariableEntities, string>(
variable,
variableSchema
)
dispatch(setVariable(variable.id, RemoteDataState.Done, _variable))
}
})
await hydration.promise
}
export const getVariable = (id: string) => async (

View File

@ -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<Props> {
testID="variable-dropdown--button"
status={dropdownStatus}
>
{selectedValue || 'No Values'}
{this.selectedText}
</Dropdown.Button>
)}
menu={onCollapse => (
@ -94,15 +95,31 @@ class VariableDropdown extends PureComponent<Props> {
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,
}
}

View File

@ -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

View File

@ -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'])

View File

@ -32,17 +32,25 @@ interface HydrateVarsOptions {
skipCache?: boolean
}
export interface EventedCancelBox<T> extends CancelBox<T> {
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<VariableValues> => {
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<Variable[]> => {
const graph = findSubgraph(createVariableGraph(allVariables), variables)
): EventedCancelBox<Variable[]> => {
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(() => {
const on = (function() {
const callbacks = {}
const ret = (evt, cb) => {
if (!callbacks.hasOwnProperty(evt)) {
callbacks[evt] = []
}
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}
return {promise: deferred.promise, cancel, on}
}