-
Notifications
You must be signed in to change notification settings - Fork 4
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
Changes from all commits
f39735f
f32f50d
f3bf86e
05cf9d7
faf06e2
0d483a1
51a1f72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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}" | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this method should examine response headers, specifically There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The last 3 lines should be replaceable with There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
end | ||
end | ||
|
||
class AdapterSettings | ||
|
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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