Ethon & Typhoeus tracing support #778
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| require 'ddtrace/contrib/configuration/settings' | ||
| require 'ddtrace/contrib/ethon/ext' | ||
|
|
||
| module Datadog | ||
| module Contrib | ||
| module Ethon | ||
| module Configuration | ||
| # Custom settings for the Ethon integration | ||
| class Settings < Contrib::Configuration::Settings | ||
| option :analytics_enabled, | ||
| default: -> { env_to_bool(Ext::ENV_ANALYTICS_ENABLED, false) }, | ||
| lazy: true | ||
|
|
||
| option :analytics_sample_rate, | ||
| default: -> { env_to_float(Ext::ENV_ANALYTICS_SAMPLE_RATE, 1.0) }, | ||
| lazy: true | ||
|
|
||
| option :distributed_tracing, default: true | ||
| option :service_name, default: Ext::SERVICE_NAME | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| require 'ddtrace/ext/net' | ||
| require 'ddtrace/ext/distributed' | ||
| require 'ddtrace/propagation/http_propagator' | ||
| require 'ddtrace/contrib/ethon/ext' | ||
|
|
||
| module Datadog | ||
| module Contrib | ||
| module Ethon | ||
| # Ethon EasyPatch | ||
| module EasyPatch | ||
| def self.included(base) | ||
| base.send(:prepend, InstanceMethods) | ||
| end | ||
|
|
||
| # InstanceMethods - implementing instrumentation | ||
| module InstanceMethods | ||
| def http_request(url, action_name, options = {}) | ||
| return super unless tracer_enabled? | ||
|
|
||
| # It's tricky to get HTTP method from libcurl | ||
| @datadog_method = action_name.to_s.upcase | ||
| super | ||
| end | ||
|
|
||
| def headers=(headers) | ||
| return super unless tracer_enabled? | ||
|
|
||
| # Store headers to call this method again when span is ready | ||
| @datadog_original_headers = headers | ||
| super headers | ||
|
There was a problem hiding this comment. Minor but can you just call |
||
| end | ||
|
|
||
| def perform | ||
| return super unless tracer_enabled? | ||
| datadog_before_request | ||
| super | ||
| end | ||
|
|
||
| def complete | ||
| return super unless tracer_enabled? | ||
| begin | ||
| response_options = mirror.options | ||
| response_code = (response_options[:response_code] || response_options[:code]).to_i | ||
| if response_code.zero? | ||
| return_code = response_options[:return_code] | ||
| message = return_code ? ::Ethon::Curl.easy_strerror(return_code) : 'unknown reason' | ||
| set_span_error_message("Request has failed: #{message}") | ||
| else | ||
| @datadog_span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, response_code) | ||
| if Datadog::Ext::HTTP::ERROR_RANGE.cover?(response_code) | ||
| set_span_error_message("Request has failed with HTTP error: #{response_code}") | ||
| end | ||
| end | ||
| ensure | ||
| @datadog_span.finish | ||
| @datadog_span = nil | ||
| end | ||
| super | ||
| end | ||
|
|
||
| def reset | ||
| super | ||
| ensure | ||
| if tracer_enabled? | ||
| @datadog_span = nil | ||
| @datadog_method = nil | ||
| @datadog_original_headers = nil | ||
| end | ||
| end | ||
|
|
||
| def datadog_before_request(parent_span: nil) | ||
| @datadog_span = datadog_configuration[:tracer].trace( | ||
delner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Ext::SPAN_REQUEST, | ||
| service: datadog_configuration[:service_name], | ||
| span_type: Datadog::Ext::HTTP::TYPE_OUTBOUND | ||
| ) | ||
| @datadog_span.parent = parent_span unless parent_span.nil? | ||
|
|
||
| datadog_tag_request | ||
|
|
||
| if datadog_configuration[:distributed_tracing] | ||
| @datadog_original_headers ||= {} | ||
| Datadog::HTTPPropagator.inject!(@datadog_span.context, @datadog_original_headers) | ||
delner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| self.headers = @datadog_original_headers | ||
| end | ||
| end | ||
|
|
||
| def datadog_span_started? | ||
| instance_variable_defined?(:@datadog_span) && !@datadog_span.nil? | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def datadog_tag_request | ||
| span = @datadog_span | ||
| uri = URI.parse(url) | ||
| method = defined?(@datadog_method) ? @datadog_method.to_s : '' | ||
|
There was a problem hiding this comment. Should this be |
||
| span.resource = "#{method} #{uri.path}".lstrip | ||
|
There was a problem hiding this comment. We only want to set the HTTP method here, e.g. Sounds crazy, and I see the value of including the path. However, the resource is meant to be a If the omission of Also, it looks like the There was a problem hiding this comment. Interesting, I was wondering why the resource contains only method name in other instrumentations. I changed the default value of |
||
|
|
||
| # Set analytics sample rate | ||
| Contrib::Analytics.set_sample_rate(span, analytics_sample_rate) if analytics_enabled? | ||
|
|
||
| span.set_tag(Datadog::Ext::HTTP::URL, uri.path) | ||
| span.set_tag(Datadog::Ext::HTTP::METHOD, method) | ||
| span.set_tag(Datadog::Ext::NET::TARGET_HOST, uri.host) | ||
| span.set_tag(Datadog::Ext::NET::TARGET_PORT, uri.port) | ||
| rescue URI::InvalidURIError | ||
| return | ||
| end | ||
|
|
||
| def set_span_error_message(message) | ||
| # Sets span error from message, in case there is no exception available | ||
| @datadog_span.status = Datadog::Ext::Errors::STATUS | ||
| @datadog_span.set_tag(Datadog::Ext::Errors::MSG, message) | ||
| end | ||
|
|
||
| def datadog_configuration | ||
| Datadog.configuration[:ethon] | ||
| end | ||
|
|
||
| def tracer_enabled? | ||
| datadog_configuration[:tracer].enabled | ||
| end | ||
|
|
||
| def analytics_enabled? | ||
| Contrib::Analytics.enabled?(datadog_configuration[:analytics_enabled]) | ||
| end | ||
|
|
||
| def analytics_sample_rate | ||
| datadog_configuration[:analytics_sample_rate] | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| module Datadog | ||
| module Contrib | ||
| module Ethon | ||
| # Ethon integration constants | ||
| module Ext | ||
| APP = 'ethon'.freeze | ||
| ENV_ANALYTICS_ENABLED = 'DD_ETHON_ANALYTICS_ENABLED'.freeze | ||
| ENV_ANALYTICS_SAMPLE_RATE = 'DD_ETHON_ANALYTICS_SAMPLE_RATE'.freeze | ||
| SERVICE_NAME = 'ethon'.freeze | ||
| SPAN_REQUEST = 'ethon.request'.freeze | ||
| SPAN_MULTI_REQUEST = 'ethon.multi.request'.freeze | ||
| end | ||
| end | ||
| end | ||
| end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| require 'ddtrace/contrib/integration' | ||
| require 'ddtrace/contrib/ethon/configuration/settings' | ||
| require 'ddtrace/contrib/ethon/patcher' | ||
|
|
||
| module Datadog | ||
| module Contrib | ||
| module Ethon | ||
| # Description of Ethon integration | ||
| class Integration | ||
| include Contrib::Integration | ||
| register_as :ethon | ||
|
|
||
| def self.version | ||
| Gem.loaded_specs['ethon'] && Gem.loaded_specs['ethon'].version | ||
| end | ||
|
|
||
| def self.present? | ||
| super && defined?(::Ethon::Easy) | ||
| end | ||
|
|
||
| def self.compatible? | ||
| super && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.0.0') | ||
| end | ||
|
|
||
| def default_configuration | ||
| Configuration::Settings.new | ||
| end | ||
|
|
||
| def patcher | ||
| Patcher | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Can you walk me through the flow of a request here a bit? Order the of the functions being called, sync vs async requests, etc.
I see a number of methods being patched here, each collecting and storing some information as instance variables. I'd like to understand how the state is built. Is this information not available on
performwithout first collecting in between all these methods?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.
For sure! I'll start with sync requests.
To make a sync request, the client uses
Easyobject. First it is created, and as a part of initialization it calls theset_attributesmethod which will in turn callheaders=, but only if headers are provided. The client can also set the headers directly usingheaders=method. The problem is that as soon as headers are set they are converted to FFI pointer, that's why I store their original version. And we don't have a span yet to inject the tracing headers.http_requestmethod is used as a helper to populate theeasywith necessary information for HTTP request. In particular, HTTP method is passed (for example,easy.http_request("www.example.com", :post, { params: { a: 1 }, body: { b: 2 } })). The problem is that this data is not preserved oneasyin a way that can be used to recover the HTTP method easily. The factories set various low-level attributes (https://github.com/typhoeus/ethon/tree/master/lib/ethon/easy/http), andlibcurlitself figures out HTTP method based on all these attributes. So I figured it is easier to just store the method instead of trying to recover it fromlibcurlattributes.The sync way to execute an
easyis callingperform. I patch theperformto create the span and to inject the headers if needed (that's why I stored the original headers on the object, to avoid reading them back from FFI).performuseslibcurl'seasy_performmethod and then calls thecompletemethod oneasythat I patch to finish the span.Async requests are executed using
Multi. Individual requests are represented by the sameEasyobjects, however the execution flow is different.Easyinstance is added to theMultiusingaddmethod. I consider this to be the beginning of the request execution, that's why the span is created there. There is no easy way to know if it is the beginning of the request or ifmultiis still on hold becausemultiexecution can be in progress in the other thread. I follow here the wayTyphoeususesMulti: it adds the easy objects toMultiright before executing theperform. It also has hooks for before-request event which are executed in theaddcall.The execution of
multihappens when theperformmethod is called. This method useslibcurl'smulti_performand callscompleteoneasyobjects which finished executing.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.
Okay this is pretty interesting, and exactly the kind of explanation I was looking for. (Thank you!)
I can see why you'd want to store this info before it gets turned into a format that's hard to read from; as long these objects are 1-1 with requests, then this seems okay. Just have to be careful in any scenario in which an
Easyobject is re-used or re-ran? Not sure if that happens, but some food for thought.If
Multiuses multipleEasyobjects, would it make sense to add an additional parent span to the multi operation? Thinking about the case if users want to see the batch as an operation, in addition to its constituent parts.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.
That's a good point. I added extra safety net by patching the
resetmethod which is used before re-using easy instance. This way I can be sure that HTTP method or headers are not mistakenly passed in the next request.