Paths

Table of Contentst

Add support for disabling logging integration in sentry
ClosedPublic
Actions

Authored by olasd on Apr 13 2022, 4:21 PM.

Details

Reviewers

anlambert
douardda

Commits

rDCOREb9b898e33753: Add support for disabling logging-based events in sentry

Summary

add a configuration option to init_sentry to disable the error reporting
from logging by sentry.

This can be set via the SWH_SENTRY_DISABLE_LOGGING_INTEGRATION env var.
Defaulti behavior remains unchanged.

Diff Detail

Repository

rDCORE Foundations and core functionalities

Branch

master

Lint

Lint Skipped

Unit

Unit Tests Skipped

Build Status

Buildable 28437
Build 44461: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 44460: arc lint + arc unit

Event Timeline

douardda created this revision.Apr 13 2022, 4:21 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptApr 13 2022, 4:21 PM

Build is green

Patch application report for D7570 (id=27423)

Rebasing onto ab170ca98c...

First, rewinding head to replay your work on top of it...
Applying: Add support for disabling logging integration in sentry

Changes applied before test

commit 80319016e43797027c9bce8a08f46935b41b3443
Author: David Douard <david.douard@sdfa3.org>
Date:   Wed Apr 13 15:54:50 2022 +0200

    Add support for disabling logging integration in sentry
    
    add a configuration option to init_sentry to disable the error reporting
    from logging by sentry.
    
    This can be set via the SWH_SENTRY_DISABLE_LOGGING_INTEGRATION env var.
    Defaulti behavior remains unchanged.

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/392/ for more details.

Harbormaster completed remote builds in B28437: Diff 27423.Apr 13 2022, 4:23 PM

douardda requested review of this revision.Apr 13 2022, 4:23 PM

We also implemented something similar in D7567 but your tests are much better than ours.

anlambert accepted this revision.Apr 13 2022, 4:32 PM

This revision is now accepted and ready to land.Apr 13 2022, 4:32 PM

anlambert added inline comments.Apr 13 2022, 4:33 PM

swh/core/sentry.py
35	documentation of that parameter seems truncated.

olasd mentioned this in D7569: statsd: add an error_type tag to @timed error counters.Apr 14 2022, 10:22 AM

Rebase

Build is green

Patch application report for D7570 (id=27441)

Rebasing onto 5723f00a2a...

Current branch diff-target is up to date.

Changes applied before test

commit 2b4c89418d27d3431baebbe06dda7886e537e81e
Author: David Douard <david.douard@sdfa3.org>
Date:   Wed Apr 13 15:54:50 2022 +0200

    Add support for disabling logging integration in sentry
    
    add a configuration option to init_sentry to disable the error reporting
    from logging by sentry.
    
    This can be set via the SWH_SENTRY_DISABLE_LOGGING_INTEGRATION env var.
    Defaulti behavior remains unchanged.

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/395/ for more details.

Harbormaster completed remote builds in B28456: Diff 27441.Apr 14 2022, 11:06 AM

olasd commandeered this revision.Apr 14 2022, 11:30 AM

olasd added a reviewer: douardda.

Improve docs, rename variable to disable_logging_events

Build is green

Patch application report for D7570 (id=27443)

Rebasing onto 5723f00a2a...

Current branch diff-target is up to date.

Changes applied before test

commit b9b898e3375374446838c8eda1fe33f64b3d232f
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 14 11:24:26 2022 +0200

    Add support for disabling logging-based events in sentry
    
    add a configuration option to init_sentry to disable the error reporting
    from logging by sentry.
    
    This can be set via the SWH_SENTRY_DISABLE_LOGGING_EVENTS env var.
    Default behavior remains unchanged.
    
    Co-Authored-By: David Douard <david.douard@sdfa3.org>

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/396/ for more details.

Harbormaster completed remote builds in B28458: Diff 27443.Apr 14 2022, 11:32 AM

olasd marked an inline comment as done.Apr 14 2022, 11:34 AM

olasd mentioned this in D7567: gunicorn: disable sentry logging event reporting by default.Apr 14 2022, 11:43 AM

olasd requested review of this revision.Apr 14 2022, 11:45 AM

olasd edited reviewers, added: douardda; removed: Reviewers.

anlambert accepted this revision.Apr 14 2022, 11:52 AM

anlambert added inline comments.

swh/core/sentry.py
53–54	`Optional[bool]` feels weird to me, using simply `bool` defaulted to `False` is easier to read.

This revision is now accepted and ready to land.Apr 14 2022, 11:52 AM

olasd added inline comments.Apr 14 2022, 11:56 AM

swh/core/sentry.py
53–54	Yeah, I agree; but currently we actually use the three states: `None` = default -> use environment variable, `True`/`False` -> use that value explicitly and ignore the environment variable.