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

[net/http] Allow overriding service name. #430

Merged
merged 8 commits into from
May 23, 2018
Merged
2 changes: 2 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,14 @@ task :ci do
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake spec:dalli'
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake spec:faraday'
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake spec:graphql'
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake spec:http'
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake spec:racecar'
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake spec:redis'
sh 'rvm $MRI_OLD_VERSIONS --verbose do appraisal contrib-old rake spec:active_record'
sh 'rvm $MRI_OLD_VERSIONS --verbose do appraisal contrib-old rake spec:active_support'
sh 'rvm $MRI_OLD_VERSIONS --verbose do appraisal contrib-old rake spec:dalli'
sh 'rvm $MRI_OLD_VERSIONS --verbose do appraisal contrib-old rake spec:faraday'
sh 'rvm $MRI_OLD_VERSIONS --verbose do appraisal contrib-old rake spec:http'
sh 'rvm $MRI_OLD_VERSIONS --verbose do appraisal contrib-old rake spec:redis'
when 2
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake test:sidekiq'
Expand Down
4 changes: 3 additions & 1 deletion docs/GettingStarted.md
Original file line number Diff line number Diff line change
Expand Up @@ -514,8 +514,10 @@ Where `options` is an optional `Hash` that accepts the following parameters:

| Key | Description | Default |
| --- | --- | --- |
| ``service_name`` | Service name used for `http` instrumentation | http |
| ``service_name`` | Service name used for `http` instrumentation | net/http |
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm glad you're making this correction in the documentation, but this coincidentally highlights maybe a poor name here (which includes a /.) Probably outside the scope of this PR, but we should probably revisit what this default name is in the future.

| ``distributed_tracing`` | Enables distributed tracing | ``false`` |
| ``tracer`` | A ``Datadog::Tracer`` instance used to instrument the application. Usually you don't need to set that. | ``Datadog.tracer`` |


If you wish to configure each connection object individually, you may use the ``Datadog.configure`` as it follows:

Expand Down
34 changes: 18 additions & 16 deletions lib/ddtrace/contrib/http/patcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ 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

Expand All @@ -64,7 +66,7 @@ def patch
require 'ddtrace/ext/net'
require 'ddtrace/ext/distributed'

patch_http()
patch_http

@patched = true
rescue StandardError => e
Expand All @@ -74,7 +76,7 @@ def patch
@patched
end

# patched? tells wether patch has been successfully applied
# patched? tells whether patch has been successfully applied
def patched?
@patched
end
Expand All @@ -84,27 +86,27 @@ def patched?
# rubocop:disable Metrics/AbcSize
def patch_http
::Net::HTTP.class_eval do
alias_method :initialize_without_datadog, :initialize
Datadog::Patcher.without_warnings do
remove_method :initialize
end

def initialize(*args)
pin = Datadog::Pin.new(SERVICE, app: APP, app_type: Datadog::Ext::AppTypes::WEB)
pin.onto(self)
initialize_without_datadog(*args)
end

alias_method :request_without_datadog, :request
remove_method :request

def datadog_pin
@datadog_pindatadog_pin ||= begin
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)
end
end

def request(req, body = nil, &block) # :yield: +response+
pin = Datadog::Pin.get_from(self)
pin = datadog_pin
return request_without_datadog(req, body, &block) unless pin && pin.tracer

transport = pin.tracer.writer.transport
return request_without_datadog(req, body, &block) if
Datadog::Contrib::HTTP.should_skip_tracing?(req, @address, @port, transport, pin)

if Datadog::Contrib::HTTP.should_skip_tracing?(req, @address, @port, transport, pin)
return request_without_datadog(req, body, &block)
end

pin.tracer.trace(NAME) do |span|
begin
Expand Down
48 changes: 48 additions & 0 deletions spec/ddtrace/contrib/http/patcher_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
require 'spec_helper'
require 'ddtrace'
require 'net/http'

RSpec.describe 'net/http patcher' do
let(:tracer) { ::Datadog::Tracer.new(writer: FauxWriter.new) }
let(:host) { 'example.com' }

before do
WebMock.disable_net_connect!(allow_localhost: true)
WebMock.enable!

stub_request(:any, host)

Datadog.registry[: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 }
end

describe 'with default configuration' do
it 'uses default service name' do
Net::HTTP.get(host, '/')

expect(request_span.service).to eq('net/http')
end
end

describe 'with changed service name' do
let(:new_service_name) { 'new_service_name' }

before do
Datadog.configure do |c|
c.use :http, tracer: tracer, service_name: new_service_name
end
end

it 'uses new service name' do
Net::HTTP.get(host, '/')

expect(request_span.service).to eq(new_service_name)
end
end
end