Skip to content

Conversation

@mydea
Copy link
Contributor

@mydea mydea commented Oct 20, 2023

After digging into this, there were a few problems here that came together:

  1. Conflicting versions of Sentry SDK - there was both @sentry/node in an older version, as well as @sentry/node-experimental installed. We only need @sentry/node-experimental
  2. We have a bug with our node-fetch support which seems to be breaking when this is not available instead of silently ignoring this. I opened a PR to hopefully fix this here: fix(node-experimental): Make node-fetch support optional getsentry/sentry-javascript#9321, for now I bumped node via volta to v18 so this works.
  3. Most importantly, you were calling the otel tracing setup before initializing sentry, which mucked up all the instrumentations etc. I removed this (from the www files) to make this work
  4. The manual instrumentation code in service-green was incorrect as far as I can tell, it failed for me. I rewrote this to work (hopefully xD)

With this, I seem to get correct traces now, I believe. Error capturing is also working I think, Sentry actually helped a lot to debug some of the issues on this list 😅

@danielkhan
Copy link
Owner

Thank you @mydea.

Most importantly, you were calling the otel tracing setup before initializing sentry, which mucked up all the instrumentations etc. I removed this (from the www files) to make this work

This is guarded via environment variable and should not be a problem - please correct me if I'm wrong.

@mydea
Copy link
Contributor Author

mydea commented Oct 20, 2023

It happened in the www files that actually was calling app.js, I think?

@danielkhan
Copy link
Owner

It happened in the www files that actually was calling app.js, I think?

Oh yeah! Good catch, thank you.

@danielkhan danielkhan merged commit d9a4f2b into danielkhan:main Oct 21, 2023
Sign up for free to 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.

2 participants