Skip to content

Commit

Permalink
feat: switch to safe-stable-stringify for safer serialization (#155)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
trentm authored Oct 24, 2023
1 parent d6d90d6 commit efb11b7
Show file tree
Hide file tree
Showing 18 changed files with 226 additions and 349 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ jobs:
- uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
- name: Update npm to at least v7 for workspaces support
if: ${{ matrix.node-version < 16 }} # node@16 comes with npm@8, node@14 with npm@6
run: npm install -g npm@7 # npm@7 supports node >=10
- name: Install dependencies
run: utils/run-install.sh
- name: Test Node.js v${{ matrix.node-version }}
Expand Down
9 changes: 8 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,14 @@ lint: check-license-headers

.PHONY: fmt
fmt:
npm --workspaces lint:fix # requires npm>=7 (aka node>=16)
npm --workspaces run lint:fix # requires npm>=7 (aka node>=16)

.PHONY: clean
clean:
rm -rf packages/*/node_modules
rm -rf node_modules
rm -rf utils/node_modules


# Build and open the rendered docs for testing.
#
Expand Down
6 changes: 6 additions & 0 deletions packages/ecs-helpers/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# @elastic/ecs-helpers Changelog

## 2.0.0

- [Breaking change.] Drop the `serialize` method. Serialization will move to
the individual `@elastic/ecs-*-format` libraries -- and they will be
switching to the `safe-stable-stringify` package.

## v1.1.0

- Fix `formatHttpRequest` and `formatHttpResponse` to be more defensive. If
Expand Down
24 changes: 0 additions & 24 deletions packages/ecs-helpers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,6 @@ npm install @elastic/ecs-helpers

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

### `stringify`

Function that serializes (very quickly!) an ECS-format log record object.

```js
const { stringify } = require('@elastic/ecs-helpers')
const ecs = {
'@timestamp': new Date().toISOString(),
'log.level': 'info',
message: 'hello world',
log: {
logger: 'test'
},
'ecs.version': '1.4.0'
}

console.log(stringify(ecs))
```

Note: This uses [fast-json-stringify](https://github.com/fastify/fast-json-stringify)
for serialization. By design this chooses speed over supporting serialization
of objects with circular references. This generally means that ecs-logging-nodejs
libraries will throw a "Converting circular structure to JSON" exception if an
attempt is made to log an object with circular references.

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

Expand Down
2 changes: 0 additions & 2 deletions packages/ecs-helpers/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@

'use strict'

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

module.exports = {
version: '1.6.0',
stringify,
...errorFormatters,
...httpFormatters
}
160 changes: 0 additions & 160 deletions packages/ecs-helpers/lib/serializer.js

This file was deleted.

5 changes: 1 addition & 4 deletions packages/ecs-helpers/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@elastic/ecs-helpers",
"version": "1.1.0",
"version": "2.0.0",
"description": "ecs-logging-nodejs helpers",
"main": "lib/index.js",
"files": [
Expand All @@ -24,9 +24,6 @@
"engines": {
"node": ">=10"
},
"dependencies": {
"fast-json-stringify": "^2.4.1"
},
"devDependencies": {
"@hapi/hapi": "^20.1.0",
"ajv": "^7.0.3",
Expand Down
57 changes: 14 additions & 43 deletions packages/ecs-helpers/test/basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ const { ecsLoggingValidate } = require('../../../utils/lib/ecs-logging-validate'

const {
version,
stringify,
formatError,
formatHttpRequest,
formatHttpResponse
Expand All @@ -42,35 +41,29 @@ const ajv = new Ajv({
addFormats(ajv)
const validate = ajv.compile(require('../../../utils/schema.json'))

test('stringify should return a valid ecs json', t => {
const ecsFields = {
test('validate valid ECS json', t => {
const rec = {
'@timestamp': new Date().toISOString(),
'log.level': 'info',
message: 'hello world',
'ecs.version': '1.4.0'
}

const line = stringify(ecsFields)
const rec = JSON.parse(line)
t.equal(validate(rec), true)
t.equal(ecsLoggingValidate(line), null)
t.equal(ecsLoggingValidate(rec), null)
t.end()
})

test('bad ECS json on purpose: @timestamp', t => {
const ecsFields = {
const rec = {
'@timestamp': 'not a date',
'log.level': 'info',
message: 'foo',
'ecs.version': '1.4.0'
}

const line = stringify(ecsFields)
const rec = JSON.parse(line)

t.equal(validate(rec), false)

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

const line = JSON.parse(stringify(ecs))
t.ok(validate(line))
t.equal(ecsLoggingValidate(line), null)
t.ok(validate(ecs))
t.equal(ecsLoggingValidate(ecs), null)

t.same(line.user_agent, { original: 'cool-agent' })
t.same(line.url, {
t.same(ecs.user_agent, { original: 'cool-agent' })
t.same(ecs.url, {
path: '/hello/world',
query: 'foo=bar',
full: `http://127.0.0.1:${server.address().port}/hello/world?foo=bar#anchor`,
fragment: 'anchor'
})
t.same(line.http, {
t.same(ecs.http, {
version: '1.1',
request: {
method: 'POST',
Expand All @@ -179,11 +171,11 @@ test('formatHttpRequest and formatHttpResponse should return a valid ecs object'
}
})
// https://www.elastic.co/guide/en/ecs/current/ecs-client.html fields
t.ok(line.client, 'client fields are set')
t.ok(line.client.address === '127.0.0.1', 'client.address is set')
t.ok(line.client.ip === line.client.address,
t.ok(ecs.client, 'client fields are set')
t.ok(ecs.client.address === '127.0.0.1', 'client.address is set')
t.ok(ecs.client.ip === ecs.client.address,
'client.address duplicated to client.ip')
t.equal(typeof (line.client.port), 'number')
t.equal(typeof (ecs.client.port), 'number')

res.end(resBody)
}
Expand Down Expand Up @@ -224,27 +216,6 @@ test('Should export a valid version', t => {
t.end()
})

test('stringify should emit valid tracing fields', t => {
const before = {
'@timestamp': new Date().toISOString(),
'log.level': 'info',
message: 'hello world',
'ecs.version': '1.4.0',
trace: { id: 1 },
transaction: { id: 2 },
span: { id: 3, extra_fields: 'are dropped' }
}

const after = JSON.parse(stringify(before))
t.ok(validate(after))
t.equal(ecsLoggingValidate(after), null)
t.same(after.trace, { id: '1' }, 'trace.id is stringified')
t.same(after.transaction, { id: '2' }, 'transaction.id is stringified')
t.same(after.span, { id: '3' },
'span.id is stringified, extra fields are excluded')
t.end()
})

test('formatError: Error', t => {
const rec = {}
formatError(rec, new Error('boom'))
Expand Down
6 changes: 6 additions & 0 deletions packages/ecs-morgan-format/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# @elastic/ecs-morgan-format Changelog

## Unreleased

- Switch to `safe-stable-stringify` for JSON serialization. This library
protects against circular references and bigints.
(https://github.com/elastic/ecs-logging-nodejs/pull/155)

## v1.4.0

- Add `service.version`, `service.environment`, and `service.node.name` log
Expand Down
Loading

0 comments on commit efb11b7

Please sign in to comment.