Skip to content

[IMP] sentry: configure through environment variables - #3724

Open
moylop260 wants to merge 1 commit into
OCA:19.0from
vauxoo-dev:19.0-imp-sentry-env-vars-moy
Open

[IMP] sentry: configure through environment variables#3724
moylop260 wants to merge 1 commit into
OCA:19.0from
vauxoo-dev:19.0-imp-sentry-env-vars-moy

Conversation

@moylop260

Copy link
Copy Markdown
Contributor

Every sentry_* option can now be set through an environment variable named
after it with an ODOO_ prefix — sentry_dsn is ODOO_SENTRY_DSN — so an
instance can be configured without a configuration file at all.

This is the same technique and the same precedence queue_job has been applying
to ODOO_QUEUE_JOB_* for years, os.environ.get(...) or <section>.get(...):
https://github.com/OCA/queue/blob/0046b8741c6820f5ca4357daa7ca2acf7e4548c9/queue_job/jobrunner/runner.py#L60-L65

Both sources stay supported and are merged per option, the environment winning
where the two disagree, so an existing [sentry] section keeps working
untouched.

Why

Since #3517 moved the options out of [options] into their own [sentry]
section, the file is quiet but it is still a file, and on a container it is the
awkward part of the deployment:

Why not the SENTRY_* variables sentry-sdk already reads

sentry-sdk does read SENTRY_DSN, SENTRY_ENVIRONMENT, SENTRY_RELEASE and
SENTRY_DEBUG on its own, but only for an option it receives as None:
_get_options guards each fallback with if rv["<option>"] is None. This
module always passes dsn explicitly, and its default is "", so SENTRY_DSN
is never consulted. sentry_enabled has no equivalent in the library at all, so
nothing in the environment could switch the module on.

Keeping the ODOO_ prefix also leaves SENTRY_DSN meaning what it means
everywhere else — the DSN of whichever process the library runs in — instead of
quietly redirecting Odoo to it.

Boolean options

Options that answer yes or no now go through a to_bool helper. A configuration
file and an environment variable can only deliver a string, and bool("False")
is True, so sentry_enabled = False used to turn Sentry on. A value left
empty is not read as "off": it carries no answer, so the default stands.

Tests

Added to sentry/tests/test_client.py: the mapping from variable name to option
name, to_bool over the strings both sources deliver, an instance configured
entirely from the environment with no section at all, the environment winning
over the file, the two sources merging per option, SENTRY_DSN not being read
as ours, and sentry_enabled = False reading as off from either source.

Every option can now come from ODOO_SENTRY_*, named after the option it
carries: sentry_dsn is ODOO_SENTRY_DSN. That is the shape queue_job has
been using for ODOO_QUEUE_JOB_* for years.

Both sources stay supported and are merged per option, the environment
winning where the two disagree, so an existing [sentry] section keeps
working untouched.

A container has two reasons to prefer the environment. Odoo reads only
[options] from its configuration file and logs "unknown option ... in
the config file" for anything else it finds there, which is why these
options were moved out to a [sentry] section to begin with. And
odoo-bin -s rewrites that file out of Odoo's own options, dropping every
other section with it.

The ODOO_ prefix keeps these apart from the SENTRY_* variables
sentry-sdk reads on its own. SENTRY_DSN addresses whichever process the
library runs in, and it cannot serve here anyway: the dsn is always
passed explicitly, and sentry-sdk only falls back to the environment for
an option it receives as None.

Options that answer yes or no now go through to_bool. A configuration
file and an environment variable can only deliver a string, and
bool("False") is True, so sentry_enabled = False used to turn sentry on.
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @barsi, @fernandahf, @versada, @naglis,
some modules you are maintaining are being modified, check this out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants