sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

Unable to configure Sentry 8.0.0 programmatically

Open mattgodbolt opened this issue 1 year ago • 6 comments

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://[email protected]/102028

Version

8.0.0

mattgodbolt avatar May 14 '24 02:05 mattgodbolt

Assigning to @getsentry/support for routing ⏲️

getsantry[bot] avatar May 14 '24 02:05 getsantry[bot]

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 😄

lforst avatar May 14 '24 17:05 lforst

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 avatar May 19 '24 17:05 mattgodbolt

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:

  • we initialise AWS, then express, then call our own SetupSentry https://github.com/compiler-explorer/compiler-explorer/blob/9cdd3a049f6a0d5511879012c2278eb2361a536e/app.ts#L518
  • the DSN comes from AWS secrets, which is why AWS is initialised first
  • SetupSentry then does some work https://github.com/compiler-explorer/compiler-explorer/blob/9cdd3a049f6a0d5511879012c2278eb2361a536e/lib/sentry.ts#L48

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 :)

mattgodbolt avatar May 19 '24 17:05 mattgodbolt

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?

lforst avatar May 21 '24 08:05 lforst

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: https://github.com/getsentry/sentry-javascript/pull/12213

It will soon be documented!

AbhiPrasad avatar May 27 '24 19:05 AbhiPrasad

Docs: https://docs.sentry.io/platforms/javascript/guides/node/install/late-initializtion/

AbhiPrasad avatar Jun 07 '24 14:06 AbhiPrasad

Thank you!

mattgodbolt avatar Jun 11 '24 01:06 mattgodbolt

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

mattgodbolt avatar Jun 11 '24 01:06 mattgodbolt

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.

mydea avatar Jun 11 '24 07:06 mydea

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...)

mattgodbolt avatar Jun 11 '24 11:06 mattgodbolt