Skip to content

Commit

Permalink
[8.15] [Synthetics] Update parsing to skip replacing missing values (e…
Browse files Browse the repository at this point in the history
…lastic#192662) (elastic#192752)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[Synthetics] Update parsing to skip replacing missing values
(elastic#192662)](elastic#192662)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"shahzad31comp@gmail.com"},"sourceCommit":{"committedDate":"2024-09-12T17:20:02Z","message":"[Synthetics]
Update parsing to skip replacing missing values (elastic#192662)\n\n##
Summary\n\nBased on user reported issue
in\nhttps://discuss.elastic.co/t/elastic-lightweight-monitor-host-name-no-longer-using-current-host-name/366421\n\nUpdate
parsing to skip replacing missing values !!\n\n###
Before\n\nhttps://${host.name} would be replaced with
\"https://\",\n\n\n### After\nVar value remain as is so that it would
look for env variable \n\"https://${host.name}\", \n\n\n###
Testing\n\ncan be tested in the ui , go to add monitor ui , user params
in url\nfield and use inspect configuration to see parsed value, enabled
inspect\nbutton from kibana advanced settings observability inspect ES
queries !!\n\n<img width=\"852\"
alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/66bf460a-7d63-4fb3-993d-1caaa6812583\">\n\n\n<img
width=\"1106\"
alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/6f5e97c6-182a-4cd1-a345-976c321d449a\">\n\n---------\n\nCo-authored-by:
Justin Kambic
<jk@elastic.co>","sha":"e94024b277779a4a6e9f786e43a4d20e56f6773f","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0"],"title":"[Synthetics]
Update parsing to skip replacing missing
values","number":192662,"url":"elastic#192662
Update parsing to skip replacing missing values (elastic#192662)\n\n##
Summary\n\nBased on user reported issue
in\nhttps://discuss.elastic.co/t/elastic-lightweight-monitor-host-name-no-longer-using-current-host-name/366421\n\nUpdate
parsing to skip replacing missing values !!\n\n###
Before\n\nhttps://${host.name} would be replaced with
\"https://\",\n\n\n### After\nVar value remain as is so that it would
look for env variable \n\"https://${host.name}\", \n\n\n###
Testing\n\ncan be tested in the ui , go to add monitor ui , user params
in url\nfield and use inspect configuration to see parsed value, enabled
inspect\nbutton from kibana advanced settings observability inspect ES
queries !!\n\n<img width=\"852\"
alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/66bf460a-7d63-4fb3-993d-1caaa6812583\">\n\n\n<img
width=\"1106\"
alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/6f5e97c6-182a-4cd1-a345-976c321d449a\">\n\n---------\n\nCo-authored-by:
Justin Kambic
<jk@elastic.co>","sha":"e94024b277779a4a6e9f786e43a4d20e56f6773f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"elastic#192662
Update parsing to skip replacing missing values (elastic#192662)\n\n##
Summary\n\nBased on user reported issue
in\nhttps://discuss.elastic.co/t/elastic-lightweight-monitor-host-name-no-longer-using-current-host-name/366421\n\nUpdate
parsing to skip replacing missing values !!\n\n###
Before\n\nhttps://${host.name} would be replaced with
\"https://\",\n\n\n### After\nVar value remain as is so that it would
look for env variable \n\"https://${host.name}\", \n\n\n###
Testing\n\ncan be tested in the ui , go to add monitor ui , user params
in url\nfield and use inspect configuration to see parsed value, enabled
inspect\nbutton from kibana advanced settings observability inspect ES
queries !!\n\n<img width=\"852\"
alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/66bf460a-7d63-4fb3-993d-1caaa6812583\">\n\n\n<img
width=\"1106\"
alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/6f5e97c6-182a-4cd1-a345-976c321d449a\">\n\n---------\n\nCo-authored-by:
Justin Kambic
<jk@elastic.co>","sha":"e94024b277779a4a6e9f786e43a4d20e56f6773f"}}]}]
BACKPORT-->

Co-authored-by: Shahzad <shahzad31comp@gmail.com>
  • Loading branch information
kibanamachine and shahzad31 committed Sep 12, 2024
1 parent 4000633 commit b376afc
Show file tree
Hide file tree
Showing 5 changed files with 412 additions and 246 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ describe('replaceStringWithParams', () => {
expect(result).toEqual('https://elastic.co');
});

it('returns empty value in case no param', () => {
it('returns same value in case no param', () => {
const result = replaceStringWithParams('${homePageUrl}', {}, logger);

expect(result).toEqual('');
expect(result).toEqual('${homePageUrl}');
});

it('works on objects', () => {
Expand Down Expand Up @@ -76,6 +76,46 @@ describe('replaceStringWithParams', () => {
expect(result).toEqual('Basic https://elastic.co https://elastic.co/product');
});

it('works on multiple without spaces', () => {
const result = replaceStringWithParams(
'Basic ${homePageUrl}${homePageUrl1}',
{ homePageUrl: 'https://elastic.co', homePageUrl1: 'https://elastic.co/product' },
logger
);

expect(result).toEqual('Basic https://elastic.cohttps://elastic.co/product');
});

it('works on multiple without spaces and one missing', () => {
const result = replaceStringWithParams(
'Basic ${homePageUrl}${homePageUrl1}',
{ homePageUrl: 'https://elastic.co', homePageUrl2: 'https://elastic.co/product' },
logger
);

expect(result).toEqual('Basic https://elastic.co${homePageUrl1}');
});

it('works on multiple without with default', () => {
const result = replaceStringWithParams(
'Basic ${homePageUrl}${homePageUrl1:test}',
{ homePageUrl: 'https://elastic.co', homePageUrl2: 'https://elastic.co/product' },
logger
);

expect(result).toEqual('Basic https://elastic.cotest');
});

it('works on multiple with multiple defaults', () => {
const result = replaceStringWithParams(
'Basic ${homePageUrl:test}${homePageUrl1:test4}',
{ homePageUrl3: 'https://elastic.co', homePageUrl2: 'https://elastic.co/product' },
logger
);

expect(result).toEqual('Basic testtest4');
});

it('works with default value', () => {
const result = replaceStringWithParams(
'Basic ${homePageUrl:https://elastic.co} ${homePageUrl1}',
Expand Down Expand Up @@ -143,6 +183,36 @@ describe('replaceStringWithParams', () => {
expect(result).toEqual('Basic ${} value');
});

it('works with ${host.name} for missing params', () => {
const result = replaceStringWithParams(
'Basic ${host.name} value',
{ homePageUrl1: 'https://elastic.co/product' },
logger
);

expect(result).toEqual('Basic ${host.name} value');
});

it('works with ${host.name} one missing params', () => {
const result = replaceStringWithParams(
'Basic ${host.name} ${homePageUrl1} value',
{ homePageUrl1: 'https://elastic.co/product' },
logger
);

expect(result).toEqual('Basic ${host.name} https://elastic.co/product value');
});

it('works with ${host.name} just missing params', () => {
const result = replaceStringWithParams(
'${host.name} ${homePageUrl1}',
{ homePageUrl1: 'https://elastic.co/product' },
logger
);

expect(result).toEqual('${host.name} https://elastic.co/product');
});

it('works with } ${abc} as part of value', () => {
const result = replaceStringWithParams(
'Basic } ${homePageUrl1} value',
Expand All @@ -162,4 +232,10 @@ describe('replaceStringWithParams', () => {

expect(result).toEqual('Basic https://elastic.co/product { value');
});

it("returns value as string | null when no params and it's an object", () => {
const result = replaceStringWithParams({}, { param: '1' }, logger);

expect(result).toEqual({});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { Logger } from '@kbn/logging';
import { isEmpty } from 'lodash';
import { ConfigKey, MonitorFields } from '../../../common/runtime_types';
import { ParsedVars, replaceVarsWithParams } from './lightweight_param_formatter';
import variableParser from './variable_parser';
Expand All @@ -20,7 +21,7 @@ export const replaceStringWithParams = (
params: Record<string, string>,
logger?: Logger
) => {
if (!value || typeof value === 'boolean') {
if (!value || typeof value === 'boolean' || isEmpty(params)) {
return value as string | null;
}

Expand All @@ -42,6 +43,10 @@ export const replaceStringWithParams = (

const parsedVars: ParsedVars = variableParser.parse(value);

if (allParamsAreMissing(parsedVars, params)) {
return value as string | null;
}

return replaceVarsWithParams(parsedVars, params);
} catch (e) {
logger?.error(`error parsing vars for value ${JSON.stringify(value)}, ${e}`);
Expand All @@ -50,6 +55,19 @@ export const replaceStringWithParams = (
return value as string | null;
};

const allParamsAreMissing = (parsedVars: ParsedVars, params: Record<string, string>) => {
const hasDefault = parsedVars.some(
(parsedVar) => parsedVar.type === 'var' && parsedVar.content.default
);
if (hasDefault) {
return false;
}
const varKeys = parsedVars
.filter((parsedVar) => parsedVar.type === 'var')
.map((v) => (typeof v.content === 'string' ? v.content : v.content.name));
return varKeys.every((v) => !params[v]);
};

const SHELL_PARAMS_REGEX = /\$\{[a-zA-Z_][a-zA-Z0-9\._\-?:]*\}/g;

export const hasNoParams = (strVal: string) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,11 @@ describe('LightweightParamFormatter', () => {
const result = replaceVarsWithParams(formatter, params);
expect(result).toEqual('https://default:1234');
});
it('wraps content name when no default', () => {
const result = replaceVarsWithParams(
[{ type: 'var', content: { name: 'missing', default: null } }],
{}
);
expect(result).toEqual('${missing}');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function replaceVarsWithParams(vars: ParsedVars, params: Record<string, s
if (v.type === 'nonvar') {
return v.content;
}
return params[v.content.name]?.trim() || v.content.default;
return params[v.content.name]?.trim() || v.content.default || '${' + v.content.name + '}';
})
.join('');
}
Loading

0 comments on commit b376afc

Please sign in to comment.