bugfix(node): Fix issue where middleware registered alone is treated as a route handler

Karibash · Karibash · commit be8604bc6a85 · 2025-09-23T17:11:31.000+09:00
diff --git a/dev-packages/node-integration-tests/suites/tracing/hono/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/hono/scenario.mjs
@@ -43,6 +43,17 @@ basePaths.forEach(basePath => {
       },
     );
 
+    // anonymous middleware
+    baseApp[method]('/middleware/separately', async (c, next) => {
+      await next();
+    });
+
+    baseApp[method]('/middleware/separately', async c => {
+      const response = c.text('response 200');
+      if (basePath === '/sync') return response;
+      return Promise.resolve(response);
+    });
+
     baseApp.all('/all', c => {
       const response = c.text('response 200');
       if (basePath === '/sync') return response;
@@ -62,6 +73,17 @@ basePaths.forEach(basePath => {
       },
     );
 
+    // anonymous middleware
+    baseApp.all('/all/middleware/separately', async (c, next) => {
+      await next();
+    });
+
+    baseApp.all('/all/middleware/separately', async c => {
+      const response = c.text('response 200');
+      if (basePath === '/sync') return response;
+      return Promise.resolve(response);
+    });
+
     baseApp.on(method, '/on', c => {
       const response = c.text('response 200');
       if (basePath === '/sync') return response;
@@ -82,6 +104,17 @@ basePaths.forEach(basePath => {
       },
     );
 
+    // anonymous middleware
+    baseApp.on(method, '/on/middleware/separately', async (c, next) => {
+      await next();
+    });
+
+    baseApp.on(method, '/on/middleware/separately', async c => {
+      const response = c.text('response 200');
+      if (basePath === '/sync') return response;
+      return Promise.resolve(response);
+    });
+
     baseApp[method]('/401', () => {
       const response = new HTTPException(401, { message: 'response 401' });
       if (basePath === '/sync') throw response;
diff --git a/dev-packages/node-integration-tests/suites/tracing/hono/test.ts b/dev-packages/node-integration-tests/suites/tracing/hono/test.ts
@@ -137,6 +137,74 @@ describe('hono tracing', () => {
             await runner.completed();
           });
 
+          test('should handle transaction with separate middleware', async () => {
+            const runner = createRunner()
+              .expect({
+                transaction: {
+                  transaction: `${method.toUpperCase()} ${route}${path === '/' ? '' : path}/middleware/separately`,
+                  spans: expect.arrayContaining([
+                    expect.objectContaining({
+                      data: expect.objectContaining({
+                        'hono.name': 'sentryRequestMiddleware',
+                        'hono.type': 'middleware',
+                      }),
+                      description: 'sentryRequestMiddleware',
+                      op: 'middleware.hono',
+                      origin: 'auto.http.otel.hono',
+                    }),
+                    expect.objectContaining({
+                      data: expect.objectContaining({
+                        'hono.name': 'sentryErrorMiddleware',
+                        'hono.type': 'middleware',
+                      }),
+                      description: 'sentryErrorMiddleware',
+                      op: 'middleware.hono',
+                      origin: 'auto.http.otel.hono',
+                    }),
+                    expect.objectContaining({
+                      data: expect.objectContaining({
+                        'hono.name': 'global',
+                        'hono.type': 'middleware',
+                      }),
+                      description: 'global',
+                      op: 'middleware.hono',
+                      origin: 'auto.http.otel.hono',
+                    }),
+                    expect.objectContaining({
+                      data: expect.objectContaining({
+                        'hono.name': 'base',
+                        'hono.type': 'middleware',
+                      }),
+                      description: 'base',
+                      op: 'middleware.hono',
+                      origin: 'auto.http.otel.hono',
+                    }),
+                    expect.objectContaining({
+                      data: expect.objectContaining({
+                        'hono.name': 'anonymous',
+                        'hono.type': 'middleware',
+                      }),
+                      description: 'anonymous',
+                      op: 'middleware.hono',
+                      origin: 'auto.http.otel.hono',
+                    }),
+                    expect.objectContaining({
+                      data: expect.objectContaining({
+                        'hono.name': `${route}${path === '/' ? '' : path}/middleware/separately`,
+                        'hono.type': 'request_handler',
+                      }),
+                      description: `${route}${path === '/' ? '' : path}/middleware/separately`,
+                      op: 'request_handler.hono',
+                      origin: 'auto.http.otel.hono',
+                    }),
+                  ]),
+                },
+              })
+              .start();
+            runner.makeRequest(method, `${route}${path === '/' ? '' : path}/middleware/separately`);
+            await runner.completed();
+          });
+
           test('should handle returned errors for %s path', async () => {
             const runner = createRunner()
               .ignore('transaction')
diff --git a/packages/node/src/integrations/tracing/hono/index.ts b/packages/node/src/integrations/tracing/hono/index.ts
@@ -4,7 +4,6 @@ import {
   captureException,
   debug,
   defineIntegration,
-  getClient,
   getDefaultIsolationScope,
   getIsolationScope,
   httpRequestToRequestData,
@@ -20,7 +19,44 @@ import type { Context, MiddlewareHandler, MiddlewareHandlerInterface, Next } fro
 
 const INTEGRATION_NAME = 'Hono';
 
-export const instrumentHono = generateInstrumentOnce(INTEGRATION_NAME, () => new HonoInstrumentation());
+function addHonoSpanAttributes(span: Span): void {
+  const attributes = spanToJSON(span).data;
+  const type = attributes[AttributeNames.HONO_TYPE];
+  if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) {
+    return;
+  }
+
+  span.setAttributes({
+    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.hono',
+    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.hono`,
+  });
+
+  const name = attributes[AttributeNames.HONO_NAME];
+  if (typeof name === 'string') {
+    span.updateName(name);
+  }
+
+  if (getIsolationScope() === getDefaultIsolationScope()) {
+    DEBUG_BUILD && debug.warn('Isolation scope is default isolation scope - skipping setting transactionName');
+    return;
+  }
+
+  const route = attributes[ATTR_HTTP_ROUTE];
+  const method = attributes[ATTR_HTTP_REQUEST_METHOD];
+  if (typeof route === 'string' && typeof method === 'string') {
+    getIsolationScope().setTransactionName(`${method} ${route}`);
+  }
+}
+
+export const instrumentHono = generateInstrumentOnce(
+  INTEGRATION_NAME,
+  () =>
+    new HonoInstrumentation({
+      responseHook: span => {
+        addHonoSpanAttributes(span);
+      },
+    }),
+);
 
 const _honoIntegration = (() => {
   return {
@@ -86,35 +122,6 @@ function honoErrorHandler(options?: Partial<HonoHandlerOptions>): MiddlewareHand
   };
 }
 
-function addHonoSpanAttributes(span: Span): void {
-  const attributes = spanToJSON(span).data;
-  const type = attributes[AttributeNames.HONO_TYPE];
-  if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) {
-    return;
-  }
-
-  span.setAttributes({
-    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.hono',
-    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.hono`,
-  });
-
-  const name = attributes[AttributeNames.HONO_NAME];
-  if (typeof name === 'string') {
-    span.updateName(name);
-  }
-
-  if (getIsolationScope() === getDefaultIsolationScope()) {
-    DEBUG_BUILD && debug.warn('Isolation scope is default isolation scope - skipping setting transactionName');
-    return;
-  }
-
-  const route = attributes[ATTR_HTTP_ROUTE];
-  const method = attributes[ATTR_HTTP_REQUEST_METHOD];
-  if (typeof route === 'string' && typeof method === 'string') {
-    getIsolationScope().setTransactionName(`${method} ${route}`);
-  }
-}
-
 /**
  * Add a Hono error handler to capture errors to Sentry.
  *
@@ -139,13 +146,5 @@ export function setupHonoErrorHandler(
 ): void {
   app.use(honoRequestHandler());
   app.use(honoErrorHandler(options));
-
-  const client = getClient();
-  if (client) {
-    client.on('spanStart', span => {
-      addHonoSpanAttributes(span);
-    });
-  }
-
   ensureIsWrapped(app.use, 'hono');
 }
diff --git a/packages/node/src/integrations/tracing/hono/instrumentation.ts b/packages/node/src/integrations/tracing/hono/instrumentation.ts
@@ -1,5 +1,6 @@
 import type { Span } from '@opentelemetry/api';
 import { context, SpanStatusCode, trace } from '@opentelemetry/api';
+import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
 import { isThenable } from '@sentry/core';
 import { AttributeNames, HonoTypes } from './constants';
@@ -18,12 +19,21 @@ import type {
 const PACKAGE_NAME = '@sentry/instrumentation-hono';
 const PACKAGE_VERSION = '0.0.1';
 
+export interface HonoResponseHookFunction {
+  (span: Span): void;
+}
+
+export interface HonoInstrumentationConfig extends InstrumentationConfig {
+  /** Function for adding custom span attributes from the response */
+  responseHook?: HonoResponseHookFunction;
+}
+
 /**
  * Hono instrumentation for OpenTelemetry
  */
-export class HonoInstrumentation extends InstrumentationBase {
-  public constructor() {
-    super(PACKAGE_NAME, PACKAGE_VERSION, {});
+export class HonoInstrumentation extends InstrumentationBase<HonoInstrumentationConfig> {
+  public constructor(config: HonoInstrumentationConfig = {}) {
+    super(PACKAGE_NAME, PACKAGE_VERSION, config);
   }
 
   /**
@@ -86,23 +96,13 @@ export class HonoInstrumentation extends InstrumentationBase {
           const handlers = args.slice(1);
           return original.apply(this, [
             path,
-            ...handlers.map((handler, index) =>
-              instrumentation._wrapHandler(
-                index + 1 === handlers.length ? HonoTypes.REQUEST_HANDLER : HonoTypes.MIDDLEWARE,
-                handler as Handler | MiddlewareHandler,
-              ),
-            ),
+            ...handlers.map(handler => instrumentation._wrapHandler(handler as Handler | MiddlewareHandler)),
           ]);
         }
 
         return original.apply(
           this,
-          args.map((handler, index) =>
-            instrumentation._wrapHandler(
-              index + 1 === args.length ? HonoTypes.REQUEST_HANDLER : HonoTypes.MIDDLEWARE,
-              handler as Handler | MiddlewareHandler,
-            ),
-          ),
+          args.map(handler => instrumentation._wrapHandler(handler as Handler | MiddlewareHandler)),
         );
       };
     };
@@ -120,12 +120,7 @@ export class HonoInstrumentation extends InstrumentationBase {
         const handlers = args.slice(2);
         return original.apply(this, [
           ...args.slice(0, 2),
-          ...handlers.map((handler, index) =>
-            instrumentation._wrapHandler(
-              index + 1 === handlers.length ? HonoTypes.REQUEST_HANDLER : HonoTypes.MIDDLEWARE,
-              handler as Handler | MiddlewareHandler,
-            ),
-          ),
+          ...handlers.map(handler => instrumentation._wrapHandler(handler as Handler | MiddlewareHandler)),
         ]);
       };
     };
@@ -149,15 +144,13 @@ export class HonoInstrumentation extends InstrumentationBase {
           const handlers = args.slice(1);
           return original.apply(this, [
             path,
-            ...handlers.map(handler =>
-              instrumentation._wrapHandler(HonoTypes.MIDDLEWARE, handler as MiddlewareHandler),
-            ),
+            ...handlers.map(handler => instrumentation._wrapHandler(handler as MiddlewareHandler)),
           ]);
         }
 
         return original.apply(
           this,
-          args.map(handler => instrumentation._wrapHandler(HonoTypes.MIDDLEWARE, handler as MiddlewareHandler)),
+          args.map(handler => instrumentation._wrapHandler(handler as MiddlewareHandler)),
         );
       };
     };
@@ -166,7 +159,7 @@ export class HonoInstrumentation extends InstrumentationBase {
   /**
    * Wraps a handler or middleware handler to apply instrumentation.
    */
-  private _wrapHandler(type: HonoTypes, handler: Handler | MiddlewareHandler): Handler | MiddlewareHandler {
+  private _wrapHandler(handler: Handler | MiddlewareHandler): Handler | MiddlewareHandler {
     // eslint-disable-next-line @typescript-eslint/no-this-alias
     const instrumentation = this;
 
@@ -176,17 +169,32 @@ export class HonoInstrumentation extends InstrumentationBase {
       }
 
       const path = c.req.path;
-      const spanName = `${type.replace('_', ' ')} - ${path}`;
-      const span = instrumentation.tracer.startSpan(spanName, {
-        attributes: {
-          [AttributeNames.HONO_TYPE]: type,
-          [AttributeNames.HONO_NAME]: type === 'request_handler' ? path : handler.name || 'anonymous',
-        },
-      });
+      const span = instrumentation.tracer.startSpan(path);
 
       return context.with(trace.setSpan(context.active(), span), () => {
         return instrumentation._safeExecute(
-          () => handler.apply(this, [c, next]),
+          () => {
+            const result = handler.apply(this, [c, next]);
+            if (isThenable(result)) {
+              return result.then(result => {
+                const type = instrumentation._determineHandlerType(result);
+                span.setAttributes({
+                  [AttributeNames.HONO_TYPE]: type,
+                  [AttributeNames.HONO_NAME]: type === HonoTypes.REQUEST_HANDLER ? path : handler.name || 'anonymous',
+                });
+                instrumentation.getConfig().responseHook?.(span);
+                return result;
+              });
+            } else {
+              const type = instrumentation._determineHandlerType(result);
+              span.setAttributes({
+                [AttributeNames.HONO_TYPE]: type,
+                [AttributeNames.HONO_NAME]: type === HonoTypes.REQUEST_HANDLER ? path : handler.name || 'anonymous',
+              });
+              instrumentation.getConfig().responseHook?.(span);
+              return result;
+            }
+          },
           () => span.end(),
           error => {
             instrumentation._handleError(span, error);
@@ -221,6 +229,15 @@ export class HonoInstrumentation extends InstrumentationBase {
     }
   }
 
+  /**
+   * Determines the handler type based on the result.
+   * @param result
+   * @private
+   */
+  private _determineHandlerType(result: unknown): HonoTypes {
+    return result === undefined ? HonoTypes.MIDDLEWARE : HonoTypes.REQUEST_HANDLER;
+  }
+
   /**
    * Handles errors by setting the span status and recording the exception.
    */
