Skip to content

Commit efb11b7

Browse files
authored
feat: switch to safe-stable-stringify for safer serialization (#155)
This drops the schema-based fast-json-stringify usage, which could potentially be faster, but will throw on circular references. The safer serialization is more appropriate for logging libs. Note that currently both pino and winston themselves use safe-stable-stringify. Add explicit dep for triple-beam, rather than getting lucky. Use a Format class to avoid needing a dep on winston or logform for the 'winston.format(...)' call. Document subtle diffs with our stringifier and winston.format.json(). Closes: #104 Closes: #144 Closes: #145 Closes: #108
1 parent d6d90d6 commit efb11b7

File tree

18 files changed

+226
-349
lines changed

18 files changed

+226
-349
lines changed

.github/workflows/test.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ jobs:
3535
- uses: actions/setup-node@v3
3636
with:
3737
node-version: ${{ matrix.node-version }}
38+
- name: Update npm to at least v7 for workspaces support
39+
if: ${{ matrix.node-version < 16 }} # node@16 comes with npm@8, node@14 with npm@6
40+
run: npm install -g npm@7 # npm@7 supports node >=10
3841
- name: Install dependencies
3942
run: utils/run-install.sh
4043
- name: Test Node.js v${{ matrix.node-version }}

Makefile

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,14 @@ lint: check-license-headers
2222

2323
.PHONY: fmt
2424
fmt:
25-
npm --workspaces lint:fix # requires npm>=7 (aka node>=16)
25+
npm --workspaces run lint:fix # requires npm>=7 (aka node>=16)
26+
27+
.PHONY: clean
28+
clean:
29+
rm -rf packages/*/node_modules
30+
rm -rf node_modules
31+
rm -rf utils/node_modules
32+
2633

2734
# Build and open the rendered docs for testing.
2835
#

packages/ecs-helpers/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# @elastic/ecs-helpers Changelog
22

3+
## 2.0.0
4+
5+
- [Breaking change.] Drop the `serialize` method. Serialization will move to
6+
the individual `@elastic/ecs-*-format` libraries -- and they will be
7+
switching to the `safe-stable-stringify` package.
8+
39
## v1.1.0
410

511
- Fix `formatHttpRequest` and `formatHttpResponse` to be more defensive. If

packages/ecs-helpers/README.md

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,30 +19,6 @@ npm install @elastic/ecs-helpers
1919

2020
The currently supported version of [Elastic Common Schema](https://www.elastic.co/guide/en/ecs/current/index.html).
2121

22-
### `stringify`
23-
24-
Function that serializes (very quickly!) an ECS-format log record object.
25-
26-
```js
27-
const { stringify } = require('@elastic/ecs-helpers')
28-
const ecs = {
29-
'@timestamp': new Date().toISOString(),
30-
'log.level': 'info',
31-
message: 'hello world',
32-
log: {
33-
logger: 'test'
34-
},
35-
'ecs.version': '1.4.0'
36-
}
37-
38-
console.log(stringify(ecs))
39-
```
40-
41-
Note: This uses [fast-json-stringify](https://github.com/fastify/fast-json-stringify)
42-
for serialization. By design this chooses speed over supporting serialization
43-
of objects with circular references. This generally means that ecs-logging-nodejs
44-
libraries will throw a "Converting circular structure to JSON" exception if an
45-
attempt is made to log an object with circular references.
4622

4723
### `formatError(obj, err) -> bool`
4824

packages/ecs-helpers/lib/index.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,11 @@
1717

1818
'use strict'
1919

20-
const stringify = require('./serializer')
2120
const errorFormatters = require('./error-formatters')
2221
const httpFormatters = require('./http-formatters')
2322

2423
module.exports = {
2524
version: '1.6.0',
26-
stringify,
2725
...errorFormatters,
2826
...httpFormatters
2927
}

packages/ecs-helpers/lib/serializer.js

Lines changed: 0 additions & 160 deletions
This file was deleted.

packages/ecs-helpers/package.json

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@elastic/ecs-helpers",
3-
"version": "1.1.0",
3+
"version": "2.0.0",
44
"description": "ecs-logging-nodejs helpers",
55
"main": "lib/index.js",
66
"files": [
@@ -24,9 +24,6 @@
2424
"engines": {
2525
"node": ">=10"
2626
},
27-
"dependencies": {
28-
"fast-json-stringify": "^2.4.1"
29-
},
3027
"devDependencies": {
3128
"@hapi/hapi": "^20.1.0",
3229
"ajv": "^7.0.3",

packages/ecs-helpers/test/basic.test.js

Lines changed: 14 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ const { ecsLoggingValidate } = require('../../../utils/lib/ecs-logging-validate'
2929

3030
const {
3131
version,
32-
stringify,
3332
formatError,
3433
formatHttpRequest,
3534
formatHttpResponse
@@ -42,35 +41,29 @@ const ajv = new Ajv({
4241
addFormats(ajv)
4342
const validate = ajv.compile(require('../../../utils/schema.json'))
4443

45-
test('stringify should return a valid ecs json', t => {
46-
const ecsFields = {
44+
test('validate valid ECS json', t => {
45+
const rec = {
4746
'@timestamp': new Date().toISOString(),
4847
'log.level': 'info',
4948
message: 'hello world',
5049
'ecs.version': '1.4.0'
5150
}
52-
53-
const line = stringify(ecsFields)
54-
const rec = JSON.parse(line)
5551
t.equal(validate(rec), true)
56-
t.equal(ecsLoggingValidate(line), null)
52+
t.equal(ecsLoggingValidate(rec), null)
5753
t.end()
5854
})
5955

6056
test('bad ECS json on purpose: @timestamp', t => {
61-
const ecsFields = {
57+
const rec = {
6258
'@timestamp': 'not a date',
6359
'log.level': 'info',
6460
message: 'foo',
6561
'ecs.version': '1.4.0'
6662
}
6763

68-
const line = stringify(ecsFields)
69-
const rec = JSON.parse(line)
70-
7164
t.equal(validate(rec), false)
7265

73-
const err = ecsLoggingValidate(line)
66+
const err = ecsLoggingValidate(rec)
7467
t.ok(err, 'got an ecsLoggingValidate error')
7568
t.equal(err.details.length, 1)
7669
t.ok(err.details[0].message)
@@ -145,18 +138,17 @@ test('formatHttpRequest and formatHttpResponse should return a valid ecs object'
145138
rv = formatHttpResponse(ecs, res)
146139
t.ok(rv, 'formatHttpResponse processed res')
147140

148-
const line = JSON.parse(stringify(ecs))
149-
t.ok(validate(line))
150-
t.equal(ecsLoggingValidate(line), null)
141+
t.ok(validate(ecs))
142+
t.equal(ecsLoggingValidate(ecs), null)
151143

152-
t.same(line.user_agent, { original: 'cool-agent' })
153-
t.same(line.url, {
144+
t.same(ecs.user_agent, { original: 'cool-agent' })
145+
t.same(ecs.url, {
154146
path: '/hello/world',
155147
query: 'foo=bar',
156148
full: `http://127.0.0.1:${server.address().port}/hello/world?foo=bar#anchor`,
157149
fragment: 'anchor'
158150
})
159-
t.same(line.http, {
151+
t.same(ecs.http, {
160152
version: '1.1',
161153
request: {
162154
method: 'POST',
@@ -179,11 +171,11 @@ test('formatHttpRequest and formatHttpResponse should return a valid ecs object'
179171
}
180172
})
181173
// https://www.elastic.co/guide/en/ecs/current/ecs-client.html fields
182-
t.ok(line.client, 'client fields are set')
183-
t.ok(line.client.address === '127.0.0.1', 'client.address is set')
184-
t.ok(line.client.ip === line.client.address,
174+
t.ok(ecs.client, 'client fields are set')
175+
t.ok(ecs.client.address === '127.0.0.1', 'client.address is set')
176+
t.ok(ecs.client.ip === ecs.client.address,
185177
'client.address duplicated to client.ip')
186-
t.equal(typeof (line.client.port), 'number')
178+
t.equal(typeof (ecs.client.port), 'number')
187179

188180
res.end(resBody)
189181
}
@@ -224,27 +216,6 @@ test('Should export a valid version', t => {
224216
t.end()
225217
})
226218

227-
test('stringify should emit valid tracing fields', t => {
228-
const before = {
229-
'@timestamp': new Date().toISOString(),
230-
'log.level': 'info',
231-
message: 'hello world',
232-
'ecs.version': '1.4.0',
233-
trace: { id: 1 },
234-
transaction: { id: 2 },
235-
span: { id: 3, extra_fields: 'are dropped' }
236-
}
237-
238-
const after = JSON.parse(stringify(before))
239-
t.ok(validate(after))
240-
t.equal(ecsLoggingValidate(after), null)
241-
t.same(after.trace, { id: '1' }, 'trace.id is stringified')
242-
t.same(after.transaction, { id: '2' }, 'transaction.id is stringified')
243-
t.same(after.span, { id: '3' },
244-
'span.id is stringified, extra fields are excluded')
245-
t.end()
246-
})
247-
248219
test('formatError: Error', t => {
249220
const rec = {}
250221
formatError(rec, new Error('boom'))

packages/ecs-morgan-format/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# @elastic/ecs-morgan-format Changelog
22

3+
## Unreleased
4+
5+
- Switch to `safe-stable-stringify` for JSON serialization. This library
6+
protects against circular references and bigints.
7+
(https://github.com/elastic/ecs-logging-nodejs/pull/155)
8+
39
## v1.4.0
410

511
- Add `service.version`, `service.environment`, and `service.node.name` log

0 commit comments

Comments
 (0)