rfc(decision): Environment Variable conventions #121
Conversation
cleptric
force-pushed
the
rfc/draft-environment-variable-conventions
branch
from
4e2da89
to
be30ec7
Compare
cleptric
changed the title
There was a problem hiding this comment.
General:
I think this makes sense and at least partially enables easier configuration of the SDK.
I have an important question though: Is the goal of supporting env variables to read them at SDK init time and store the values as long as the application is alive? Or should SDKs also observe updates to env variables after they're initialized?
- If only at init time, all good, we can make this work fairly easily.
- In case of the latter, we need to be aware though that this has deeper implications on how SDKs currently deal with options. For instance, we currently don't guarantee support for "updating" of
getClient().getOptions(). It's possible (at least in JS) but very likely, we have some instrumentation (e.g. an integration) that hard-copied e.g. sample rates when they initialized, stored them away and wouldn't receive the update value. If we were to support dynamic updates of options, we need to ensure that this doesn't happen anywhere. IOW, we'll need to re-query the env variables a lot.
Also, users (and we) need to be aware that setting options via env variables will only work for primitive values. Adding integrations or callbacks (beforeSend) like this will not work.
JS Specific and also dependent on my initial question:
- Since Browser and Server SDKs inherit from a core SDK package, we need to find a way to a) only add env variable reads in the server side or b) make these reads a no-op in the browser
- Both but especially b) will have a bundle size impact. Negligible in server-land, something we need to keep an eye on in browser-land.
|
Just for init, everything else, as you rightly pointed out, has too many side effects. |
|
Sorry, tapped something while reviewing that marked it as ready and I don't see a way to unmark it on GH mobile now. |
| - `SENTRY_DSN` | ||
| - `SENTRY_RELEASE` | ||
| - `SENTRY_ENVIRONMENT` | ||
| - `SENTRY_SAMPLE_RATE` | ||
| - `SENTRY_TRACES_SAMPLE_RATE` | ||
| - `SENTRY_PROFILES_SAMPLE_RATE` | ||
| - `SENTRY_DEBUG` |
There was a problem hiding this comment.
Just stumbled across this RFC. We could also list SENTRY_TRACE and SENTRY_BAGGAGE, which could be picked up to continue traces.
There was a problem hiding this comment.
Yep, there is/was another RFC for that, but I can add it and actually move this RFC forward 😬
TODO. Rendered RFC