Skip to content

Commit

Permalink
[Discover] Show ignored field values (#115040) (#115584)
Browse files Browse the repository at this point in the history
* WIP replacing indexPattern.flattenHit by tabify

* Fix jest tests

* Read metaFields from index pattern

* Remove old test code

* remove unnecessary changes

* Remove flattenHitWrapper APIs

* Fix imports

* Fix missing metaFields

* Add all meta fields to allowlist

* Improve inline comments

* Move flattenHit test to new implementation

* Add deprecation comment to implementation

* WIP - Show ignored field values

* Disable filters in doc_table

* remove redundant comments

* No, it wasn't

* start warning message

* Enable ignored values in CSV reports

* Add help tooltip

* Better styling with warning plus collapsible button

* Disable filtering within table for ignored values

* Fix jest tests

* Fix types in tests

* Add more tests and documentation

* Remove comment

* Move dangerouslySetInnerHTML into helper method

* Extract document formatting into common utility

* Remove HTML source field formatter

* Move formatHit to Discover

* Change wording of ignored warning

* Add cache for formatted hits

* Remove dead type

* Fix row_formatter for objects

* Improve mobile layout

* Fix jest tests

* Fix typo

* Remove additional span again

* Change mock to revert test

* Improve tests

* More jest tests

* Fix typo

* Change wording

* Remove dead comment

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	src/plugins/data_views/public/index.ts
  • Loading branch information
Tim Roes authored Oct 19, 2021
1 parent 33de724 commit a50fb5a
Show file tree
Hide file tree
Showing 44 changed files with 858 additions and 406 deletions.
49 changes: 49 additions & 0 deletions src/plugins/data/common/search/tabify/tabify_docs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ function create(id: string) {
});
}

const meta = {
_index: 'index-name',
_id: '1',
};

describe('tabify_docs', () => {
describe('flattenHit', () => {
let indexPattern: DataView;
Expand Down Expand Up @@ -70,6 +75,50 @@ describe('tabify_docs', () => {
expect(Object.keys(response)).toEqual(expectedOrder);
expect(Object.entries(response).map(([key]) => key)).toEqual(expectedOrder);
});

it('does merge values from ignored_field_values and fields correctly', () => {
const flatten = flattenHit(
{
...meta,
fields: { 'extension.keyword': ['foo'], extension: ['foo', 'ignored'] },
ignored_field_values: {
'extension.keyword': ['ignored'],
fully_ignored: ['some', 'value'],
},
},
indexPattern,
{ includeIgnoredValues: true }
);
expect(flatten).toHaveProperty(['extension.keyword'], ['foo', 'ignored']);
expect(flatten).toHaveProperty('extension', ['foo', 'ignored']);
expect(flatten).toHaveProperty('fully_ignored', ['some', 'value']);
});

it('does not merge values from ignored_field_values into _source', () => {
const flatten = flattenHit(
{
...meta,
_source: { 'extension.keyword': ['foo', 'ignored'] },
ignored_field_values: { 'extension.keyword': ['ignored'] },
},
indexPattern,
{ includeIgnoredValues: true, source: true }
);
expect(flatten).toHaveProperty(['extension.keyword'], ['foo', 'ignored']);
});

it('does merge ignored_field_values when no _source was present, even when parameter was on', () => {
const flatten = flattenHit(
{
...meta,
fields: { 'extension.keyword': ['foo'] },
ignored_field_values: { 'extension.keyword': ['ignored'] },
},
indexPattern,
{ includeIgnoredValues: true, source: true }
);
expect(flatten).toHaveProperty(['extension.keyword'], ['foo', 'ignored']);
});
});

describe('tabifyDocs', () => {
Expand Down
38 changes: 33 additions & 5 deletions src/plugins/data/common/search/tabify/tabify_docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,18 @@ export interface TabifyDocsOptions {
* merged into the flattened document.
*/
source?: boolean;
/**
* If set to `true` values that have been ignored in ES (ignored_field_values)
* will be merged into the flattened document. This will only have an effect if
* the `hit` has been retrieved using the `fields` option.
*/
includeIgnoredValues?: boolean;
}

// This is an overwrite of the SearchHit type to add the ignored_field_values.
// Can be removed once the estypes.SearchHit knows about ignored_field_values
type Hit<T = unknown> = estypes.SearchHit<T> & { ignored_field_values?: Record<string, unknown[]> };

/**
* Flattens an individual hit (from an ES response) into an object. This will
* create flattened field names, like `user.name`.
Expand All @@ -65,11 +75,7 @@ export interface TabifyDocsOptions {
* @param indexPattern The index pattern for the requested index if available.
* @param params Parameters how to flatten the hit
*/
export function flattenHit(
hit: estypes.SearchHit,
indexPattern?: IndexPattern,
params?: TabifyDocsOptions
) {
export function flattenHit(hit: Hit, indexPattern?: IndexPattern, params?: TabifyDocsOptions) {
const flat = {} as Record<string, any>;

function flatten(obj: Record<string, any>, keyPrefix: string = '') {
Expand Down Expand Up @@ -109,6 +115,28 @@ export function flattenHit(
flatten(hit.fields || {});
if (params?.source !== false && hit._source) {
flatten(hit._source as Record<string, any>);
} else if (params?.includeIgnoredValues && hit.ignored_field_values) {
// If enabled merge the ignored_field_values into the flattened hit. This will
// merge values that are not actually indexed by ES (i.e. ignored), e.g. because
// they were above the `ignore_above` limit or malformed for specific types.
// This API will only contain the values that were actually ignored, i.e. for the same
// field there might exist another value in the `fields` response, why this logic
// merged them both together. We do not merge this (even if enabled) in case source has been
// merged, since we would otherwise duplicate values, since ignore_field_values and _source
// contain the same values.
Object.entries(hit.ignored_field_values).forEach(([fieldName, fieldValue]) => {
if (flat[fieldName]) {
// If there was already a value from the fields API, make sure we're merging both together
if (Array.isArray(flat[fieldName])) {
flat[fieldName] = [...flat[fieldName], ...fieldValue];
} else {
flat[fieldName] = [flat[fieldName], ...fieldValue];
}
} else {
// If no previous value was assigned we can simply use the value from `ignored_field_values` as it is
flat[fieldName] = fieldValue;
}
});
}

// Merge all valid meta fields into the flattened object
Expand Down
13 changes: 0 additions & 13 deletions src/plugins/data_views/common/data_views/data_view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { DuplicateField } from '../../../kibana_utils/common';

import { IIndexPattern, IFieldType } from '../../common';
import { DataViewField, IIndexPatternFieldList, fieldList } from '../fields';
import { formatHitProvider } from './format_hit';
import { flattenHitWrapper } from './flatten_hit';
import {
FieldFormatsStartCommon,
Expand Down Expand Up @@ -45,8 +44,6 @@ interface SavedObjectBody {
type?: string;
}

type FormatFieldFn = (hit: Record<string, any>, fieldName: string) => any;

export class DataView implements IIndexPattern {
public id?: string;
public title: string = '';
Expand All @@ -67,11 +64,6 @@ export class DataView implements IIndexPattern {
* Type is used to identify rollup index patterns
*/
public type: string | undefined;
public formatHit: {
(hit: Record<string, any>, type?: string): any;
formatField: FormatFieldFn;
};
public formatField: FormatFieldFn;
/**
* @deprecated Use `flattenHit` utility method exported from data plugin instead.
*/
Expand Down Expand Up @@ -103,11 +95,6 @@ export class DataView implements IIndexPattern {
this.fields = fieldList([], this.shortDotsEnable);

this.flattenHit = flattenHitWrapper(this, metaFields);
this.formatHit = formatHitProvider(
this,
fieldFormats.getDefaultInstance(KBN_FIELD_TYPES.STRING)
);
this.formatField = this.formatHit.formatField;

// set values
this.id = spec.id;
Expand Down
74 changes: 0 additions & 74 deletions src/plugins/data_views/common/data_views/format_hit.ts

This file was deleted.

1 change: 0 additions & 1 deletion src/plugins/data_views/common/data_views/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,5 @@

export * from './_pattern_cache';
export * from './flatten_hit';
export * from './format_hit';
export * from './data_view';
export * from './data_views';
6 changes: 1 addition & 5 deletions src/plugins/data_views/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ export {
ILLEGAL_CHARACTERS,
validateDataView,
} from '../common/lib';
export {
formatHitProvider,
onRedirectNoIndexPattern,
onUnsupportedTimePattern,
} from './data_views';
export { onRedirectNoIndexPattern, onUnsupportedTimePattern } from './data_views';

export { IndexPatternField, IIndexPatternFieldList, TypeMeta } from '../common';

Expand Down
3 changes: 2 additions & 1 deletion src/plugins/discover/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"data",
"embeddable",
"inspector",
"fieldFormats",
"kibanaLegacy",
"urlForwarding",
"navigation",
Expand All @@ -16,7 +17,7 @@
"indexPatternFieldEditor"
],
"optionalPlugins": ["home", "share", "usageCollection", "spaces"],
"requiredBundles": ["kibanaUtils", "home", "kibanaReact", "fieldFormats", "dataViews"],
"requiredBundles": ["kibanaUtils", "home", "kibanaReact", "dataViews"],
"extraPublicDirs": ["common"],
"owner": {
"name": "Data Discovery",
Expand Down
18 changes: 7 additions & 11 deletions src/plugins/discover/public/__mocks__/index_pattern.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
* Side Public License, v 1.
*/

import type { estypes } from '@elastic/elasticsearch';
import { flattenHit, IIndexPatternFieldList } from '../../../data/common';
import { IIndexPatternFieldList } from '../../../data/common';
import { IndexPattern } from '../../../data/common';

const fields = [
Expand All @@ -28,33 +27,38 @@ const fields = [
{
name: 'message',
type: 'string',
displayName: 'message',
scripted: false,
filterable: false,
aggregatable: false,
},
{
name: 'extension',
type: 'string',
displayName: 'extension',
scripted: false,
filterable: true,
aggregatable: true,
},
{
name: 'bytes',
type: 'number',
displayName: 'bytesDisplayName',
scripted: false,
filterable: true,
aggregatable: true,
},
{
name: 'scripted',
type: 'number',
displayName: 'scripted',
scripted: true,
filterable: false,
},
{
name: 'object.value',
type: 'number',
displayName: 'object.value',
scripted: false,
filterable: true,
aggregatable: true,
Expand All @@ -73,23 +77,15 @@ const indexPattern = {
id: 'the-index-pattern-id',
title: 'the-index-pattern-title',
metaFields: ['_index', '_score'],
formatField: jest.fn(),
flattenHit: undefined,
formatHit: jest.fn((hit) => (hit.fields ? hit.fields : hit._source)),
fields,
getComputedFields: () => ({ docvalueFields: [], scriptFields: {}, storedFields: ['*'] }),
getSourceFiltering: () => ({}),
getFieldByName: jest.fn(() => ({})),
timeFieldName: '',
docvalueFields: [],
getFormatterForField: () => ({ convert: () => 'formatted' }),
getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })),
} as unknown as IndexPattern;

indexPattern.isTimeBased = () => !!indexPattern.timeFieldName;
indexPattern.formatField = (hit: Record<string, unknown>, fieldName: string) => {
return fieldName === '_source'
? hit._source
: flattenHit(hit as unknown as estypes.SearchHit, indexPattern)[fieldName];
};

export const indexPatternMock = indexPattern;
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
* Side Public License, v 1.
*/

import { flattenHit, IIndexPatternFieldList } from '../../../data/common';
import { IIndexPatternFieldList } from '../../../data/common';
import { IndexPattern } from '../../../data/common';
import type { estypes } from '@elastic/elasticsearch';

const fields = [
{
Expand Down Expand Up @@ -64,23 +63,16 @@ const indexPattern = {
id: 'index-pattern-with-timefield-id',
title: 'index-pattern-with-timefield',
metaFields: ['_index', '_score'],
flattenHit: undefined,
formatHit: jest.fn((hit) => hit._source),
fields,
getComputedFields: () => ({}),
getSourceFiltering: () => ({}),
getFieldByName: (name: string) => fields.getByName(name),
timeFieldName: 'timestamp',
getFormatterForField: () => ({ convert: () => 'formatted' }),
getFormatterForField: () => ({ convert: (value: unknown) => value }),
isTimeNanosBased: () => false,
popularizeField: () => {},
} as unknown as IndexPattern;

indexPattern.isTimeBased = () => !!indexPattern.timeFieldName;
indexPattern.formatField = (hit: Record<string, unknown>, fieldName: string) => {
return fieldName === '_source'
? hit._source
: flattenHit(hit as unknown as estypes.SearchHit, indexPattern)[fieldName];
};

export const indexPatternWithTimefieldMock = indexPattern;
Loading

0 comments on commit a50fb5a

Please sign in to comment.