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

[CIVIS-9901] accept gzipped responses from API #170

Merged
merged 7 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lib/datadog/ci/ext/transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module Transport
DEFAULT_DD_SITE = "datadoghq.com"

HEADER_DD_API_KEY = "DD-API-KEY"
HEADER_ACCEPT_ENCODING = "Accept-Encoding"
HEADER_CONTENT_TYPE = "Content-Type"
HEADER_CONTENT_ENCODING = "Content-Encoding"
HEADER_EVP_SUBDOMAIN = "X-Datadog-EVP-Subdomain"
Expand Down Expand Up @@ -48,6 +49,8 @@ module Transport
CONTENT_TYPE_JSON = "application/json"
CONTENT_TYPE_MULTIPART_FORM_DATA = "multipart/form-data"
CONTENT_ENCODING_GZIP = "gzip"

GZIP_MAGIC_NUMBER = "\x1F\x8B".b
end
end
end
Expand Down
14 changes: 11 additions & 3 deletions lib/datadog/ci/transport/api/agentless.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@ def citestcycle_request(path:, payload:, headers: {}, verb: "post")
def api_request(path:, payload:, headers: {}, verb: "post")
super

perform_request(@api_http, path: path, payload: payload, headers: headers, verb: verb)
perform_request(
@api_http,
path: path,
payload: payload,
headers: headers,
verb: verb,
accept_compressed_response: true
)
end

def citestcov_request(path:, payload:, headers: {}, verb: "post")
Expand All @@ -37,12 +44,13 @@ def citestcov_request(path:, payload:, headers: {}, verb: "post")

private

def perform_request(http_client, path:, payload:, headers:, verb:)
def perform_request(http_client, path:, payload:, headers:, verb:, accept_compressed_response: false)
http_client.request(
path: path,
payload: payload,
headers: headers_with_default(headers),
verb: verb
verb: verb,
accept_compressed_response: accept_compressed_response
)
end

Expand Down
12 changes: 12 additions & 0 deletions lib/datadog/ci/transport/gzip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@ def compress(input)
gzip_writer.close
sio.string
end

def decompress(input)
sio = StringIO.new(input)
gzip_reader = Zlib::GzipReader.new(
sio,
external_encoding: Encoding::UTF_8,
internal_encoding: Encoding::UTF_8
)
gzip_reader.read || ""

Choose a reason for hiding this comment

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

Should read not always return a string already? When would it not?

Copy link
Member Author

Choose a reason for hiding this comment

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

RBS believes that read returns (String | nil), so I need to somehow convince it that Gzip.decompress will never return nil

ensure
gzip_reader&.close
end
end
end
end
Expand Down
38 changes: 35 additions & 3 deletions lib/datadog/ci/transport/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,24 @@ def initialize(host:, timeout: DEFAULT_TIMEOUT, port: nil, ssl: true, compress:
@compress = compress.nil? ? false : compress
end

def request(path:, payload:, headers:, verb: "post", retries: MAX_RETRIES, backoff: INITIAL_BACKOFF)
def request(
path:,
payload:,
headers:,
verb: "post",
retries: MAX_RETRIES,
backoff: INITIAL_BACKOFF,
accept_compressed_response: false
)
if compress
headers[Ext::Transport::HEADER_CONTENT_ENCODING] = Ext::Transport::CONTENT_ENCODING_GZIP
payload = Gzip.compress(payload)
end

if accept_compressed_response
headers[Ext::Transport::HEADER_ACCEPT_ENCODING] = Ext::Transport::CONTENT_ENCODING_GZIP
end

Datadog.logger.debug do
"Sending #{verb} request: host=#{host}; port=#{port}; ssl_enabled=#{ssl}; " \
"compression_enabled=#{compress}; path=#{path}; payload_size=#{payload.size}"
Expand Down Expand Up @@ -91,12 +103,32 @@ def adapter
@adapter ||= Datadog::Core::Transport::HTTP::Adapters::Net.new(settings)
end

# this is needed because Datadog::Tracing::Writer is not fully compatiple with Datadog::Core::Transport
# TODO: remove when CI implements its own worker
# adds compatibility with Datadog::Tracing transport and
# provides ungzipping capabilities
class ResponseDecorator < ::SimpleDelegator
def payload
return @decompressed_payload if defined?(@decompressed_payload)

if gzipped?(__getobj__.payload)
Datadog.logger.debug("Decompressing gzipped response payload")
@decompressed_payload = Gzip.decompress(__getobj__.payload)
else
__getobj__.payload
end
end

def trace_count
0
end

def gzipped?(payload)

Choose a reason for hiding this comment

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

I think this method should examine response headers, specifically Content-Encoding: gzip (https://www.rfc-editor.org/rfc/rfc2616#page-118)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I will add this check in the follow up to this PR as soon as I add headers method to https://github.com/DataDog/dd-trace-rb/blob/9810fe00d76a2ff2407f4772fa2363c3f0623522/lib/datadog/core/transport/http/response.rb

I would still keep the GZIP_MAGIC_NUMBER check even then as I want to be double sure that I don't try to decode not gzipped payload (between Agent, Rapid API backend and any proxies that could exist in customer environments I expect all possible headers/payload mismatches to happen.

return false if payload.nil? || payload.empty?

first_bytes = payload[0, 2]
return false if first_bytes.nil? || first_bytes.empty?

first_bytes.b == Datadog::CI::Ext::Transport::GZIP_MAGIC_NUMBER

Choose a reason for hiding this comment

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

The last 3 lines should be replaceable with payload.start_with?(GZIP_MAGIC_NUMBER)

Copy link
Member Author

Choose a reason for hiding this comment

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

Not exactly, I need to convert the string to ASCII_BINARY before comparing bytes with payload.b, I am not sure if this operation takes constant time. Payload could be very large (for my biggest test repos it is about 10Mb, our customers have repos with many more tests, so it could be easily a 50Mb string)

end
end

class AdapterSettings
Expand Down
4 changes: 4 additions & 0 deletions sig/datadog/ci/ext/transport.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ module Datadog

HEADER_DD_API_KEY: "DD-API-KEY"

HEADER_ACCEPT_ENCODING: "Accept-Encoding"

HEADER_CONTENT_TYPE: "Content-Type"

HEADER_CONTENT_ENCODING: "Content-Encoding"
Expand Down Expand Up @@ -63,6 +65,8 @@ module Datadog
CONTENT_TYPE_MULTIPART_FORM_DATA: "multipart/form-data"

CONTENT_ENCODING_GZIP: "gzip"

GZIP_MAGIC_NUMBER: String
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion sig/datadog/ci/transport/api/agentless.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ module Datadog

private

def perform_request: (Datadog::CI::Transport::HTTP client, path: String, payload: String, headers: Hash[String, String], verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator
def perform_request: (Datadog::CI::Transport::HTTP client, path: String, payload: String, headers: Hash[String, String], verb: ::String, ?accept_compressed_response: bool) -> Datadog::CI::Transport::HTTP::ResponseDecorator

def build_http_client: (String url, compress: bool) -> Datadog::CI::Transport::HTTP

Expand Down
1 change: 1 addition & 0 deletions sig/datadog/ci/transport/gzip.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module Datadog
module Transport
module Gzip
def self?.compress: (String input) -> String
def self?.decompress: (String input) -> String
end
end
end
Expand Down
7 changes: 6 additions & 1 deletion sig/datadog/ci/transport/http.rbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
class SimpleDelegator
@decompressed_payload: String

def __getobj__: () -> Datadog::Core::Transport::Response
def gzipped?: (String payload) -> bool
def payload: () -> String
end

module Datadog
Expand All @@ -19,7 +24,7 @@ module Datadog

def initialize: (host: String, ?port: Integer?, ?ssl: bool, ?timeout: Integer, ?compress: bool) -> void

def request: (?verb: String, payload: String, headers: Hash[String, String], path: String, ?retries: Integer, ?backoff: Integer) -> ResponseDecorator
def request: (?verb: String, payload: String, headers: Hash[String, String], path: String, ?retries: Integer, ?backoff: Integer, ?accept_compressed_response: bool) -> ResponseDecorator

private

Expand Down
13 changes: 9 additions & 4 deletions spec/datadog/ci/transport/api/agentless_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@
path: "path",
payload: "payload",
verb: "post",
headers: expected_headers
headers: expected_headers,
accept_compressed_response: false
)

subject.citestcycle_request(path: "path", payload: "payload")
Expand Down Expand Up @@ -120,7 +121,8 @@
path: "path",
payload: "payload",
verb: "post",
headers: expected_headers
headers: expected_headers,
accept_compressed_response: false
)

subject.citestcycle_request(path: "path", payload: "payload")
Expand All @@ -131,7 +133,8 @@
path: "path",
payload: "payload",
verb: "post",
headers: expected_headers.merge({"Content-Type" => "application/json"})
headers: expected_headers.merge({"Content-Type" => "application/json"}),
accept_compressed_response: false
)

subject.citestcycle_request(path: "path", payload: "payload", headers: {"Content-Type" => "application/json"})
Expand All @@ -154,7 +157,8 @@
headers: {
"DD-API-KEY" => "api_key",
"Content-Type" => "application/json"
}
},
accept_compressed_response: true
)

subject.api_request(path: "path", payload: "payload")
Expand Down Expand Up @@ -199,6 +203,7 @@
expect(args[:verb]).to eq("post")
expect(args[:headers]).to eq(expected_headers)
expect(args[:payload]).to eq(expected_payload)
expect(args[:accept_compressed_response]).to eq(false)
end
end
end
Expand Down
38 changes: 29 additions & 9 deletions spec/datadog/ci/transport/gzip_spec.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
require_relative "../../../../lib/datadog/ci/transport/gzip"

RSpec.describe Datadog::CI::Transport::Gzip do
subject { described_class.compress(input) }

describe ".compress" do
let(:input) do
<<-LOREM
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc et est eu dui dignissim tempus. Aliquam
let(:input) do
<<-LOREM
❤️ Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc et est eu dui dignissim tempus. Aliquam
scelerisque posuere odio id sollicitudin. Etiam dolor lorem, interdum sed mollis consectetur, sagittis a massa.
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec gravida, libero ac gravida ornare, leo elit
facilisis nunc, in pharetra odio lectus sit amet augue. Cras fermentum interdum tortor, pulvinar laoreet massa
Expand All @@ -18,17 +15,40 @@
commodo sapien vel, consequat felis. Aenean velit turpis, rhoncus in ipsum ut, tempor iaculis nisi. Fusce
faucibus consequat blandit. Nam maximus augue quis tellus cursus eleifend. Suspendisse auctor, orci sit amet
vehicula molestie, magna nibh rutrum metus, eget sagittis felis mauris eu quam. Vivamus ut vulputate est.
LOREM
end
LOREM
end

describe ".compress" do
subject { described_class.compress(input) }

it "compresses" do
expect(input.size).to be > subject.size
end

it "can be decompressed with gzip" do
Zlib::GzipReader.new(StringIO.new(subject)) do |gzip|
Zlib::GzipReader.new(
StringIO.new(subject),
external_encoding: Encoding::UTF_8,
internal_encoding: Encoding::UTF_8
) do |gzip|
expect(gzip.read).to eq(input)
end
end
end

describe ".decompress" do
subject { described_class.decompress(compressed_input) }

let(:compressed_input) do
sio = StringIO.new
writer = Zlib::GzipWriter.new(sio)
writer << input
writer.close
sio.string
end

it "decompresses" do
expect(subject).to eq(input)
end
end
end
38 changes: 33 additions & 5 deletions spec/datadog/ci/transport/http_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,32 +89,60 @@
let(:path) { "/api/v1/intake" }
let(:payload) { '{ "key": "value" }' }
let(:headers) { {"Content-Type" => "application/json"} }
let(:request_options) { {} }
let(:expected_headers) { headers }
let(:request_options) { {accept_compressed_response: false} }

let(:http_response) { double("http_response", code: 200) }
let(:response_payload) { "sample payload" }
let(:http_response) { double("http_response", code: 200, payload: response_payload) }

subject(:response) { transport.request(path: path, payload: payload, headers: headers, **request_options) }

context "when request is successful" do
let(:env) do
let(:expected_env) do
env = Datadog::Core::Transport::HTTP::Env.new(
Datadog::Core::Transport::Request.new
)
env.body = payload
env.path = path
env.headers = headers
env.headers = expected_headers
env.verb = "post"
env
end

before do
expect(adapter).to receive(:call).with(env).and_return(http_response)
expect(adapter).to receive(:call).with(expected_env).and_return(http_response)
end

it "produces a response" do
is_expected.to be_a_kind_of(described_class::ResponseDecorator)

expect(response.code).to eq(200)
expect(response.payload).to eq("sample payload")
end

context "when accepting gzipped response" do
let(:expected_headers) { {"Content-Type" => "application/json", "Accept-Encoding" => "gzip"} }
let(:request_options) { {accept_compressed_response: true} }

it "adds Accept-Encoding header" do
is_expected.to be_a_kind_of(described_class::ResponseDecorator)

expect(response.code).to eq(200)
expect(response.payload).to eq("sample payload")
end

context "when response is gzipped" do
let(:response_payload) do
Datadog::CI::Transport::Gzip.compress("sample payload")
end

it "decompressed response payload" do
is_expected.to be_a_kind_of(described_class::ResponseDecorator)

expect(response.code).to eq(200)
expect(response.payload).to eq("sample payload")
end
end
end
end

Expand Down
Loading