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

Check envelope size before sending it #1747

Merged
merged 6 commits into from
Mar 7, 2022
Merged

Check envelope size before sending it #1747

merged 6 commits into from
Mar 7, 2022

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented Feb 25, 2022

Because we now need to check if a serialized envelope item is oversized (see #1603 (comment)), envelope serialization is better performed by Transport to have more control to filter oversized items.

@st0012
Copy link
Collaborator Author

st0012 commented Mar 4, 2022

@sl0thentr0py I know the code could still be refactored (which I'll do later), but can you give it a quick scan and see if the approach looks good?

Copy link
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

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

logic looks ok 👍

@codecov-commenter
Copy link

codecov-commenter commented Mar 5, 2022

Codecov Report

Merging #1747 (23652c9) into master (e727167) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1747      +/-   ##
==========================================
+ Coverage   98.43%   98.45%   +0.01%     
==========================================
  Files         144      144              
  Lines        8312     8412     +100     
==========================================
+ Hits         8182     8282     +100     
  Misses        130      130              
Impacted Files Coverage Δ
sentry-ruby/lib/sentry/envelope.rb 95.23% <ø> (-4.77%) ⬇️
sentry-ruby/lib/sentry/event.rb 98.90% <100.00%> (+0.01%) ⬆️
sentry-ruby/lib/sentry/transport.rb 99.08% <100.00%> (+0.19%) ⬆️
...-ruby/spec/sentry/transport/http_transport_spec.rb 100.00% <100.00%> (ø)
sentry-ruby/spec/sentry/transport_spec.rb 100.00% <100.00%> (ø)
sentry-resque/lib/sentry/resque.rb 100.00% <0.00%> (+2.85%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e727167...23652c9. Read the comment docs.

@st0012 st0012 removed the wip label Mar 5, 2022
@st0012 st0012 added this to the 5.2.0 milestone Mar 5, 2022
@st0012 st0012 requested a review from sl0thentr0py March 5, 2022 15:17
st0012 added 2 commits March 7, 2022 19:05
Because we now need to check if a serialized envelope item is oversized
(see
#1603 (comment)),
envelope serialization is better performed by Transport to have more
control to filter oversized items.
@st0012 st0012 changed the title Add Transport#serialize_envelope to control envelope serialization Check envelope size before sending it Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants