Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(derived_code_mappings): Report errors with warning level #76212

Merged
merged 4 commits into from
Aug 26, 2024

Conversation

armenzg
Copy link
Member

@armenzg armenzg commented Aug 14, 2024

This reports the exceptions as warning-level errors rather than error-level.

Deployments do not get paused if the error is warning or lower:

--additional-query="issue.type:error !level:info !level:warning"

Rendering of a warning-level exception:
image

Using info level errors or exceptions avoids creating alerts and possibly showing up in releases, thus, not blocking deployments.

All of these errors are harmless and I'm aiming to investigate them over time to determine if any of them contain a real bug.
@armenzg armenzg requested a review from JoshFerge August 14, 2024 20:27
@armenzg armenzg self-assigned this Aug 14, 2024
@armenzg armenzg requested review from a team as code owners August 14, 2024 20:27
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 14, 2024
Copy link

codecov bot commented Aug 14, 2024

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
21646 1 21645 202
View the top 1 failed tests by shortest run time
tests.sentry.integrations.github.test_integration.GitHubIntegrationTest�test_get_trees_for_org_rate_limit_401
Stack Traces | 5.02s run time
#x1B[1m#x1B[.../shared_integrations/client/base.py#x1B[0m:252: in _request
resp.raise_for_status()
#x1B[1m#x1B[31m.venv/lib/python3.11.../site-packages/requests/models.py#x1B[0m:1021: in raise_for_status
raise HTTPError(http_error_msg, response=self)
#x1B[1m#x1B[31mE requests.exceptions.HTTPError: 401 Client Error: Unauthorized for url: https://api.github.com/rate_limit#x1B[0m

#x1B[33mThe above exception was the direct cause of the following exception:#x1B[0m
#x1B[1m#x1B[.../integrations/github/client.py#x1B[0m:447: in _populate_trees
rate_limit = self.get_rate_limit()
#x1B[1m#x1B[.../integrations/github/client.py#x1B[0m:286: in get_rate_limit
return GithubRateLimitInfo(self.get("/rate_limit")["resources"][specific_resource])
#x1B[1m#x1B[.../shared_integrations/client/base.py#x1B[0m:349: in get
return self.request("GET", *args, **kwargs)
#x1B[1m#x1B[.../shared_integrations/client/base.py#x1B[0m:315: in request
return self._request(*args, **kwargs)
#x1B[1m#x1B[.../shared_integrations/client/base.py#x1B[0m:280: in _request
raise ApiError.from_response(error_resp, url=full_url) from e
#x1B[1m#x1B[31mE sentry.shared_integrations.exceptions.ApiUnauthorized: {"message": "Bad credentials"}#x1B[0m

#x1B[33mDuring handling of the above exception, another exception occurred:#x1B[0m
#x1B[1m#x1B[.../integrations/github/test_integration.py#x1B[0m:901: in test_get_trees_for_org_rate_limit_401
trees = installation.get_trees_for_org()
#x1B[1m#x1B[.../integrations/github/integration.py#x1B[0m:290: in get_trees_for_org
trees = self.get_client().get_trees_for_org(gh_org=gh_org, cache_seconds=cache_seconds)
#x1B[1m#x1B[.../integrations/github/client.py#x1B[0m:357: in get_trees_for_org
trees = self._populate_trees(repositories)
#x1B[1m#x1B[.../integrations/github/client.py#x1B[0m:453: in _populate_trees
logger.exception(
#x1B[1m#x1B[.../hostedtoolcache/Python/3.11.8....../x64/lib/python3.11/logging/__init__.py#x1B[0m:1524: in exception
self.error(msg, *args, exc_info=exc_info, **kwargs)
#x1B[1m#x1B[.../hostedtoolcache/Python/3.11.8....../x64/lib/python3.11/logging/__init__.py#x1B[0m:1518: in error
self._log(ERROR, msg, args, **kwargs)
#x1B[1m#x1B[31mE TypeError: Logger._log() got multiple values for argument 'level'#x1B[0m

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

Copy link
Member

@JoshFerge JoshFerge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure if this works? given the test failure. i think my idea was to use sentry APIs directly.

@armenzg armenzg removed the request for review from a team August 16, 2024 12:41
This reports the exception as warning level rather than error level.

Deployments do not get paused if the error is warning or lower.
Copy link
Member Author

@armenzg armenzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are different execution paths depending on the method used. The logging integration does not allow for it.

@armenzg armenzg requested a review from JoshFerge August 16, 2024 16:32
@armenzg armenzg changed the title chore(derived_code_mappings): Set info level issues chore(derived_code_mappings): Report errors with warning level Aug 16, 2024
@armenzg armenzg requested a review from a team August 16, 2024 17:22
@armenzg armenzg enabled auto-merge (squash) August 16, 2024 17:23
@armenzg armenzg merged commit 491dd96 into master Aug 26, 2024
50 of 51 checks passed
@armenzg armenzg deleted the feat/lower-logging-level/armenzg branch August 26, 2024 12:24
Copy link

sentry-io bot commented Aug 26, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: capture_message() got multiple values for argument 'level' sentry.tasks.derive_code_mappings.derive_code_m... View Issue

Did you find this useful? React with a 👍 or 👎

@armenzg armenzg added the Trigger: Revert add to a merged PR to revert it (skips CI) label Aug 26, 2024
@getsentry-bot
Copy link
Contributor

PR reverted: 47dd90f

getsentry-bot added a commit that referenced this pull request Aug 26, 2024
#76212)"

This reverts commit 491dd96.

Co-authored-by: armenzg <44410+armenzg@users.noreply.github.com>
armenzg added a commit that referenced this pull request Aug 27, 2024
This reports the exceptions as warning-level errors rather than
error-level.

Deployments do not get paused if the error is [warning or
lower](https://github.com/getsentry/getsentry/blob/b08907d9da3e8856c6cbedbd86d4a722c6f89dd9/gocd/templates/bash/backend/check-sentry-new-errors.sh#L12):
```
--additional-query="issue.type:error !level:info !level:warning"
```

Rendering of a warning-level exception:
<img width="265" alt="image"
src="https://github.com/user-attachments/assets/cd630c9b-9ac7-4894-aa33-736789c17c0b">
armenzg added a commit that referenced this pull request Aug 29, 2024
This reports the exceptions as warning-level errors rather than
error-level.

Deployments do not get paused if the error is [warning or
lower](https://github.com/getsentry/getsentry/blob/b08907d9da3e8856c6cbedbd86d4a722c6f89dd9/gocd/templates/bash/backend/check-sentry-new-errors.sh#L12):
```
--additional-query="issue.type:error !level:info !level:warning"
```

Rendering of a warning-level exception:
<img width="265" alt="image"
src="https://github.com/user-attachments/assets/cd630c9b-9ac7-4894-aa33-736789c17c0b">

PS: This is a redo of #76212 but without passing a level, thus, making
it an info-level message.
@github-actions github-actions bot locked and limited conversation to collaborators Sep 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Trigger: Revert add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants