From 768e0981a4179192db14a4229df37ed52f292b48 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 b74a0cf80c47b935f27d1ade5eff4ead1a6d94a5 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 a832ef52618a4c51e136c2ecb797e0c463ddf138 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 b81cff1071c5388f19408b37041b142736cf56fb 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/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap b/x-pack/legacy/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap index 9957f13fc1334..b6f636131bdfc 100644 --- a/x-pack/legacy/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap +++ b/x-pack/legacy/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap @@ -3,7 +3,7 @@ exports[`ML JobLink renders without errors 1`] = ` From 627a074a52cd55f704dc449c62256b26e5b41460 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 c034e1d14c7b9cdd930907b457fd7c17222194d7 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 675030c307e049c18402ae39fd7f4034defdcc20 Mon Sep 17 00:00:00 2001 From: shahzad Date: Fri, 24 Apr 2020 13:19:09 +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/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap b/x-pack/legacy/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap index b6f636131bdfc..9957f13fc1334 100644 --- a/x-pack/legacy/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap +++ b/x-pack/legacy/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_job_link.test.tsx.snap @@ -3,7 +3,7 @@ exports[`ML JobLink renders without errors 1`] = ` From db9c05abe26650e16b7a31df9c157edbcfa9dba0 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 c4737638cefd4e0f48fc2d7890816a0de4ccad77 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 + '_'; };