Using both Bun and Express leads to "express is not instrumented" warning (Bun not publishing to http Diagnostic Channel)
Opened this issue · 12 comments
Is there an existing issue for this?
- I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
- I have reviewed the documentation https://docs.sentry.io/
- I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases
How do you use Sentry?
Sentry Saas (sentry.io)
Which SDK are you using?
@sentry/bun
SDK Version
10.15.0
Framework Version
Express 5.1.0
Link to Sentry event
No response
Reproduction Example/SDK Setup
https://github.com/RudolfSchreier/sentry-express-bun-demo
Steps to Reproduce
- Get a working SENTRY_DSN for some project
- start the demo using
env SENTRY_DSN=... bun run dev
Expected Result
The express server starts up with working Sentry integration and no warnings.
Actual Result
The express server starts up, but with a Sentry warning about missing instrumentation:
[Sentry] express is not instrumented. Please make sure to initialize Sentry in a separate file that you
--importwhen running node, see: https://docs.sentry.io/platforms/javascript/guides/express/install/esm/.
Additional Context
When using both Bun and Express, it is unclear which SDK to preferably use (@sentry/node or @sentry/bun), but both appear to show this issue.
It is unclear if the warning is correct or erroneous (and Express integration is actually working).
It is unclear to me from the description of #13674 if this the expected result for Express v5.
Tip: React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it.
Hello, and thanks for your reproduction!
The OTel-based Node SDKs need the instrument.ts file early-imported so it can run before the application starts. In Node, this flag is called --import, but in bun it's --preload.
We probably need to update the docs and the log for this.
Hi and thanks for the triage :)
The call to bun already uses --preload, and I am not quite sure what else to configure to make Bun behave more node-y here.
Ah sorry, I missed this. I can have a closer look at this next week :)
But this check if it's wrapped is throwing the warning:
The function is using this OTel function under the hood. Either it's not instrumented correctly or the check is not correct.
Hey folks, wanted to report that I think the issue is even deeper than this. I believe http is not instrumented either, which is likely cascading into express not being instrumented. If you take the same setup as the reproduction repo and instead just try to do this:
import http from "http"
const server = http.createServer((_req, res) => {
res.statusCode = 200
res.end("Hello World")
})
server.listen(3020, () => {
console.info(`App listening on http://localhost:3020`)
})
Then hit http://localhost:3020 in a browser you'll note nothing is traced. The exact same setup with node works as expected.
The documentation and tutorials I've found all imply that this should be working, so I'm unclear if this is a new issue or just never worked.
I think that this issue is probably connected. I'll look into this.
What I found out so far is that the patch callback (in the OTel code) that is defined as a third parameter of InstrumentationNodeModuleDefinition is never called. This correctly happens when using express with Node.
When adding this array to the array which init() returns, it will at least call the patch function:
[
// this below won't call patch (current code)
InstrumentationNodeModuleDefinition('express', ['>=4.0.0 <6'], /* patch and unpatch callbacks */)
// this below will call patch
[InstrumentationNodeModuleFile('express/lib/express.js', ['>=4.0.0 <6'], /* patch and unpatch callbacks */)]
]I found the issue but as I am out for a conference next week, I cannot fix it right away.
- The OTel http integration patches the incoming requests only when
disableIncomingRequestInstrumentationisfalse(http patching code - also below)
if (!this.getConfig().disableIncomingRequestInstrumentation) {
this._wrap(
moduleExports.Server.prototype,
'emit',
this._getPatchIncomingRequestFunction('http')
);- The Sentry SDK exposes its own OTel http integration for patching incoming requests so we disable this patching
sentry-javascript/packages/node/src/integrations/http.ts
Lines 157 to 158 in 7b40a95
- The Sentry SDK wraps and patches the http handlers by subscribing to the diagnostics channels of http. And while the
wrap()method is called during SDK setup, the channel subscriptions are somehow dismissed (<--- this is the problem). This could be, that the channels are not yet created at this time and therefore, it doesn't subscribe to anything.
To debug this further, we would need to check why it's not possible to subscribe to the diagnostics channels and if this is maybe a timing problem. Maybe those http diagnostic channels are not firing in Bun. Will keep you updated on this.
If you create a channel yourself, you can see that the channels are firing in general (if you create them yourself):
const channel = diagnostics_channel.channel('http.server.request.start');
subscribe('http.server.request.start', ()=> {console.log('http.server.request.start fired')});
app.get('/test-channel', function (req, res) {
channel.publish({
some: 'message',
});
});@s1gr1d So the problem is that unless I'm missing something very nuanced, Bun does NOT actually publish those http specific diagnostic events. I was able to verify this with a simple test:
import { subscribe } from "node:diagnostics_channel"
import http from "node:http"
subscribe("http.server.request.start", (message, name) => {
console.warn("http.server.request: ", message, name)
})
subscribe("http.client.request.start", (message, name) => {
console.warn("http.client.request: ", message, name)
})
const server = http.createServer((_req, res) => {
res.statusCode = 200
res.end("Hello World")
})
server.listen(3020, "0.0.0.0", () => {
console.info(`App listening on http://localhost:3020/`)
})
http.get("http://localhost:3020/", () => {
console.info("response from server")
})Running this with node (node server.js) works as expected and logs the diagnostic messages . Running with Bun (bun server.js) produces nothing. No error, just no messages. I'm not convinced this is actually a bug on the Bun side, it's just that they simply have not implemented publishing those diagnostic messages, and perhaps Sentry assumed they had.
This shuold work in bun though: https://bun.sh/reference/node/diagnostics_channel
@chargome I think the issue is that people are assuming the existence and availability of the diagnostics_channel mechanism and code means that all of the same diagnostic events are being fired that node fires. After reviewing the bun codebase I am convinced that is not the case. It does not appear that many (if any at all) diagnostics_channel events are being fired from the standard libraries. So I believe this is a case of confusing documentation and people (arguably rightly) jumping to conclusions about the availability of specific events.
Thanks for investigating @mhodgson, mind taking this to bun or should we take care of this? In case you do please reference this issue here so we get notified 🙏
I have a question in the Bun discord. Let's maybe confirm if I'm right first. Interestingly enough if you downgrade all the way to bun 1.2.5 the http instrumentation works, but the diagnostic events are still not firing. So I'm not sure we actually have the full picture here yet. Worth deeper investigation on Sentry side around the boundary of bun 1.2.5 -> 1.2.6.