Skip to content

Commit

Permalink
fix(dashboard): analysis modal crashed when value not valid (#3881)
Browse files Browse the repository at this point in the history 
* handle the parse issue
Signed-off-by: ashutosh16 <[email protected]>

* handle the parse error

Signed-off-by: ashutosh16 <[email protected]>

* handle the parse error

Signed-off-by: ashutosh16 <[email protected]>

* handle the parse error

Signed-off-by: ashutosh16 <[email protected]>

---------

Signed-off-by: ashutosh16 <[email protected]>
  • Loading branch information
@ashutosh16
ashutosh16 authored Oct 9, 2024
1 parent d49b3fa commit 4afdb85
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 15 deletions.
72 changes: 66 additions & 6 deletions ui/src/app/components/analysis-modal/transforms.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1CloudWatchMetric,
GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1MetricProvider,
GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1MetricResult,
GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Measurement,
} from '../../../models/rollout/generated';
import {
analysisEndTime,
Expand All @@ -25,6 +26,7 @@ import {
metricSubstatus,
printableCloudWatchQuery,
printableDatadogQuery,
transformMeasurements,
} from './transforms';
import {AnalysisStatus, FunctionalStatus} from './types';

Expand Down Expand Up @@ -376,35 +378,35 @@ describe('analysis modal transforms', () => {
});
test('interpolateQuery() for prometheus query and args', () => {
expect(interpolateQuery(MOCK_QUERY_PROMETHEUS, MOCK_ARGS_PROMETHEUS)).toBe(
'sum(irate(istio_requests_total{reporter="source",destination_service=~"istio-host-split-canary",response_code!~"5.*"}[5m])) / sum(irate(istio_requests_total{reporter="source",destination_service=~"istio-host-split-canary"}[5m]))'
'sum(irate(istio_requests_total{reporter="source",destination_service=~"istio-host-split-canary",response_code!~"5.*"}[5m])) / sum(irate(istio_requests_total{reporter="source",destination_service=~"istio-host-split-canary"}[5m]))',
);
});
test('interpolateQuery() for newrelic query and args', () => {
expect(interpolateQuery(MOCK_QUERY_NEWRELIC, MOCK_ARGS_NEWRELIC)).toBe(
"FROM Transaction SELECT percentage(count(*), WHERE httpResponseCode != 500) as successRate where appName = 'myApp'"
"FROM Transaction SELECT percentage(count(*), WHERE httpResponseCode != 500) as successRate where appName = 'myApp'",
);
});
test('interpolateQuery() for simple datadog query and args', () => {
expect(interpolateQuery(MOCK_QUERY_DATADOG, MOCK_ARGS_DATADOG)).toBe('sum:requests.error.rate{service:istio-host-split-canary}');
});
test('interpolateQuery() for wavefront query and args', () => {
expect(interpolateQuery(MOCK_QUERY_WAVEFRONT, MOCK_ARGS_WAVEFRONT)).toBe(
'sum(rate(5m, ts("istio.requestcount.count", response_code!=500 and destination_service="istio-host-split-canary"))) / sum(rate(5m, ts("istio.requestcount.count", reporter=client and destination_service="istio-host-split-canary")))'
'sum(rate(5m, ts("istio.requestcount.count", response_code!=500 and destination_service="istio-host-split-canary"))) / sum(rate(5m, ts("istio.requestcount.count", reporter=client and destination_service="istio-host-split-canary")))',
);
});
test('interpolateQuery() for graphite query and args', () => {
expect(interpolateQuery(MOCK_QUERY_GRAPHITE, MOCK_ARGS_GRAPHITE)).toBe(
"target=summarize(asPercent(sumSeries(stats.timers.httpServerRequests.app.istio-host-split-canary.exception.*.method.*.outcome.{CLIENT_ERROR,INFORMATIONAL,REDIRECTION,SUCCESS}.status.*.uri.*.count), sumSeries(stats.timers.httpServerRequests.app.istio-host-split-canary.exception.*.method.*.outcome.*.status.*.uri.*.count)),'5min','avg')"
"target=summarize(asPercent(sumSeries(stats.timers.httpServerRequests.app.istio-host-split-canary.exception.*.method.*.outcome.{CLIENT_ERROR,INFORMATIONAL,REDIRECTION,SUCCESS}.status.*.uri.*.count), sumSeries(stats.timers.httpServerRequests.app.istio-host-split-canary.exception.*.method.*.outcome.*.status.*.uri.*.count)),'5min','avg')",
);
});
test('interpolateQuery() for influxdb query and args', () => {
expect(interpolateQuery(MOCK_QUERY_INFLUXDB, MOCK_ARGS_INFLUXDB)).toBe(
'from(bucket: "app_istio") range(start: -15m) filter(fn: (r) => r["destination_workload"] == "myApp")|> filter(fn: (r) => r["_measurement"] == "istio:istio_requests_errors_percentage:rate1m:5xx")'
'from(bucket: "app_istio") range(start: -15m) filter(fn: (r) => r["destination_workload"] == "myApp")|> filter(fn: (r) => r["_measurement"] == "istio:istio_requests_errors_percentage:rate1m:5xx")',
);
});
test('interpolateQuery() for skywalking query and args', () => {
expect(interpolateQuery(MOCK_QUERY_SKYWALKING, MOCK_ARGS_SKYWALKING)).toBe(
'query queryData($duration: Duration!) { service_apdex: readMetricsValues(condition: { name: "service_apdex", entity: { scope: Service, serviceName: "istio-host-split-canary", normal: true } }, duration: $duration) { label values { values { value } } } }'
'query queryData($duration: Duration!) { service_apdex: readMetricsValues(condition: { name: "service_apdex", entity: { scope: Service, serviceName: "istio-host-split-canary", normal: true } }, duration: $duration) { label values { values { value } } } }',
);
});

Expand Down Expand Up @@ -545,4 +547,62 @@ describe('analysis modal transforms', () => {
tableValue: {latency: null, cpuUsage: null},
});
});
const MOCK_MEASUREMENTS: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Measurement[] = [{value: '[5]'}, {value: '[10]'}, {value: '[15]'}];
const MOCK_MEASUREMENTS_WITH_NAN: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Measurement[] = [{value: '[NaN]'}, {value: '[10]'}, {value: '[15]'}];
const MOCK_MEASUREMENTS_WITH_STRING: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Measurement[] = [{value: 'NaN'}, {value: '10'}, {value: '15'}];

it('transforms measurements correctly with valid numbers', () => {
expect(transformMeasurements(['0'], MOCK_MEASUREMENTS)).toEqual({
chartable: true,
min: 0,
max: null,
measurements: [
{value: '[5]', chartValue: {'0': 5}, tableValue: {'0': 5}},
{value: '[10]', chartValue: {'0': 10}, tableValue: {'0': 10}},
{value: '[15]', chartValue: {'0': 15}, tableValue: {'0': 15}},
],
});
});
it('transforms measurements correctly with NaN numbers in list', () => {
expect(transformMeasurements(['0'], MOCK_MEASUREMENTS_WITH_NAN)).toEqual({
chartable: true,
min: 0,
max: null,
measurements: [
{value: '[NaN]', chartValue: null, tableValue: null},
{value: '[10]', chartValue: {'0': 10}, tableValue: {'0': 10}},
{value: '[15]', chartValue: {'0': 15}, tableValue: {'0': 15}},
],
});
});
it('returns default values when measurements are undefined', () => {
expect(transformMeasurements(['0'])).toEqual({
chartable: false,
min: 0,
max: null,
measurements: [],
});
});

it('returns default values when measurements are empty', () => {
expect(transformMeasurements(['0'], [])).toEqual({
chartable: false,
min: 0,
max: null,
measurements: [],
});
});

it('handles measurements with string values', () => {
expect(transformMeasurements(['0'], MOCK_MEASUREMENTS_WITH_STRING)).toEqual({
chartable: true,
min: 0,
max: 15,
measurements: [
{value: 'NaN', chartValue: null, tableValue: null},
{value: '10', chartValue: 10, tableValue: 10},
{value: '15', chartValue: 15, tableValue: 15},
],
});
});
});
27 changes: 18 additions & 9 deletions ui/src/app/components/analysis-modal/transforms.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// eslint-disable-file @typescript-eslint/ban-ts-comment
import JSON5 from 'json5'
import * as moment from 'moment';

import {
Expand Down Expand Up @@ -132,7 +131,7 @@ const PROVIDER_CONDITION_SUPPORT: {
export const conditionDetails = (
condition?: string,
args: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Argument[] = [],
provider?: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1MetricProvider
provider?: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1MetricProvider,
): {
label: string | null;
thresholds: number[];
Expand Down Expand Up @@ -209,7 +208,7 @@ export const chartMax = (valueMax: number, failThresholds: number[] | null, succ
* @param phase analysis phase
* @returns analysis phase adjusted to render the UI status with a more accurate functional status
*/
export const getAdjustedMetricPhase = (phase?: AnalysisStatus): AnalysisStatus => (phase === AnalysisStatus.Error ? AnalysisStatus.Failed : phase ?? AnalysisStatus.Unknown);
export const getAdjustedMetricPhase = (phase?: AnalysisStatus): AnalysisStatus => (phase === AnalysisStatus.Error ? AnalysisStatus.Failed : (phase ?? AnalysisStatus.Unknown));

/**
*
Expand Down Expand Up @@ -379,7 +378,7 @@ export const interpolateQuery = (query?: string, args?: GithubComArgoprojArgoRol
*/
export const printableDatadogQuery = (
datadog: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1DatadogMetric,
args: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Argument[]
args: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Argument[],
): string[] | undefined => {
if ((datadog.apiVersion ?? '').toLowerCase() === 'v1' && 'query' in datadog) {
return [interpolateQuery(datadog.query, args)];
Expand Down Expand Up @@ -418,7 +417,7 @@ export const printableCloudWatchQuery = (cloudWatch: GithubComArgoprojArgoRollou
*/
export const metricQueries = (
provider?: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1MetricProvider | null,
args: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Argument[] = []
args: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Argument[] = [],
): string[] | undefined => {
if (provider === undefined || provider === null) {
return undefined;
Expand Down Expand Up @@ -468,7 +467,7 @@ export const transformMeasurements = (conditionKeys: string[], measurements?: Gi
return measurements.reduce(
(
acc: {chartable: boolean; min: number; max: number | null; measurements: TransformedMeasurement[]},
currMeasurement: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Measurement
currMeasurement: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1Measurement,
) => {
const transformedValue = transformMeasurementValue(conditionKeys, currMeasurement.value);
const {canChart, tableValue} = transformedValue;
Expand All @@ -488,7 +487,7 @@ export const transformMeasurements = (conditionKeys: string[], measurements?: Gi
],
};
},
{chartable: true, min: 0, max: null, measurements: [] as TransformedMeasurement[]}
{chartable: true, min: 0, max: null, measurements: [] as TransformedMeasurement[]},
);
};

Expand All @@ -508,7 +507,7 @@ type FormattedMeasurementValue = number | string | null;
*/
export const formattedValue = (value: any): FormattedMeasurementValue => {
const isNum = isFiniteNumber(value);
return isNum ? roundNumber(Number(value)) : value?.toString() ?? null;
return isNum ? roundNumber(Number(value)) : (value?.toString() ?? null);
};

/**
Expand All @@ -534,6 +533,7 @@ const formatNumberMeasurement = (value: number): MeasurementValueInfo => {
export const formatSingleItemArrayMeasurement = (value: FormattedMeasurementValue[], accessor: number): MeasurementValueInfo => {
if (isFiniteNumber(accessor)) {
const measurementValue = value?.[accessor] ?? null;

// if it's a number or null, chart it
if (isFiniteNumber(measurementValue) || measurementValue === null) {
const displayValue = formattedValue(measurementValue);
Expand Down Expand Up @@ -618,7 +618,16 @@ const transformMeasurementValue = (conditionKeys: string[], value?: string): Mea
};
}

const parsedValue = JSON5.parse(value);
let parsedValue;
try {
parsedValue = JSON.parse(value);
} catch {
return {
canChart: true,
chartValue: null,
tableValue: null,
};
}

// single number measurement value
if (isFiniteNumber(parsedValue)) {
Expand Down
3 changes: 3 additions & 0 deletions ui/src/app/components/pods/pods.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ export enum PodStatus {
}

const isPodReady = (ready: string) => {
if (!ready) {
return false;
}
// Ready is a string in the format "0/1", "1/1", etc.
const [current, total] = ready.split('/');
return current === total;
Expand Down

0 comments on commit 4afdb85

Please sign in to comment.