feat(node): Add an instrumentation interface for Hono#17366
feat(node): Add an instrumentation interface for Hono#17366s1gr1d merged 5 commits intogetsentry:developfrom
Conversation
There was a problem hiding this comment.
I extracted and defined only the minimal types necessary for instrumentation from Hono source code.
| function Hono(this: HonoInstance, ...args: any): HonoInstance { | ||
| const app: HonoInstance = moduleExports.Hono.apply(this, args); | ||
|
|
||
| instrumentation._wrap(app, 'get', instrumentation._patchHandler()); | ||
| instrumentation._wrap(app, 'post', instrumentation._patchHandler()); | ||
| instrumentation._wrap(app, 'put', instrumentation._patchHandler()); | ||
| instrumentation._wrap(app, 'delete', instrumentation._patchHandler()); | ||
| instrumentation._wrap(app, 'options', instrumentation._patchHandler()); | ||
| instrumentation._wrap(app, 'patch', instrumentation._patchHandler()); | ||
| instrumentation._wrap(app, 'all', instrumentation._patchHandler()); | ||
| instrumentation._wrap(app, 'on', instrumentation._patchOnHandler()); | ||
| instrumentation._wrap(app, 'use', instrumentation._patchMiddlewareHandler()); | ||
|
|
||
| return app; | ||
| } |
There was a problem hiding this comment.
Hono dynamically defines these methods within its constructor, which prevented overriding the prototype, so I decided to patch the constructor instead.
| /** | ||
| * Patches the route handler to instrument it. | ||
| */ | ||
| private _patchHandler(): (original: HandlerInterface) => HandlerInterface { | ||
| return function(original: HandlerInterface) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| return function wrappedHandler(this: HonoInstance, ...args: any) { | ||
| // TODO: Add OpenTelemetry tracing logic here | ||
| return original.apply(this, args); | ||
| }; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Patches the 'on' handler to instrument it. | ||
| */ | ||
| private _patchOnHandler(): (original: OnHandlerInterface) => OnHandlerInterface { | ||
| return function(original: OnHandlerInterface) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| return function wrappedHandler(this: HonoInstance, ...args: any) { | ||
| // TODO: Add OpenTelemetry tracing logic here | ||
| return original.apply(this, args); | ||
| }; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Patches the middleware handler to instrument it. | ||
| */ | ||
| private _patchMiddlewareHandler(): (original: MiddlewareHandlerInterface) => MiddlewareHandlerInterface { | ||
| return function(original: MiddlewareHandlerInterface) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| return function wrappedHandler(this: HonoInstance, ...args: any) { | ||
| // TODO: Add OpenTelemetry tracing logic here | ||
| return original.apply(this, args); | ||
| }; | ||
| }; | ||
| } |
There was a problem hiding this comment.
The implementation of these patches will be submitted later across several PRs.
75251f3 to
0ddfe17
Compare
… instead of function replacement
b40e85a to
cb1a52e
Compare
6a3abac to
21fba19
Compare
There was a problem hiding this comment.
We don't use enums in our repository (also see related PR). However, I think this file can be deleted, as those enums are not used anywhere.
There was a problem hiding this comment.
I have removed the enums as you pointed out.
These values will be needed when specifying span attributes, so at that time I plan to define them as constants with primitive values.
2a6cd6e
| @@ -0,0 +1,88 @@ | |||
| import { InstrumentationBase,InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; | |||
There was a problem hiding this comment.
Run yarn fix to fix linting and the code formatting with prettier.
There was a problem hiding this comment.
I’ve fixed it. I’ll be careful not to forget next time.
98a2c37
| /** | ||
| * Patches the module exports to instrument Hono. | ||
| */ | ||
| private _patch(moduleExports: { Hono: Hono }): { Hono: Hono } { |
There was a problem hiding this comment.
Lowercase the hono variable so it's easier to differentiate between the type and the variable.
| private _patch(moduleExports: { Hono: Hono }): { Hono: Hono } { | |
| private _patch(moduleExports: { hono: Hono }): { hono: Hono } { |
There was a problem hiding this comment.
Since we are patching the class exported by hono, wouldn’t it need to be uppercase here, same as the exported class name?
https://github.com/honojs/hono/blob/main/src/hono.ts#L16
|
Thank you for this contribution 🙌 |
98a2c37 to
e694aae
Compare
| instrumentation._wrap(this, 'use', instrumentation._patchMiddlewareHandler()); | ||
| } | ||
| }; | ||
| return moduleExports; |
There was a problem hiding this comment.
Bug: Hono Instrumentation Fails on Default Imports
The Hono instrumentation only patches the named Hono export, ignoring the default export. This prevents instrumentation for applications importing Hono as a default. Additionally, the Hono class is replaced in-place without using InstrumentationBase._wrap, and instance methods are wrapped within the subclass constructor. This design prevents proper unpatching, leading to persistent instrumentation where new Hono instances remain wrapped even after the instrumentation is disabled, breaking enable/disable semantics.
|
Hi @s1gr1d, |
|
Sorry, we were busy with Hackweek last week (an internal hackathon) but I merged it now :) |
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #17366 Co-authored-by: s1gr1d <32902192+s1gr1d@users.noreply.github.com>
This PR introduces the scaffolding for Sentry’s tracing integration with Hono by adding interface-only implementations and wiring needed to verify the overall approach before filling in the tracing logic.
Summary
Intent & scope
The goal is to confirm the wrapping points, attribute schema, and initialization flow before we add any span creation, context propagation, or attribute setting.
That follow-up will include span start/finish, setting hono.type/hono.name, request path/method extraction, and trace context propagation.
Rationale
There is an existing Hono OTel package (@hono/otel), but it currently lacks several features we need for a robust Sentry integration—especially middleware instrumentation and Sentry-specific integration points (e.g., seamless correlation with Sentry transactions/spans and future Sentry error handler wiring).
Given these gaps, we’re proceeding with an in-repo implementation tailored for Sentry’s needs.
Related Issue
#15260