Skip to content

Commit

Permalink
Ensure rule message do not span multiple lines (#62391) (#62480)
Browse files Browse the repository at this point in the history
Because these messages are used for logging, we should ensure they do
not span multiple lines and confuse log parsers. Since the frontend does
not currently display these newlines, anyway, there is no impact to the
UI.
  • Loading branch information
rylnd authored Apr 3, 2020
1 parent 8501d11 commit 9f7e2cd
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,23 @@ describe('buildRuleMessageFactory', () => {
expect(message).toEqual(expect.stringContaining('signals index: "index"'));
});

it('joins message parts with newlines', () => {
it('joins message parts with spaces', () => {
const buildMessage = buildRuleMessageFactory(factoryParams);

const message = buildMessage('my message');
const messageParts = message.split('\n');
expect(messageParts).toContain('my message');
expect(messageParts).toContain('name: "name"');
expect(messageParts).toContain('id: "id"');
expect(messageParts).toContain('rule id: "ruleId"');
expect(messageParts).toContain('signals index: "index"');
expect(message).toEqual(expect.stringContaining('my message '));
expect(message).toEqual(expect.stringContaining(' name: "name" '));
expect(message).toEqual(expect.stringContaining(' id: "id" '));
expect(message).toEqual(expect.stringContaining(' rule id: "ruleId" '));
expect(message).toEqual(expect.stringContaining(' signals index: "index"'));
});

it('joins multiple arguments with newlines', () => {
it('joins multiple arguments with spaces', () => {
const buildMessage = buildRuleMessageFactory(factoryParams);

const message = buildMessage('my message', 'here is more');
const messageParts = message.split('\n');
expect(messageParts).toContain('my message');
expect(messageParts).toContain('here is more');
expect(message).toEqual(expect.stringContaining('my message '));
expect(message).toEqual(expect.stringContaining(' here is more'));
});

it('defaults the rule ID if not provided ', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ export const buildRuleMessageFactory = ({
`id: "${id}"`,
`rule id: "${ruleId ?? '(unknown rule id)'}"`,
`signals index: "${index}"`,
].join('\n');
].join(' ');
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export const signalRulesAlertType = ({
'Machine learning rule is missing job id and/or anomaly threshold:',
`job id: "${machineLearningJobId}"`,
`anomaly threshold: "${anomalyThreshold}"`,
].join('\n')
].join(' ')
);
}

Expand Down

0 comments on commit 9f7e2cd

Please sign in to comment.