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

Implement HTTP integration configuration #556

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
2 changes: 1 addition & 1 deletion lib/ddtrace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def configure(target = configuration, opts = {})
require 'ddtrace/contrib/grape/patcher'
require 'ddtrace/contrib/graphql/patcher'
require 'ddtrace/contrib/grpc/patcher'
require 'ddtrace/contrib/http/patcher'
require 'ddtrace/contrib/http/integration'
require 'ddtrace/contrib/integration'
require 'ddtrace/contrib/mongodb/patcher'
require 'ddtrace/contrib/mysql2/patcher'
Expand Down
39 changes: 39 additions & 0 deletions lib/ddtrace/contrib/http/circuit_breaker.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@

module Datadog
module Contrib
module HTTP
# HTTP integration circuit breaker behavior
# For avoiding recursive traces.
module CircuitBreaker
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be a bit confusing with CircuitBreaker pattern. But I don't have better suggestion :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a "circuit breaker" pattern? I was trying to think of something that encapsulated the idea of "if looping, escape", like a short-circuit where the circuit breaker trips to prevent an overload. But maybe a metaphor here isn't the best choice of naming scheme.

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI https://martinfowler.com/bliki/CircuitBreaker.html

on a high level it also breaks circuit, but its a bit more dynamic and serves a bit different purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right... a microservices related pattern. Forgot about this one.

def should_skip_tracing?(req, address, port, transport, pin)
# we don't want to trace our own call to the API (they use net/http)
# when we know the host & port (from the URI) we use it, else (most-likely
# called with a block) rely on the URL at the end.
if req.respond_to?(:uri) && req.uri
if req.uri.host.to_s == transport.hostname.to_s &&
req.uri.port.to_i == transport.port.to_i
return true
end
elsif address && port &&
address.to_s == transport.hostname.to_s &&
port.to_i == transport.port.to_i
return true
end
# we don't want a "shotgun" effect with two nested traces for one
# logical get, and request is likely to call itself recursively
active = pin.tracer.active_span
return true if active && (active.name == Ext::SPAN_REQUEST)
false
end

def should_skip_distributed_tracing?(pin)
if pin.config && pin.config.key?(:distributed_tracing)
return !pin.config[:distributed_tracing]
end

!Datadog.configuration[:http][:distributed_tracing]
end
end
end
end
end
17 changes: 17 additions & 0 deletions lib/ddtrace/contrib/http/configuration/settings.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
require 'ddtrace/contrib/configuration/settings'
require 'ddtrace/contrib/http/ext'

module Datadog
module Contrib
module HTTP
module Configuration
# Custom settings for the HTTP integration
class Settings < Contrib::Configuration::Settings
option :distributed_tracing, default: false
option :service_name, default: Ext::SERVICE_NAME
option :tracer, default: Datadog.tracer
end
end
end
end
end
13 changes: 13 additions & 0 deletions lib/ddtrace/contrib/http/ext.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module Datadog
module Contrib
module HTTP
# HTTP integration constants
module Ext
APP = 'net/http'.freeze
SERVICE_NAME = 'net/http'.freeze

SPAN_REQUEST = 'http.request'.freeze
end
end
end
end
32 changes: 32 additions & 0 deletions lib/ddtrace/contrib/http/integration.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
require 'ddtrace/contrib/integration'
require 'ddtrace/contrib/http/configuration/settings'
require 'ddtrace/contrib/http/patcher'
require 'ddtrace/contrib/http/circuit_breaker'

module Datadog
module Contrib
# HTTP integration
module HTTP
extend CircuitBreaker

# Description of HTTP integration
class Integration
include Contrib::Integration

register_as :http, auto_patch: true

def self.version
Gem::Version.new(RUBY_VERSION)
end

def default_configuration
Configuration::Settings.new
end

def patcher
Patcher
end
end
end
end
end
68 changes: 10 additions & 58 deletions lib/ddtrace/contrib/http/patcher.rb
Original file line number Diff line number Diff line change
@@ -1,63 +1,23 @@
# requirements should be kept minimal as Patcher is a shared requirement.
require 'ddtrace/contrib/patcher'
require 'ddtrace/contrib/http/ext'

module Datadog
module Contrib
# Datadog Net/HTTP integration.
module HTTP
URL = 'http.url'.freeze
METHOD = 'http.method'.freeze
BODY = 'http.body'.freeze

NAME = 'http.request'.freeze
APP = 'net/http'.freeze
SERVICE = 'net/http'.freeze

module_function

def should_skip_tracing?(req, address, port, transport, pin)
# we don't want to trace our own call to the API (they use net/http)
# when we know the host & port (from the URI) we use it, else (most-likely
# called with a block) rely on the URL at the end.
if req.respond_to?(:uri) && req.uri
if req.uri.host.to_s == transport.hostname.to_s &&
req.uri.port.to_i == transport.port.to_i
return true
end
elsif address && port &&
address.to_s == transport.hostname.to_s &&
port.to_i == transport.port.to_i
return true
end
# we don't want a "shotgun" effect with two nested traces for one
# logical get, and request is likely to call itself recursively
active = pin.tracer.active_span()
return true if active && (active.name == NAME)
false
end

def should_skip_distributed_tracing?(pin)
if pin.config && pin.config.key?(:distributed_tracing)
return !pin.config[:distributed_tracing]
end

!Datadog.configuration[:http][:distributed_tracing]
end

# Patcher enables patching of 'net/http' module.
module Patcher
include Base
register_as :http, auto_patch: true
option :distributed_tracing, default: false
option :service_name, default: SERVICE
option :tracer, default: Datadog.tracer

@patched = false
include Contrib::Patcher

module_function

def patched?
done?(:http)
end

# patch applies our patch if needed
def patch
unless @patched
do_once(:http) do
begin
require 'uri'
require 'ddtrace/pin'
Expand All @@ -67,18 +27,10 @@ def patch
require 'ddtrace/ext/distributed'

patch_http

@patched = true
rescue StandardError => e
Datadog::Tracer.log.error("Unable to apply net/http integration: #{e}")
end
end
@patched
end

# patched? tells whether patch has been successfully applied
def patched?
@patched
end

# rubocop:disable Metrics/MethodLength
Expand All @@ -94,7 +46,7 @@ def datadog_pin
service = Datadog.configuration[:http][:service_name]
tracer = Datadog.configuration[:http][:tracer]

Datadog::Pin.new(service, app: APP, app_type: Datadog::Ext::AppTypes::WEB, tracer: tracer)
Datadog::Pin.new(service, app: Ext::APP, app_type: Datadog::Ext::AppTypes::WEB, tracer: tracer)
end
end

Expand All @@ -108,7 +60,7 @@ def request(req, body = nil, &block) # :yield: +response+
return request_without_datadog(req, body, &block)
end

pin.tracer.trace(NAME) do |span|
pin.tracer.trace(Ext::SPAN_REQUEST) do |span|
begin
span.service = pin.service
span.span_type = Datadog::Ext::HTTP::TYPE
Expand Down
4 changes: 2 additions & 2 deletions spec/ddtrace/contrib/http/patcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@

stub_request(:any, host)

Datadog.registry[:http].reset_options!
Datadog.configuration[:http].reset_options!
Datadog.configure do |c|
c.use :http, tracer: tracer
end
end

let(:request_span) do
tracer.writer.spans(:keep).find { |span| span.name == Datadog::Contrib::HTTP::NAME }
tracer.writer.spans(:keep).find { |span| span.name == Datadog::Contrib::HTTP::Ext::SPAN_REQUEST }
end

describe 'with default configuration' do
Expand Down