Skip to content

Commit

Permalink
[ML] Anomaly Detection: Consolidate severity colors (elastic#204803)
Browse files Browse the repository at this point in the history
## Summary

Part of elastic#140695.

We defined the hex codes for anomaly detection severity colors in
several places. This PR consolidates this and makes sure the hex codes
are defined in just one place. Note this PR doesn't change any of the
colors or styling related to anomaly detection, it is pure refactoring
to make future updates to these colors more easy.

- Renames `BLANK` to `UNKNOWN` to be in line with severity labels.
- Uses `ML_SEVERITY_COLORS.*` in test assertions so future color changes
won't need updating every assertion.
- Migrates all SCSS that made use of severity colors to emotion so it
can reuse `ML_SEVERITY_COLORS`. Therefor the SCSS based severity colors
get removed in this PR.

### Checklist

- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
walterra authored and CAWilson94 committed Jan 10, 2025
1 parent 6fedf7d commit adc4eda
Show file tree
Hide file tree
Showing 24 changed files with 222 additions and 248 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,37 @@
*/

import { getSeverityColor } from './get_severity_color';
import { ML_SEVERITY_COLORS } from './severity_colors';

describe('getSeverityColor', () => {
test('returns correct hex code for low for 0 <= score < 3', () => {
expect(getSeverityColor(0)).toBe('#d2e9f7');
expect(getSeverityColor(0.001)).toBe('#d2e9f7');
expect(getSeverityColor(2.99)).toBe('#d2e9f7');
expect(getSeverityColor(0)).toBe(ML_SEVERITY_COLORS.LOW);
expect(getSeverityColor(0.001)).toBe(ML_SEVERITY_COLORS.LOW);
expect(getSeverityColor(2.99)).toBe(ML_SEVERITY_COLORS.LOW);
});

test('returns correct hex code for warning for 3 <= score < 25', () => {
expect(getSeverityColor(3)).toBe('#8bc8fb');
expect(getSeverityColor(24.99)).toBe('#8bc8fb');
expect(getSeverityColor(3)).toBe(ML_SEVERITY_COLORS.WARNING);
expect(getSeverityColor(24.99)).toBe(ML_SEVERITY_COLORS.WARNING);
});

test('returns correct hex code for minor for 25 <= score < 50', () => {
expect(getSeverityColor(25)).toBe('#fdec25');
expect(getSeverityColor(49.99)).toBe('#fdec25');
expect(getSeverityColor(25)).toBe(ML_SEVERITY_COLORS.MINOR);
expect(getSeverityColor(49.99)).toBe(ML_SEVERITY_COLORS.MINOR);
});

test('returns correct hex code for major for 50 <= score < 75', () => {
expect(getSeverityColor(50)).toBe('#fba740');
expect(getSeverityColor(74.99)).toBe('#fba740');
expect(getSeverityColor(50)).toBe(ML_SEVERITY_COLORS.MAJOR);
expect(getSeverityColor(74.99)).toBe(ML_SEVERITY_COLORS.MAJOR);
});

test('returns correct hex code for critical for score >= 75', () => {
expect(getSeverityColor(75)).toBe('#fe5050');
expect(getSeverityColor(100)).toBe('#fe5050');
expect(getSeverityColor(1000)).toBe('#fe5050');
expect(getSeverityColor(75)).toBe(ML_SEVERITY_COLORS.CRITICAL);
expect(getSeverityColor(100)).toBe(ML_SEVERITY_COLORS.CRITICAL);
expect(getSeverityColor(1000)).toBe(ML_SEVERITY_COLORS.CRITICAL);
});

test('returns correct hex code for unknown for scores less than 0', () => {
expect(getSeverityColor(-10)).toBe('#ffffff');
expect(getSeverityColor(-10)).toBe(ML_SEVERITY_COLORS.UNKNOWN);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ML_ANOMALY_THRESHOLD } from './anomaly_threshold';
import { ML_SEVERITY_COLORS } from './severity_colors';

/**
* Returns a severity RGB color (one of critical, major, minor, warning, low or blank)
* Returns a severity RGB color (one of critical, major, minor, warning, low or unknown)
* for the supplied normalized anomaly score (a value between 0 and 100).
* @param normalizedScore - A normalized score between 0-100, which is based on the probability of the anomalousness of this record
*/
Expand All @@ -25,6 +25,6 @@ export function getSeverityColor(normalizedScore: number): string {
} else if (normalizedScore >= ML_ANOMALY_THRESHOLD.LOW) {
return ML_SEVERITY_COLORS.LOW;
} else {
return ML_SEVERITY_COLORS.BLANK;
return ML_SEVERITY_COLORS.UNKNOWN;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,5 @@ export const ML_SEVERITY_COLORS = {
/**
* Color used in the UI to indicate an anomaly for which the score is unknown.
*/
BLANK: '#ffffff',
UNKNOWN: '#ffffff',
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@
* 2.0.
*/

import React from 'react';

import { EuiHealth } from '@elastic/eui';

import { type FieldFormatsRegistry } from '@kbn/field-formats-plugin/common';
import { fieldFormatsServiceMock } from '@kbn/field-formats-plugin/public/mocks';
import React from 'react';
import { ML_SEVERITY_COLORS } from '@kbn/ml-anomaly-utils';

import { ALERT_ANOMALY_SCORE } from '../../../common/constants/alerts';

import { getAlertFormatters } from './render_cell_value';

describe('getAlertFormatters', () => {
Expand All @@ -19,31 +24,31 @@ describe('getAlertFormatters', () => {

test('format anomaly score correctly', () => {
expect(alertFormatter(ALERT_ANOMALY_SCORE, 50.3)).toEqual(
<EuiHealth color="#fba740" textSize="xs">
<EuiHealth color={ML_SEVERITY_COLORS.MAJOR} textSize="xs">
50
</EuiHealth>
);

expect(alertFormatter(ALERT_ANOMALY_SCORE, '50.3,89.6')).toEqual(
<EuiHealth color="#fe5050" textSize="xs">
<EuiHealth color={ML_SEVERITY_COLORS.CRITICAL} textSize="xs">
89
</EuiHealth>
);

expect(alertFormatter(ALERT_ANOMALY_SCORE, '0.7')).toEqual(
<EuiHealth color="#d2e9f7" textSize="xs">
<EuiHealth color={ML_SEVERITY_COLORS.LOW} textSize="xs">
&lt; 1
</EuiHealth>
);

expect(alertFormatter(ALERT_ANOMALY_SCORE, '0')).toEqual(
<EuiHealth color="#d2e9f7" textSize="xs">
<EuiHealth color={ML_SEVERITY_COLORS.LOW} textSize="xs">
&lt; 1
</EuiHealth>
);

expect(alertFormatter(ALERT_ANOMALY_SCORE, '')).toEqual(
<EuiHealth color="#d2e9f7" textSize="xs">
<EuiHealth color={ML_SEVERITY_COLORS.LOW} textSize="xs">
&lt; 1
</EuiHealth>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// ML has it's own variables for coloring
@import 'variables';

// Protect the rest of Kibana from ML generic namespacing
// SASSTODO: Prefix ml selectors instead
.ml-app {
Expand All @@ -14,4 +11,4 @@
@import 'components/job_selector/index';
@import 'components/rule_editor/index'; // SASSTODO: This file overwrites EUI directly

}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import React from 'react';
import { render } from '@testing-library/react';
import { ML_SEVERITY_COLORS } from '@kbn/ml-anomaly-utils/severity_colors';
import { SeverityCell } from './severity_cell';

describe('SeverityCell', () => {
Expand All @@ -18,7 +19,7 @@ describe('SeverityCell', () => {
const { container } = render(<SeverityCell {...props} />);
expect(container.textContent).toBe('75');
const svgEl = container.querySelectorAll('[data-euiicon-type]')[0];
expect(svgEl && svgEl.getAttribute('color')).toBe('#fe5050');
expect(svgEl && svgEl.getAttribute('color')).toBe(ML_SEVERITY_COLORS.CRITICAL);
});

test('should render a multi-bucket marker with low severity score', () => {
Expand All @@ -29,6 +30,6 @@ describe('SeverityCell', () => {
const { container } = render(<SeverityCell {...props} />);
expect(container.textContent).toBe('< 1');
const svgEl = container.getElementsByTagName('svg').item(0);
expect(svgEl && svgEl.getAttribute('fill')).toBe('#d2e9f7');
expect(svgEl && svgEl.getAttribute('fill')).toBe(ML_SEVERITY_COLORS.LOW);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { css } from '@emotion/react';

import { useEuiFontSize, useEuiTheme } from '@elastic/eui';

import { mlColors } from '../../styles';
import { ML_SEVERITY_COLORS } from '@kbn/ml-anomaly-utils';

export const useInfluencersListStyles = () => {
const { euiTheme } = useEuiTheme();
Expand Down Expand Up @@ -50,12 +50,12 @@ export const useInfluencersListStyles = () => {
width: `${barScore}%`,
backgroundColor:
severity === 'critical'
? mlColors.critical
? ML_SEVERITY_COLORS.CRITICAL
: severity === 'major'
? mlColors.major
? ML_SEVERITY_COLORS.MAJOR
: severity === 'minor'
? mlColors.minor
: mlColors.warning,
? ML_SEVERITY_COLORS.MINOR
: ML_SEVERITY_COLORS.WARNING,
}),
scoreLabel: (severity: string) =>
css({
Expand All @@ -67,12 +67,12 @@ export const useInfluencersListStyles = () => {
display: 'inline',
borderColor:
severity === 'critical'
? mlColors.critical
? ML_SEVERITY_COLORS.CRITICAL
: severity === 'major'
? mlColors.major
? ML_SEVERITY_COLORS.MAJOR
: severity === 'minor'
? mlColors.minor
: mlColors.warning,
? ML_SEVERITY_COLORS.MINOR
: ML_SEVERITY_COLORS.WARNING,
}),
totalScoreLabel: css({
width: euiTheme.size.xl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import React from 'react';
import { i18n } from '@kbn/i18n';
import type { EuiRangeProps } from '@elastic/eui';
import { EuiFieldNumber, EuiFlexGroup, EuiFlexItem, EuiFormRow, EuiRange } from '@elastic/eui';
import { ML_ANOMALY_THRESHOLD } from '@kbn/ml-anomaly-utils';
import { ML_ANOMALY_THRESHOLD, ML_SEVERITY_COLORS } from '@kbn/ml-anomaly-utils';

export interface SeveritySelectorProps {
value: number | undefined;
Expand All @@ -24,22 +24,22 @@ export const SeverityControl: FC<SeveritySelectorProps> = React.memo(({ value, o
{
min: ML_ANOMALY_THRESHOLD.LOW,
max: ML_ANOMALY_THRESHOLD.MINOR,
color: '#8BC8FB',
color: ML_SEVERITY_COLORS.WARNING,
},
{
min: ML_ANOMALY_THRESHOLD.MINOR,
max: ML_ANOMALY_THRESHOLD.MAJOR,
color: '#FDEC25',
color: ML_SEVERITY_COLORS.MINOR,
},
{
min: ML_ANOMALY_THRESHOLD.MAJOR,
max: ML_ANOMALY_THRESHOLD.CRITICAL,
color: '#FBA740',
color: ML_SEVERITY_COLORS.MAJOR,
},
{
min: ML_ANOMALY_THRESHOLD.CRITICAL,
max: MAX_ANOMALY_SCORE,
color: '#FE5050',
color: ML_SEVERITY_COLORS.CRITICAL,
},
];

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
@import '../../../application/variables';
@import 'components/explorer_chart_label/index';
@import 'explorer_chart';
Loading

0 comments on commit adc4eda

Please sign in to comment.