From aa080e1c25a519b399a92e8582b7c65223c94c0c Mon Sep 17 00:00:00 2001 From: shahzad Date: Fri, 24 Apr 2020 05:59:37 +0200 Subject: [PATCH 1/9] update uptime ml job id to limit to 64 char --- .../plugins/uptime/common/constants/ui.ts | 2 +- .../uptime/public/state/api/ml_anomaly.ts | 20 ++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/x-pack/legacy/plugins/uptime/common/constants/ui.ts b/x-pack/legacy/plugins/uptime/common/constants/ui.ts index 29e8dabf53f92..9d17952477ff1 100644 --- a/x-pack/legacy/plugins/uptime/common/constants/ui.ts +++ b/x-pack/legacy/plugins/uptime/common/constants/ui.ts @@ -15,7 +15,7 @@ export enum STATUS { DOWN = 'down', } -export const ML_JOB_ID = 'high_latency_by_geo'; +export const ML_JOB_ID = 'high_latency'; export const ML_MODULE_ID = 'uptime_heartbeat'; diff --git a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts index f10745a50f56a..11ad7c85353d1 100644 --- a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts +++ b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts @@ -18,7 +18,20 @@ import { import { DataRecognizerConfigResponse } from '../../../../../../plugins/ml/common/types/modules'; import { JobExistResult } from '../../../../../../plugins/ml/common/types/data_recognizer'; -export const getMLJobId = (monitorId: string) => `${monitorId}_${ML_JOB_ID}`.toLowerCase(); +const getJobPrefix = (monitorId: string) => { + // ML App doesn't support upper case characters in job name + const lowerCaseMonitorId = monitorId.toLowerCase(); + + // ML Job ID can't be greater than 64 length, so will be substring it, and hope + // At such big length, there is minimum chance of having duplicate monitor id + // Subtracting ML_JOB_ID constant and _ char as well + if ((lowerCaseMonitorId + ML_JOB_ID + 1).length > 64) { + return lowerCaseMonitorId.substring(64 - ML_JOB_ID.length - 1); + } + return lowerCaseMonitorId; +}; + +export const getMLJobId = (monitorId: string) => `${getJobPrefix(monitorId)}_${ML_JOB_ID}`; export const getMLCapabilities = async (): Promise => { return await apiService.get(API_URLS.ML_CAPABILITIES); @@ -34,11 +47,8 @@ export const createMLJob = async ({ }: MonitorIdParam & HeartbeatIndicesParam): Promise => { const url = API_URLS.ML_SETUP_MODULE + ML_MODULE_ID; - // ML App doesn't support upper case characters in job name - const lowerCaseMonitorId = monitorId.toLowerCase(); - const data = { - prefix: `${lowerCaseMonitorId}_`, + prefix: `${getJobPrefix(monitorId)}_`, useDedicatedIndex: false, startDatafeed: true, start: moment() From b488cb226dd87ac05c29cb553bd211a3bd5b989e Mon Sep 17 00:00:00 2001 From: shahzad Date: Fri, 24 Apr 2020 06:33:28 +0200 Subject: [PATCH 2/9] added a test --- .../state/api/__tests__/ml_anomaly.test.ts | 31 +++++++++++++++++++ .../uptime/public/state/api/ml_anomaly.ts | 5 ++- 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 x-pack/legacy/plugins/uptime/public/state/api/__tests__/ml_anomaly.test.ts diff --git a/x-pack/legacy/plugins/uptime/public/state/api/__tests__/ml_anomaly.test.ts b/x-pack/legacy/plugins/uptime/public/state/api/__tests__/ml_anomaly.test.ts new file mode 100644 index 0000000000000..76c59f20134c4 --- /dev/null +++ b/x-pack/legacy/plugins/uptime/public/state/api/__tests__/ml_anomaly.test.ts @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { getMLJobId } from '../ml_anomaly'; + +describe('ML Anomaly API', () => { + it('it generates a valid ML job ID', async () => { + const monitorId = 'ABC1334haa'; + + const jobId = getMLJobId(monitorId); + + expect(jobId).toEqual(jobId.toLowerCase()); + + const longAndWeirdMonitorId = + 'https://auto-mmmmxhhhhhccclongAndWeirdMonitorId123yyyyyrereauto-xcmpa-1345555454646'; + const jobId1 = getMLJobId(longAndWeirdMonitorId); + + expect(jobId1.length <= 64).toBe(true); + + const monIdSpecialChars = '/ ? , " < > | * a'; + + const jobId2 = getMLJobId(monIdSpecialChars); + + const format = /[/?,"<>|*]+/; + + expect(format.test(jobId2)).toBe(false); + }); +}); diff --git a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts index 11ad7c85353d1..3ca783f7dacfc 100644 --- a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts +++ b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts @@ -20,7 +20,10 @@ import { JobExistResult } from '../../../../../../plugins/ml/common/types/data_r const getJobPrefix = (monitorId: string) => { // ML App doesn't support upper case characters in job name - const lowerCaseMonitorId = monitorId.toLowerCase(); + // Also Spaces and the characters / ? , " < > | * are not allowed + // so we will replace all special chars with _ + + const lowerCaseMonitorId = monitorId.replace(/[^A-Z0-9]+/gi, '_').toLowerCase(); // ML Job ID can't be greater than 64 length, so will be substring it, and hope // At such big length, there is minimum chance of having duplicate monitor id From 6f1ecd38f22d909a57f291972ba6f7118ed7fb67 Mon Sep 17 00:00:00 2001 From: shahzad Date: Fri, 24 Apr 2020 08:22:47 +0200 Subject: [PATCH 3/9] fixed issue --- x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts index 3ca783f7dacfc..2dc8323fd65a0 100644 --- a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts +++ b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts @@ -61,7 +61,7 @@ export const createMLJob = async ({ query: { bool: { filter: [ - { term: { 'monitor.id': lowerCaseMonitorId } }, + { term: { 'monitor.id': monitorId } }, { range: { 'monitor.duration.us': { gt: 0 } } }, ], }, From 2a7fa9e02bd6d01ecd247f05f5fb5f999372f264 Mon Sep 17 00:00:00 2001 From: shahzad Date: Fri, 24 Apr 2020 10:57:46 +0200 Subject: [PATCH 4/9] snap --- .../ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/uptime/public/components/monitor_details/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap b/x-pack/legacy/plugins/uptime/public/components/monitor_details/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap index 9957f13fc1334..b6f636131bdfc 100644 --- a/x-pack/legacy/plugins/uptime/public/components/monitor_details/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap +++ b/x-pack/legacy/plugins/uptime/public/components/monitor_details/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap @@ -3,7 +3,7 @@ exports[`ML JobLink renders without errors 1`] = ` From f97d0a955778404de909cc0890383d5b28eea774 Mon Sep 17 00:00:00 2001 From: shahzad Date: Fri, 24 Apr 2020 12:19:11 +0200 Subject: [PATCH 5/9] revert change --- x-pack/legacy/plugins/uptime/common/constants/ui.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/uptime/common/constants/ui.ts b/x-pack/legacy/plugins/uptime/common/constants/ui.ts index 9d17952477ff1..29e8dabf53f92 100644 --- a/x-pack/legacy/plugins/uptime/common/constants/ui.ts +++ b/x-pack/legacy/plugins/uptime/common/constants/ui.ts @@ -15,7 +15,7 @@ export enum STATUS { DOWN = 'down', } -export const ML_JOB_ID = 'high_latency'; +export const ML_JOB_ID = 'high_latency_by_geo'; export const ML_MODULE_ID = 'uptime_heartbeat'; From 0fd7150ad4b636d46de7d4212869322423967a32 Mon Sep 17 00:00:00 2001 From: shahzad Date: Fri, 24 Apr 2020 12:50:53 +0200 Subject: [PATCH 6/9] fix substr issue --- x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts index 2dc8323fd65a0..b3e73bab013da 100644 --- a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts +++ b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts @@ -29,7 +29,7 @@ const getJobPrefix = (monitorId: string) => { // At such big length, there is minimum chance of having duplicate monitor id // Subtracting ML_JOB_ID constant and _ char as well if ((lowerCaseMonitorId + ML_JOB_ID + 1).length > 64) { - return lowerCaseMonitorId.substring(64 - ML_JOB_ID.length - 1); + return lowerCaseMonitorId.substring(0, 64 - ML_JOB_ID.length - 1); } return lowerCaseMonitorId; }; From 6b6abdf717d2011eb7a550035f2ff9c88efb7ccd Mon Sep 17 00:00:00 2001 From: shahzad Date: Fri, 24 Apr 2020 13:11:39 +0200 Subject: [PATCH 7/9] snapshot --- .../ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/uptime/public/components/monitor_details/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap b/x-pack/legacy/plugins/uptime/public/components/monitor_details/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap index b6f636131bdfc..9957f13fc1334 100644 --- a/x-pack/legacy/plugins/uptime/public/components/monitor_details/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap +++ b/x-pack/legacy/plugins/uptime/public/components/monitor_details/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap @@ -3,7 +3,7 @@ exports[`ML JobLink renders without errors 1`] = ` From 11ce521d732b27496d19b2641bffc0c76c46bc9a Mon Sep 17 00:00:00 2001 From: shahzad Date: Fri, 24 Apr 2020 16:02:58 +0200 Subject: [PATCH 8/9] update tests --- .../state/api/__tests__/ml_anomaly.test.ts | 13 ++++++++----- .../uptime/public/state/api/ml_anomaly.ts | 16 +++++++++------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/x-pack/legacy/plugins/uptime/public/state/api/__tests__/ml_anomaly.test.ts b/x-pack/legacy/plugins/uptime/public/state/api/__tests__/ml_anomaly.test.ts index 76c59f20134c4..838e5b8246b4b 100644 --- a/x-pack/legacy/plugins/uptime/public/state/api/__tests__/ml_anomaly.test.ts +++ b/x-pack/legacy/plugins/uptime/public/state/api/__tests__/ml_anomaly.test.ts @@ -7,25 +7,28 @@ import { getMLJobId } from '../ml_anomaly'; describe('ML Anomaly API', () => { - it('it generates a valid ML job ID', async () => { + it('it generates a lowercase job id', async () => { const monitorId = 'ABC1334haa'; const jobId = getMLJobId(monitorId); expect(jobId).toEqual(jobId.toLowerCase()); + }); + it('should truncate long monitor IDs', () => { const longAndWeirdMonitorId = 'https://auto-mmmmxhhhhhccclongAndWeirdMonitorId123yyyyyrereauto-xcmpa-1345555454646'; - const jobId1 = getMLJobId(longAndWeirdMonitorId); - expect(jobId1.length <= 64).toBe(true); + expect(getMLJobId(longAndWeirdMonitorId)).toHaveLength(64); + }); + it('should remove special characters and replace them with underscore', () => { const monIdSpecialChars = '/ ? , " < > | * a'; - const jobId2 = getMLJobId(monIdSpecialChars); + const jobId = getMLJobId(monIdSpecialChars); const format = /[/?,"<>|*]+/; - expect(format.test(jobId2)).toBe(false); + expect(format.test(jobId)).toBe(false); }); }); diff --git a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts index b3e73bab013da..57792d96b28e5 100644 --- a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts +++ b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts @@ -23,18 +23,20 @@ const getJobPrefix = (monitorId: string) => { // Also Spaces and the characters / ? , " < > | * are not allowed // so we will replace all special chars with _ - const lowerCaseMonitorId = monitorId.replace(/[^A-Z0-9]+/gi, '_').toLowerCase(); + const prefix = monitorId.replace(/[^A-Z0-9]+/gi, '_').toLowerCase(); // ML Job ID can't be greater than 64 length, so will be substring it, and hope // At such big length, there is minimum chance of having duplicate monitor id - // Subtracting ML_JOB_ID constant and _ char as well - if ((lowerCaseMonitorId + ML_JOB_ID + 1).length > 64) { - return lowerCaseMonitorId.substring(0, 64 - ML_JOB_ID.length - 1); + // Subtracting ML_JOB_ID constant as well + const postfix = '_' + ML_JOB_ID; + + if ((prefix + postfix).length >= 64) { + return prefix.substring(0, 64 - postfix.length - 1) + '_'; } - return lowerCaseMonitorId; + return prefix + '_'; }; -export const getMLJobId = (monitorId: string) => `${getJobPrefix(monitorId)}_${ML_JOB_ID}`; +export const getMLJobId = (monitorId: string) => `${getJobPrefix(monitorId)}${ML_JOB_ID}`; export const getMLCapabilities = async (): Promise => { return await apiService.get(API_URLS.ML_CAPABILITIES); @@ -51,7 +53,7 @@ export const createMLJob = async ({ const url = API_URLS.ML_SETUP_MODULE + ML_MODULE_ID; const data = { - prefix: `${getJobPrefix(monitorId)}_`, + prefix: `${getJobPrefix(monitorId)}`, useDedicatedIndex: false, startDatafeed: true, start: moment() From 086fe2f3921b6354fd571d3e8e5e7083e94e486c Mon Sep 17 00:00:00 2001 From: shahzad Date: Fri, 24 Apr 2020 17:33:56 +0200 Subject: [PATCH 9/9] fix test --- x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts index 57792d96b28e5..3022c45d8dd91 100644 --- a/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts +++ b/x-pack/legacy/plugins/uptime/public/state/api/ml_anomaly.ts @@ -30,8 +30,8 @@ const getJobPrefix = (monitorId: string) => { // Subtracting ML_JOB_ID constant as well const postfix = '_' + ML_JOB_ID; - if ((prefix + postfix).length >= 64) { - return prefix.substring(0, 64 - postfix.length - 1) + '_'; + if ((prefix + postfix).length > 64) { + return prefix.substring(0, 64 - postfix.length) + '_'; } return prefix + '_'; };