feat(hono): Instrument middlewares app.use()#19611
Conversation
![cursor[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| expect(fakeApp._capturedThis).toBe(fakeApp); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing integration or E2E test for feat PR
Medium Severity
This feat PR introduces middleware span instrumentation but only includes unit tests (patchAppUse.test.ts). No integration or E2E test is added to verify that middleware spans actually appear in transaction payloads sent to Sentry. The existing integration test suite in dev-packages/cloudflare-integration-tests/suites/hono-sdk/ does not include any assertions about middleware spans (e.g., spans with op: 'middleware.hono' in the transaction's span list). An integration or E2E test covering the new behavior is recommended.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
size-limit report 📦
|
# Conflicts: # packages/hono/src/cloudflare/middleware.ts # packages/hono/test/cloudflare/middleware.test.ts
|
Regarding your question: It just looks parallel, as all the middlewares had the artificial timeout only before calling 
|
| const MIDDLEWARE_ORIGIN = 'auto.middleware.hono'; | ||
|
|
||
| // Module-level counter for anonymous middleware span names | ||
| let MIDDLEWARE_IDX = 0; |
There was a problem hiding this comment.
l/q: In Cloudflare one isolate could handle more requests. Currently I am not 100% sure every if every request has their own bundled code (ergo starting at 0). Have you tried it out on Cloudflare how it is behaving under load when deployed?
There was a problem hiding this comment.
I tested it and it always starts at 0 👍
However, I'm changing it a bit to only increment if we apply the anonymous name. Right now, it increments on every middleware call which results in counting from 0 to 2 e.g. in this case, where timeout2 is in-between:
There was a problem hiding this comment.
I actually removed the numbering again. OTel also just uses <anonymous> as a name for anonymous functions and using a numbering here was just a best-effort to better distinguish them. But as it creates potentially wrong data, it's probably better if people just name their middleware (and most often, it's actually named).
![sentry[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
3 participants
Middleware spans are named either after the function name or they are numbered.
Middleware in Hono is onion-shaped (see docs) and technically, this shape would create a nested children-based span structure. This however, is not as intuitive and so I decided (after also talking to @andreiborza and @JPeer264) to create a sibiling-like structure:
Closes #19585