diff --git a/docs/sources/auth/gitlab.md b/docs/sources/auth/gitlab.md index e3a450f9fc7..56fc3b131a5 100644 --- a/docs/sources/auth/gitlab.md +++ b/docs/sources/auth/gitlab.md @@ -100,12 +100,12 @@ display name, especially if the display name contains spaces or special characters. Make sure you always use the group or subgroup name as it appears in the URL of the group or subgroup. -Here's a complete example with `alloed_sign_up` enabled, and access limited to +Here's a complete example with `allow_sign_up` enabled, and access limited to the `example` and `foo/bar` groups: ```ini [auth.gitlab] -enabled = false +enabled = true allow_sign_up = true client_id = GITLAB_APPLICATION_ID client_secret = GITLAB_SECRET diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx index 680cd1e6685..f29d38d283a 100644 --- a/public/app/features/explore/Explore.tsx +++ b/public/app/features/explore/Explore.tsx @@ -373,9 +373,10 @@ export class Explore extends React.PureComponent { this.onModifyQueries({ type: 'ADD_FILTER', key: columnKey, value: rowValue }); }; - onModifyQueries = (action: object, index?: number) => { + onModifyQueries = (action, index?: number) => { const { datasource } = this.state; if (datasource && datasource.modifyQuery) { + const preventSubmit = action.preventSubmit; this.setState( state => { const { queries, queryTransactions } = state; @@ -391,16 +392,26 @@ export class Explore extends React.PureComponent { nextQueryTransactions = []; } else { // Modify query only at index - nextQueries = [ - ...queries.slice(0, index), - { - key: generateQueryKey(index), - query: datasource.modifyQuery(this.queryExpressions[index], action), - }, - ...queries.slice(index + 1), - ]; - // Discard transactions related to row query - nextQueryTransactions = queryTransactions.filter(qt => qt.rowIndex !== index); + nextQueries = queries.map((q, i) => { + // Synchronise all queries with local query cache to ensure consistency + q.query = this.queryExpressions[i]; + return i === index + ? { + key: generateQueryKey(index), + query: datasource.modifyQuery(q.query, action), + } + : q; + }); + nextQueryTransactions = queryTransactions + // Consume the hint corresponding to the action + .map(qt => { + if (qt.hints != null && qt.rowIndex === index) { + qt.hints = qt.hints.filter(hint => hint.fix.action !== action); + } + return qt; + }) + // Preserve previous row query transaction to keep results visible if next query is incomplete + .filter(qt => preventSubmit || qt.rowIndex !== index); } this.queryExpressions = nextQueries.map(q => q.query); return { @@ -408,7 +419,8 @@ export class Explore extends React.PureComponent { queryTransactions: nextQueryTransactions, }; }, - () => this.onSubmit() + // Accepting certain fixes do not result in a well-formed query which should not be submitted + !preventSubmit ? () => this.onSubmit() : null ); } }; diff --git a/public/app/features/explore/PlaceholdersBuffer.test.ts b/public/app/features/explore/PlaceholdersBuffer.test.ts new file mode 100644 index 00000000000..2ce31e79b05 --- /dev/null +++ b/public/app/features/explore/PlaceholdersBuffer.test.ts @@ -0,0 +1,72 @@ +import PlaceholdersBuffer from './PlaceholdersBuffer'; + +describe('PlaceholdersBuffer', () => { + it('does nothing if no placeholders are defined', () => { + const text = 'metric'; + const buffer = new PlaceholdersBuffer(text); + + expect(buffer.hasPlaceholders()).toBe(false); + expect(buffer.toString()).toBe(text); + expect(buffer.getNextMoveOffset()).toBe(0); + }); + + it('respects the traversal order of placeholders', () => { + const text = 'sum($2 offset $1) by ($3)'; + const buffer = new PlaceholdersBuffer(text); + + expect(buffer.hasPlaceholders()).toBe(true); + expect(buffer.toString()).toBe('sum( offset ) by ()'); + expect(buffer.getNextMoveOffset()).toBe(12); + + buffer.setNextPlaceholderValue('1h'); + + expect(buffer.hasPlaceholders()).toBe(true); + expect(buffer.toString()).toBe('sum( offset 1h) by ()'); + expect(buffer.getNextMoveOffset()).toBe(-10); + + buffer.setNextPlaceholderValue('metric'); + + expect(buffer.hasPlaceholders()).toBe(true); + expect(buffer.toString()).toBe('sum(metric offset 1h) by ()'); + expect(buffer.getNextMoveOffset()).toBe(16); + + buffer.setNextPlaceholderValue('label'); + + expect(buffer.hasPlaceholders()).toBe(false); + expect(buffer.toString()).toBe('sum(metric offset 1h) by (label)'); + expect(buffer.getNextMoveOffset()).toBe(0); + }); + + it('respects the traversal order of adjacent placeholders', () => { + const text = '$1$3$2$4'; + const buffer = new PlaceholdersBuffer(text); + + expect(buffer.hasPlaceholders()).toBe(true); + expect(buffer.toString()).toBe(''); + expect(buffer.getNextMoveOffset()).toBe(0); + + buffer.setNextPlaceholderValue('1'); + + expect(buffer.hasPlaceholders()).toBe(true); + expect(buffer.toString()).toBe('1'); + expect(buffer.getNextMoveOffset()).toBe(0); + + buffer.setNextPlaceholderValue('2'); + + expect(buffer.hasPlaceholders()).toBe(true); + expect(buffer.toString()).toBe('12'); + expect(buffer.getNextMoveOffset()).toBe(-1); + + buffer.setNextPlaceholderValue('3'); + + expect(buffer.hasPlaceholders()).toBe(true); + expect(buffer.toString()).toBe('132'); + expect(buffer.getNextMoveOffset()).toBe(1); + + buffer.setNextPlaceholderValue('4'); + + expect(buffer.hasPlaceholders()).toBe(false); + expect(buffer.toString()).toBe('1324'); + expect(buffer.getNextMoveOffset()).toBe(0); + }); +}); diff --git a/public/app/features/explore/PlaceholdersBuffer.ts b/public/app/features/explore/PlaceholdersBuffer.ts new file mode 100644 index 00000000000..9a0db18ef04 --- /dev/null +++ b/public/app/features/explore/PlaceholdersBuffer.ts @@ -0,0 +1,112 @@ +/** + * Provides a stateful means of managing placeholders in text. + * + * Placeholders are numbers prefixed with the `$` character (e.g. `$1`). + * Each number value represents the order in which a placeholder should + * receive focus if multiple placeholders exist. + * + * Example scenario given `sum($3 offset $1) by($2)`: + * 1. `sum( offset |) by()` + * 2. `sum( offset 1h) by(|)` + * 3. `sum(| offset 1h) by (label)` + */ +export default class PlaceholdersBuffer { + private nextMoveOffset: number; + private orders: number[]; + private parts: string[]; + + constructor(text: string) { + const result = this.parse(text); + const nextPlaceholderIndex = result.orders.length ? result.orders[0] : 0; + this.nextMoveOffset = this.getOffsetBetween(result.parts, 0, nextPlaceholderIndex); + this.orders = result.orders; + this.parts = result.parts; + } + + clearPlaceholders() { + this.nextMoveOffset = 0; + this.orders = []; + } + + getNextMoveOffset(): number { + return this.nextMoveOffset; + } + + hasPlaceholders(): boolean { + return this.orders.length > 0; + } + + setNextPlaceholderValue(value: string) { + if (this.orders.length === 0) { + return; + } + const currentPlaceholderIndex = this.orders[0]; + this.parts[currentPlaceholderIndex] = value; + this.orders = this.orders.slice(1); + if (this.orders.length === 0) { + this.nextMoveOffset = 0; + return; + } + const nextPlaceholderIndex = this.orders[0]; + // Case should never happen but handle it gracefully in case + if (currentPlaceholderIndex === nextPlaceholderIndex) { + this.nextMoveOffset = 0; + return; + } + const backwardMove = currentPlaceholderIndex > nextPlaceholderIndex; + const indices = backwardMove + ? { start: nextPlaceholderIndex + 1, end: currentPlaceholderIndex + 1 } + : { start: currentPlaceholderIndex + 1, end: nextPlaceholderIndex }; + this.nextMoveOffset = (backwardMove ? -1 : 1) * this.getOffsetBetween(this.parts, indices.start, indices.end); + } + + toString(): string { + return this.parts.join(''); + } + + private getOffsetBetween(parts: string[], startIndex: number, endIndex: number) { + return parts.slice(startIndex, endIndex).reduce((offset, part) => offset + part.length, 0); + } + + private parse(text: string): ParseResult { + const placeholderRegExp = /\$(\d+)/g; + const parts = []; + const orders = []; + let textOffset = 0; + while (true) { + const match = placeholderRegExp.exec(text); + if (!match) { + break; + } + const part = text.slice(textOffset, match.index); + parts.push(part); + // Accounts for placeholders at text boundaries + if (part !== '') { + parts.push(''); + } + const order = parseInt(match[1], 10); + orders.push({ index: parts.length - 1, order }); + textOffset += part.length + match.length; + } + // Ensures string serialisation still works if no placeholders were parsed + // and also accounts for the remainder of text with placeholders + parts.push(text.slice(textOffset)); + return { + // Placeholder values do not necessarily appear sequentially so sort the + // indices to traverse in priority order + orders: orders.sort((o1, o2) => o1.order - o2.order).map(o => o.index), + parts, + }; + } +} + +type ParseResult = { + /** + * Indices to placeholder items in `parts` in traversal order. + */ + orders: number[]; + /** + * Parts comprising the original text with placeholders occupying distinct items. + */ + parts: string[]; +}; diff --git a/public/app/features/explore/QueryField.tsx b/public/app/features/explore/QueryField.tsx index 86daaa43eac..9350682f1a0 100644 --- a/public/app/features/explore/QueryField.tsx +++ b/public/app/features/explore/QueryField.tsx @@ -12,6 +12,7 @@ import NewlinePlugin from './slate-plugins/newline'; import Typeahead from './Typeahead'; import { makeFragment, makeValue } from './Value'; +import PlaceholdersBuffer from './PlaceholdersBuffer'; export const TYPEAHEAD_DEBOUNCE = 100; @@ -61,12 +62,15 @@ export interface TypeaheadInput { class QueryField extends React.PureComponent { menuEl: HTMLElement | null; + placeholdersBuffer: PlaceholdersBuffer; plugins: any[]; resetTimer: any; constructor(props, context) { super(props, context); + this.placeholdersBuffer = new PlaceholdersBuffer(props.initialValue || ''); + // Base plugins this.plugins = [ClearPlugin(), NewlinePlugin(), ...props.additionalPlugins]; @@ -76,7 +80,7 @@ class QueryField extends React.PureComponent operation.type === 'insert_text'); + if (insertTextOperation) { + const suggestionText = insertTextOperation.text; + this.placeholdersBuffer.setNextPlaceholderValue(suggestionText); + if (this.placeholdersBuffer.hasPlaceholders()) { + nextChange.move(this.placeholdersBuffer.getNextMoveOffset()).focus(); + } + } + return true; } break; @@ -336,6 +352,8 @@ class QueryField extends React.PureComponent = null; const suggestions: CompletionItemGroup[] = []; - // sum(foo{bar="1"}) by (|) - const line = value.anchorBlock.getText(); - const cursorOffset: number = value.anchorOffset; - // sum(foo{bar="1"}) by ( - const leftSide = line.slice(0, cursorOffset); + // Stitch all query lines together to support multi-line queries + let queryOffset; + const queryText = value.document.getBlocks().reduce((text, block) => { + const blockText = block.getText(); + if (value.anchorBlock.key === block.key) { + // Newline characters are not accounted for but this is irrelevant + // for the purpose of extracting the selector string + queryOffset = value.anchorOffset + text.length; + } + text += blockText; + return text; + }, ''); + + const leftSide = queryText.slice(0, queryOffset); const openParensAggregationIndex = leftSide.lastIndexOf('('); const openParensSelectorIndex = leftSide.slice(0, openParensAggregationIndex).lastIndexOf('('); const closeParensSelectorIndex = leftSide.slice(openParensSelectorIndex).indexOf(')') + openParensSelectorIndex; - // foo{bar="1"} - const selectorString = leftSide.slice(openParensSelectorIndex + 1, closeParensSelectorIndex); + + let selectorString = leftSide.slice(openParensSelectorIndex + 1, closeParensSelectorIndex); + + // Range vector syntax not accounted for by subsequent parse so discard it if present + selectorString = selectorString.replace(/\[[^\]]+\]$/, ''); + const selector = parseSelector(selectorString, selectorString.length - 2).selector; const labelKeys = this.labelKeys[selector]; diff --git a/public/app/plugins/datasource/prometheus/query_hints.ts b/public/app/plugins/datasource/prometheus/query_hints.ts index 388a9be48d1..ea505832468 100644 --- a/public/app/plugins/datasource/prometheus/query_hints.ts +++ b/public/app/plugins/datasource/prometheus/query_hints.ts @@ -2,6 +2,11 @@ import _ from 'lodash'; import { QueryHint } from 'app/types/explore'; +/** + * Number of time series results needed before starting to suggest sum aggregation hints + */ +export const SUM_HINT_THRESHOLD_COUNT = 20; + export function getQueryHints(query: string, series?: any[], datasource?: any): QueryHint[] { const hints = []; @@ -90,5 +95,24 @@ export function getQueryHints(query: string, series?: any[], datasource?: any): }); } } + + if (series.length >= SUM_HINT_THRESHOLD_COUNT) { + const simpleMetric = query.trim().match(/^\w+$/); + if (simpleMetric) { + hints.push({ + type: 'ADD_SUM', + label: 'Many time series results returned.', + fix: { + label: 'Consider aggregating with sum().', + action: { + type: 'ADD_SUM', + query: query, + preventSubmit: true, + }, + }, + }); + } + } + return hints.length > 0 ? hints : null; } diff --git a/public/app/plugins/datasource/prometheus/specs/language_provider.test.ts b/public/app/plugins/datasource/prometheus/specs/language_provider.test.ts index 3a46e2efaf3..20e148efd57 100644 --- a/public/app/plugins/datasource/prometheus/specs/language_provider.test.ts +++ b/public/app/plugins/datasource/prometheus/specs/language_provider.test.ts @@ -198,5 +198,76 @@ describe('Language completion provider', () => { expect(result.context).toBe('context-aggregation'); expect(result.suggestions).toEqual([{ items: [{ label: 'bar' }], label: 'Labels' }]); }); + + it('returns label suggestions inside a multi-line aggregation context', () => { + const instance = new LanguageProvider(datasource, { + labelKeys: { '{__name__="metric"}': ['label1', 'label2', 'label3'] }, + }); + const value = Plain.deserialize('sum(\nmetric\n)\nby ()'); + const aggregationTextBlock = value.document.getBlocksAsArray()[3]; + const range = value.selection.moveToStartOf(aggregationTextBlock).merge({ anchorOffset: 4 }); + const valueWithSelection = value.change().select(range).value; + const result = instance.provideCompletionItems({ + text: '', + prefix: '', + wrapperClasses: ['context-aggregation'], + value: valueWithSelection, + }); + expect(result.context).toBe('context-aggregation'); + expect(result.suggestions).toEqual([ + { + items: [{ label: 'label1' }, { label: 'label2' }, { label: 'label3' }], + label: 'Labels', + }, + ]); + }); + + it('returns label suggestions inside an aggregation context with a range vector', () => { + const instance = new LanguageProvider(datasource, { + labelKeys: { '{__name__="metric"}': ['label1', 'label2', 'label3'] }, + }); + const value = Plain.deserialize('sum(rate(metric[1h])) by ()'); + const range = value.selection.merge({ + anchorOffset: 26, + }); + const valueWithSelection = value.change().select(range).value; + const result = instance.provideCompletionItems({ + text: '', + prefix: '', + wrapperClasses: ['context-aggregation'], + value: valueWithSelection, + }); + expect(result.context).toBe('context-aggregation'); + expect(result.suggestions).toEqual([ + { + items: [{ label: 'label1' }, { label: 'label2' }, { label: 'label3' }], + label: 'Labels', + }, + ]); + }); + + it('returns label suggestions inside an aggregation context with a range vector and label', () => { + const instance = new LanguageProvider(datasource, { + labelKeys: { '{__name__="metric",label1="value"}': ['label1', 'label2', 'label3'] }, + }); + const value = Plain.deserialize('sum(rate(metric{label1="value"}[1h])) by ()'); + const range = value.selection.merge({ + anchorOffset: 42, + }); + const valueWithSelection = value.change().select(range).value; + const result = instance.provideCompletionItems({ + text: '', + prefix: '', + wrapperClasses: ['context-aggregation'], + value: valueWithSelection, + }); + expect(result.context).toBe('context-aggregation'); + expect(result.suggestions).toEqual([ + { + items: [{ label: 'label1' }, { label: 'label2' }, { label: 'label3' }], + label: 'Labels', + }, + ]); + }); }); }); diff --git a/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts b/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts index 7eba54536fe..3b782d3dd09 100644 --- a/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts +++ b/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts @@ -1,4 +1,4 @@ -import { getQueryHints } from '../query_hints'; +import { getQueryHints, SUM_HINT_THRESHOLD_COUNT } from '../query_hints'; describe('getQueryHints()', () => { it('returns no hints for no series', () => { @@ -79,4 +79,25 @@ describe('getQueryHints()', () => { }, }); }); + + it('returns a sum hint when many time series results are returned for a simple metric', () => { + const seriesCount = SUM_HINT_THRESHOLD_COUNT; + const series = Array.from({ length: seriesCount }, _ => ({ + datapoints: [[0, 0], [0, 0]], + })); + const hints = getQueryHints('metric', series); + expect(hints.length).toBe(1); + expect(hints[0]).toMatchObject({ + type: 'ADD_SUM', + label: 'Many time series results returned.', + fix: { + label: 'Consider aggregating with sum().', + action: { + type: 'ADD_SUM', + query: 'metric', + preventSubmit: true, + }, + }, + }); + }); }); diff --git a/public/app/plugins/datasource/stackdriver/datasource.ts b/public/app/plugins/datasource/stackdriver/datasource.ts index 4a81eb8a619..034333cbb86 100644 --- a/public/app/plugins/datasource/stackdriver/datasource.ts +++ b/public/app/plugins/datasource/stackdriver/datasource.ts @@ -114,7 +114,6 @@ export default class StackdriverDatasource { if (!queryRes.series) { return; } - this.projectName = queryRes.meta.defaultProject; const unit = this.resolvePanelUnitFromTargets(options.targets); queryRes.series.forEach(series => { let timeSerie: any = { diff --git a/public/app/types/explore.ts b/public/app/types/explore.ts index 807769212f3..1db37392b0b 100644 --- a/public/app/types/explore.ts +++ b/public/app/types/explore.ts @@ -119,6 +119,7 @@ export interface QueryFix { export interface QueryFixAction { type: string; query?: string; + preventSubmit?: boolean; } export interface QueryHint {