From 998b5917afde34cedd4ae69b60bf5e505db20291 Mon Sep 17 00:00:00 2001 From: st0012 Date: Thu, 4 Mar 2021 23:16:18 +0800 Subject: [PATCH 1/2] Compress event payload by default Rules: - If `config.transport.encoding == "gzip"` and the event envelope's size is larger than 30kb, the envelope will be compressed. - If `config.transport.encoding == "gzip"` but the event envelope's size is smaller than 30kb, the envelope won't be compressed. - If `config.transport.encoding != "gzip"`, the event won't be compressed regardless how but it is. --- .../lib/sentry/transport/configuration.rb | 4 +- .../lib/sentry/transport/http_transport.rb | 18 +++- .../sentry/transport/http_transport_spec.rb | 88 +++++++++++++++---- 3 files changed, 90 insertions(+), 20 deletions(-) diff --git a/sentry-ruby/lib/sentry/transport/configuration.rb b/sentry-ruby/lib/sentry/transport/configuration.rb index 10dbad87b..f630068f5 100644 --- a/sentry-ruby/lib/sentry/transport/configuration.rb +++ b/sentry-ruby/lib/sentry/transport/configuration.rb @@ -1,12 +1,14 @@ module Sentry class Transport class Configuration - attr_accessor :timeout, :open_timeout, :proxy, :ssl, :ssl_ca_file, :ssl_verification, :http_adapter, :faraday_builder, :transport_class + attr_accessor :timeout, :open_timeout, :proxy, :ssl, :ssl_ca_file, :ssl_verification, :http_adapter, :faraday_builder, + :transport_class, :encoding def initialize @ssl_verification = true @open_timeout = 1 @timeout = 2 + @encoding = HTTPTransport::GZIP_ENCODING end def transport_class=(klass) diff --git a/sentry-ruby/lib/sentry/transport/http_transport.rb b/sentry-ruby/lib/sentry/transport/http_transport.rb index 590d2803d..7f5ed74f6 100644 --- a/sentry-ruby/lib/sentry/transport/http_transport.rb +++ b/sentry-ruby/lib/sentry/transport/http_transport.rb @@ -1,8 +1,12 @@ require 'faraday' +require 'zlib' module Sentry class HTTPTransport < Transport - CONTENT_TYPE = 'application/json' + GZIP_ENCODING = "gzip" + GZIP_THRESHOLD = 1024 * 30 + CONTENT_TYPE = 'application/x-sentry-envelope' + attr_reader :conn, :adapter def initialize(*args) @@ -13,8 +17,16 @@ def initialize(*args) end def send_data(data) + encoding = "" + + if should_compress?(data) + data = Zlib.gzip(data) + encoding = GZIP_ENCODING + end + conn.post @endpoint do |req| req.headers['Content-Type'] = CONTENT_TYPE + req.headers['Content-Encoding'] = encoding req.headers['X-Sentry-Auth'] = generate_auth_header req.body = data end @@ -31,6 +43,10 @@ def send_data(data) private + def should_compress?(data) + @transport_configuration.encoding == GZIP_ENCODING && data.bytesize >= GZIP_THRESHOLD + end + def set_conn server = @dsn.server diff --git a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb index 75164b2a8..d46940f42 100644 --- a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb @@ -1,17 +1,20 @@ require 'spec_helper' RSpec.describe Sentry::HTTPTransport do - let(:client) { Sentry::Client.new(Sentry.configuration) } - let(:event) { client.event_from_message("test") } - subject { described_class.new(Sentry.configuration) } - - describe "customizations" do - before do - Sentry.init do |c| - c.dsn = 'http://12345@sentry.localdomain/sentry/42' - end + let(:configuration) do + Sentry::Configuration.new.tap do |config| + config.dsn = 'http://12345@sentry.localdomain/sentry/42' end + end + let(:client) { Sentry::Client.new(configuration) } + let(:event) { client.event_from_message("foobarbaz") } + let(:data) do + subject.encode(event.to_hash) + end + subject { described_class.new(configuration) } + + describe "customizations" do it 'sets a custom User-Agent' do expect(subject.conn.headers[:user_agent]).to eq("sentry-ruby/#{Sentry::VERSION}") end @@ -19,7 +22,7 @@ it 'allows to customise faraday' do builder = spy('faraday_builder') expect(Faraday).to receive(:new).and_yield(builder) - Sentry.configuration.transport.faraday_builder = proc { |b| b.request :instrumentation } + configuration.transport.faraday_builder = proc { |b| b.request :instrumentation } subject @@ -27,14 +30,63 @@ end end + describe "request payload" do + let(:compressed_stubs) do + Faraday::Adapter::Test::Stubs.new do |stub| + stub.post('sentry/api/42/envelope/') do |env| + expect(env.request_headers["Content-Type"]).to eq("application/x-sentry-envelope") + expect(env.request_headers["Content-Encoding"]).to eq("gzip") + + envelope = Zlib.gunzip(env.body) + expect(envelope).to include(event.event_id) + expect(envelope).to include("foobarbaz") + end + end + end + + let(:uncompressed_stubs) do + Faraday::Adapter::Test::Stubs.new do |stub| + stub.post('sentry/api/42/envelope/') do |env| + expect(env.request_headers["Content-Type"]).to eq("application/x-sentry-envelope") + expect(env.request_headers["Content-Encoding"]).to eq("") + + envelope = env.body + expect(envelope).to include(event.event_id) + expect(envelope).to include("foobarbaz") + end + end + end + + it "compresses data by default" do + configuration.transport.http_adapter = [:test, compressed_stubs] + + subject.send_data(data) + compressed_stubs.verify_stubbed_calls + end + + it "doesn't compress small event" do + configuration.transport.http_adapter = [:test, uncompressed_stubs] + + event.instance_variable_set(:@threads, nil) # shrink event + + subject.send_data(data) + uncompressed_stubs.verify_stubbed_calls + end + + it "doesn't compress data if the encoding is not gzip" do + configuration.transport.http_adapter = [:test, uncompressed_stubs] + configuration.transport.encoding = "json" + + subject.send_data(data) + uncompressed_stubs.verify_stubbed_calls + end + end + describe "failed request handling" do before do - Sentry.init do |c| - c.dsn = 'http://12345@sentry.localdomain/sentry/42' - c.transport.http_adapter = [:test, stubs] - c.transport.transport_class = described_class - end + configuration.transport.http_adapter = [:test, stubs] end + context "receive 4xx responses" do let(:stubs) do Faraday::Adapter::Test::Stubs.new do |stub| @@ -43,7 +95,7 @@ end it 'raises an error' do - expect { subject.send_data(event.to_hash) }.to raise_error(Sentry::Error, /the server responded with status 404/) + expect { subject.send_data(data) }.to raise_error(Sentry::Error, /the server responded with status 404/) stubs.verify_stubbed_calls end @@ -57,7 +109,7 @@ end it 'raises an error' do - expect { subject.send_data(event.to_hash) }.to raise_error(Sentry::Error, /the server responded with status 500/) + expect { subject.send_data(data) }.to raise_error(Sentry::Error, /the server responded with status 500/) stubs.verify_stubbed_calls end @@ -71,7 +123,7 @@ end it 'raises an error with header' do - expect { subject.send_data(event.to_hash) }.to raise_error(Sentry::Error, /error_in_header/) + expect { subject.send_data(data) }.to raise_error(Sentry::Error, /error_in_header/) stubs.verify_stubbed_calls end From 88b5e466cb0e224472008856dc46be24b6e88fc9 Mon Sep 17 00:00:00 2001 From: st0012 Date: Fri, 5 Mar 2021 18:39:53 +0800 Subject: [PATCH 2/2] Update changelog --- sentry-ruby/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry-ruby/CHANGELOG.md b/sentry-ruby/CHANGELOG.md index b3be75d3d..da744be6c 100644 --- a/sentry-ruby/CHANGELOG.md +++ b/sentry-ruby/CHANGELOG.md @@ -10,6 +10,7 @@ - Refactor tracing implementation [#1309](https://github.com/getsentry/sentry-ruby/pull/1309) - Allow configuring BreadcrumbBuffer's size limit [#1310](https://github.com/getsentry/sentry-ruby/pull/1310) - Return nil from logger methods instead of breadcrumb buffer [#1299](https://github.com/getsentry/sentry-ruby/pull/1299) +- Compress event payload by default [#1314](https://github.com/getsentry/sentry-ruby/pull/1314) ## 4.2.2