mirror of
https://github.com/hyperdxio/hyperdx
synced 2026-04-21 13:37:15 +00:00
improve alert precision to match the threshold value (#1387)
Users reported that the precision was way off to what the threshold value was, this helps ensure the two numbers have the same precision. Before: <img width="1280" height="363" alt="image" src="https://github.com/user-attachments/assets/fc1bc72c-a70e-4068-aa06-3a01d6c65b2b" /> After: <img width="1446" height="618" alt="Screenshot 2025-11-19 at 4 20 38 PM" src="https://github.com/user-attachments/assets/49be78eb-dac9-49f4-b490-a354fb69fb71" /> **Note:** One thing that could be better is if we instead used the Number Format specified on the frontend, this would require us to move the Numbro dependency and logic into common-utils, and we would also probably want to update the alert value UI to also use numbro.. I can take a stab at this if we think it's better. I figured this was a good interim solution. Fixes HDX-2847
This commit is contained in:
parent
562dd7ea28
commit
e838436d20
3 changed files with 183 additions and 5 deletions
5
.changeset/new-scissors-complain.md
Normal file
5
.changeset/new-scissors-complain.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@hyperdx/api": patch
|
||||
---
|
||||
|
||||
Improve value rounding on alerts to match thresholds
|
||||
|
|
@ -42,6 +42,7 @@ import {
|
|||
AlertMessageTemplateDefaultView,
|
||||
buildAlertMessageTemplateHdxLink,
|
||||
buildAlertMessageTemplateTitle,
|
||||
formatValueToMatchThreshold,
|
||||
getDefaultExternalAction,
|
||||
isAlertResolved,
|
||||
renderAlertTemplate,
|
||||
|
|
@ -225,6 +226,93 @@ describe('checkAlerts', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('formatValueToMatchThreshold', () => {
|
||||
// Test with integer threshold - value should be formatted as integer
|
||||
expect(formatValueToMatchThreshold(1111.11111111, 1)).toBe('1111');
|
||||
expect(formatValueToMatchThreshold(5, 1)).toBe('5');
|
||||
expect(formatValueToMatchThreshold(5.9, 1)).toBe('6');
|
||||
|
||||
// Test scientific notation threshold - value should be formatted as integer
|
||||
expect(formatValueToMatchThreshold(0.00001, 0.0000001)).toBe('0.0000100');
|
||||
|
||||
// Test with single decimal threshold - value should have 1 decimal place
|
||||
expect(formatValueToMatchThreshold(1111.11111111, 1.5)).toBe('1111.1');
|
||||
expect(formatValueToMatchThreshold(5.555, 1.5)).toBe('5.6');
|
||||
|
||||
// Test with multiple decimal places in threshold
|
||||
expect(formatValueToMatchThreshold(1.1234, 0.1234)).toBe('1.1234');
|
||||
expect(formatValueToMatchThreshold(5.123456789, 0.1234)).toBe('5.1235');
|
||||
expect(formatValueToMatchThreshold(10, 0.12)).toBe('10.00');
|
||||
|
||||
// Test with very long decimal threshold
|
||||
expect(formatValueToMatchThreshold(1111.11111111, 0.123456)).toBe(
|
||||
'1111.111111',
|
||||
);
|
||||
|
||||
// Test edge cases
|
||||
expect(formatValueToMatchThreshold(0, 1)).toBe('0');
|
||||
expect(formatValueToMatchThreshold(0.5, 1)).toBe('1');
|
||||
expect(formatValueToMatchThreshold(0.123456, 0.1234)).toBe('0.1235');
|
||||
|
||||
// Test negative values
|
||||
expect(formatValueToMatchThreshold(-5.555, 1.5)).toBe('-5.6');
|
||||
expect(formatValueToMatchThreshold(-1111.11111111, 1)).toBe('-1111');
|
||||
|
||||
// Test when value is already an integer and threshold is integer
|
||||
expect(formatValueToMatchThreshold(100, 50)).toBe('100');
|
||||
expect(formatValueToMatchThreshold(0, 0)).toBe('0');
|
||||
|
||||
// Test rounding behavior
|
||||
expect(formatValueToMatchThreshold(1.5, 0.1)).toBe('1.5');
|
||||
expect(formatValueToMatchThreshold(1.55, 0.1)).toBe('1.6');
|
||||
expect(formatValueToMatchThreshold(1.449, 0.1)).toBe('1.4');
|
||||
|
||||
// Test very large numbers (main benefit of NumberFormat over toFixed)
|
||||
expect(formatValueToMatchThreshold(9999999999999.5, 1)).toBe(
|
||||
'10000000000000',
|
||||
);
|
||||
expect(formatValueToMatchThreshold(1234567890123.456, 0.1)).toBe(
|
||||
'1234567890123.5',
|
||||
);
|
||||
expect(formatValueToMatchThreshold(999999999999999, 1)).toBe(
|
||||
'999999999999999',
|
||||
);
|
||||
|
||||
// Test that thousand separators are NOT added
|
||||
expect(formatValueToMatchThreshold(123456.789, 1)).toBe('123457');
|
||||
expect(formatValueToMatchThreshold(1000000.5, 0.1)).toBe('1000000.5');
|
||||
|
||||
// Test precision at JavaScript's safe integer boundary
|
||||
expect(formatValueToMatchThreshold(9007199254740991, 1)).toBe(
|
||||
'9007199254740991',
|
||||
);
|
||||
|
||||
// Test very small numbers in different notations
|
||||
expect(formatValueToMatchThreshold(0.000000001, 0.0000000001)).toBe(
|
||||
'0.0000000010',
|
||||
);
|
||||
expect(formatValueToMatchThreshold(1.23e-8, 1e-9)).toBe('0.000000012');
|
||||
|
||||
// Test mixed magnitude (large value with small precision threshold)
|
||||
expect(formatValueToMatchThreshold(1000000.123456, 0.0001)).toBe(
|
||||
'1000000.1235',
|
||||
);
|
||||
expect(formatValueToMatchThreshold(99999.999999, 0.01)).toBe('100000.00');
|
||||
|
||||
// Test threshold with trailing zeros vs without
|
||||
expect(formatValueToMatchThreshold(5.5, 1.0)).toBe('6'); // 1.0 should be treated as integer
|
||||
expect(formatValueToMatchThreshold(5.55, 0.1)).toBe('5.6'); // 0.10 has 1 decimal place
|
||||
|
||||
// Test edge case: very small threshold, large value
|
||||
expect(formatValueToMatchThreshold(1234567.89, 0.000001)).toBe(
|
||||
'1234567.890000',
|
||||
);
|
||||
|
||||
// Test rounding at different magnitudes
|
||||
expect(formatValueToMatchThreshold(999.9999, 0.001)).toBe('1000.000');
|
||||
expect(formatValueToMatchThreshold(0.9999, 0.001)).toBe('1.000');
|
||||
});
|
||||
|
||||
it('buildAlertMessageTemplateTitle', () => {
|
||||
expect(
|
||||
buildAlertMessageTemplateTitle({
|
||||
|
|
@ -280,6 +368,62 @@ describe('checkAlerts', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('buildAlertMessageTemplateTitle formats value to match threshold precision', () => {
|
||||
// Test with decimal threshold - value should be formatted to match
|
||||
const decimalChartView: AlertMessageTemplateDefaultView = {
|
||||
...defaultChartView,
|
||||
alert: {
|
||||
...defaultChartView.alert,
|
||||
threshold: 1.5,
|
||||
},
|
||||
value: 1111.11111111,
|
||||
};
|
||||
|
||||
expect(
|
||||
buildAlertMessageTemplateTitle({
|
||||
view: decimalChartView,
|
||||
}),
|
||||
).toMatchInlineSnapshot(
|
||||
`"🚨 Alert for \\"Test Chart\\" in \\"My Dashboard\\" - 1111.1 exceeds 1.5"`,
|
||||
);
|
||||
|
||||
// Test with multiple decimal places
|
||||
const multiDecimalChartView: AlertMessageTemplateDefaultView = {
|
||||
...defaultChartView,
|
||||
alert: {
|
||||
...defaultChartView.alert,
|
||||
threshold: 0.1234,
|
||||
},
|
||||
value: 1.123456789,
|
||||
};
|
||||
|
||||
expect(
|
||||
buildAlertMessageTemplateTitle({
|
||||
view: multiDecimalChartView,
|
||||
}),
|
||||
).toMatchInlineSnapshot(
|
||||
`"🚨 Alert for \\"Test Chart\\" in \\"My Dashboard\\" - 1.1235 exceeds 0.1234"`,
|
||||
);
|
||||
|
||||
// Test with integer value and decimal threshold
|
||||
const integerValueView: AlertMessageTemplateDefaultView = {
|
||||
...defaultChartView,
|
||||
alert: {
|
||||
...defaultChartView.alert,
|
||||
threshold: 0.12,
|
||||
},
|
||||
value: 10,
|
||||
};
|
||||
|
||||
expect(
|
||||
buildAlertMessageTemplateTitle({
|
||||
view: integerValueView,
|
||||
}),
|
||||
).toMatchInlineSnapshot(
|
||||
`"🚨 Alert for \\"Test Chart\\" in \\"My Dashboard\\" - 10.00 exceeds 0.12"`,
|
||||
);
|
||||
});
|
||||
|
||||
it('isAlertResolved', () => {
|
||||
// Test OK state returns true
|
||||
expect(isAlertResolved(AlertState.OK)).toBe(true);
|
||||
|
|
@ -2199,14 +2343,14 @@ describe('checkAlerts', () => {
|
|||
1,
|
||||
'https://hooks.slack.com/services/123',
|
||||
{
|
||||
text: '🚨 Alert for "CPU" in "My Dashboard" - 6.25 exceeds 1',
|
||||
text: '🚨 Alert for "CPU" in "My Dashboard" - 6 exceeds 1',
|
||||
blocks: [
|
||||
{
|
||||
text: {
|
||||
text: [
|
||||
`*<http://app:8080/dashboards/${dashboard._id}?from=1700170200000&granularity=5+minute&to=1700174700000 | 🚨 Alert for "CPU" in "My Dashboard" - 6.25 exceeds 1>*`,
|
||||
`*<http://app:8080/dashboards/${dashboard._id}?from=1700170200000&granularity=5+minute&to=1700174700000 | 🚨 Alert for "CPU" in "My Dashboard" - 6 exceeds 1>*`,
|
||||
'',
|
||||
'6.25 exceeds 1',
|
||||
'6 exceeds 1',
|
||||
'Time Range (UTC): [Nov 16 10:05:00 PM - Nov 16 10:10:00 PM)',
|
||||
'',
|
||||
].join('\n'),
|
||||
|
|
|
|||
|
|
@ -87,6 +87,33 @@ export const isAlertResolved = (state?: AlertState): boolean => {
|
|||
return state === AlertState.OK;
|
||||
};
|
||||
|
||||
/**
|
||||
* Formats the value to match the decimal precision of the threshold.
|
||||
* This ensures consistent display of numbers in alert messages.
|
||||
* Uses Intl.NumberFormat for better precision handling with large numbers.
|
||||
*/
|
||||
export const formatValueToMatchThreshold = (
|
||||
value: number,
|
||||
threshold: number,
|
||||
): string => {
|
||||
// Format threshold with NumberFormat to get its string representation
|
||||
const thresholdFormatted = new Intl.NumberFormat('en-US', {
|
||||
maximumSignificantDigits: 21,
|
||||
useGrouping: false,
|
||||
}).format(threshold);
|
||||
|
||||
// Count decimal places in the formatted threshold
|
||||
const decimalIndex = thresholdFormatted.indexOf('.');
|
||||
const decimalPlaces =
|
||||
decimalIndex === -1 ? 0 : thresholdFormatted.length - decimalIndex - 1;
|
||||
|
||||
return new Intl.NumberFormat('en-US', {
|
||||
minimumFractionDigits: decimalPlaces,
|
||||
maximumFractionDigits: decimalPlaces,
|
||||
useGrouping: false,
|
||||
}).format(value);
|
||||
};
|
||||
|
||||
export const notifyChannel = async ({
|
||||
channel,
|
||||
message,
|
||||
|
|
@ -339,9 +366,10 @@ export const buildAlertMessageTemplateTitle = ({
|
|||
`Tile with id ${alert.tileId} not found in dashboard ${dashboard.name}`,
|
||||
);
|
||||
}
|
||||
const formattedValue = formatValueToMatchThreshold(value, alert.threshold);
|
||||
const baseTitle = template
|
||||
? handlebars.compile(template)(view)
|
||||
: `Alert for "${tile.config.name}" in "${dashboard.name}" - ${value} ${
|
||||
: `Alert for "${tile.config.name}" in "${dashboard.name}" - ${formattedValue} ${
|
||||
doesExceedThreshold(alert.thresholdType, alert.threshold, value)
|
||||
? alert.thresholdType === AlertThresholdType.ABOVE
|
||||
? 'exceeds'
|
||||
|
|
@ -614,8 +642,9 @@ ${truncatedResults}
|
|||
if (dashboard == null) {
|
||||
throw new Error(`Source is ${alert.source} but dashboard is null`);
|
||||
}
|
||||
const formattedValue = formatValueToMatchThreshold(value, alert.threshold);
|
||||
rawTemplateBody = `${group ? `Group: "${group}"` : ''}
|
||||
${value} ${
|
||||
${formattedValue} ${
|
||||
doesExceedThreshold(alert.thresholdType, alert.threshold, value)
|
||||
? alert.thresholdType === AlertThresholdType.ABOVE
|
||||
? 'exceeds'
|
||||
|
|
|
|||
Loading…
Reference in a new issue