Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable KQL support for TSVB #26006

Merged
merged 18 commits into from
Jan 30, 2019
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
display: flex;
align-items: center;
justify-content: center;

flex-direction: column;
// Calculate colors similar to EuiCallout
$tempBackgroundColor: tintOrShade($euiColorDanger, 90%, 70%);
background-color: $tempBackgroundColor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import React from 'react';
import { render, unmountComponentAtNode } from 'react-dom';
import { I18nProvider } from '@kbn/i18n/react';
import chrome from 'ui/chrome';

function ReactEditorControllerProvider(Private, config) {
class ReactEditorController {
Expand All @@ -31,6 +32,7 @@ function ReactEditorControllerProvider(Private, config) {

async render(params) {
const Component = this.vis.type.editorConfig.component;
await this.setDefaultIndexPattern(params.appState);
render(
<I18nProvider>
<Component
Expand All @@ -45,6 +47,16 @@ function ReactEditorControllerProvider(Private, config) {
</I18nProvider>, this.el);
}

setDefaultIndexPattern = async (appState) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain what the purpose of this change is? It looks like you're trying to handle the case where the index pattern input is empty, but that probably isn't necessary any more now that we handle null index patterns properly in the KQL serializers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me try to check if it can work now without that part.

Copy link
Member Author

@markov00 markov00 Jan 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was introduced to fix the empty/null pattern error. I'm updating the PR removing that.

if (this.vis.params.index_pattern === '') {
// set the default index pattern if none is defined.
const savedObjectsClient = chrome.getSavedObjectsClient();
const indexPattern = await savedObjectsClient.get('index-pattern', config.get('defaultIndex'));
this.vis.params.index_pattern = indexPattern.attributes.title;
appState.vis.params.index_pattern = indexPattern.attributes.title;
}
}

resize() {
if (this.visData) {
this.render(this.visData);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import { validateInterval } from '../lib/validate_interval';
import { timezoneProvider } from 'ui/vis/lib/timezone';
import { timefilter } from 'ui/timefilter';
import { buildEsQuery } from '@kbn/es-query';

const MetricsRequestHandlerProvider = function (Private, Notifier, config, $http, i18n) {
const notify = new Notifier({ location: i18n('tsvb.requestHandler.notifier.locationNameTitle', { defaultMessage: 'Metrics' }) });
Expand All @@ -35,14 +34,11 @@ const MetricsRequestHandlerProvider = function (Private, Notifier, config, $http
const parsedTimeRange = timefilter.calculateBounds(timeRange);
const scaledDataFormat = config.get('dateFormat:scaled');
const dateFormat = config.get('dateFormat');
const esQueryConfigs = {
allowLeadingWildcards: config.get('query:allowLeadingWildcards'),
queryStringOptions: config.get('query:queryString:options'),
};
if (panel && panel.id) {
const params = {
timerange: { timezone, ...parsedTimeRange },
filters: [buildEsQuery(undefined, [query], filters, esQueryConfigs)],
query,
filters,
panels: [panel],
state: uiStateObj
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import buildProcessorFunction from './build_processor_function';
import processors from './request_processors/annotations';

export default function buildAnnotationRequest(req, panel, annotation) {
const processor = buildProcessorFunction(processors, req, panel, annotation);
export default function buildAnnotationRequest(req, panel, annotation, esQueryConfig, indexPattern) {
const processor = buildProcessorFunction(processors, req, panel, annotation, esQueryConfig, indexPattern);
const doc = processor({});
return doc;
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import buildAnnotationRequest from './build_annotation_request';
import handleAnnotationResponse from './handle_annotation_response';
import { getIndexPatternObject } from './helpers/get_index_pattern';

function validAnnotation(annotation) {
return annotation.index_pattern &&
Expand All @@ -28,27 +29,19 @@ function validAnnotation(annotation) {
annotation.template;
}

export default async (req, panel) => {
export default async (req, panel, esQueryConfig) => {
const { callWithRequest } = req.server.plugins.elasticsearch.getCluster('data');
const bodies = panel.annotations
const bodiesPromises = panel.annotations
.filter(validAnnotation)
.map(annotation => {

const indexPattern = annotation.index_pattern;
const bodies = [];

bodies.push({
index: indexPattern,
ignoreUnavailable: true,
});

const body = buildAnnotationRequest(req, panel, annotation);
body.timeout = '90s';
bodies.push(body);
return bodies;
return getAnnotationBody(req, panel, annotation, esQueryConfig);
});

if (!bodies.length) return { responses: [] };
const bodies = await Promise.all(bodiesPromises);
if (!bodies.length) {
return {
responses: [],
};
}
try {
const resp = await callWithRequest(req, 'msearch', {
rest_total_hits_as_int: true,
Expand All @@ -66,6 +59,18 @@ export default async (req, panel) => {
if (error.message === 'missing-indices') return { responses: [] };
throw error;
}

};

async function getAnnotationBody(req, panel, annotation, esQueryConfig) {
const indexPatternString = annotation.index_pattern;
const indexPatternObject = await getIndexPatternObject(req, indexPatternString);
const request = buildAnnotationRequest(req, panel, annotation, esQueryConfig, indexPatternObject);
request.timeout = '90s';
return [
{
index: indexPatternString,
ignoreUnavailable: true,
},
request,
];
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import { getTableData } from './get_table_data';
import { getSeriesData } from './get_series_data';
export default function getPanelData(req) {
return panel => {
if (panel.type === 'table') return getTableData(req, panel);
if (panel.type === 'table') {
return getTableData(req, panel);
}
return getSeriesData(req, panel);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,34 +21,43 @@ import getRequestParams from './series/get_request_params';
import handleResponseBody from './series/handle_response_body';
import handleErrorResponse from './handle_error_response';
import getAnnotations from './get_annotations';
export function getSeriesData(req, panel) {
import { getEsQueryConfig } from './helpers/get_es_query_uisettings';

export async function getSeriesData(req, panel) {
const { callWithRequest } = req.server.plugins.elasticsearch.getCluster('data');
const bodies = panel.series.map(series => getRequestParams(req, panel, series));
const params = {
rest_total_hits_as_int: true,
body: bodies.reduce((acc, items) => acc.concat(items), [])
};
return callWithRequest(req, 'msearch', params)
.then(resp => {
const series = resp.responses.map(handleResponseBody(panel));
return {
[panel.id]: {
id: panel.id,
series: series.reduce((acc, series) => acc.concat(series), [])
}
};
})
.then(resp => {
if (!panel.annotations || panel.annotations.length === 0) return resp;
return getAnnotations(req, panel).then(annotations => {
resp[panel.id].annotations = annotations;
const esQueryConfig = await getEsQueryConfig(req);

try {
const bodiesPromises = panel.series.map(series => getRequestParams(req, panel, series, esQueryConfig));
const bodies = await Promise.all(bodiesPromises);
const params = {
rest_total_hits_as_int: true,
body: bodies.reduce((acc, items) => acc.concat(items), [])
};
return callWithRequest(req, 'msearch', params)
.then(resp => {
const series = resp.responses.map(handleResponseBody(panel));
return {
[panel.id]: {
id: panel.id,
series: series.reduce((acc, series) => acc.concat(series), [])
}
};
})
.then(resp => {
if (!panel.annotations || panel.annotations.length === 0) return resp;
return getAnnotations(req, panel, esQueryConfig).then(annotations => {
resp[panel.id].annotations = annotations;
return resp;
});
})
.then(resp => {
resp.type = panel.type;
return resp;
});
})
.then(resp => {
resp.type = panel.type;
return resp;
})
.catch(handleErrorResponse(panel));
})
.catch(handleErrorResponse(panel));
} catch(e) {
return handleErrorResponse(e);
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,23 @@
* specific language governing permissions and limitations
* under the License.
*/

import { get } from 'lodash';
import buildRequestBody from './table/build_request_body';
import handleErrorResponse from './handle_error_response';
import { get } from 'lodash';
import processBucket from './table/process_bucket';
import { getIndexPatternObject } from './helpers/get_index_pattern';
import { getEsQueryConfig } from './helpers/get_es_query_uisettings';


export async function getTableData(req, panel) {
const { callWithRequest } = req.server.plugins.elasticsearch.getCluster('data');
const indexPatternString = panel.index_pattern;

const esQueryConfig = await getEsQueryConfig(req);
const indexPatternObject = await getIndexPatternObject(req, indexPatternString);
const params = {
index: panel.index_pattern,
body: buildRequestBody(req, panel)
index: indexPatternString,
body: buildRequestBody(req, panel, esQueryConfig, indexPatternObject)
};
try {
const resp = await callWithRequest(req, 'search', params);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export async function getEsQueryConfig(req) {
const uiSettings = req.getUiSettingsService();
const allowLeadingWildcards = await uiSettings.get('query:allowLeadingWildcards');
const queryStringOptions = await uiSettings.get('query:queryString:options');
return {
allowLeadingWildcards,
queryStringOptions: JSON.parse(queryStringOptions),
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export async function getIndexPatternObject(req, indexPatternString) {
// getting the matching index pattern
const savedObjectClient = req.getSavedObjectsClient();
const indexPatternObjects = await savedObjectClient.find({
type: 'index-pattern',
fields: ['title', 'fields'],
search: `"${indexPatternString}"`,
search_fields: ['title'],
});

// getting the index pattern fields
const indexPatterns = indexPatternObjects.saved_objects
.filter(obj => obj.attributes.title === indexPatternString)
.map(indexPattern => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a util function you can use to do this instead if you want

export function getFromSavedObject(savedObject) {
if (!savedObject) {
return null;
}
return {
fields: JSON.parse(savedObject.attributes.fields),
title: savedObject.attributes.title,
};
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, but reside on public and we need to use it on server side. I should move that function in a shared folder and reuse it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh duh, I missed the fact that one was server and one public. No sweat if you don't wanna move that around in this PR.

const { title, fields } = indexPattern.attributes;
return {
title,
fields: JSON.parse(fields),
};
});
return indexPatterns.length === 1 ? indexPatterns[0] : null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,18 @@

import getBucketSize from '../../helpers/get_bucket_size';
import getTimerange from '../../helpers/get_timerange';
export default function query(req, panel, annotation) {
import { buildEsQuery } from '@kbn/es-query';

export default function query(req, panel, annotation, esQueryConfig, indexPattern) {
return next => doc => {
const timeField = annotation.time_field;
const {
bucketSize
} = getBucketSize(req, 'auto');
const { bucketSize } = getBucketSize(req, 'auto');
const { from, to } = getTimerange(req);

doc.size = 0;
doc.query = {
bool: {
must: []
}
};

const queries = req.payload.query ? [req.payload.query] : [];
Copy link
Contributor

@Bargs Bargs Dec 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const queries = req.payload.query ? [req.payload.query] : [];
const queries = req.payload.query || [];

@markov00 we're just getting back into reviewing the PR after fixing that bug in master that I mentioned in our zoom sync. Wanted to let you know that you'll need to fix it for TSVB in this PR as well since the relevant file changed. All you have to do is apply the suggestion above. Our PR is #27636 in case you want a little more info.

const filters = !annotation.ignore_global_filters ? req.payload.filters : [];
doc.query = buildEsQuery(indexPattern, queries, filters, esQueryConfig);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the query bar really affect the annotations? I think annotations are likely to come from a different index, so queries in the global query bar will likely filter out all docs in the annotation index. Annotations also have options to ignore the global filter and the panel filter by which are enabled by default so if we did want to allow the query bar to affect annotations, I’d think we would want an option to ignore it as well. @alexfrancoeur or @AlonaNadler do you have any opinion here? I'm not super familiar with how annotations are generally used in the wild so I'm just guessing.

Copy link

@alexfrancoeur alexfrancoeur Jan 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite timely, I literally just closed two enhancement requests this week around annotations being filterable. They have been filterable for awhile now but it isn't too discoverable. https://github.com/elastic/enhancements/issues/4007, https://github.com/elastic/enhancements/issues/5141. I'd say it's important, we have controls in TSVB for it as of 5.5 #11260

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexfrancoeur Cool, so just to make sure I understand, you're saying we should allow the query bar to filter annotations, and add an additional boolean option called something like Ignore global query bar?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And what about the visualization, don't we need to add the Ignore global query bar also here? So you can opt-out from the query bar as it works before this PR

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think KQL should filter out TSVB just as the existing query bar and filters does today. There are options to ignore the filters or to persist annotations there. This functionality is generally applied to metrics as well in TSVB, not just annotations.

These boolean options should already be available in the TSVB UI today. If it's easier to sync live, I'm happy to touch base over zoom.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexfrancoeur yes please a zoom sync will help :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexfrancoeur @Bargs @lukasolson with this commit 9384a2d I've updated the PR to mimic exactly how TSVB works with lucene syntax:

  • flagging yes on Ignore global filters on data, any filter added to a dashboard or any search written on the query bar will be ignored by the visualization
  • flagging yes on Ignore global filters on annotations, any filter added to a dashboard or any search written on the query bar will be ignored by the annotations
  • flagging no on Ignore global filters on data, any filter added to a dashboard or any search written on the query bar will filter the visualization data
  • flagging no on Ignore global filters on annotations, any filter added to a dashboard or any search written on the query bar will filter the annotations

The current label Ignore global filters is, IMHO, a bit misleading because doesn't seems to include queries on the querybar, but I think we can change that on a different PR.

If this is ok for you, we can merge this PR and backport it to 6.7

const timerange = {
range: {
[timeField]: {
Expand All @@ -54,11 +51,6 @@ export default function query(req, panel, annotation) {
});
}

const globalFilters = req.payload.filters;
if (!annotation.ignore_global_filters) {
doc.query.bool.must = doc.query.bool.must.concat(globalFilters);
}

if (!annotation.ignore_panel_filters && panel.filter) {
doc.query.bool.must.push({
query_string: {
Expand All @@ -76,7 +68,5 @@ export default function query(req, panel, annotation) {
}

return next(doc);

};
}

Loading