Skip to content

Commit 92abf6c

Browse files
authored
[Lens][ES|QL] Fix splitValue nullability coercion when constructing ColorSeries (elastic#262217)
Fixes elastic#255333. ## Summary The reported bug was a combination of two problems in getColorSeriesAccessorFn(): 1. Missing raw values were being lost during the formatted-to-raw lookup. The code used: `rawValueMap.get(splitValue) ?? splitValue` although for a missing split value, the map can legitimately contain: `key: '(null)'`,`value: undefined`. Using `??` throws that `undefined` away and falls back to the formatted string `'(null)'`. 2. Multi-field keys were then built from those raw/formatted values directly. But `getColorCategories()` builds category keys using `getValueKey()`, where missing values become `__missing__`. This PR preserves mapped missing raw values using `Map.has()` instead of `??`, and normalizes each split component with `getValueKey()` before creating the `MultiFieldKey`. |Before|After| |:--:|:--:| |<img width="1260" height="997" alt="image" src="https://github.com/user-attachments/assets/0bb778fe-eb58-4ddb-ac59-b8b3ded69530" />|<img width="1256" height="998" alt="image" src="https://github.com/user-attachments/assets/2b14412f-458e-4a92-9a13-1bc1d924c158" />|
1 parent 4d6fd17 commit 92abf6c

2 files changed

Lines changed: 119 additions & 14 deletions

File tree

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
import type { XYChartSeriesIdentifier } from '@elastic/charts';
11+
import { getColorCategories } from '@kbn/chart-expressions-common';
12+
import { DEFAULT_COLOR_MAPPING_CONFIG } from '@kbn/coloring';
13+
import type { RawValue } from '@kbn/data-plugin/common';
14+
import { KbnPalette, getKbnPalettes } from '@kbn/palettes';
15+
import type { DatatableRow } from '@kbn/expressions-plugin/common';
16+
import type { InvertedRawValueMap } from '../data_layers';
17+
import { getColorSeriesAccessorFn } from './color_mapping_accessor';
18+
19+
const createSeriesIdentifier = (agentName: string, ownerLabel: string): XYChartSeriesIdentifier =>
20+
({
21+
key: `${agentName}-${ownerLabel}`,
22+
specId: 'spec',
23+
yAccessor: 'metric',
24+
splitAccessors: new Map([
25+
['agent.name', agentName],
26+
['aspnetcore.memory_pool.owner', ownerLabel],
27+
]),
28+
seriesKeys: [agentName, ownerLabel],
29+
} as unknown as XYChartSeriesIdentifier);
30+
31+
describe('getColorSeriesAccessorFn', () => {
32+
it('uses palette colors for multi-split missing values mapped from formatted null labels', () => {
33+
const palettes = getKbnPalettes({ name: 'amsterdam', darkMode: false });
34+
const defaultPaletteColors = palettes
35+
.get(KbnPalette.Default)
36+
.colors()
37+
.map((color) => color.toLowerCase());
38+
const neutralPaletteColors = palettes
39+
.get(KbnPalette.Neutral)
40+
.colors()
41+
.map((color) => color.toLowerCase());
42+
const neutralFallbackColor = neutralPaletteColors[1];
43+
44+
const rows: DatatableRow[] = [
45+
{
46+
'agent.name': 'mysql-integrations',
47+
'beat.stats.libbeat.pipeline.queue.filled.events': 8,
48+
},
49+
{
50+
'agent.name': 'apache-integrations',
51+
'beat.stats.libbeat.pipeline.queue.filled.events': 6,
52+
},
53+
];
54+
55+
const invertedRawValueMap: InvertedRawValueMap = new Map<string, Map<string, RawValue>>([
56+
[
57+
'agent.name',
58+
new Map<string, RawValue>([
59+
['mysql-integrations', 'mysql-integrations'],
60+
['apache-integrations', 'apache-integrations'],
61+
]),
62+
],
63+
['aspnetcore.memory_pool.owner', new Map<string, RawValue>([['(null)', undefined]])],
64+
]);
65+
66+
const getColor = getColorSeriesAccessorFn(
67+
DEFAULT_COLOR_MAPPING_CONFIG,
68+
invertedRawValueMap,
69+
palettes,
70+
false,
71+
{
72+
type: 'categories',
73+
categories: getColorCategories(rows, ['agent.name', 'aspnetcore.memory_pool.owner']),
74+
},
75+
['agent.name', 'aspnetcore.memory_pool.owner']
76+
);
77+
78+
const mysqlColor = getColor(createSeriesIdentifier('mysql-integrations', '(null)'));
79+
const apacheColor = getColor(createSeriesIdentifier('apache-integrations', '(null)'));
80+
81+
expect(mysqlColor?.toLowerCase()).toBe(defaultPaletteColors[0]);
82+
expect(apacheColor?.toLowerCase()).toBe(defaultPaletteColors[1]);
83+
expect(mysqlColor?.toLowerCase()).not.toBe(neutralFallbackColor);
84+
expect(apacheColor?.toLowerCase()).not.toBe(neutralFallbackColor);
85+
});
86+
});

src/platform/plugins/shared/chart_expressions/expression_xy/public/helpers/color/color_mapping_accessor.ts

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,32 @@
88
*/
99

1010
import type { SeriesColorAccessorFn } from '@elastic/charts';
11-
import { getColorFactory, type ColorMapping, type ColorMappingInputData } from '@kbn/coloring';
11+
import {
12+
getColorFactory,
13+
getValueKey,
14+
type ColorMapping,
15+
type ColorMappingInputData,
16+
} from '@kbn/coloring';
1217
import type { KbnPalettes } from '@kbn/palettes';
13-
import { MultiFieldKey } from '@kbn/data-plugin/common';
18+
import { MultiFieldKey, type RawValue } from '@kbn/data-plugin/common';
1419
import type { InvertedRawValueMap } from '../data_layers';
1520

1621
/**
1722
* Return a color accessor function for XY charts depending on the split accessors received.
1823
*/
24+
function getRawSplitValue(
25+
fieldId: string,
26+
splitValue: string | number | null | undefined,
27+
invertedRawValueMap: InvertedRawValueMap
28+
): RawValue {
29+
if (typeof splitValue !== 'string') {
30+
return splitValue;
31+
}
32+
33+
const rawValueMap = invertedRawValueMap.get(fieldId) ?? new Map<string, RawValue>();
34+
return rawValueMap.has(splitValue) ? rawValueMap.get(splitValue) : splitValue;
35+
}
36+
1937
export function getColorSeriesAccessorFn(
2038
config: ColorMapping.Config,
2139
invertedRawValueMap: InvertedRawValueMap,
@@ -30,24 +48,25 @@ export function getColorSeriesAccessorFn(
3048
if (configuredSplitAccessors.length > 1) {
3149
// if we have more then 1 split accessor we are in an ESQL multi-term context
3250
// we need to reconstruct back the MultiFieldKey from the original set of raw values to get the right color
33-
const rawValues = configuredSplitAccessors.map((fieldId) => {
34-
const splitValue = splitAccessors.get(fieldId);
35-
if (splitValue === undefined) return null;
36-
const rawValueMap = invertedRawValueMap.get(fieldId) ?? new Map<string, unknown>();
37-
return typeof splitValue === 'string'
38-
? rawValueMap.get(splitValue) ?? splitValue
39-
: splitValue;
40-
});
41-
return getColor(new MultiFieldKey({ key: rawValues }));
51+
const rawValues = configuredSplitAccessors
52+
.map((fieldId) => {
53+
const splitValue = splitAccessors.get(fieldId);
54+
if (splitValue === undefined) return null;
55+
return getRawSplitValue(fieldId, splitValue, invertedRawValueMap);
56+
})
57+
.map((raw) => getValueKey(raw));
58+
return getColor(
59+
new MultiFieldKey({
60+
key: rawValues,
61+
})
62+
);
4263
}
4364

4465
const fieldId = configuredSplitAccessors[0];
4566
const splitValue = splitAccessors.get(fieldId);
4667
// No category associated in the split accessor, use the default color
4768
if (splitValue === undefined) return null;
48-
const rawValueMap = invertedRawValueMap.get(fieldId) ?? new Map<string, unknown>();
49-
const rawValue =
50-
typeof splitValue === 'string' ? rawValueMap.get(splitValue) ?? splitValue : splitValue;
69+
const rawValue = getRawSplitValue(fieldId, splitValue, invertedRawValueMap);
5170

5271
return getColor(rawValue);
5372
};

0 commit comments

Comments
 (0)