Skip to content

[18.0][MIG] sentry: sentry_sdk version update - #3174

Merged
OCA-git-bot merged 42 commits into
OCA:18.0from
kencove:18.0-mig-sentry
Jan 31, 2025
Merged

[18.0][MIG] sentry: sentry_sdk version update#3174
OCA-git-bot merged 42 commits into
OCA:18.0from
kencove:18.0-mig-sentry

Conversation

@dnplkndll

Copy link
Copy Markdown
Contributor

No description provided.

naglis and others added 17 commits December 31, 2024 12:56
* [ADD] sentry module

* [FIX] updated sentry module according to PR comments
- [FIX] sentry: fixes missing `raven` library preventing loading of modules
- [FIX] 2to3 script on py file
- [FIX] add requirements.txt
sentry: It is not always possible to read commit information from
a production environment. In those cases it is useful to be able
to set a release version manually.

[UPD] Update sentry.pot
[UPD] Update sentry.pot
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-14.0/server-tools-14.0-sentry
Translate-URL: https://translation.odoo-community.org/projects/server-tools-14-0/server-tools-14-0-sentry/
@dnplkndll dnplkndll changed the title [18.0][MIG] sentry with sentry version update [18.0][MIG] sentry: sentry_sdk version update Jan 22, 2025
@dnplkndll

Copy link
Copy Markdown
Contributor Author

for review:
@barsi @naglis @versada @moylop260 @fernandahf

luisg123v and others added 10 commits January 22, 2025 20:17
The following warning is fixed:

    DeprecationWarning: Using or importing the ABCs from 'collections' instead of
    from 'collections.abc' is deprecated since Python 3.3, and in 3.9 it will stop
    working
Allow using `sentry_release` or `sentry_odoo_dir` in the Odoo
configuration file.

Previously, the `sentry_odoo_dir` was never actually respected. It would
always be overridden by `sentry_release`. Even if `sentry_release` is
not set, it will use an empty value instead of using `sentry_odoo_dir`
to find the Git commit hash.

After this commit, the `sentry_release` parameter still takes
precedence. However, if `sentry_release` is not set and
`sentry_odoo_dir` is set, then `sentry_odoo_dir` will be used to find
the appropriate Git commit hash, which will be used as the `release`
value.

Both cases are covered by the added unit tests.
Because post_load is called after odoo.service.server start
It has already registered the unpatched   odoo.service.wsgi_server.application
So patch it here too.
This enables wsgi performance reporting with sentry_traces_sample_rate
The test code uses a "mock" Transport object to ensure that events are
stored locally in memory, instead of triggering network requests.

The Sentry client is cleaned up once done, and this triggers a call to
capture_envelope, a different way of sending events to Sentry. Since
our mock class did not fully complete initialization, and also did
not provide an overriding method, the original was called, which
depends on proper initialization to work.

We introduce an override for capture_envelope: as it is meant to be
a "sibling" to capture_event, it makes sense for us to also make sure
events registrered in this way are intercepted, even if we don't
currently expect any of our tests to explicitly cause it to be used.
mymage and others added 10 commits January 22, 2025 20:17
Before this fix, the Sentry module sent events for WARNING-
level logs, even if sentry_logging_level was registered as
"error" or higher.

The fix itself is minor: setup of the integration mistakenly
set the hardcoded WARNING level to the event handler and the
sentry_logging_level to the breadcrumb handler, when they
should have been the other way around.

The largest part of the diff is a reworking of the tests in
order to properly replicate the issue:

* The test previously emitted a fake log event directly using
  the integration's handler's emit-method, which skipped the
  part of the logic that actually filters based on logging level.
  This has been changed to use a bespoke NoopHandler and dedicated
  Logger, so that the tests can emit "actual logs" and test Sentry
  as accurately as possible.
* The tests were not configured to use a non-default logging level,
  thus making it so that none of them caught the fact we were basically
  hard-coding the setting to WARNING-level.
  The tests now set the logging level to ERROR in order to make sure
  the configuration parameter works when it is non-default.
* Changes to configuration (especially ignored loggers) were leaking
  from one test into others. The tests were directly mutating the
  `odoo.tools.config.options` mapping, without resetting it afterward,
  leaving the changes in place for subsequent tests.
  Introduced a helper method `patch_config` that can be used to patch
  the config object so that the patch is undone at the end of the test.

NOTE: this commit was cherry-picked from d24f3d7,
and includes some changes to test code that was not in the original due
to conflicts.
@moylop260

Copy link
Copy Markdown
Contributor

/ocabot merge nobump

@OCA-git-bot

Copy link
Copy Markdown
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 18.0-ocabot-merge-pr-3174-by-moylop260-bump-nobump, awaiting test results.

@moylop260

Copy link
Copy Markdown
Contributor

Hi @dnplkndll

Thank you for your PR

Could you check if you could get the following fix and test, please?

#3178

@OCA-git-bot
OCA-git-bot merged commit d7fd9c2 into OCA:18.0 Jan 31, 2025
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at 7442e31. Thanks a lot for contributing to OCA. ❤️

@StefanRijnhart

Copy link
Copy Markdown
Member

@moylop260 did you mean to do

/ocabot migration sentry

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Feb 1, 2025
@OCA-git-bot OCA-git-bot mentioned this pull request Feb 1, 2025
42 tasks
@dnplkndll

Copy link
Copy Markdown
Contributor Author

Hi @dnplkndll

Thank you for your PR

Could you check if you could get the following fix and test, please?

#3178

I noticed that but did not see why needed. something a test missed? will take a look.
I thought the redis_session worked but it is not stable, starts to write local again with some trigger. although that needs worked out on the other side.

@dnplkndll

Copy link
Copy Markdown
Contributor Author

still green #3181

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.