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

[Lens] Support histogram mapping type for all numeric functions #90357

Merged
merged 12 commits into from
Feb 16, 2021
Merged
2 changes: 1 addition & 1 deletion src/plugins/data/common/search/aggs/metrics/max.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const getMaxMetricAgg = () => {
{
name: 'field',
type: 'field',
filterFieldTypes: [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.DATE],
filterFieldTypes: [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.DATE, KBN_FIELD_TYPES.HISTOGRAM],
},
],
});
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/common/search/aggs/metrics/min.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const getMinMetricAgg = () => {
{
name: 'field',
type: 'field',
filterFieldTypes: [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.DATE],
filterFieldTypes: [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.DATE, KBN_FIELD_TYPES.HISTOGRAM],
},
],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ export const fieldMappings = {
bytes: {
type: 'long',
},
bytes_histogram: {
type: 'histogram',
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reviewers: You can test the histogram data in the sample logs, but I think this is probably not a good idea to merge. So I'll plan to remove these changes before merging.

tags: {
type: 'text',
fields: {
Expand Down
Binary file not shown.
11 changes: 10 additions & 1 deletion x-pack/plugins/lens/public/indexpattern_datasource/datapanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,15 @@ function sortFields(fieldA: IndexPatternField, fieldB: IndexPatternField) {
return fieldA.displayName.localeCompare(fieldB.displayName, undefined, { sensitivity: 'base' });
}

const supportedFieldTypes = new Set(['string', 'number', 'boolean', 'date', 'ip', 'document']);
const supportedFieldTypes = new Set([
'string',
'number',
'boolean',
'date',
'ip',
'histogram',
'document',
]);

const fieldTypeNames: Record<DataType, string> = {
document: i18n.translate('xpack.lens.datatypes.record', { defaultMessage: 'record' }),
Expand All @@ -66,6 +74,7 @@ const fieldTypeNames: Record<DataType, string> = {
boolean: i18n.translate('xpack.lens.datatypes.boolean', { defaultMessage: 'boolean' }),
date: i18n.translate('xpack.lens.datatypes.date', { defaultMessage: 'date' }),
ip: i18n.translate('xpack.lens.datatypes.ipAddress', { defaultMessage: 'IP' }),
histogram: i18n.translate('xpack.lens.datatypes.histogram', { defaultMessage: 'histogram' }),
};

// Wrapper around esQuery.buildEsQuery, handling errors (e.g. because a query can't be parsed) by
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ const typeToFn: Record<string, string> = {
median: 'aggMedian',
};

const supportedTypes = ['number', 'histogram'];

function buildMetricOperation<T extends MetricColumn<string>>({
type,
displayName,
Expand Down Expand Up @@ -61,7 +63,7 @@ function buildMetricOperation<T extends MetricColumn<string>>({
timeScalingMode: optionalTimeScaling ? 'optional' : undefined,
getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type: fieldType }) => {
if (
fieldType === 'number' &&
supportedTypes.includes(fieldType) &&
aggregatable &&
(!aggregationRestrictions || aggregationRestrictions[type])
) {
Expand All @@ -77,7 +79,7 @@ function buildMetricOperation<T extends MetricColumn<string>>({

return Boolean(
newField &&
newField.type === 'number' &&
supportedTypes.includes(newField.type) &&
newField.aggregatable &&
(!newField.aggregationRestrictions || newField.aggregationRestrictions![type])
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,52 @@ describe('percentile', () => {
};
});

describe('getPossibleOperationForField', () => {
it('should accept number', () => {
expect(
percentileOperation.getPossibleOperationForField({
name: 'bytes',
displayName: 'bytes',
type: 'number',
esTypes: ['long'],
aggregatable: true,
})
).toEqual({
dataType: 'number',
isBucketed: false,
scale: 'ratio',
});
});

it('should accept histogram', () => {
expect(
percentileOperation.getPossibleOperationForField({
name: 'response_time',
displayName: 'response_time',
type: 'histogram',
esTypes: ['histogram'],
aggregatable: true,
})
).toEqual({
dataType: 'number',
isBucketed: false,
scale: 'ratio',
});
});

it('should reject keywords', () => {
expect(
percentileOperation.getPossibleOperationForField({
name: 'origin',
displayName: 'origin',
type: 'string',
esTypes: ['keyword'],
aggregatable: true,
})
).toBeUndefined();
});
});

describe('toEsAggsFn', () => {
it('should reflect params correctly', () => {
const percentileColumn = layer.columns.col2 as PercentileIndexPatternColumn;
Expand Down Expand Up @@ -134,6 +180,34 @@ describe('percentile', () => {
});
});

describe('isTransferable', () => {
it('should transfer from number to histogram', () => {
const indexPattern = createMockedIndexPattern();
indexPattern.getFieldByName = jest.fn().mockReturnValue({
name: 'response_time',
displayName: 'response_time',
type: 'histogram',
esTypes: ['histogram'],
aggregatable: true,
});
expect(
percentileOperation.isTransferable(
{
label: '',
sourceField: 'response_time',
isBucketed: false,
dataType: 'number',
operationType: 'percentile',
params: {
percentile: 95,
},
},
indexPattern
)
).toBeTruthy();
});
});

describe('param editor', () => {
it('should render current percentile', () => {
const updateLayerSpy = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,16 @@ function ofName(name: string, percentile: number) {

const DEFAULT_PERCENTILE_VALUE = 95;

const supportedFieldTypes = ['number', 'histogram'];

export const percentileOperation: OperationDefinition<PercentileIndexPatternColumn, 'field'> = {
type: 'percentile',
displayName: i18n.translate('xpack.lens.indexPattern.percentile', {
defaultMessage: 'Percentile',
}),
input: 'field',
getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type: fieldType }) => {
if (fieldType === 'number' && aggregatable && !aggregationRestrictions) {
if (supportedFieldTypes.includes(fieldType) && aggregatable && !aggregationRestrictions) {
return {
dataType: 'number',
isBucketed: false,
Expand All @@ -62,7 +64,7 @@ export const percentileOperation: OperationDefinition<PercentileIndexPatternColu

return Boolean(
newField &&
newField.type === 'number' &&
supportedFieldTypes.includes(newField.type) &&
newField.aggregatable &&
!newField.aggregationRestrictions
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { getInvalidFieldMessage } from './operations/definitions/helpers';
* produce 'number')
*/
export function normalizeOperationDataType(type: DataType) {
if (type === 'histogram') return 'number';
return type === 'document' ? 'number' : type;
}

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/lens/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ export type DatasourceDimensionDropHandlerProps<T> = DatasourceDimensionDropProp
dropType: DropType;
};

export type DataType = 'document' | 'string' | 'number' | 'date' | 'boolean' | 'ip';
export type FieldOnlyDataType = 'document' | 'ip' | 'histogram';
export type DataType = 'string' | 'number' | 'date' | 'boolean' | FieldOnlyDataType;

// An operation represents a column in a table, not any information
// about how the column was created such as whether it is a sum or average.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const columnSortOrder = {
ip: 3,
boolean: 4,
number: 5,
histogram: 6,
};

/**
Expand Down
81 changes: 80 additions & 1 deletion x-pack/plugins/lens/server/routes/field_stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,11 @@ export async function initFieldsRoute(setup: CoreSetup<PluginStartContract>) {
return result;
};

if (field.type === 'number') {
if (field.type === 'histogram') {
return res.ok({
body: await getNumberOnlyHistogram(search, field),
});
} else if (field.type === 'number') {
return res.ok({
body: await getNumberHistogram(search, field),
});
Expand Down Expand Up @@ -205,6 +209,81 @@ export async function getNumberHistogram(
};
}

export async function getNumberOnlyHistogram(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It looks like this code is almost the same as getNumberHistogram (except for the top values part). Can we reduce the redundancy somehow in a nice way (maybe some conditional branches)? Not a blocker, just stumbled over it a little.

aggSearchWithBody: (body: unknown) => Promise<unknown>,
field: IFieldType
): Promise<FieldStatsResponse> {
const fieldRef = getFieldRef(field);

const searchBody = {
sample: {
sampler: { shard_size: SHARD_SIZE },
aggs: {
min_value: {
min: { field: field.name },
},
max_value: {
max: { field: field.name },
},
sample_count: { value_count: { ...fieldRef } },
},
},
};

const minMaxResult = (await aggSearchWithBody(searchBody)) as ESSearchResponse<
unknown,
{ body: { aggs: typeof searchBody } }
>;

const minValue = minMaxResult.aggregations!.sample.min_value.value;
const maxValue = minMaxResult.aggregations!.sample.max_value.value;

let histogramInterval = (maxValue! - minValue!) / 10;

if (Number.isInteger(minValue!) && Number.isInteger(maxValue!)) {
histogramInterval = Math.ceil(histogramInterval);
}

if (histogramInterval === 0) {
return {
totalDocuments: minMaxResult.hits.total.value,
sampledValues: minMaxResult.aggregations!.sample.sample_count.value!,
sampledDocuments: minMaxResult.aggregations!.sample.doc_count,
histogram: { buckets: [] },
};
}

const histogramBody = {
sample: {
sampler: { shard_size: SHARD_SIZE },
aggs: {
histo: {
histogram: {
field: field.name,
interval: histogramInterval,
},
},
},
},
};
const histogramResult = (await aggSearchWithBody(histogramBody)) as ESSearchResponse<
unknown,
{ body: { aggs: typeof histogramBody } }
>;

return {
totalDocuments: minMaxResult.hits.total.value,
sampledDocuments: minMaxResult.aggregations!.sample.doc_count,
sampledValues: minMaxResult.aggregations!.sample.sample_count.value!,
histogram: {
buckets: histogramResult.aggregations!.sample.histo.buckets.map((bucket) => ({
count: bucket.doc_count,
key: bucket.key,
})),
},
};
}

export async function getStringSamples(
aggSearchWithBody: (body: unknown) => unknown,
field: IFieldType
Expand Down
Loading