Skip to content
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

Unable to configure Sentry 8.0.0 programmatically #12033

Closed
mattgodbolt opened this issue May 14, 2024 · 11 comments
Closed

Unable to configure Sentry 8.0.0 programmatically #12033

mattgodbolt opened this issue May 14, 2024 · 11 comments
Labels
Package: node Issues related to the Sentry Node SDK

Comments

@mattgodbolt
Copy link

mattgodbolt commented May 14, 2024

Environment

SaaS (https://sentry.io/)

What are you trying to accomplish?

Trying to update to 8.0.0 with configuration for the DSN read from our in-house configuration system. Currently we do that in 7.* by (early on) reading our secret store and then using that to get the DSN and configure sentry. Now trying to accomplish this in 8.*

How are you getting stuck?

8.* requires a pre-include (for ESM apps), which is:

  • unwieldy and breaks our existing setup
  • makes it unclear we can import our configuration system and get the DSN from it before "first thing" .

Our program is open source so we need this to be configurable so that folks running our product locally don't post their errors to our DSN.

Where in the product are you?

Sign In

Link

No response

DSN

https://8e4614f649ad4e3faf3e7e8827b935f9@sentry.io/102028

Version

8.0.0

@getsantry
Copy link

getsantry bot commented May 14, 2024

Assigning to @getsentry/support for routing ⏲️

@JLuse JLuse transferred this issue from getsentry/sentry May 14, 2024
@lforst
Copy link
Member

lforst commented May 14, 2024

Hi, thanks for writing in! We'll do our best to address your concerns and maybe we can find some fixes and improvements.

I hope it's ok if I ask you some questions so that we better understand what's going on:

  • What prevents you from reading the DSN from your secret store in v8?
  • You mentioned that the "pre-include" (I assume you mean the --import flag) is unwieldy and breaks your existing setup. Can you share why it is unwieldy for you and how exactly it breaks your setup?
  • What would make it more clear that you can import your configuration system from the pre-import?

FWIW, and this may be important for your case, you don't strictly need to pre-import a file with Sentry.init(). As long as you run node as follows node --import @sentry/node/import your-app.js (which registers import-in-the-middle so that opentelemetry can patch modules), AND you call Sentry.init() as the first thing in your app, everything should still work.

We are still kinda figuring out withing the team how to document all of this because there is a bit of complexity to it with a compatibility matrix of Node.js versions and there are a lot of conflicting opinions with regards to complexity/correctness trade-offs. Just so you can follow our thought process a bit 😄

@mattgodbolt
Copy link
Author

Thanks for the reply!

  • What prevents you from reading the DSN from your secret store in v8?

The documentation states that the first thing has to be a call to the configuration, before anything is imported, so I figured that meant I couldn't include the code for our config management system (which transitively picks up lots of things like AWS libraries etc) before we then configured Sentry without losing all the AWS integrations (if any?).

  • You mentioned that the "pre-include" (I assume you mean the --import flag) is unwieldy and breaks your existing setup. Can you share why it is unwieldy for you and how exactly it breaks your setup?

Absolutely. Yes I did mean the --import flag :) We have a bunch of scripts that deploy and run our code in our production environment. Changing those in lock step with the version of the code we deploy is tricky: the command-line arguments to node effectively change between our code versions. It's not impossible to make this change of course, but it's a pain to do, and if I can avoid doing so I will prefer having a simple "you just run app.js and everything works" approach.

  • What would make it more clear that you can import your configuration system from the pre-import?

I guess understanding what is and is not covered by the "you must have imported sentry first".

FWIW, and this may be important for your case, you don't strictly need to pre-import a file with Sentry.init(). As long as you run node as follows node --import @sentry/node/import your-app.js (which registers import-in-the-middle so that opentelemetry can patch modules), AND you call Sentry.init() as the first thing in your app, everything should still work.

I'm still not following this. If I support --import, but I still "AND you call Sentry.init() as the first thing in your app" - what is "first thing" ? If I've called a bunch of code to manage our secrets, that's not first thing?

Can you elaborate on what "first thing" really means?

We are still kinda figuring out withing the team how to document all of this because there is a bit of complexity to it with a compatibility matrix of Node.js versions and there are a lot of conflicting opinions with regards to complexity/correctness trade-offs. Just so you can follow our thought process a bit 😄

Thanks

@mattgodbolt
Copy link
Author

For context, our app is open source so has to work both with and without sentry, and with user-configurabl sentry stuff.

In our current 7.x setup:

The packaged release is run from a separate repo entirely which has the node.js flags: https://github.com/compiler-explorer/infra/blob/48c1128457f0c58cec5818312fb221837696b064/init/start.sh#L34 so this is why adding a --import is tricky. Adding it first would break (I think), and adding it after moving to 8.x would mean there's a period we're not tracking sentry stuff. Which is probably ok.

As you can see our existing design is far from "first thing" initialsation of Sentry :)

@lforst
Copy link
Member

lforst commented May 21, 2024

Thanks for elaborating! I think all of your questions are answered by the following: We are simply a bit strict with our wording in the docs because we need to be terse. Generally speaking, you should call Sentry.init() before any code you want to have instrumented. If you cannot call Sentry.init() before some parts of your application, that is a chicken-egg problem. It is likely a hard problem, but you can solve it by re-architecting. You can also decide not to do anything about it.

You are totally allowed to hard code your DSN, meaning you don't have to import other code before init(). DSNs are not designed to be secrets, and it is generally fine to share them. We very rarely see attacks and we will be happy to gift you back events if you are maliciously affected by spam through a public DSN.

Does this answer your question?

@AbhiPrasad
Copy link
Member

With https://github.com/getsentry/sentry-javascript/releases/tag/8.5.0 we've released the ability to defer calling Sentry.init.

Here's the PR that describes how the feature works: #12213

It will soon be documented!

@AbhiPrasad AbhiPrasad added the Package: node Issues related to the Sentry Node SDK label May 27, 2024
@AbhiPrasad AbhiPrasad added this to the v8 Instrumentation Bugs milestone May 27, 2024
@AbhiPrasad
Copy link
Member

@mattgodbolt
Copy link
Author

Thank you!

@mattgodbolt
Copy link
Author

This still needs a bunch of command-line parameter changes to my node install... so this is still a significant change for us.

@mydea
Copy link
Member

mydea commented Jun 11, 2024

If you are running it in ESM, there is sadly no way around using --import for instrumentation to fully work - the only alternative is https://docs.sentry.io/platforms/javascript/guides/node/install/esm-without-import/, which only provides limited instrumentation.

@mattgodbolt
Copy link
Author

I see. How did things work pre-8.0? I will schedule the changes we need to try and do a lock-step release of our software and the container that runs it (which provides the cmdline flags to node...)

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

No branches or pull requests

4 participants