Skip to content

feat(hono): Support middleware spans defined in app groups - #20465

Merged
s1gr1d merged 7 commits into
developfrom
sig/sub-app-middleware
Apr 24, 2026
Merged

feat(hono): Support middleware spans defined in app groups#20465
s1gr1d merged 7 commits into
developfrom
sig/sub-app-middleware

Conversation

@s1gr1d

Copy link
Copy Markdown
Member

Patches app.route to also support middleware defined in Hono route groups.

The Node instrumentation still produces too many spans, this will be fixed in another PR - one test is skipped for now.

Closes#20449

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those are basically the tests like before, but split in two scenarios, with a different route prefix (/test-middleware and /test-subapp-middleware)

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 46f78a2. Configure here.

Comment threadpackages/hono/test/shared/patchAppUse.test.ts

expect(middlewareSpans).toHaveLength(2);
expect(middlewareSpans[0]?.description).toBe('middlewareA');
expect(middlewareSpans[1]?.description).toBe('middlewareB');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing .filter() makes span assertion too broad

Medium Severity

The original test filtered spans with .filter(span => span.op === 'middleware.hono' && span.origin === 'auto.middleware.hono') before sorting. The refactored version drops the .filter() entirely, so middlewareSpans now contains all transaction spans, not just middleware spans. The subsequent toHaveLength(2) and description checks operate on the wrong set, making the test fragile and semantically incorrect.

Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 46f78a2. Configure here.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not filtered on purpose - makes the test too weak. We want to specifically check for all spans that are here

Comment threaddev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts Outdated
@s1gr1d
s1gr1d merged commit 39ad06b into developApr 24, 2026
43 checks passed
@s1gr1d
s1gr1d deleted the sig/sub-app-middleware branch April 24, 2026 07:25
@s1gr1ds1gr1d changed the title test(hono): Add E2E tests for middleware spansfeat(hono): Support middleware spans defined in app groupsApr 24, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Hono] Support middleware spans defined in app groups

2 participants

@s1gr1d@nicohrubec