feat(mongoose): Instrument mongoose >= 9.7 via native tracing channels#21803
Conversation
logaretm
force-pushed
the
awad/mongoose-tracing-channels
branch
from
668581c to
0e1e4c1
Compare
size-limit report 📦
|
more a general thing, but I think I'd just call this |
mydea
force-pushed
the
fn/better-integration-extend
branch
2 times, most recently
from
2407b9e to
31520be
Compare
|
Yea I was thinking about this yesterday, we ideally would export those as-is for the environments that do not support module hooks. Will make that change |
logaretm
force-pushed
the
awad/mongoose-tracing-channels
branch
from
5a990f3 to
3e8b992
Compare
logaretm
requested review from
JPeer264,
andreiborza,
isaacs,
mydea and
nicohrubec
and removed request for
a team
logaretm
force-pushed
the
awad/mongoose-tracing-channels
branch
from
ccbdd92 to
9cbf9df
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
Reviewed by Cursor Bugbot for commit 9cbf9df. Configure here.
| instrumentMongoose(); | ||
| }, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Preload skips mongoose channel subscribe
Medium Severity
For mongoose >=9.7, diagnostics-channel subscription runs only in the server-utils integration setupOnce, while preloadOpenTelemetry invokes instrumentMongoose alone. The IITM patcher no longer covers 9.7+, so mongoose work between preload and Sentry.init() (or any preload-only usage) is not traced.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9cbf9df. Configure here.
JPeer264
left a comment
JPeer264
left a comment
There was a problem hiding this comment.
Code wise LGTM. I think this one has not been done yet? #21803 (comment) (I would vote for that too)
| ...(collection != null ? { [DB_COLLECTION_NAME]: collection } : {}), | ||
| ...(data.database != null ? { [DB_NAMESPACE]: data.database } : {}), | ||
| ...(queryText != null ? { [DB_QUERY_TEXT]: queryText } : {}), | ||
| ...(batchSize != null ? { [DB_OPERATION_BATCH_SIZE]: batchSize } : {}), |
There was a problem hiding this comment.
l: undefined values are skipped, null not, so it seems like this was done on purpose. I think following is a little easier to read:
| ...(batchSize != null ? { [DB_OPERATION_BATCH_SIZE]: batchSize } : {}), | |
| [DB_OPERATION_BATCH_SIZE]: batchSize ?? undefined |
There was a problem hiding this comment.
Sounds good, that was ugly
Subscribe to mongoose 9.7's native diagnostics_channel tracing channels via bindTracingChannelToSpan, emitting stable OTel DB semconv. Narrow the vendored OTel patcher to < 9.7.0 to avoid double instrumentation.
…gration Runtimes that use the server-utils integration directly expose it to users, so 'channel' in the name leaks an implementation detail. Node imports it under an internal alias since it keeps its own composed mongooseIntegration.
Replace the conditional-spread pattern for optional attributes with `?? undefined`. Undefined-valued keys are dropped by the span backends, so this is equivalent while keeping all six attribute lines consistent.
Replace the ad-hoc `void Promise.resolve().then(...)` deferral with the shared `waitForTracingChannelBinding` util, matching the vercel-ai and redis integrations. It retries until the async context binding is available, so it also handles custom OTel setups where the binding isn't ready on the first tick.
logaretm
force-pushed
the
awad/mongoose-tracing-channels
branch
from
9cbf9df to
6e85b2f
Compare
3 participants
Instruments mongoose >= 9.7 through its native
diagnostics_channeltracing channels instead of monkey-patching, the same way we did for redis/ioredis. The subscription lives inserver-utilsasmongooseChannelIntegration, and the nodemongooseIntegrationextends it (viaextendIntegration) to keep running the IITM patcher for mongoose < 9.7.The channel path emits the latest stable DB semconv, so there is an attribute drift. We did that for redis so I thought to do the same here, but we could match the legacy attributes and drop them later for v11 if that's what we want.