Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for messageFormat strings containing conditionals #442

Merged
merged 7 commits into from
Jul 25, 2023
8 changes: 8 additions & 0 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,14 @@ const levelPrettifier = logLevel => `LEVEL: ${levelColorize(logLevel)}`
}
```

In addition to this, if / end statement blocks can also be specified. Else statements and nested conditions are not supported.

```js
{
messageFormat: '{levelLabel} - {if pid}{pid} - {end}url:{req.url}'
}
```

This option can also be defined as a `function` with this prototype:

```js
Expand Down
61 changes: 48 additions & 13 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ module.exports.internals = {
deleteLogProperty,
splitPropertyKey,
createDate,
isValidDate
isValidDate,
parseMessageFormat
}

/**
Expand Down Expand Up @@ -263,16 +264,21 @@ function prettifyLevel ({ log, colorizer = defaultColorizer, levelKey = LEVEL_KE
*/
function prettifyMessage ({ log, messageFormat, messageKey = MESSAGE_KEY, colorizer = defaultColorizer, levelLabel = LEVEL_LABEL, levelKey = LEVEL_KEY, customLevels, useOnlyCustomProps }) {
if (messageFormat && typeof messageFormat === 'string') {
const message = String(messageFormat).replace(/{([^{}]+)}/g, function (match, p1) {
// return log level as string instead of int
let level
if (p1 === levelLabel && (level = getPropertyValue(log, levelKey)) !== undefined) {
const condition = useOnlyCustomProps ? customLevels === undefined : customLevels[level] === undefined
return condition ? LEVELS[level] : customLevels[level]
}
// Parse nested key access, e.g. `{keyA.subKeyB}`.
return getPropertyValue(log, p1) || ''
})
const parsedMessageFormat = parseMessageFormat(messageFormat, log)

const message = String(parsedMessageFormat).replace(
/{([^{}]+)}/g,
function (match, p1) {
// return log level as string instead of int
let level
if (p1 === levelLabel && (level = getPropertyValue(log, levelKey)) !== undefined) {
const condition = useOnlyCustomProps ? customLevels === undefined : customLevels[level] === undefined
return condition ? LEVELS[level] : customLevels[level]
}

// Parse nested key access, e.g. `{keyA.subKeyB}`.
return getPropertyValue(log, p1) || ''
})
return colorizer.message(message)
}
if (messageFormat && typeof messageFormat === 'function') {
Expand Down Expand Up @@ -383,7 +389,7 @@ function prettifyObject ({
// Split object keys into two categories: error and non-error
const { plain, errors } = Object.entries(input).reduce(({ plain, errors }, [k, v]) => {
if (keysToIgnore.includes(k) === false) {
// Pre-apply custom prettifiers, because all 3 cases below will need this
// Pre-apply custom prettifiers, because all 3 cases below will need this
const pretty = typeof customPrettifiers[k] === 'function'
? customPrettifiers[k](v, k, input)
: v
Expand Down Expand Up @@ -557,7 +563,7 @@ function splitPropertyKey (key) {
*
* @param {object} obj The object to be searched.
* @param {string|string[]} property A string, or an array of strings, identifying
* the property to be retrieved from the object.
* the property to be retrieved from the object.
* Accepts nested properties delimited by a `.`.
* Delimiter can be escaped to preserve property names that contain the delimiter.
* e.g. `'prop1.prop2'` or `'prop2\.domain\.corp.prop2'`.
Expand Down Expand Up @@ -766,3 +772,32 @@ function handleCustomlevelNamesOpts (cLevels) {
return {}
}
}

/**
* Translates all conditional blocks from within the messageFormat. Translates any matching
* {if key}{key}{end} statements and returns everything between if and else blocks if the key provided
* was found in log.
*
* @param {string} messageFormat A format string or function that defines how the
* logged message should be conditionally formatted, e.g. `'{if level}{level}{end} - {if req.id}{req.id}{end}'`.
* @param {object} log The log object to be modified.
*
* @returns {string} The parsed messageFormat.
*/
function parseMessageFormat (messageFormat, log) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be a better name:

Suggested change
function parseMessageFormat (messageFormat, log) {
function interpretConditionals (messageFormat, log) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed accordingly.

messageFormat = messageFormat.replace(/{if (.*?)}(.*?){end}/g, function (_, key, value) {
const propertyValue = getPropertyValue(log, key)
if (propertyValue && value.includes(key)) {
return value.replace(new RegExp('{' + key + '}', 'g'), propertyValue)
} else {
return ''
}
})

// Remove unescaped if blocks
messageFormat = messageFormat.replace(/{if (.*?)}/g, '')
// Remove unescaped end blocks
messageFormat = messageFormat.replace(/{end}/g, '')
timlohse1104 marked this conversation as resolved.
Show resolved Hide resolved

return messageFormat.replace(/\s+/g, ' ').trim()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be a bit easier to read:

Suggested change
function parseMessageFormat (messageFormat, log) {
messageFormat = messageFormat.replace(/{if (.*?)}(.*?){end}/g, function (_, key, value) {
const propertyValue = getPropertyValue(log, key)
if (propertyValue && value.includes(key)) {
return value.replace(new RegExp('{' + key + '}', 'g'), propertyValue)
} else {
return ''
}
})
// Remove unescaped if blocks
messageFormat = messageFormat.replace(/{if (.*?)}/g, '')
// Remove unescaped end blocks
messageFormat = messageFormat.replace(/{end}/g, '')
return messageFormat.replace(/\s+/g, ' ').trim()
}
function parseMessageFormat (messageFormat, log) {
messageFormat = messageFormat.replace(/{if (.*?)}(.*?){end}/g, replacer)
// Remove unescaped if blocks
messageFormat = messageFormat.replace(/{if (.*?)}/g, '')
// Remove unescaped end blocks
messageFormat = messageFormat.replace(/{end}/g, '')
return messageFormat.replace(/\s+/g, ' ').trim()
function replacer (_, key, value) {
const propertyValue = getPropertyValue(log, key)
if (propertyValue && value.includes(key)) {
return value.replace(new RegExp('{' + key + '}', 'g'), propertyValue)
} else {
return ''
}
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated accordingly.

62 changes: 62 additions & 0 deletions test/lib/utils.internals.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,65 @@ tap.test('#getPropertyValue', t => {

t.end()
})

tap.test('#parseMessageFormat', t => {
const logData = {
level: 30,
data1: {
data2: 'bar'
},
msg: 'foo'
}

t.test('parseMessageFormat translates if / else statement to found property value', async t => {
const log = fastCopy(logData)
t.equal(internals.parseMessageFormat('{level} - {if data1.data2}{data1.data2}{end}', log), '{level} - bar')
})

t.test('parseMessageFormat translates if / else statement to found property value and leave unmatched property key untouched', async t => {
const log = fastCopy(logData)
t.equal(internals.parseMessageFormat('{level} - {if data1.data2}{data1.data2} ({msg}){end}', log), '{level} - bar ({msg})')
})

t.test('parseMessageFormat removes unescaped if statements', async t => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you mean non-terminated instead of unescaped?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed wording.

const log = fastCopy(logData)
t.equal(internals.parseMessageFormat('{level} - {if data1.data2}{data1.data2}', log), '{level} - {data1.data2}')
})

t.test('parseMessageFormat removes unescaped end statements', async t => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "floating" is a better term than "unescaped" here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed wording.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the format string is?:

{level} - {if foo}something{end}{end}

(i.e. someone tried to add an unsupported nested condition but failed to add the leading if)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The additional {end} within the if / end statement will be removed. Added test scenario for this.

const log = fastCopy(logData)
t.equal(internals.parseMessageFormat('{level} - {data1.data2}{end}', log), '{level} - {data1.data2}')
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a few tests for not-matching if/else/end pair? What error would you get?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah for sure. Thanks for the fast code review.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added further test scenarios.


t.test('parseMessageFormat removes if / end blocks if existent condition key does not match existent property key', async t => {
const log = fastCopy(logData)
t.equal(internals.parseMessageFormat('{level}{if msg}{data1.data2}{end}', log), '{level}')
})

t.test('parseMessageFormat removes if / end blocks if non-existent condition key does not match existent property key', async t => {
const log = fastCopy(logData)
t.equal(internals.parseMessageFormat('{level}{if foo}{msg}{end}', log), '{level}')
})

t.test('parseMessageFormat removes if / end blocks if existent condition key does not match non-existent property key', async t => {
const log = fastCopy(logData)
t.equal(internals.parseMessageFormat('{level}{if msg}{foo}{end}', log), '{level}')
})

t.test('parseMessageFormat removes if / end blocks if non-existent condition key does not match non-existent property key', async t => {
const log = fastCopy(logData)
t.equal(internals.parseMessageFormat('{level}{if foo}{bar}{end}', log), '{level}')
})

t.test('parseMessageFormat removes if / end blocks if nested condition key does not match property key', async t => {
const log = fastCopy(logData)
t.equal(internals.parseMessageFormat('{level}{if data1.msg}{data1.data2}{end}', log), '{level}')
})

t.test('parseMessageFormat removes nested if / end statement blocks', async t => {
const log = fastCopy(logData)
t.equal(internals.parseMessageFormat('{if msg}{if data1.data2}{msg}{data1.data2}{end}{end}', log), 'foo{data1.data2}')
})

t.end()
})
5 changes: 5 additions & 0 deletions test/lib/utils.public.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ tap.test('prettifyMessage', t => {
t.equal(str, 'localhost/test - param: - foo')
})

t.test('`messageFormat` supports conditional blocks', async t => {
const str = prettifyMessage({ log: { level: 30, req: { id: 'foo' } }, messageFormat: '{level} | {if req.id}({req.id}){end}{if msg}{msg}{end}' })
t.equal(str, '30 | (foo)')
})

t.test('`messageFormat` supports function definition', async t => {
const str = prettifyMessage({
log: { level: 30, request: { url: 'localhost/test' }, msg: 'incoming request' },
Expand Down