Skip to content

Commit

Permalink
Fix handling of special characters in categorical values (#757)
Browse files Browse the repository at this point in the history
This PR addresses a bug that occurred when categorical values contained special characters such as colons. Previously, entities were stored as a comma-separated name-value pair string in the heatmap cell's custom data field. When a cell was clicked, the custom data was retrieved and the string was split via colons. This caused errors if the categorical value itself contained colons or if multiple name-value pairs included commas.

To fix this issue, this PR changes the storage format to a JSON string in the cell's custom data. When a cell is clicked, the JSON object is restored. Since JSON naturally handles special characters, this eliminates the need for additional special character handling.

Additionally, the previous implementation stored a newline-separated name-value pair in the y-axis and a comma-separated name-value pair in the cell tooltip. With entities now stored as JSON in custom data, we no longer have the flexibility to store these different formats. This PR standardizes the format to newline-separated name-value pairs for both the y-axis and cell tooltip.

Testing Done:
* Conducted end-to-end testing in preview, historical, and real-time use cases.
* Added unit tests.

Signed-off-by: Kaituo Li <[email protected]>
  • Loading branch information
kaituo authored May 17, 2024
1 parent e509a03 commit 9cb2b00
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
getAnomalySummary,
convertAlerts,
generateAlertAnnotations,
buildHeatmapPlotData,
ANOMALY_HEATMAP_COLORSCALE,
} from '../anomalyChartUtils';
import { httpClientMock, coreServicesMock } from '../../../../../test/mocks';
import { MonitorAlert } from '../../../../models/interfaces';
Expand Down Expand Up @@ -158,3 +160,91 @@ describe('anomalyChartUtils function tests', () => {
]);
});
});


describe('buildHeatmapPlotData', () => {
it('should build the heatmap plot data correctly', () => {
const x = ['05-13 06:58:52 2024'];
const y = ['Exception while fetching data\napp_2'];
const z = [0.1];
const anomalyOccurrences = [1];
const entityLists = [
[
{ name: 'error', value: 'Exception while fetching data' },
{ name: 'service', value: 'app_2' },
],
];
const cellTimeInterval = 10;

const expected = {
x: x,
y: y,
z: z,
colorscale: ANOMALY_HEATMAP_COLORSCALE,
zmin: 0,
zmax: 1,
type: 'heatmap',
showscale: false,
xgap: 2,
ygap: 2,
opacity: 1,
text: anomalyOccurrences,
customdata: entityLists,
hovertemplate:
'<b>Entities</b>: %{y}<br>' +
'<b>Time</b>: %{x}<br>' +
'<b>Max anomaly grade</b>: %{z}<br>' +
'<b>Anomaly occurrences</b>: %{text}' +
'<extra></extra>',
cellTimeInterval: cellTimeInterval,
};

const result = buildHeatmapPlotData(x, y, z, anomalyOccurrences, entityLists, cellTimeInterval);
expect(result).toEqual(expected);
});

it('should handle multiple entries correctly', () => {
const x = ['05-13 06:58:52 2024', '05-13 07:58:52 2024'];
const y = ['Exception while fetching data\napp_2', 'Network error\napp_3'];
const z = [0.1, 0.2];
const anomalyOccurrences = [1, 2];
const entityLists = [
[
{ name: 'error', value: 'Exception while fetching data' },
{ name: 'service', value: 'app_2' },
],
[
{ name: 'error', value: 'Network error' },
{ name: 'service', value: 'app_3' },
],
];
const cellTimeInterval = 10;

const expected = {
x: x,
y: y,
z: z,
colorscale: ANOMALY_HEATMAP_COLORSCALE,
zmin: 0,
zmax: 1,
type: 'heatmap',
showscale: false,
xgap: 2,
ygap: 2,
opacity: 1,
text: anomalyOccurrences,
customdata: entityLists,
hovertemplate:
'<b>Entities</b>: %{y}<br>' +
'<b>Time</b>: %{x}<br>' +
'<b>Max anomaly grade</b>: %{z}<br>' +
'<b>Anomaly occurrences</b>: %{text}' +
'<extra></extra>',
cellTimeInterval: cellTimeInterval,
};

const result = buildHeatmapPlotData(x, y, z, anomalyOccurrences, entityLists, cellTimeInterval);
expect(result).toEqual(expected);
});
});

16 changes: 14 additions & 2 deletions public/pages/AnomalyCharts/utils/anomalyChartUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,19 @@ export const getSampleAnomaliesHeatmapData = (
return [resultPlotData];
};

const buildHeatmapPlotData = (

/**
* Builds the data for a heatmap plot representing anomalies.
*
* @param {any[]} x - The x coordinate value for the cell representing time.
* @param {any[]} y - Array of newline-separated name-value pairs representing entities. This is used for the y-axis labels and displayed in the mouse hover tooltip.
* @param {any[]} z - Array representing the maximum anomaly grades.
* @param {any[]} anomalyOccurrences - Array representing the number of anomalies.
* @param {any[]} entityLists - JSON representation of name-value pairs. Note that the values may contain special characters such as commas and newlines. JSON is used here because it naturally handles special characters and nested structures.
* @param {number} cellTimeInterval - The interval covered by each heatmap cell.
* @returns {PlotData} - The data structure required for plotting the heatmap.
*/
export const buildHeatmapPlotData = (
x: any[],
y: any[],
z: any[],
Expand All @@ -334,7 +346,7 @@ const buildHeatmapPlotData = (
text: anomalyOccurrences,
customdata: entityLists,
hovertemplate:
'<b>Entities</b>: %{customdata}<br>' +
'<b>Entities</b>: %{y}<br>' +
'<b>Time</b>: %{x}<br>' +
'<b>Max anomaly grade</b>: %{z}<br>' +
'<b>Anomaly occurrences</b>: %{text}' +
Expand Down
54 changes: 54 additions & 0 deletions public/pages/utils/__tests__/anomalyResultUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
getFeatureDataPointsForDetector,
parsePureAnomalies,
buildParamsForGetAnomalyResultsWithDateRange,
transformEntityListsForHeatmap,
convertHeatmapCellEntityStringToEntityList,
} from '../anomalyResultUtils';
import { getRandomDetector } from '../../../redux/reducers/__tests__/utils';
import {
Expand All @@ -25,6 +27,8 @@ import {
import { ANOMALY_RESULT_SUMMARY, PARSED_ANOMALIES } from './constants';
import { MAX_ANOMALIES } from '../../../utils/constants';
import { SORT_DIRECTION, AD_DOC_FIELDS } from '../../../../server/utils/constants';
import { Entity } from '../../../../server/models/interfaces';
import { NUM_CELLS } from '../../AnomalyCharts/utils/anomalyChartUtils'

describe('anomalyResultUtils', () => {
let randomDetector_20_min: Detector;
Expand Down Expand Up @@ -636,4 +640,54 @@ describe('anomalyResultUtils', () => {
expect(parsedPureAnomalies).toStrictEqual(PARSED_ANOMALIES);
});
});

describe('transformEntityListsForHeatmap', () => {
it('should transform an empty entityLists array to an empty array', () => {
const entityLists: Entity[][] = [];
const result = transformEntityListsForHeatmap(entityLists);
expect(result).toEqual([]);
const convertedBack = convertHeatmapCellEntityStringToEntityList("[]");
expect([]).toEqual(convertedBack);
});

it('should transform a single entity list correctly', () => {
const entityLists: Entity[][] = [
[
{ name: 'entity1', value: 'value1' },
{ name: 'entity2', value: 'value2' },
],
];

const json = JSON.stringify(entityLists[0]);

const expected = [
new Array(NUM_CELLS).fill(json),
];

const result = transformEntityListsForHeatmap(entityLists);
expect(result).toEqual(expected);
const convertedBack = convertHeatmapCellEntityStringToEntityList(json);
expect(entityLists[0]).toEqual(convertedBack);
});

it('should handle special characters in entity values', () => {
const entityLists: Entity[][] = [
[
{ name: 'entity1', value: 'value1, with comma' },
{ name: 'entity2', value: 'value2\nwith newline' },
],
];

const json = JSON.stringify(entityLists[0]);

const expected = [
new Array(NUM_CELLS).fill(json),
];

const result = transformEntityListsForHeatmap(entityLists);
expect(result).toEqual(expected);
const convertedBack = convertHeatmapCellEntityStringToEntityList(json);
expect(entityLists[0]).toEqual(convertedBack);
});
});
});
22 changes: 2 additions & 20 deletions public/pages/utils/anomalyResultUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1820,22 +1820,7 @@ export const convertToCategoryFieldAndEntityString = (
export const convertHeatmapCellEntityStringToEntityList = (
heatmapCellEntityString: string
) => {
let entityList = [] as Entity[];
const entitiesAsStringList = heatmapCellEntityString.split(
HEATMAP_CELL_ENTITY_DELIMITER
);
var i;
for (i = 0; i < entitiesAsStringList.length; i++) {
const entityAsString = entitiesAsStringList[i];
const entityAsFieldValuePair = entityAsString.split(
HEATMAP_CALL_ENTITY_KEY_VALUE_DELIMITER
);
entityList.push({
name: entityAsFieldValuePair[0],
value: entityAsFieldValuePair[1],
});
}
return entityList;
return JSON.parse(heatmapCellEntityString);
};

export const entityListsMatch = (
Expand Down Expand Up @@ -1895,10 +1880,7 @@ const appendEntityFilters = (requestBody: any, entityList: Entity[]) => {
export const transformEntityListsForHeatmap = (entityLists: any[]) => {
let transformedEntityLists = [] as any[];
entityLists.forEach((entityList: Entity[]) => {
const listAsString = convertToCategoryFieldAndEntityString(
entityList,
', '
);
const listAsString = JSON.stringify(entityList);
let row = [];
var i;
for (i = 0; i < NUM_CELLS; i++) {
Expand Down

0 comments on commit 9cb2b00

Please sign in to comment.