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

TSVB should use "histogram:maxBars" and "histogram:barTarget" settings for auto instead of a default 100 buckets #83628

Merged
merged 13 commits into from
Nov 24, 2020
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ search: {
siblingPipelineType: string;
termsAggFilter: string[];
toAbsoluteDates: typeof toAbsoluteDates;
calcAutoIntervalLessThan: typeof calcAutoIntervalLessThan;
};
getRequestInspectorStats: typeof getRequestInspectorStats;
getResponseInspectorStats: typeof getResponseInspectorStats;
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/common/search/aggs/buckets/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@ export * from './lib/ip_range';
export * from './migrate_include_exclude_format';
export * from './significant_terms';
export * from './terms';
export * from './lib/time_buckets/calc_auto_interval';
2 changes: 2 additions & 0 deletions src/plugins/data/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ import {
includeTotalLoaded,
toKibanaSearchResponse,
getTotalLoaded,
calcAutoIntervalLessThan,
} from '../common';

export {
Expand Down Expand Up @@ -282,6 +283,7 @@ export const search = {
siblingPipelineType,
termsAggFilter,
toAbsoluteDates,
calcAutoIntervalLessThan,
},
getRequestInspectorStats,
getResponseInspectorStats,
Expand Down
32 changes: 17 additions & 15 deletions src/plugins/data/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,7 @@ export const search: {
siblingPipelineType: string;
termsAggFilter: string[];
toAbsoluteDates: typeof toAbsoluteDates;
calcAutoIntervalLessThan: typeof calcAutoIntervalLessThan;
};
getRequestInspectorStats: typeof getRequestInspectorStats;
getResponseInspectorStats: typeof getResponseInspectorStats;
Expand Down Expand Up @@ -1246,21 +1247,22 @@ export function usageProvider(core: CoreSetup_2): SearchUsage;
// src/plugins/data/server/index.ts:111:26 - (ae-forgotten-export) The symbol "TruncateFormat" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:137:27 - (ae-forgotten-export) The symbol "isFilterable" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:137:27 - (ae-forgotten-export) The symbol "isNestedField" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:253:20 - (ae-forgotten-export) The symbol "getRequestInspectorStats" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:253:20 - (ae-forgotten-export) The symbol "getResponseInspectorStats" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:253:20 - (ae-forgotten-export) The symbol "tabifyAggResponse" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:253:20 - (ae-forgotten-export) The symbol "tabifyGetColumns" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:268:5 - (ae-forgotten-export) The symbol "getTotalLoaded" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:269:5 - (ae-forgotten-export) The symbol "toSnakeCase" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:273:1 - (ae-forgotten-export) The symbol "CidrMask" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:274:1 - (ae-forgotten-export) The symbol "dateHistogramInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:283:1 - (ae-forgotten-export) The symbol "InvalidEsCalendarIntervalError" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:284:1 - (ae-forgotten-export) The symbol "InvalidEsIntervalFormatError" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:285:1 - (ae-forgotten-export) The symbol "Ipv4Address" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:289:1 - (ae-forgotten-export) The symbol "isValidEsInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:290:1 - (ae-forgotten-export) The symbol "isValidInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:294:1 - (ae-forgotten-export) The symbol "propFilter" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:297:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:254:20 - (ae-forgotten-export) The symbol "getRequestInspectorStats" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:254:20 - (ae-forgotten-export) The symbol "getResponseInspectorStats" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:254:20 - (ae-forgotten-export) The symbol "tabifyAggResponse" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:254:20 - (ae-forgotten-export) The symbol "tabifyGetColumns" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:269:5 - (ae-forgotten-export) The symbol "getTotalLoaded" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:270:5 - (ae-forgotten-export) The symbol "toSnakeCase" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:274:1 - (ae-forgotten-export) The symbol "CidrMask" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:275:1 - (ae-forgotten-export) The symbol "dateHistogramInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:284:1 - (ae-forgotten-export) The symbol "InvalidEsCalendarIntervalError" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:285:1 - (ae-forgotten-export) The symbol "InvalidEsIntervalFormatError" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:286:1 - (ae-forgotten-export) The symbol "Ipv4Address" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:290:1 - (ae-forgotten-export) The symbol "isValidEsInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:291:1 - (ae-forgotten-export) The symbol "isValidInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:295:1 - (ae-forgotten-export) The symbol "propFilter" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:298:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:299:1 - (ae-forgotten-export) The symbol "calcAutoIntervalLessThan" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index_patterns/index_patterns_service.ts:58:14 - (ae-forgotten-export) The symbol "IndexPatternsService" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/plugin.ts:88:66 - (ae-forgotten-export) The symbol "DataEnhancements" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/search/types.ts:104:5 - (ae-forgotten-export) The symbol "ISearchStartSearchSource" needs to be exported by the entry point index.d.ts
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_type_timeseries/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

export const MAX_BUCKETS_SETTING = 'metrics:max_buckets';
export const INDEXES_SEPARATOR = ',';

export const AUTO_INTERVAL = 'auto';
export const ROUTES = {
VIS_DATA: '/api/metrics/vis/data',
};
2 changes: 2 additions & 0 deletions src/plugins/vis_type_timeseries/common/vis_schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ export const seriesItems = schema.object({
separate_axis: numberIntegerOptional,
seperate_axis: numberIntegerOptional,
series_index_pattern: stringOptionalNullable,
series_max_bars: numberIntegerOptional,
series_time_field: stringOptionalNullable,
series_interval: stringOptionalNullable,
series_drop_last_bucket: numberIntegerOptional,
Expand Down Expand Up @@ -229,6 +230,7 @@ export const panel = schema.object({
ignore_global_filters: numberOptional,
ignore_global_filter: numberOptional,
index_pattern: stringRequired,
max_bars: numberIntegerOptional,
interval: stringRequired,
isModelInvalid: schema.maybe(schema.boolean()),
legend_position: stringOptionalNullable,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,35 +19,39 @@

import { get } from 'lodash';
import PropTypes from 'prop-types';
import React, { useContext } from 'react';
import React, { useContext, useCallback } from 'react';
import {
htmlIdGenerator,
EuiFieldText,
EuiFlexGroup,
EuiFlexItem,
EuiFormRow,
EuiComboBox,
EuiRange,
EuiIconTip,
EuiText,
EuiFormLabel,
} from '@elastic/eui';
import { FieldSelect } from './aggs/field_select';
import { createSelectHandler } from './lib/create_select_handler';
import { createTextHandler } from './lib/create_text_handler';
import { YesNo } from './yes_no';
import { KBN_FIELD_TYPES } from '../../../../../plugins/data/public';
import { FormValidationContext } from '../contexts/form_validation_context';
import {
isGteInterval,
validateReInterval,
isAutoInterval,
AUTO_INTERVAL,
} from './lib/get_interval';
import { isGteInterval, validateReInterval, isAutoInterval } from './lib/get_interval';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { TIME_RANGE_DATA_MODES, TIME_RANGE_MODE_KEY } from '../../../common/timerange_data_modes';
import { PANEL_TYPES } from '../../../common/panel_types';
import { isTimerangeModeEnabled } from '../lib/check_ui_restrictions';
import { VisDataContext } from '../contexts/vis_data_context';
import { getUISettings } from '../../services';
import { AUTO_INTERVAL } from '../../../common/constants';
import { UI_SETTINGS } from '../../../../data/common';

const RESTRICT_FIELDS = [KBN_FIELD_TYPES.DATE];
const LEVEL_OF_DETAIL_STEPS = 10;
const LEVEL_OF_DETAIL_MIN_BUCKETS = 1;

const validateIntervalValue = (intervalValue) => {
const isAutoOrGteInterval = isGteInterval(intervalValue) || isAutoInterval(intervalValue);
Expand All @@ -65,15 +69,36 @@ const htmlId = htmlIdGenerator();
const isEntireTimeRangeActive = (model, isTimeSeries) =>
!isTimeSeries && model[TIME_RANGE_MODE_KEY] === TIME_RANGE_DATA_MODES.ENTIRE_TIME_RANGE;

export const IndexPattern = ({ fields, prefix, onChange, disabled, model: _model }) => {
export const IndexPattern = ({
fields,
prefix,
onChange,
disabled,
model: _model,
allowLevelofDetail,
}) => {
const config = getUISettings();

const handleSelectChange = createSelectHandler(onChange);
const handleTextChange = createTextHandler(onChange);

const timeFieldName = `${prefix}time_field`;
const indexPatternName = `${prefix}index_pattern`;
const intervalName = `${prefix}interval`;
const maxBarsName = `${prefix}max_bars`;
const dropBucketName = `${prefix}drop_last_bucket`;
const updateControlValidity = useContext(FormValidationContext);
const uiRestrictions = get(useContext(VisDataContext), 'uiRestrictions');
const maxBarsUiSettings = config.get(UI_SETTINGS.HISTOGRAM_MAX_BARS);

const handleMaxBarsChange = useCallback(
({ target }) => {
onChange({
[maxBarsName]: Math.max(LEVEL_OF_DETAIL_MIN_BUCKETS, target.value),
});
},
[onChange, maxBarsName]
);

const timeRangeOptions = [
{
Expand All @@ -97,10 +122,12 @@ export const IndexPattern = ({ fields, prefix, onChange, disabled, model: _model
[indexPatternName]: '*',
[intervalName]: AUTO_INTERVAL,
[dropBucketName]: 1,
[maxBarsName]: config.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET),
[TIME_RANGE_MODE_KEY]: timeRangeOptions[0].value,
};

const model = { ...defaults, ..._model };

const isDefaultIndexPatternUsed = model.default_index_pattern && !model[indexPatternName];
const intervalValidation = validateIntervalValue(model[intervalName]);
const selectedTimeRangeOption = timeRangeOptions.find(
Expand Down Expand Up @@ -229,6 +256,77 @@ export const IndexPattern = ({ fields, prefix, onChange, disabled, model: _model
</EuiFormRow>
</EuiFlexItem>
</EuiFlexGroup>
{allowLevelofDetail && (
<EuiFlexGroup>
<EuiFlexItem>
<EuiFormRow
id={htmlId('detailLevel')}
label={
<>
<FormattedMessage
id="visTypeTimeseries.indexPattern.detailLevel"
defaultMessage="Level of detail"
/>{' '}
<EuiIconTip
position="right"
content={
<FormattedMessage
id="visTypeTimeseries.indexPattern.detailLevelHelpText"
defaultMessage="Controls the auto interval based on the time range. The default interval is affected by the advanced settings {histogramTargetBars} and {histogramMaxBars}."
values={{
histogramTargetBars: UI_SETTINGS.HISTOGRAM_MAX_BARS,
histogramMaxBars: UI_SETTINGS.HISTOGRAM_BAR_TARGET,
}}
/>
}
type="questionInCircle"
/>
</>
}
>
<EuiFlexGroup responsive={false} alignItems="center">
<EuiFlexItem grow={false}>
<EuiFormLabel>
<FormattedMessage
id="visTypeTimeseries.indexPattern.сoarse"
defaultMessage="Coarse"
/>
</EuiFormLabel>
</EuiFlexItem>
<EuiFlexItem grow={true}>
<EuiRange
id={htmlIdGenerator()()}
value={model[maxBarsName]}
onChange={handleMaxBarsChange}
disabled={
disabled ||
isEntireTimeRangeActive(model, isTimeSeries) ||
!(model[intervalName] === AUTO_INTERVAL || !model[intervalName])
}
min={0}
Copy link
Contributor

Choose a reason for hiding this comment

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

This value is definitely wrong because it will never generate data. I think a value of 1 is probably too low as well. What do you think about 10?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wylieconlon I needed it earlier to correctly mark the scale from 0 to 100 with a step of 10. When entering a number other than 0, the scale would be shifted. I handle that case here:
image
Let's create a constant to make it a little obvious

Unfortunately even without a scale I cannot set1as a min value cause the max - min value must be a multiple of the step property, otherwise I see an error. Looks like a problem with EUI component.
image

Also I'm not sure that 10 is a good default value for this property. I think for some cases 1 value can be useful and allows to emulate "Entire timerange mode" which we have on other tabs in TSVB. Please keep in mind that to see data for that case you should set
image

max={maxBarsUiSettings}
step={maxBarsUiSettings / LEVEL_OF_DETAIL_STEPS}
aria-label={i18n.translate(
'visTypeTimeseries.indexPattern.detailLevelAriaLabel',
{
defaultMessage: 'Level of detail',
}
)}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFormLabel>
<FormattedMessage
id="visTypeTimeseries.indexPattern.finest"
defaultMessage="Finest"
/>
</EuiFormLabel>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFormRow>
</EuiFlexItem>
</EuiFlexGroup>
)}
</div>
);
};
Expand All @@ -245,4 +343,5 @@ IndexPattern.propTypes = {
prefix: PropTypes.string,
disabled: PropTypes.bool,
className: PropTypes.string,
allowLevelofDetail: PropTypes.bool,
};
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import { get } from 'lodash';
import { search } from '../../../../../../plugins/data/public';
const { parseEsInterval } = search.aggs;
import { GTE_INTERVAL_RE } from '../../../../common/interval_regexp';

export const AUTO_INTERVAL = 'auto';
import { AUTO_INTERVAL } from '../../../../common/constants';

export const unitLookup = {
s: i18n.translate('visTypeTimeseries.getInterval.secondsLabel', { defaultMessage: 'seconds' }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ class TimeseriesPanelConfigUi extends Component {
fields={this.props.fields}
model={this.props.model}
onChange={this.props.onChange}
allowLevelofDetail={true}
/>

<EuiHorizontalRule />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import {
convertIntervalIntoUnit,
isAutoInterval,
isGteInterval,
AUTO_INTERVAL,
} from './lib/get_interval';
import { AUTO_INTERVAL } from '../../../common/constants';
import { PANEL_TYPES } from '../../../common/panel_types';

const MIN_CHART_HEIGHT = 300;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ export const TimeseriesConfig = injectI18n(function (props) {
{...props}
prefix="series_"
disabled={!model.override_index_pattern}
with-interval={true}
allowLevelofDetail={true}
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import { buildAnnotationRequest } from './build_request_body';
import { getEsShardTimeout } from '../helpers/get_es_shard_timeout';
import { getIndexPatternObject } from '../helpers/get_index_pattern';
import { UI_SETTINGS } from '../../../../../data/common';

export async function getAnnotationRequestParams(
req,
Expand All @@ -27,6 +28,7 @@ export async function getAnnotationRequestParams(
esQueryConfig,
capabilities
) {
const uiSettings = req.getUiSettingsService();
const esShardTimeout = await getEsShardTimeout(req);
const indexPattern = annotation.index_pattern;
const { indexPatternObject, indexPatternString } = await getIndexPatternObject(req, indexPattern);
Expand All @@ -36,7 +38,11 @@ export async function getAnnotationRequestParams(
annotation,
esQueryConfig,
indexPatternObject,
capabilities
capabilities,
{
maxBarsUiSettings: await uiSettings.get(UI_SETTINGS.HISTOGRAM_MAX_BARS),
barTargetUiSettings: await uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET),
}
);

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
* under the License.
*/

import { AUTO_INTERVAL } from '../../../common/constants';

const DEFAULT_TIME_FIELD = '@timestamp';

export function getIntervalAndTimefield(panel, series = {}, indexPatternObject) {
Expand All @@ -26,10 +28,18 @@ export function getIntervalAndTimefield(panel, series = {}, indexPatternObject)
(series.override_index_pattern && series.series_time_field) ||
panel.time_field ||
getDefaultTimeField();
const interval = (series.override_index_pattern && series.series_interval) || panel.interval;

let interval = panel.interval;
let maxBars = panel.max_bars;

if (series.override_index_pattern) {
interval = series.series_interval;
maxBars = series.series_max_bars;
}

return {
timeField,
interval,
interval: interval || AUTO_INTERVAL,
maxBars,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { get } from 'lodash';
import { processBucket } from './table/process_bucket';
import { getEsQueryConfig } from './helpers/get_es_query_uisettings';
import { getIndexPatternObject } from './helpers/get_index_pattern';
import { UI_SETTINGS } from '../../../../data/common';

export async function getTableData(req, panel) {
const panelIndexPattern = panel.index_pattern;
Expand All @@ -39,7 +40,12 @@ export async function getTableData(req, panel) {
};

try {
const body = buildRequestBody(req, panel, esQueryConfig, indexPatternObject, capabilities);
const uiSettings = req.getUiSettingsService();
const body = buildRequestBody(req, panel, esQueryConfig, indexPatternObject, capabilities, {
maxBarsUiSettings: await uiSettings.get(UI_SETTINGS.HISTOGRAM_MAX_BARS),
barTargetUiSettings: await uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET),
});

const [resp] = await searchStrategy.search(req, [
{
body,
Expand Down
Loading