Skip to content

@sentry/bun not reporting any span data #12705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3 tasks done
nzapponi opened this issue Jun 30, 2024 · 16 comments · Fixed by #15586
Closed
3 tasks done

@sentry/bun not reporting any span data #12705

nzapponi opened this issue Jun 30, 2024 · 16 comments · Fixed by #15586
Assignees
Labels
Package: bun Issues related to the Sentry Bun SDK

Comments

@nzapponi
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/bun

SDK Version

8.13.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup/Reproduction Example

Entrypoint line 1:

import "./sentry.ts"

sentry.ts

import { SocketIoInstrumentation } from "@opentelemetry/instrumentation-socket.io";
import * as Sentry from "@sentry/bun";
import data from "../../../package.json";
import { SENTRY_DSN, SENTRY_ENVIRONMENT } from "./const";

Sentry.init({
  dsn: SENTRY_DSN,
  tracesSampleRate: 1.0,
  environment: SENTRY_ENVIRONMENT,
  release: data.version,
});

Sentry.addOpenTelemetryInstrumentation(new SocketIoInstrumentation());

Steps to Reproduce

  1. Start the bun server (which uses Express)
  2. Make an API call from the client (or curl) to the server

Expected Result

A trace of the API call from request to response, including all the middleware calls, database queries (I'm using pg), etc.

Actual Result

I get a trace, but every transaction just says "Empty - Transaction did not report any span data"
This also happens on server-side events that I instrumented using await Sentry.startSpan(...): I get a trace that the span is there, but no data within it.

Screenshot 2024-06-30 at 16 05 21

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jun 30, 2024
@github-actions github-actions bot added the Package: bun Issues related to the Sentry Bun SDK label Jun 30, 2024
@nzapponi nzapponi changed the title @sentry/bun or reporting any span data @sentry/bun not reporting any span data Jun 30, 2024
@andreiborza
Copy link
Member

Hi, thank you for reporting this. Could you please add debug: true to your sentry options and paste some output?

@nzapponi
Copy link
Author

nzapponi commented Jul 1, 2024

Thanks @andreiborza !

Here it is:

$ bun run --hot src/server.ts
Sentry Logger [log]: Initializing Sentry: process: 72557, thread: main.
Sentry Logger [log]: Integration installed: InboundFilters
Sentry Logger [log]: Integration installed: FunctionToString
Sentry Logger [log]: Integration installed: LinkedErrors
Sentry Logger [log]: Integration installed: RequestData
Sentry Logger [log]: Integration installed: Console
Sentry Logger [log]: Integration installed: Http
Sentry Logger [log]: Integration installed: NodeFetch
Sentry Logger [log]: Integration installed: OnUncaughtException
Sentry Logger [log]: Integration installed: OnUnhandledRejection
Sentry Logger [log]: Integration installed: ContextLines
Sentry Logger [log]: Integration installed: Context
Sentry Logger [log]: Integration installed: Modules
Sentry Logger [log]: Integration installed: BunServer
Sentry Logger [log]: Running in CommonJS mode.
Sentry Logger [debug]: @opentelemetry/api: Registered a global for diag v1.9.0.
Sentry Logger [debug]: @opentelemetry/api: Registered a global for trace v1.9.0.
Sentry Logger [debug]: @opentelemetry/api: Registered a global for context v1.9.0.
Sentry Logger [debug]: @opentelemetry/api: Registered a global for propagation v1.9.0.
Sentry Logger [warn]: @opentelemetry/instrumentation-socket.io Module socket.io has been loaded before @opentelemetry/instrumentation-socket.io so it might not work, please initialize it before requiring socket.io
Sentry Logger [warn]: @opentelemetry/instrumentation-socket.io Module socket.io has been loaded before @opentelemetry/instrumentation-socket.io so it might not work, please initialize it before requiring socket.io
  server:app Registered data model +0ms
[Sentry] express is not instrumented. This is likely because you required/imported express before calling `Sentry.init()`.
  server:app Bootstrapping services +0ms
  server:pg Initializing database +0ms
  server:pg Running migrations +0ms
  server:app Listening on port 28001... +0ms

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jul 1, 2024
@nzapponi
Copy link
Author

nzapponi commented Jul 1, 2024

Weird that it says Running in CommonJS mode... isn't it?

@lforst
Copy link
Member

lforst commented Jul 2, 2024

@nzapponi The logic for that log message basically just checks whether require is in the global namespace. This is a bit weird in bun, because it maximizes compatibility.

Could you provide a reproduction of where this is not working? Thanks! It's a bit hard to gauge things from looking afar.

@nzapponi
Copy link
Author

nzapponi commented Jul 2, 2024

Here is a sample repo to reproduce: https://github.com/nzapponi/bun-sentry-repro
I keep getting no span data.

Thanks!

Screenshot 2024-07-02 at 11 53 10

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jul 2, 2024
@chargome chargome self-assigned this Jul 2, 2024
@chargome
Copy link
Member

chargome commented Jul 2, 2024

Thank you @nzapponi, we'll look into it!

@chargome
Copy link
Member

chargome commented Jul 2, 2024

@nzapponi it seems that there are issues regarding the auto-instrumentation with express and bun.

I keep getting no span data.

For now, although you do not receive auto-generated span data you can still make use of custom instrumentation and error monitoring for your app with your current setup.

We'll reference this issue once we have a fix.

@nzapponi
Copy link
Author

nzapponi commented Jul 2, 2024

Ok thanks

@nzapponi
Copy link
Author

nzapponi commented Jan 9, 2025

Solved by replacing Sentry.setupExpressErrorHandler(app) with app.use(Sentry.expressErrorHandler())

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jan 9, 2025
@chargome
Copy link
Member

chargome commented Jan 9, 2025

Hmm strange, we basically do the same thing within setupExpressErrorHandler:

export function setupExpressErrorHandler(
  app: { use: (middleware: ExpressMiddleware | ExpressErrorMiddleware) => unknown },
  options?: ExpressHandlerOptions,
): void {
  app.use(expressRequestHandler());
  app.use(expressErrorHandler(options));
  ensureIsWrapped(app.use, 'express');
}

Thanks for sharing your findings @nzapponi !

@nzapponi
Copy link
Author

nzapponi commented Jan 9, 2025

Weird!

Given that, it feels like I should also add the expressRequestHandler.

And shouldn't that be called before any routes and not after (like it is with your current implementation)? The way you implemented looks odd because any successful request will never hit the request handler.

Finally, I guess I may be wrong and it's not working any differently - the reason why I'm not seeing the warning anymore is likely because ensureIsWrapped is not being called anymore.

I'll do more digging and report back.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jan 9, 2025
@chargome
Copy link
Member

chargome commented Jan 9, 2025

What does ensureIsWrapped do?

This just checks if express is properly wrapped and emits a warning if not.

Given that, it feels like I should also add the expressRequestHandler then?

Yes I would say so.

Yes I think this is likely a timing/esm problem, but there are still issues with bun and preloading the instrumentation: oven-sh/bun#13165

@nzapponi
Copy link
Author

nzapponi commented Jan 9, 2025

Sorry, I just updated my comment above.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jan 9, 2025
@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Jan 9, 2025
@linear linear bot added the Migrated label Jan 9, 2025
@nzapponi
Copy link
Author

nzapponi commented Jan 9, 2025

Alright, so here's how I fixed it actually - I went back to using Sentry.setupExpressErrorHandler(app) as my suggestion above was just hiding the console.log and not actually solving anything.

To get Express to work with Bun, I had to manually enable the Express integration during Sentry.init:

Sentry.init({
    dsn,
    tracesSampleRate: 1.0,
    integrations: [Sentry.expressIntegration()],
  });

I can now see the full Express data!
Tested with Bun v1.1.43.

Image

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jan 9, 2025
@mydea
Copy link
Member

mydea commented Jan 10, 2025

Huh, that is a good point - we actually do not add any node performance integrations by default in bun. I wonder if we should do that 🤔

Copy link
Contributor

github-actions bot commented Mar 6, 2025

A PR closing this issue has just been released 🚀

This issue was referenced by PR #15586, which was included in the 9.5.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: bun Issues related to the Sentry Bun SDK
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants