Skip to content

Commit

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

## Summary

Based on user reported issue in
https://discuss.elastic.co/t/elastic-lightweight-monitor-host-name-no-longer-using-current-host-name/366421

Update parsing to skip replacing missing values !!

### Before

https://${host.name} would be replaced with "https://",


### After
Var value remain as is so that it would look for env variable 
"https://${host.name}", 


### Testing

can be tested in the ui , go to add monitor ui , user params in url
field and use inspect configuration to see parsed value, enabled inspect
button from kibana advanced settings observability inspect ES queries !!

<img width="852" alt="image"
src="https://github.com/user-attachments/assets/66bf460a-7d63-4fb3-993d-1caaa6812583">


<img width="1106" alt="image"
src="https://github.com/user-attachments/assets/6f5e97c6-182a-4cd1-a345-976c321d449a">

---------

Co-authored-by: Justin Kambic <jk@elastic.co>
  • Loading branch information
shahzad31 and justinkambic authored Sep 12, 2024
1 parent d801f5d commit e94024b
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 e94024b

Please sign in to comment.