mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 17:28:26 -04:00
[Lens] Relax counter field checks for saved visualizations with unsupported operations (#163515)
## Summary Fix #163473 This PR relaxes a bit the checks on the Lens side for old/saved visualizations with unsupported operations for the `counter` field type, while preserving those checks for newer visualizations. Dashboards with "meaningless" operations will now show a warning message: <img width="556" alt="Screenshot 2023-08-09 at 18 31 21" src="7c8f3739-4957-4d1d-8aaa-e9457b8a4426"> When in editor the warning is shown at the top-right corner as well: <img width="845" alt="Screenshot 2023-08-09 at 18 30 31" src="c52a7823-d4b9-4efd-9c5d-ca654f3f03a0"> New visualizations still prevent the user from using the unsupported operations: <img width="410" alt="Screenshot 2023-08-09 at 18 30 55" src="d2364a01-0dc3-409a-9c0b-3e3a77cb2566"> <img width="848" alt="Screenshot 2023-08-09 at 18 31 48" src="086a7360-6b1a-40a2-90d9-f4e8c7bf3f3a"> There's theoretically a last case where users in old SOs might create a new metric dimension trying to force to use a unsupported operation for a counter field: in this case the logic for a "new" visualization will kick-in, clean the data in the workspace and show a full error. Cancelling such metric dimension will lead to the previous "relaxed" state. Messages are grouped by field and by top referencing column (i.e. a formula): this means that if a formula uses the same `counter` field with two different dimensions (i.e. `sum(counter_field) + median(counter_field)` as `myFormula`) will show up as a single column (`myFormula`). The wording of the message mimics the same documentation copy provided in the ES documentation. Ref: https://github.com/elastic/elasticsearch/pull/97974 Testing SO: [export.ndjson.txt](12304924/export.ndjson.txt) ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
This commit is contained in:
Marco Liberati
GitHub
parent
81ca020ad7
commit
4c812e3b6d
4 changed files with 326 additions and 19 deletions
|
|
@ -69,6 +69,7 @@ import {
|
|||
isColumnInvalid,
|
||||
cloneLayer,
|
||||
getNotifiableFeatures,
|
||||
getUnsupportedOperationsWarningMessage,
|
||||
} from './utils';
|
||||
import { getUniqueLabelGenerator, isDraggedDataViewField, nonNullable } from '../../utils';
|
||||
import { hasField, normalizeOperationDataType } from './pure_utils';
|
||||
|
|
@ -801,6 +802,7 @@ export function getFormBasedDatasource({
|
|||
core.docLinks,
|
||||
setState
|
||||
),
|
||||
...getUnsupportedOperationsWarningMessage(state, frameDatasourceAPI, core.docLinks),
|
||||
];
|
||||
|
||||
const infoMessages = getNotifiableFeatures(state, frameDatasourceAPI, visualizationInfo);
|
||||
|
|
|
|||
|
|
@ -126,7 +126,6 @@ function buildMetricOperation<T extends MetricColumn<string>>({
|
|||
newField &&
|
||||
supportedTypes.includes(newField.type) &&
|
||||
newField.aggregatable &&
|
||||
isTimeSeriesCompatible(type, newField.timeSeriesMetric) &&
|
||||
(!newField.aggregationRestrictions || newField.aggregationRestrictions![type])
|
||||
);
|
||||
},
|
||||
|
|
|
|||
|
|
@ -8,15 +8,20 @@
|
|||
import React from 'react';
|
||||
import { shallow } from 'enzyme';
|
||||
import { createDatatableUtilitiesMock } from '@kbn/data-plugin/common/mocks';
|
||||
import { getPrecisionErrorWarningMessages, cloneLayer } from './utils';
|
||||
import {
|
||||
getPrecisionErrorWarningMessages,
|
||||
cloneLayer,
|
||||
getUnsupportedOperationsWarningMessage,
|
||||
} from './utils';
|
||||
import type { FormBasedPrivateState, GenericIndexPatternColumn } from './types';
|
||||
import type { FramePublicAPI } from '../../types';
|
||||
import type { FramePublicAPI, IndexPattern } from '../../types';
|
||||
import type { DocLinksStart } from '@kbn/core/public';
|
||||
import { EuiLink } from '@elastic/eui';
|
||||
import { TermsIndexPatternColumn } from './operations';
|
||||
import { mountWithIntl } from '@kbn/test-jest-helpers';
|
||||
import { FormattedMessage } from '@kbn/i18n-react';
|
||||
import { FormBasedLayer } from './types';
|
||||
import { createMockedIndexPatternWithAdditionalFields } from './mocks';
|
||||
|
||||
describe('indexpattern_datasource utils', () => {
|
||||
describe('getPrecisionErrorWarningMessages', () => {
|
||||
|
|
@ -240,4 +245,195 @@ describe('indexpattern_datasource utils', () => {
|
|||
).toMatchSnapshot();
|
||||
});
|
||||
});
|
||||
|
||||
describe('getUnsupportedOperationsWarningMessage', () => {
|
||||
let docLinks: DocLinksStart;
|
||||
const affectedOperations = [
|
||||
'sum',
|
||||
'average',
|
||||
'percentile',
|
||||
'percentile_rank',
|
||||
'count',
|
||||
'unique_count',
|
||||
'standard_deviation',
|
||||
];
|
||||
|
||||
function createColumnsForField(field: string, colOffset: number = 0) {
|
||||
return Object.fromEntries(
|
||||
affectedOperations.map((operationType, i) => [
|
||||
`col_${i + colOffset}`,
|
||||
{ operationType, sourceField: field, label: `${operationType} of ${field}` },
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
function createState(fields: string[]) {
|
||||
return {
|
||||
layers: {
|
||||
id: {
|
||||
indexPatternId: '0',
|
||||
columns: Object.assign(
|
||||
{},
|
||||
...fields.map((field, i) =>
|
||||
createColumnsForField(field, i * affectedOperations.length)
|
||||
)
|
||||
),
|
||||
},
|
||||
},
|
||||
} as unknown as FormBasedPrivateState;
|
||||
}
|
||||
|
||||
function createFramePublic(indexPattern: IndexPattern): FramePublicAPI {
|
||||
return {
|
||||
dataViews: {
|
||||
indexPatterns: Object.fromEntries([indexPattern].map((dataView, i) => [i, dataView])),
|
||||
},
|
||||
} as unknown as FramePublicAPI;
|
||||
}
|
||||
|
||||
function createFormulaColumns(formulaParts: string[], field: string, colOffset: number = 0) {
|
||||
const fullFormula = formulaParts.map((part) => `${part}(${field})`).join(' + ');
|
||||
// just assume it's a sum of all the parts for testing
|
||||
const rootId = `col-formula${colOffset}`;
|
||||
return Object.fromEntries([
|
||||
[
|
||||
rootId,
|
||||
{
|
||||
operationType: 'formula',
|
||||
label: `Formula: ${fullFormula}`,
|
||||
params: { formula: fullFormula },
|
||||
},
|
||||
],
|
||||
...formulaParts.map((part, i) => [
|
||||
`${rootId}X${i}`,
|
||||
{ operationType: part, sourceField: field, label: 'Part of formula' },
|
||||
]),
|
||||
[
|
||||
`${rootId}X${formulaParts.length}`,
|
||||
{ operationType: 'math', references: formulaParts.map((_, i) => `${rootId}X${i}`) },
|
||||
],
|
||||
]);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
docLinks = {
|
||||
links: {
|
||||
fleet: {
|
||||
datastreamsTSDSMetrics: 'http://tsdb_metric_doc',
|
||||
},
|
||||
},
|
||||
} as DocLinksStart;
|
||||
});
|
||||
|
||||
it.each([['bytes'], ['bytes_gauge']])(
|
||||
'should return no warning for non-counter fields: %s',
|
||||
(fieldName: string) => {
|
||||
const warnings = getUnsupportedOperationsWarningMessage(
|
||||
createState([fieldName]),
|
||||
createFramePublic(
|
||||
createMockedIndexPatternWithAdditionalFields([
|
||||
{
|
||||
name: 'bytes_gauge',
|
||||
displayName: 'bytes_gauge',
|
||||
type: 'number',
|
||||
aggregatable: true,
|
||||
searchable: true,
|
||||
timeSeriesMetric: 'gauge',
|
||||
},
|
||||
])
|
||||
),
|
||||
docLinks
|
||||
);
|
||||
expect(warnings).toHaveLength(0);
|
||||
}
|
||||
);
|
||||
|
||||
it('should return a warning for a counter field grouped by field', () => {
|
||||
const warnings = getUnsupportedOperationsWarningMessage(
|
||||
createState(['bytes_counter']),
|
||||
createFramePublic(
|
||||
createMockedIndexPatternWithAdditionalFields([
|
||||
{
|
||||
name: 'bytes_counter',
|
||||
displayName: 'bytes_counter',
|
||||
type: 'number',
|
||||
aggregatable: true,
|
||||
searchable: true,
|
||||
timeSeriesMetric: 'counter',
|
||||
},
|
||||
])
|
||||
),
|
||||
docLinks
|
||||
);
|
||||
expect(warnings).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should group multiple warnings by field', () => {
|
||||
const warnings = getUnsupportedOperationsWarningMessage(
|
||||
createState(['bytes_counter', 'bytes_counter2']),
|
||||
createFramePublic(
|
||||
createMockedIndexPatternWithAdditionalFields([
|
||||
{
|
||||
name: 'bytes_counter',
|
||||
displayName: 'bytes_counter',
|
||||
type: 'number',
|
||||
aggregatable: true,
|
||||
searchable: true,
|
||||
timeSeriesMetric: 'counter',
|
||||
},
|
||||
{
|
||||
name: 'bytes_counter2',
|
||||
displayName: 'bytes_counter2',
|
||||
type: 'number',
|
||||
aggregatable: true,
|
||||
searchable: true,
|
||||
timeSeriesMetric: 'counter',
|
||||
},
|
||||
])
|
||||
),
|
||||
docLinks
|
||||
);
|
||||
expect(warnings).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('should handle formula reporting only the top visible dimension', () => {
|
||||
const warnings = getUnsupportedOperationsWarningMessage(
|
||||
{
|
||||
layers: {
|
||||
id: {
|
||||
indexPatternId: '0',
|
||||
columns: Object.assign(
|
||||
{},
|
||||
...['bytes_counter', 'bytes_counter2'].map((field, i) =>
|
||||
createFormulaColumns(affectedOperations, field, i * affectedOperations.length)
|
||||
)
|
||||
),
|
||||
},
|
||||
},
|
||||
} as unknown as FormBasedPrivateState,
|
||||
createFramePublic(
|
||||
createMockedIndexPatternWithAdditionalFields([
|
||||
{
|
||||
name: 'bytes_counter',
|
||||
displayName: 'bytes_counter',
|
||||
type: 'number',
|
||||
aggregatable: true,
|
||||
searchable: true,
|
||||
timeSeriesMetric: 'counter',
|
||||
},
|
||||
{
|
||||
name: 'bytes_counter2',
|
||||
displayName: 'bytes_counter2',
|
||||
type: 'number',
|
||||
aggregatable: true,
|
||||
searchable: true,
|
||||
timeSeriesMetric: 'counter',
|
||||
},
|
||||
])
|
||||
),
|
||||
docLinks
|
||||
);
|
||||
expect(warnings).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import { TimeRange } from '@kbn/es-query';
|
|||
import { EuiLink, EuiSpacer, EuiText } from '@elastic/eui';
|
||||
|
||||
import type { DatatableColumn } from '@kbn/expressions-plugin/common';
|
||||
import { groupBy, escape, uniq } from 'lodash';
|
||||
import { groupBy, escape, uniq, uniqBy } from 'lodash';
|
||||
import type { Query } from '@kbn/data-plugin/common';
|
||||
import { SearchRequest } from '@kbn/data-plugin/common';
|
||||
|
||||
|
|
@ -40,16 +40,19 @@ import type { ReferenceBasedIndexPatternColumn } from './operations/definitions/
|
|||
|
||||
import {
|
||||
operationDefinitionMap,
|
||||
GenericIndexPatternColumn,
|
||||
TermsIndexPatternColumn,
|
||||
CountIndexPatternColumn,
|
||||
getReferenceRoot,
|
||||
updateColumnParam,
|
||||
updateDefaultLabels,
|
||||
RangeIndexPatternColumn,
|
||||
FormulaIndexPatternColumn,
|
||||
DateHistogramIndexPatternColumn,
|
||||
MaxIndexPatternColumn,
|
||||
MinIndexPatternColumn,
|
||||
type GenericIndexPatternColumn,
|
||||
type TermsIndexPatternColumn,
|
||||
type CountIndexPatternColumn,
|
||||
type RangeIndexPatternColumn,
|
||||
type FormulaIndexPatternColumn,
|
||||
type DateHistogramIndexPatternColumn,
|
||||
type MaxIndexPatternColumn,
|
||||
type MinIndexPatternColumn,
|
||||
type GenericOperationDefinition,
|
||||
type FieldBasedIndexPatternColumn,
|
||||
} from './operations';
|
||||
|
||||
import { getInvalidFieldMessage, isColumnOfType } from './operations/definitions/helpers';
|
||||
|
|
@ -338,6 +341,113 @@ export function getShardFailuresWarningMessages(
|
|||
return [];
|
||||
}
|
||||
|
||||
export function getUnsupportedOperationsWarningMessage(
|
||||
state: FormBasedPrivateState,
|
||||
{ dataViews }: FramePublicAPI,
|
||||
docLinks: DocLinksStart
|
||||
) {
|
||||
const warningMessages: UserMessage[] = [];
|
||||
const columnsWithUnsupportedOperations: Array<
|
||||
[FieldBasedIndexPatternColumn, ReferenceBasedIndexPatternColumn | undefined]
|
||||
> = Object.values(state.layers)
|
||||
// filter layers without dataView loaded yet
|
||||
.filter(({ indexPatternId }) => dataViews.indexPatterns[indexPatternId])
|
||||
.flatMap((layer) => {
|
||||
const dataView = dataViews.indexPatterns[layer.indexPatternId];
|
||||
const columnsEntries = Object.entries(layer.columns);
|
||||
return columnsEntries
|
||||
.filter(([_, column]) => {
|
||||
if (!hasField(column)) {
|
||||
return false;
|
||||
}
|
||||
const field = dataView.getFieldByName(column.sourceField);
|
||||
if (!field) {
|
||||
return false;
|
||||
}
|
||||
return (
|
||||
!(
|
||||
operationDefinitionMap[column.operationType] as Extract<
|
||||
GenericOperationDefinition,
|
||||
{ input: 'field' }
|
||||
>
|
||||
).getPossibleOperationForField(field) && field?.timeSeriesMetric === 'counter'
|
||||
);
|
||||
})
|
||||
.map(
|
||||
([id, fieldColumn]) =>
|
||||
[fieldColumn, layer.columns[getReferenceRoot(layer, id)]] as [
|
||||
FieldBasedIndexPatternColumn,
|
||||
ReferenceBasedIndexPatternColumn | undefined
|
||||
]
|
||||
);
|
||||
});
|
||||
if (columnsWithUnsupportedOperations.length) {
|
||||
// group the columns by field
|
||||
// then group together columns of a formula/referenced operation who use the same field
|
||||
const columnsGroupedByField = Object.values(
|
||||
groupBy(columnsWithUnsupportedOperations, ([column]) => column.sourceField)
|
||||
).map((columnsList) => uniqBy(columnsList, ([column, rootColumn]) => rootColumn ?? column));
|
||||
|
||||
for (const columnsGrouped of columnsGroupedByField) {
|
||||
const sourceField = columnsGrouped[0][0].sourceField;
|
||||
warningMessages.push({
|
||||
severity: 'warning',
|
||||
fixableInEditor: false,
|
||||
displayLocations: [{ id: 'toolbar' }, { id: 'embeddableBadge' }],
|
||||
shortMessage: i18n.translate(
|
||||
'xpack.lens.indexPattern.tsdbErrorWarning.unsupportedCounterOperationErrorWarning.shortMessage',
|
||||
{
|
||||
defaultMessage:
|
||||
'The result of {count} {count, plural, one {operation} other {operations}} might be meaningless for {field}: {operations}',
|
||||
values: {
|
||||
count: columnsGrouped.length,
|
||||
operations: columnsGrouped
|
||||
.map(([affectedColumn, rootColumn]) => (rootColumn ?? affectedColumn).label)
|
||||
.join(', '),
|
||||
field: sourceField,
|
||||
},
|
||||
}
|
||||
),
|
||||
longMessage: (
|
||||
<>
|
||||
<FormattedMessage
|
||||
id="xpack.lens.indexPattern.unsupportedCounterOperationErrorWarning"
|
||||
defaultMessage="While {count} {count, plural, one {operation} other {operations}} for {field} {count, plural, one {is} other {are}} allowed the result might be meaningless: {operations}. To learn more about this, {link}."
|
||||
values={{
|
||||
count: columnsGrouped.length,
|
||||
operations: (
|
||||
<>
|
||||
{columnsGrouped.map(([affectedColumn, rootColumn], i) => (
|
||||
<React.Fragment key={(rootColumn ?? affectedColumn).label}>
|
||||
<strong>{(rootColumn ?? affectedColumn).label}</strong>
|
||||
{i < columnsGrouped.length - 1 ? ', ' : ''}
|
||||
</React.Fragment>
|
||||
))}
|
||||
</>
|
||||
),
|
||||
field: sourceField,
|
||||
link: (
|
||||
<EuiLink
|
||||
href={docLinks.links.fleet.datastreamsTSDSMetrics}
|
||||
target="_blank"
|
||||
external={true}
|
||||
>
|
||||
<FormattedMessage
|
||||
defaultMessage="visit the Time series documentation"
|
||||
id="xpack.lens.indexPattern.unsupportedCounterOperationErrorWarning.link"
|
||||
/>
|
||||
</EuiLink>
|
||||
),
|
||||
}}
|
||||
/>
|
||||
</>
|
||||
),
|
||||
});
|
||||
}
|
||||
}
|
||||
return warningMessages;
|
||||
}
|
||||
|
||||
export function getPrecisionErrorWarningMessages(
|
||||
datatableUtilities: DatatableUtilitiesService,
|
||||
state: FormBasedPrivateState,
|
||||
|
|
@ -349,18 +459,18 @@ export function getPrecisionErrorWarningMessages(
|
|||
|
||||
if (state && activeData) {
|
||||
Object.entries(activeData)
|
||||
.reduce(
|
||||
(acc, [layerId, { columns }]) => [
|
||||
...acc,
|
||||
...columns.map((column) => ({ layerId, column })),
|
||||
],
|
||||
[] as Array<{ layerId: string; column: DatatableColumn }>
|
||||
)
|
||||
.reduce((acc, [layerId, { columns }]) => {
|
||||
acc.push(...columns.map((column) => ({ layerId, column })));
|
||||
return acc;
|
||||
}, [] as Array<{ layerId: string; column: DatatableColumn }>)
|
||||
.forEach(({ layerId, column }) => {
|
||||
const currentLayer = state.layers[layerId];
|
||||
const currentColumn = currentLayer?.columns[column.id];
|
||||
if (currentLayer && currentColumn && datatableUtilities.hasPrecisionError(column)) {
|
||||
const indexPattern = dataViews.indexPatterns[currentLayer.indexPatternId];
|
||||
if (!indexPattern) {
|
||||
return;
|
||||
}
|
||||
// currentColumnIsTerms is mostly a type guard. If there's a precision error,
|
||||
// we already know that we're dealing with a terms-based operation (at least for now).
|
||||
const currentColumnIsTerms = isColumnOfType<TermsIndexPatternColumn>(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue