feature(node): Add instrumentation to the handler in Hono #17428
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Karibash
force-pushed
the
feature/hono-instrumentation-handler
branch
from
32e3a23 to
505c480
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Karibash
force-pushed
the
feature/hono-instrumentation-handler
branch
from
505c480 to
a7ef204
Compare
| } | ||
|
|
||
| const path = c.req.path; | ||
| const spanName = `${type.replace('_', ' ')} - ${path}`; |
There was a problem hiding this comment.
What would be an example span name here? I think there are no tests that check this name so I couldn't find an example.
Would be good to also have tests for this.
| if ( | ||
| result && | ||
| typeof result === 'object' && | ||
| typeof Object.getOwnPropertyDescriptor(result, 'then')?.value === 'function' |
There was a problem hiding this comment.
You can use isThenable here from @sentry/core
| * Safely executes a function and handles errors. | ||
| */ | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| private _safeExecute(execute: () => any, onSuccess: () => void, onFailure: (error: unknown) => void): () => any { |
There was a problem hiding this comment.
I'm wondering if it would be possible to type this instead of relying on any here?
| path, | ||
| ...handlers.map((handler, index) => | ||
| instrumentation._wrapHandler( | ||
| index + 1 === handlers.length ? HonoTypes.REQUEST_HANDLER : HonoTypes.MIDDLEWARE, |
There was a problem hiding this comment.
Can we be sure that the handlers are always those two in this order?
|
@Karibash just a general heads-up: It's awesome that you are working on this instrumentation right now. Keep in mind that we are planning to start working on a dedicated hono SDK in about a month. So your integration can be used in the meantime but we might need to deprecate it once the new SDK is stable :) |
|
@s1gr1d Thanks for the heads-up! |
|
It will use OTel under the hood but we'll focus on making it work on Cloudflare as a first priority. |
|
Got it — so in that sense, the approach in this PR won’t differ too much from what you’re planning for the new SDK (aside from the implementation details that will naturally evolve), right? |
Summary
This PR enhances the Hono integration by adding comprehensive handler instrumentation, error handling capabilities, and thorough test coverage. The changes build upon the basic Hono integration to provide a complete tracing and error monitoring solution.
New Features
Bug Fixes
Implementation Details
Testing
Related Issue
close #15260