ref(event_manager): Fix typing issues for event_manager by armenzg · Pull Request #52888 · getsentry/sentry

armenzg · 2023-07-14T16:24:02Z

We want to make a lot of changes to event_manager and we need to have backend typing in place for the upcoming work.

We want to make a lot of changes to event_manager and we need to have backend typing in place for the upcoming work. Fixes #52877

codecov · 2023-07-14T16:54:18Z

Codecov Report

Merging #52888 (5495512) into master (6d2d078) will increase coverage by 5.07%.
The diff coverage is 100.00%.

❗ Current head 5495512 differs from pull request most recent head 12a66f8. Consider uploading reports for the commit 12a66f8 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #52888      +/-   ##
==========================================
+ Coverage   74.29%   79.36%   +5.07%     
==========================================
  Files        4931     4930       -1     
  Lines      207058   207070      +12     
  Branches    35388    35388              
==========================================
+ Hits       153829   164348   +10519     
+ Misses      48232    37691   -10541     
- Partials     4997     5031      +34

Impacted Files	Coverage Δ
...rmance_issues/detectors/io_main_thread_detector.py	`90.90% <ø> (ø)`
src/sentry/attachments/__init__.py	`100.00% <100.00%> (ø)`
src/sentry/eventtypes/__init__.py	`100.00% <100.00%> (ø)`
src/sentry/tsdb/__init__.py	`100.00% <100.00%> (ø)`

... and 633 files with indirect coverage changes

armenzg · 2023-07-14T17:47:10Z

        headers={"Content-Type": attachment.content_type},
    )
-    file.putfile(BytesIO(data), blob_size=settings.SENTRY_ATTACHMENT_BLOB_SIZE)
+    file.putfile(BytesIO(data), blob_size=settings.SENTRY_ATTACHMENT_BLOB_SIZE)  # type: ignore


I was getting this src/sentry/event_manager.py:2158: error: Argument 1 to "BytesIO" has incompatible type "Callable[[CachedAttachment], Any]"; expected "Buffer" [arg-type]

I could not figure it out.

armenzg · 2023-07-14T17:50:40Z


-    hashes.write_to_event(event.data)
+    # Using cast to satisfy mypy
+    hashes.write_to_event(cast(Dict[str, Any], event.data))


I was not sure how to fix this one without getting complicated.
I was getting this message before: src/sentry/event_manager.py:2269: error: Argument 1 to "write_to_event" of "CalculatedHashes" has incompatible type "NodeData"; expected "Dict[str, Any]" [arg-type]

Here's the code for write_to_event

cast should only be used for mypy bugs -- you'll need to fix the types here or convert -- probably by fixing the signature for write_to_event

asottile-sentry · 2023-07-14T18:50:11Z

+__all__ = [
+    "CspEvent",
+    "DefaultEvent",
+    "ErrorEvent",
+    "ExpectCTEvent",
+    "ExpectStapleEvent",
+    "HpkpEvent",
+    "TransactionEvent",
+]


I would remove this personally -- it's better to import explicitly since __init__.py makes it much easier to introduce import cycles and to worsen startup performance

Consider the following situation: 1. Symbolicator needs the bundle for release `foo`, dist `1` that contains `https://example.com/main.dart.js`. 2. URL search is not exact. Symbolicator asks the `artifact-lookup` endpoint for a bundle with release `foo`, dist `1`, URL `/main`. 3. There are two bundles for the release/dist. Bundle 1 contains `main.dart`, bundle 2 contains `main.dart.js`. It so happens that bundle 1 is newer. 4. Bundle 1 gets returned to Symbolicator. 5. The bundle Symbolicator got back doesn't contain the file it wanted. We fix this by returning not just the first bundle that contains the URL, but up to `MAX_BUNDLES_QUERY` (=5) bundles.

resolves a migration warning: `fields.W904` ``` sentry.ReleaseActivity.data: (fields.W904) django.contrib.postgres.fields.JSONField is deprecated. Support for it (except in historical migrations) will be removed in Django 4.0. HINT: Use django.db.models.JSONField instead. ``` --------- Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>

### Summary Don't show the invalid_data errors in the UI as to not cause false alarm as this has since been fixed. ### Screenshots #### Before ![Screenshot 2023-06-14 at 12 08 45 PM](https://github.com/getsentry/sentry/assets/6111995/ccbef4f6-023e-4e1c-80cb-921f7d68ea55) #### After ![Screenshot 2023-06-14 at 12 04 51 PM](https://github.com/getsentry/sentry/assets/6111995/24582d7a-058e-4b00-9740-9c88f1f513f6) --------- Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>

we are now on django 3+

We were improperly mapping the dataset resulting in all calls with `query` string filtering on the `IssuePlatform` dataset. Resolves SENTRY-1379

- Remove unnecessary prop - Convert teamResolutionTime to functional

We should let self-hosted Sentry use [team-level roles](#49028).

To run the consumer issue the following command. `ingest-replay-recordings --processes NUM_CORES * 4 --max-batch-size 100`. Additional options can be specified but currently have sane defaults.

…52846)

…e numeric timestamps correctly (#52828)

Closes #50697 Update sort order wording in user settings to match issue details Before: <img width="555" alt="Screenshot 2023-07-14 at 9 46 50 AM" src="https://github.com/getsentry/sentry/assets/22582037/d61e854a-91d9-4641-bfed-d97f38ef4647"> After: <img width="555" alt="Screenshot 2023-07-14 at 9 45 25 AM" src="https://github.com/getsentry/sentry/assets/22582037/8aba23a8-049e-4bac-bd31-9678895cd04c">

…52871)

### Summary Adds a new detector (flags aren't going to be flipped until after audit). Servers using http/1.1 might be causing queueing behaviour on the browser when the connections reach a certain amount. This detector roughly detects overlapping spans against the same location (connections are limited per host), assuming that queue depth is causing monotonically increasing requestStart. #### Other - We'll likely need to change the network timing (`requestStart`) this currently targets, but sdk needs to be cut before that. - Our sdk collects absolute paths, so we skip url parsing in that case and use location '/' - Didn't want to have `/1.1` in filenames or enums so change the file/vars to `http overhead`. - We'll try a `500` delay threshold for now, meaning the peak of the increasing times should be 500ms. I've found existing real events that fire on this. --------- Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>

github-actions · 2023-07-17T12:48:30Z

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

armenzg · 2023-07-17T12:51:05Z

My apologies everyone.

This is what I did in case we want to figure out if there's a way to prevent this:

I pulled from master and I merged into my branch
I tried to push but there's some automation that added one more commit to this branch
I pulled this remote branch to merge locally
I pushed again

Now I see all of these other teams tagged for the review and a bunch of code that I never added.

I will close and re-open.

armenzg added 2 commits July 14, 2023 09:20

feat(typing): Type event_manager

9ef98f9

ref(event_manager): Fix typing issues for event_manager

52b65d6

We want to make a lot of changes to event_manager and we need to have backend typing in place for the upcoming work. Fixes #52877

armenzg self-assigned this Jul 14, 2023

github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 14, 2023

vercel Bot deployed to Preview July 14, 2023 16:25 View deployment

More fixes

41dcdfb

armenzg marked this pull request as ready for review July 14, 2023 17:46

armenzg requested review from a team as code owners July 14, 2023 17:46

armenzg requested a review from a team July 14, 2023 17:46

vercel Bot deployed to Preview July 14, 2023 17:48 View deployment

armenzg commented Jul 14, 2023

View reviewed changes

🔪 regenerate mypy module blocklist

5495512

vercel Bot deployed to Preview July 14, 2023 17:53 View deployment

asottile-sentry reviewed Jul 14, 2023

View reviewed changes

armenzg and others added 14 commits July 17, 2023 08:47

Address Anthony's feedback

0fc0ec6

fix(getting-started-docs): Fix path native qt (#52878)

8cceb81

ref: remove django_compat (#52879)

7c2d28c

we are now on django 3+

fix(helpful-event): fix querying for title for helpful event (#52838)

cf33905

We were improperly mapping the dataset resulting in all calls with `query` string filtering on the `IssuePlatform` dataset. Resolves SENTRY-1379

ref(stats): Convert TeamResolutionTime to functional (#52845)

e6e655b

- Remove unnecessary prop - Convert teamResolutionTime to functional

chore(roles): Default team-roles flag to True (#52830)

1cf90b9

We should let self-hosted Sentry use [team-level roles](#49028).

chore(hybrid-cloud): Stabilize slack command test (#52767)

fb838a1

feat(replays): Add multiprocessing consumer step (#48276)

697642e

To run the consumer issue the following command. `ingest-replay-recordings --processes NUM_CORES * 4 --max-batch-size 100`. Additional options can be specified but currently have sane defaults.

fix(hybrid-cloud): auth attach identity unsets idp:provisioned flag (#…

33e2597

…52846)

fix(replay): Fix an issue where useInitialTimeOffsetMs doesn't consum…

8e9f190

…e numeric timestamps correctly (#52828)

priscilawebdev and others added 2 commits July 17, 2023 08:47

ref(getting-started-docs): Migrate minidump doc to sentry main repo (#…

16a4533

…52871)

armenzg requested a review from a team as a code owner July 17, 2023 12:48

armenzg requested a review from a team July 17, 2023 12:48

armenzg requested review from a team as code owners July 17, 2023 12:48

armenzg requested review from a team July 17, 2023 12:48

armenzg requested review from a team as code owners July 17, 2023 12:48

github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 17, 2023

armenzg removed request for a team July 17, 2023 12:48

vercel Bot deployed to Preview July 17, 2023 12:50 View deployment

armenzg closed this Jul 17, 2023

armenzg deleted the event-manager-typing/armenzg branch July 17, 2023 12:51

github-actions Bot locked and limited conversation to collaborators Aug 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ref(event_manager): Fix typing issues for event_manager#52888

ref(event_manager): Fix typing issues for event_manager#52888
armenzg wants to merge 43 commits into
masterfrom
event-manager-typing/armenzg

armenzg commented Jul 14, 2023

Uh oh!

codecov Bot commented Jul 14, 2023 •

edited

Loading

Uh oh!

armenzg Jul 14, 2023

Uh oh!

armenzg Jul 14, 2023

Uh oh!

asottile-sentry Jul 14, 2023

Uh oh!

asottile-sentry Jul 14, 2023

Uh oh!

github-actions Bot commented Jul 17, 2023

Uh oh!

armenzg commented Jul 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

Uh oh!

Conversation

armenzg commented Jul 14, 2023

Uh oh!

codecov Bot commented Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

armenzg Jul 14, 2023

Choose a reason for hiding this comment

Uh oh!

armenzg Jul 14, 2023

Choose a reason for hiding this comment

Uh oh!

asottile-sentry Jul 14, 2023

Choose a reason for hiding this comment

Uh oh!

asottile-sentry Jul 14, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 17, 2023

Uh oh!

armenzg commented Jul 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

codecov Bot commented Jul 14, 2023 •

edited

Loading