Skip to content

Commit

Permalink
Migrate to SignalFX error tags (signalfx#50)
Browse files Browse the repository at this point in the history
* Replace OpenTracing error logs with SignalFx tags

This PR changes how errors are recorded on spans. It uses the new
SignalFx semantic convention and records the errors as attributes
instead of logs.

In addition to updating errors, also fixed postgresql and elasticsearch
failing tests.

Other changes:

- Explicitly limits support for Koa to <8.0. Will submit another PR to
add support for >8.0

* removed error.object tag
  • Loading branch information
owais authored Mar 31, 2020
1 parent 75ae35d commit 2a640f9
Show file tree
Hide file tree
Showing 46 changed files with 266 additions and 189 deletions.
2 changes: 2 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,8 @@ jobs:
- PLUGINS=pg
- PG_TEST_NATIVE=true
- image: postgres:9.5
environment:
- POSTGRES_HOST_AUTH_METHOD=trust

node-q:
<<: *node-plugin-base
Expand Down
2 changes: 1 addition & 1 deletion docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"license": "BSD-3-Clause",
"private": true,
"devDependencies": {
"typedoc": "^0.13.0",
"typedoc": "^0.17.3",
"typescript": "^3.3.3"
}
}
12 changes: 6 additions & 6 deletions src/format.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ function extractTags (trace, span) {
trace.error = 1
}
break
case 'error.type':
case 'error.msg':
case 'error.stack':
case 'sfx.error.kind':
case 'sfx.error.message':
case 'sfx.error.stack':
trace.error = 1
default: // eslint-disable-line no-fallthrough
addTag(trace.meta, tag, tags[tag])
Expand All @@ -85,9 +85,9 @@ function extractError (trace, span) {
const error = span.context()._tags['error']

if (error instanceof Error) {
trace.meta['error.msg'] = error.message
trace.meta['error.type'] = error.name
trace.meta['error.stack'] = error.stack
trace.meta['sfx.error.message'] = error.message
trace.meta['sfx.error.kind'] = error.name
trace.meta['sfx.error.stack'] = error.stack
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/plugins/amqp10.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,9 @@ function addTags (tracer, config, span, link) {
function finish (span, error) {
if (error) {
span.addTags({
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})
}

Expand Down
6 changes: 3 additions & 3 deletions src/plugins/amqplib.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ function getResourceName (method, fields) {

function addError (span, error) {
span.addTags({
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})

return error
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/cassandra-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,9 @@ function addError (span, error) {
if (error && error instanceof Error) {
span.addTags({
'error': 'true',
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})
}

Expand Down
8 changes: 4 additions & 4 deletions src/plugins/elasticsearch.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ function wrapCallback (tracer, span, done) {
function finish (span, err) {
if (err) {
span.addTags({
'error.type': err.name,
'error.msg': err.message,
'error.stack': err.stack
'sfx.error.kind': err.name,
'sfx.error.message': err.message,
'sfx.error.stack': err.stack
})
}

Expand All @@ -94,7 +94,7 @@ module.exports = [
{
name: '@elastic/elasticsearch',
file: 'lib/Transport.js',
versions: ['>=5.6.16'], // initial version of this module
versions: ['>=5.6.16', '>=7.6.0'], // initial version of this module
patch (Transport, tracer, config) {
this.wrap(Transport.prototype, 'request', createWrapRequest(tracer, config))
},
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/graphql.js
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,9 @@ function startResolveSpan (tracer, config, childOf, path, info, contextValue) {
function finish (error, span, finishTime) {
if (error) {
span.addTags({
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})
}

Expand Down
6 changes: 3 additions & 3 deletions src/plugins/http/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ function patch (http, methodName, tracer, config) {

function addError (span, error) {
span.addTags({
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})

return error
Expand Down
10 changes: 5 additions & 5 deletions src/plugins/knex.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ function createWrapRunnerQuery (tracer, config) {
}

function addError (span, error) {
span.setTag('error', 'true')
span.log({
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
span.addTags({
'error': true,
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})
return error
}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/koa.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ module.exports = [
},
{
name: 'koa-router',
versions: ['>=7'],
versions: ['>=7 <8'],
patch (Router, tracer, config) {
this.wrap(Router.prototype, 'register', createWrapRegister(tracer, config))
},
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/memcached.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ function addHost (span, client, server, query) {
function addError (span, error) {
if (error) {
span.addTags({
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})
}

Expand Down
6 changes: 3 additions & 3 deletions src/plugins/mongodb-core.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ function wrapCallback (tracer, span, done, cursor) {
return tracer.scope().bind((err, res) => {
if (err) {
span.addTags({
'error.type': err.name,
'error.msg': err.message,
'error.stack': err.stack
'sfx.error.kind': err.name,
'sfx.error.message': err.message,
'sfx.error.stack': err.stack
})
}

Expand Down
6 changes: 3 additions & 3 deletions src/plugins/mysql.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ function wrapCallback (tracer, span, parent, done) {
return tracer.scope().bind((err, res) => {
if (err) {
span.addTags({
'error.type': err.name,
'error.msg': err.message,
'error.stack': err.stack
'sfx.error.kind': err.name,
'sfx.error.message': err.message,
'sfx.error.stack': err.stack
})
}

Expand Down
6 changes: 3 additions & 3 deletions src/plugins/mysql2.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ function wrapCallback (tracer, span, parent, done) {
return tracer.scope().bind((err, res) => {
if (err) {
span.addTags({
'error.type': err.name,
'error.msg': err.message,
'error.stack': err.stack
'sfx.error.kind': err.name,
'sfx.error.message': err.message,
'sfx.error.stack': err.stack
})
}

Expand Down
9 changes: 4 additions & 5 deletions src/plugins/nest.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,10 @@ function createGuardsTrace (tracer, args, guards, instance, callback, fn) {
}

function addError (span, error) {
span.setTag('error', 'true')
span.log({
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
span.addTags({
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})
return error
}
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/pg.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ function createWrapQuery (tracer, config) {
pgQuery.callback = scope.bind((err, res) => {
if (err) {
span.addTags({
'error.type': err.name,
'error.msg': err.message,
'error.stack': err.stack
'sfx.error.kind': err.name,
'sfx.error.message': err.message,
'sfx.error.stack': err.stack
})
}

Expand Down
5 changes: 3 additions & 2 deletions src/plugins/sails.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ function wrapActionFunction (tracer, actionFn, identity) {
if (span) {
span.addTags({
'error': true,
'error.msg': e.message,
'error.stack': e.stack
'sfx.error.kind': e.name,
'sfx.error.message': e.message,
'sfx.error.stack': e.stack
})
}

Expand Down
6 changes: 3 additions & 3 deletions src/plugins/util/tx.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ function wrapPromise (span, promise) {
function finish (span, error) {
if (error) {
span.addTags({
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})
}

Expand Down
6 changes: 3 additions & 3 deletions src/plugins/util/web.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ const web = {
if (span) {
if (error) {
span.addTags({
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})
}

Expand Down
6 changes: 3 additions & 3 deletions src/scope/new/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ class Scope extends Base {
} catch (e) {
if (span && typeof span.addTags === 'function') {
span.addTags({
'error.type': e.name,
'error.msg': e.message,
'error.stack': e.stack
'sfx.error.kind': e.name,
'sfx.error.message': e.message,
'sfx.error.stack': e.stack
})
}

Expand Down
6 changes: 3 additions & 3 deletions src/tracer.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ class SignalFxTracer extends Tracer {
function addError (span, error) {
if (error && error instanceof Error) {
span.addTags({
'error.type': error.name,
'error.msg': error.message,
'error.stack': error.stack
'sfx.error.kind': error.name,
'sfx.error.message': error.message,
'sfx.error.stack': error.stack
})
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/zipkin/format.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ function formatTags (formatted, span) {
formattedTags.error = String(tags[tag])
}
break
case 'error.type':
case 'error.msg':
case 'error.stack':
case 'sfx.error.kind':
case 'sfx.error.message':
case 'sfx.error.stack':
formattedTags.error = 'true'
formattedTags[tag] = String(tags[tag])
break
Expand Down
12 changes: 6 additions & 6 deletions test/format.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ describe('format', () => {
spanContext._tags['error'] = error
trace = format(span)

expect(trace.meta['error.msg']).to.equal(error.message)
expect(trace.meta['error.type']).to.equal(error.name)
expect(trace.meta['error.stack']).to.equal(error.stack)
expect(trace.meta['sfx.error.message']).to.equal(error.message)
expect(trace.meta['sfx.error.kind']).to.equal(error.name)
expect(trace.meta['sfx.error.stack']).to.equal(error.stack)
})

it('should extract the origin', () => {
Expand Down Expand Up @@ -188,9 +188,9 @@ describe('format', () => {
})

it('should set the error flag when there is an error-related tag', () => {
spanContext._tags['error.type'] = 'Error'
spanContext._tags['error.msg'] = 'boom'
spanContext._tags['error.stack'] = ''
spanContext._tags['sfx.error.kind'] = 'Error'
spanContext._tags['sfx.error.message'] = 'boom'
spanContext._tags['sfx.error.stack'] = ''

trace = format(span)

Expand Down
7 changes: 3 additions & 4 deletions test/plugins/adonis.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ describe('Plugin', () => {
expect(spans[0]).to.have.property('name', 'adonis.middleware')
expect(spans[0].meta).to.have.property('component', 'adonis')
expect(spans[0].meta).to.have.property('error', 'true')
expect(spans[0].meta).to.have.property('error.type', 'Error')
expect(spans[0].meta).to.have.property('error.msg', 'custom error')
expect(spans[0].meta).to.have.property('error.stack')
expect(spans[0].meta).to.have.property('sfx.error.kind', 'Error')
expect(spans[0].meta).to.have.property('sfx.error.message', 'custom error')
expect(spans[0].meta).to.have.property('sfx.error.stack')
expect(spans[0].parent_id.toString()).to.equal(spans[1].span_id.toString())

expect(spans[1]).to.have.property('service', 'test')
Expand All @@ -150,7 +150,6 @@ describe('Plugin', () => {
expect(spans[1].meta).to.have.property('http.url', `http://localhost:${port}/api/user/${randId}`)
expect(spans[1].meta).to.have.property('http.method', 'GET')
expect(spans[1].meta).to.have.property('http.status_code', '500')
expect(spans[1].meta).to.have.property('error', 'true')
}).then(done)
.catch(done)

Expand Down
6 changes: 3 additions & 3 deletions test/plugins/amqp10.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ describe('Plugin', () => {
const span = traces[0][0]

expect(span.meta).to.have.property('error', 'true')
expect(span.meta).to.have.property('error.type', error.name)
expect(span.meta).to.have.property('error.msg', error.message)
expect(span.meta).to.have.property('error.stack', error.stack)
expect(span.meta).to.have.property('sfx.error.kind', error.name)
expect(span.meta).to.have.property('sfx.error.message', error.message)
expect(span.meta).to.have.property('sfx.error.stack', error.stack)
}, 2)
.then(done)
.catch(done)
Expand Down
Loading

0 comments on commit 2a640f9

Please sign in to comment.