feat(server-utils): Expose channel-based, streamlined fastifyIntegration#21706
feat(server-utils): Expose channel-based, streamlined fastifyIntegration#21706mydea wants to merge 8 commits into
fastifyIntegration#21706Conversation
mydea
changed the title
fastifyIntegration
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| instrumentSentryHttp, | ||
| instrumentExpress, | ||
| instrumentConnect, | ||
| instrumentFastify, |
There was a problem hiding this comment.
Preload misses early Fastify instances
High Severity
Removing v5 Fastify instrumentation from the preload list means fastify.initialization is only subscribed during fastifyIntegration setupOnce at Sentry.init(). Any Fastify instance created before init (common with @sentry/node/preload and deferred init) never receives the tracing plugin.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit faecc64. Configure here.
There was a problem hiding this comment.
this is fine, init should still be called before other stuff
mydea
force-pushed
the
ref/streamline-fastify-otel-vendored
branch
from
faecc64 to
7edd803
Compare
mydea
requested review from
JPeer264 and
andreiborza
and removed request for
a team
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7edd803. Configure here.
|
|
||
| const hookType = spanAttributes[ATTRIBUTE_FASTIFY_TYPE]; | ||
| const op = | ||
| hookType === HOOK_TYPE_INSTANCE ? HOOK_OP : hookType === HOOK_TYPE_HANDLER ? REQUEST_HANDLER_OP : undefined; |
There was a problem hiding this comment.
Route hooks lack span op
Medium Severity
Route-level Fastify hooks wired in otelWireRoute use fastify.type route-hook, but handlerWrapper only assigns hook.fastify / request_handler.fastify for instance hooks and handlers. Route hooks such as route preHandler spans therefore get no sentry.op, unlike the integration test expectations and prior span enrichment behavior.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7edd803. Configure here.
There was a problem hiding this comment.
seems to be fine in all tests. not sure what a reasonable fallback op would be here...
|
@isaacs we can likely also use this as-is in deno and bun (IMHO we can do that in a follow up) |
2 participants
This is the first part of #20745, streamlining fastify v5 instrumentation:
Some other general notes:
instrumentFastifymethod and no longer calls it in preload, as this does not actually need to be preloaded anymore.