sentry_runner(feat): Report development issues #24512

armenzg · 2021-03-17T20:39:43Z

If the developer has SENTRY_DEVENV_DSN set (if the developer uses direnv it will be set) we can report issues faced by the Sentry runner.

…ment If the developer uses `direnv` it will load the `SENTRY_DEVENV_DSN` env variable. When using the `sentry` CLI, this will enable reporting development environment issues to Sentry.io

If the developer has SENTRY_DEVENV_DSN set we can report issues faced by the Sentry runner.

armenzg · 2021-03-17T20:43:02Z

src/sentry/runner/__init__.py

+            )
+        cli(prog_name=get_prog(), obj={}, max_content_width=100)
+    except Exception as e:
+        if os.environ.get("SENTRY_DEVENV_DSN"):


@untitaker @billyvg For me, this block is sufficient. The changes in the sdk.py did not work to catch issues raised and we can investigate later if we need to make changes there; works for you? (If you say so, I will go and remove all changes from sdk.py).

You can test this change by shutting down the devservices and running sentry cleanup.

You can see the issue reported here:
https://sentry.io/organizations/sentry/issues/2279751970/?project=1492057&query=is%3Aunresolved+is%3Afor_review+assigned_or_suggested%3Ame_or_none

I've decided to remove the changes to sdk.py. This will be good enough for now.

untitaker · 2021-03-19T14:34:09Z

src/sentry/runner/__init__.py

put this in a with-stmt:

with init(): # capture_exception etc

then the old client will be restored at the end

actually it won't (it just blocks until the error has been sent) but I'd still recommend

untitaker · 2021-03-19T14:34:18Z

src/sentry/runner/__init__.py

Suggested change

raise (e)

raise e

src/sentry/runner/__init__.py

armenzg · 2021-03-19T20:20:54Z

src/sentry/runner/__init__.py

 from sentry.utils.imports import import_string
 from sentry.utils.compat import map

+logging.basicConfig(format="%(asctime)s [%(levelname)s] %(message)s", level=logging.INFO)


I've tried to understand how other pieces of code do logging.

I tried using setLevel(logging.INFO) on my logger so I could see logger.info("We have reported the error below to Sentry") at the end with no avail. Adding basicConfig() does the trick but I don't know why it would need that.

I've also tried using logging practices for the project but it's unclear to me.
I've looked at the modules that generate this output:

20:11:30 [WARNING] sentry.utils.geo: settings.GEOIP_PATH_MMDB not configured. 20:11:32 [INFO] sentry.plugins.github: apps-not-configured

and I know they listen to SENTRY_LOG_LEVEL=INFO and SENTRY_LOG_FORMAT='MACHINE', however, their code seems typical import logging and using a getLogger(); what am I missing?

untitaker · 2021-03-22T16:08:39Z

src/sentry/runner/__init__.py

 from sentry.utils.imports import import_string
 from sentry.utils.compat import map

+logging.basicConfig(format="%(asctime)s [%(levelname)s] %(message)s", level=logging.INFO)


would only run this based on SENTRY_DEVENV_DSN to avoid potential impact on prod

armenzg · 2021-03-22T19:48:03Z

Tagging @evanpurkhiser for the last blessing since he's touched the file few times in the last year.

armenzg · 2021-03-22T19:49:30Z

.envrc


+### You can override the exported variables with a .env file
+# All exports should happen before here unless they're safeguarded (see devenv error reporting below)
+if [ -f '.env' ]; then


Moving this up here works better when I want to load "SENTRY_DEVENV_NO_REPORT" through the .env file.

armenzg · 2021-03-22T19:50:59Z

.envrc

 else
    info "Development errors will be reported to Sentry.io."$'\n'"        If you wish to opt-out, set SENTRY_DEVENV_NO_REPORT as an env variable."
-    export SENTRY_DSN=https://[email protected]/1492057
+    export SENTRY_DEVENV_DSN=https://[email protected]/1492057


I'm renaming this variable since SENTRY_DSN since that is the default variable and I would not want it to conflict with what some engineer would have defined by hand or in their own start-up scripts.

armenzg · 2021-03-22T19:52:55Z

src/sentry/runner/__init__.py

+            "Use SENTRY_DEVENV_NO_REPORT to avoid reporting issues."
+        )
+        try:
+            cli(prog_name=get_prog(), obj={}, max_content_width=100)


In the previous commit, I only had cli() once, however, I had 2-3 if conditions. This makes it easier to read but will require the next person to ever touch the command to also update it below.

My apologies for the multiple comments (and probably multiple emails). I was only expecting to make 1 single comment.

Should we refactor this to a new function so that we don't need to update both cli() call sites?

@billyvg I've used this:

diff --git a/src/sentry/runner/__init__.py b/src/sentry/runner/__init__.py index 6522718a7d..436c5b1e0a 100644 --- a/src/sentry/runner/__init__.py +++ b/src/sentry/runner/__init__.py @@ -163,6 +163,12 @@ def call_command(name, obj=None, **kwargs): def main(): + func = cli + kwargs = { + "prog_name": get_prog(), + "obj": {}, + "max_content_width": 100, + } # This variable is *only* set as part of direnv/.envrc, thus, we cannot affect production if os.environ.get("SENTRY_DEVENV_DSN"): # We do this here because `configure_structlog` executes later @@ -174,7 +180,7 @@ def main(): "Use SENTRY_DEVENV_NO_REPORT to avoid reporting issues." ) try: - cli(prog_name=get_prog(), obj={}, max_content_width=100) + func(**kwargs) except Exception as e: # This reports to the project sentry-dev-env with sentry_sdk.init(dsn=os.environ["SENTRY_DEVENV_DSN"]): @@ -184,5 +190,4 @@ def main(): logger.info("We have reported the error below to Sentry") raise e else: - # If you change this command here make sure to also change it in the block above - cli(prog_name=get_prog(), obj={}, max_content_width=100) + func(**kwargs)

billyvg · 2021-03-23T16:30:12Z

src/sentry/runner/__init__.py

+            "Use SENTRY_DEVENV_NO_REPORT to avoid reporting issues."
+        )
+        try:
+            func(**kwargs)


cli(**kwargs) is probably sufficient

This change enables error reporting issues when using `sentry`'s CLI runner.

armenzg added 2 commits March 17, 2021 15:57

devenv(feat): Report sentry issues encountered in development environ…

61a706e

…ment If the developer uses `direnv` it will load the `SENTRY_DEVENV_DSN` env variable. When using the `sentry` CLI, this will enable reporting development environment issues to Sentry.io

sentry_runner(feat): Report development issues

99d8b44

If the developer has SENTRY_DEVENV_DSN set we can report issues faced by the Sentry runner.

vercel bot had a problem deploying to Preview – storybook March 17, 2021 20:39 Failure

vercel bot deployed to Preview – sentry March 17, 2021 20:39 View deployment

armenzg self-assigned this Mar 17, 2021

armenzg added the Component: Developer Environment This covers issues related to setting up a developer's environment label Mar 17, 2021

armenzg commented Mar 17, 2021

View reviewed changes

armenzg added this to the Report dev env errors to Sentry.io milestone Mar 17, 2021

armenzg mentioned this pull request Mar 17, 2021

dev_env(feat): Report issues from sentry runner #24447

Closed

Merge branch 'master' into armenzg/sentry-runner-report-issues

1757b5a

vercel bot had a problem deploying to Preview – storybook March 19, 2021 14:19 Failure

vercel bot deployed to Preview – sentry March 19, 2021 14:19 View deployment

armenzg marked this pull request as ready for review March 19, 2021 14:20

armenzg requested a review from a team as a code owner March 19, 2021 14:20

armenzg requested review from billyvg and untitaker March 19, 2021 14:21

untitaker approved these changes Mar 19, 2021

View reviewed changes

Revert changes to sdk

6d7fafb

armenzg force-pushed the armenzg/sentry-runner-report-issues branch from 19709ca to 6d7fafb Compare March 19, 2021 20:15

vercel bot deployed to Preview – sentry March 19, 2021 20:15 View deployment

vercel bot had a problem deploying to Preview – storybook March 19, 2021 20:15 Failure

armenzg commented Mar 19, 2021

View reviewed changes

src/sentry/runner/__init__.py Show resolved Hide resolved

armenzg commented Mar 19, 2021

View reviewed changes

untitaker approved these changes Mar 22, 2021

View reviewed changes

armenzg added 3 commits March 22, 2021 14:00

Merge branch 'master' into armenzg/sentry-runner-report-issues

52d3295

Better location for loading values from .env file

e72a612

Clean up logging block

5919cc7

vercel bot deployed to Preview – storybook March 22, 2021 19:46 View deployment

vercel bot deployed to Preview – sentry March 22, 2021 19:46 View deployment

armenzg requested a review from evanpurkhiser March 22, 2021 19:47

armenzg commented Mar 22, 2021

View reviewed changes

armenzg added 2 commits March 23, 2021 09:40

Merge branch 'master' into armenzg/sentry-runner-report-issues

5ae0ca8

Use func/kwargs

c61c05f

vercel bot deployed to Preview – storybook March 23, 2021 13:55 View deployment

vercel bot deployed to Preview – sentry March 23, 2021 13:55 View deployment

billyvg approved these changes Mar 23, 2021

View reviewed changes

armenzg merged commit 4f5c640 into master Mar 23, 2021

armenzg deleted the armenzg/sentry-runner-report-issues branch March 23, 2021 16:51

ahmedetefy pushed a commit that referenced this pull request Mar 26, 2021

sentry_runner(feat): Report development issues (#24512)

294ef89

This change enables error reporting issues when using `sentry`'s CLI runner.

github-actions bot locked and limited conversation to collaborators Apr 8, 2021

Uh oh!

sentry_runner(feat): Report development issues #24512

sentry_runner(feat): Report development issues #24512

Uh oh!

Conversation

armenzg commented Mar 17, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

armenzg commented Mar 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants