From 82cdd5074f1b6f083b8c5cfb556e26f00c6ddd00 Mon Sep 17 00:00:00 2001 From: Will Date: Fri, 4 Sep 2026 11:03:47 -0400 Subject: [PATCH] fix: preserve the error description on the request span for 5xx `recordErrorInSpanHook` (onError) records the error status with `error.message` as the description. `finalizeResponseSpanHook` (onSend) runs afterwards and, for `statusCode >= 500`, called `setStatus` again with no message. Per the OTel spec `setStatus` replaces the status, so the description was cleared immediately before `span.end()` and the root request span was exported as ERROR with an empty description. Because the branch was gated on `>= 500`, 4xx kept its description and 5xx always lost it; child spans were unaffected. Track whether the error hook already recorded a status and only apply the bare status when it did not, so a 5xx that never threw (e.g. `reply.code(500).send()`) still gets an error status. --- index.js | 11 +++++- test/index.test.js | 98 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 6885e29..a75af5e 100644 --- a/index.js +++ b/index.js @@ -44,6 +44,7 @@ const ANONYMOUS_FUNCTION_NAME = 'anonymous' const kInstrumentation = Symbol('fastify otel instance') const kRequestSpan = Symbol('fastify otel request spans') const kRequestContext = Symbol('fastify otel request context') +const kErrorStatusSet = Symbol('fastify otel error status set') const kAddHookOriginal = Symbol('fastify otel addhook original') const kSetNotFoundOriginal = Symbol('fastify otel setnotfound original') const kIgnorePaths = Symbol('fastify otel ignore path') @@ -284,6 +285,7 @@ class FastifyOtelInstrumentation extends InstrumentationBase { }) instance.decorateRequest(kRequestSpan, null) instance.decorateRequest(kRequestContext, null) + instance.decorateRequest(kErrorStatusSet, false) instance.addHook('onRoute', function otelWireRoute (routeOptions) { if (instrumentation[kIgnorePaths]?.(routeOptions) === true) { @@ -460,6 +462,7 @@ class FastifyOtelInstrumentation extends InstrumentationBase { } request[kRequestSpan] = null + request[kErrorStatusSet] = false hookDone() }) @@ -474,7 +477,11 @@ class FastifyOtelInstrumentation extends InstrumentationBase { const span = request[kRequestSpan] if (span != null) { - if (reply.statusCode >= 500) { + // setStatus replaces the status, so setting a bare ERROR here would + // drop the description recorded by recordErrorInSpanHook. A 5xx that + // never threw (e.g. reply.code(500).send()) has no description of its + // own, so it still needs the bare status. + if (reply.statusCode >= 500 && request[kErrorStatusSet] === false) { span.setStatus({ code: SpanStatusCode.ERROR }) } @@ -485,6 +492,7 @@ class FastifyOtelInstrumentation extends InstrumentationBase { } request[kRequestSpan] = null + request[kErrorStatusSet] = false hookDone(null, payload) } @@ -498,6 +506,7 @@ class FastifyOtelInstrumentation extends InstrumentationBase { code: SpanStatusCode.ERROR, message: error.message }) + request[kErrorStatusSet] = true if (instrumentation[kRecordExceptions] !== false) { span.recordException(error) } diff --git a/test/index.test.js b/test/index.test.js index 1cc052d..6596798 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -1151,6 +1151,104 @@ describe('FastifyInstrumentation', () => { assert.equal(response.status, 500) }) + test('should keep the error description on the request span (5xx)', async t => { + const app = Fastify() + const plugin = instrumentation.plugin() + + await app.register(plugin) + + app.get('/', async function helloworld () { + throw new Error('kaboom') + }) + + await app.listen() + + after(() => app.close()) + + const response = await fetch( + `http://localhost:${app.server.address().port}/` + ) + + const spans = memoryExporter + .getFinishedSpans() + .filter(span => span.instrumentationScope.name === '@fastify/otel') + + const handler = spans.find(span => span.name.startsWith('handler')) + const start = spans.find(span => span.name === 'request') + + assert.equal(spans.length, 2) + assert.equal(response.status, 500) + assert.deepStrictEqual(handler.status, { + code: SpanStatusCode.ERROR, + message: 'kaboom' + }) + assert.deepStrictEqual(start.status, { + code: SpanStatusCode.ERROR, + message: 'kaboom' + }) + }) + + test('should keep the error description on the request span (4xx)', async t => { + const app = Fastify() + const plugin = instrumentation.plugin() + + await app.register(plugin) + + app.get('/', async function helloworld () { + const error = new Error('kaboom') + error.statusCode = 400 + throw error + }) + + await app.listen() + + after(() => app.close()) + + const response = await fetch( + `http://localhost:${app.server.address().port}/` + ) + + const spans = memoryExporter + .getFinishedSpans() + .filter(span => span.instrumentationScope.name === '@fastify/otel') + + const start = spans.find(span => span.name === 'request') + + assert.equal(response.status, 400) + assert.deepStrictEqual(start.status, { + code: SpanStatusCode.ERROR, + message: 'kaboom' + }) + }) + + test('should set an error status on a 5xx reply that did not throw', async t => { + const app = Fastify() + const plugin = instrumentation.plugin() + + await app.register(plugin) + + app.get('/', async function helloworld (request, reply) { + return reply.code(500).send('not an exception') + }) + + await app.listen() + + after(() => app.close()) + + const response = await fetch( + `http://localhost:${app.server.address().port}/` + ) + + const spans = memoryExporter + .getFinishedSpans() + .filter(span => span.instrumentationScope.name === '@fastify/otel') + + const start = spans.find(span => span.name === 'request') + + assert.equal(response.status, 500) + assert.deepStrictEqual(start.status, { code: SpanStatusCode.ERROR }) + }) + test('should create named span (404)', async t => { const app = Fastify() const plugin = instrumentation.plugin()