fix: preserve the error description on the request span for 5xx

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)
           }

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()