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.

Sign up for GitHub

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

Jump to bottom

feat(replays): Add user-agent parsing to replays processor #1420

Merged
merged 57 commits into from
Aug 29, 2022
Merged

feat(replays): Add user-agent parsing to replays processor #1420

merged 57 commits into from
Aug 29, 2022

Conversation

cmanallen
Copy link
Member

@cmanallen cmanallen commented Aug 16, 2022 •
edited
Loading

No description provided.

@cmanallen
Copy link
Member Author

cmanallen commented Aug 17, 2022 •
edited
Loading

Edit: resolved.

Test test_replay_events_without_processing is failing in CI but not locally. Possible timeout issue?

Traceback (most recent call last):
[758](https://github.com/getsentry/relay/runs/7880246249?check_suite_focus=true#step:10:759)
  File "/home/runner/work/relay/relay/tests/integration/test_replay_events.py", line 118, in test_replay_events_without_processing
[759](https://github.com/getsentry/relay/runs/7880246249?check_suite_focus=true#step:10:760)
    envelope = mini_sentry.captured_events.get(timeout=10)
[760](https://github.com/getsentry/relay/runs/7880246249?check_suite_focus=true#step:10:761)
  File "/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/queue.py", line 178, in get
[761](https://github.com/getsentry/relay/runs/7880246249?check_suite_focus=true#step:10:762)
    raise Empty
[762](https://github.com/getsentry/relay/runs/7880246249?check_suite_focus=true#step:10:763)
_queue.Empty

@cmanallen cmanallen marked this pull request as ready for review August 17, 2022 14:28
@cmanallen cmanallen requested a review from a team August 17, 2022 14:28
jjbayer
jjbayer reviewed Aug 18, 2022
View reviewed changes
relay-general/src/user_agent.rs Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
tests/integration/test_replay_events.py Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
@cmanallen
Copy link
Member Author

@jjbayer I add unit coverage and updated this branch to include some ip-address handling behavior. Basically defaulting the user's ip-address to the envelope's ip-address if it wasn't included. Since that branch would have had a dependency on this one it made sense to just merge them. All else remains the same.

https://github.com/getsentry/relay/pull/1420/files#diff-ff64e5542d3e0a29fab7ed5550d22511a6ca00cefaae3bc9a8de7b158499d792R914-R916

jan-auer
jan-auer reviewed Aug 19, 2022
View reviewed changes
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
relay-server/Cargo.toml Outdated Show resolved Hide resolved
relay-server/src/actors/processor.rs Outdated Show resolved Hide resolved
relay-server/src/actors/processor.rs Outdated Show resolved Hide resolved
cmanallen and others added 16 commits August 19, 2022 08:02
@cmanallen @jan-auer
Update relay-replays/src/lib.rs
3713598 
Co-authored-by: Jan Michael Auer <mail@jauer.org>
@cmanallen @jan-auer
Update relay-replays/src/lib.rs
9970906 
Co-authored-by: Jan Michael Auer <mail@jauer.org>
@cmanallen
Permit vectors to default to empty list
20cb0ed
@cmanallen
Merge branch 'replays-parse-user-agent' of https://github.com/getsent…
4f5c846 
…ry/relay into replays-parse-user-agent
@cmanallen @jan-auer
Allow compiler to infer type
6204a26 
Co-authored-by: Jan Michael Auer <mail@jauer.org>
@cmanallen
Fix import order
8ebc850
@cmanallen
Merge branch 'replays-parse-user-agent' of https://github.com/getsent…
a095bf2 
…ry/relay into replays-parse-user-agent
@cmanallen
Log out failed deserialization
28d9cd3
@cmanallen
Return false
9c5c51f
@cmanallen
Make ReplayInput private
03cd143
@cmanallen
Allow requests struct to default
c2b6fe5
@cmanallen
Make ReplayInput methods private
ac2a3c1
@cmanallen
Add docstring to normalize_replay_event function
4d529cb
@cmanallen
Add default for user struct and update ip-address setter with simplif…
0f7e8b1 
…ied behavior
@cmanallen
Merge branch 'master' into replays-parse-user-agent
d7105fb
@cmanallen
Merge branch 'master' into replays-parse-user-agent
31dae59
@cmanallen cmanallen requested a review from jan-auer August 29, 2022 14:37
jjbayer
jjbayer approved these changes Aug 29, 2022
View reviewed changes
Copy link
Member

@jjbayer jjbayer left a comment

Choose a reason for hiding this comment

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

Looks good to me, some minor nitpicks.

relay-replays/src/lib.rs Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
relay-replays/src/lib.rs Outdated Show resolved Hide resolved
@cmanallen cmanallen merged commit 9f03ea5 into master Aug 29, 2022
@cmanallen cmanallen deleted the replays-parse-user-agent branch August 29, 2022 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jjbayer jjbayer jjbayer approved these changes

@jan-auer jan-auer Awaiting requested review from jan-auer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cmanallen @jan-auer @jjbayer