From 2b523b5f4f11145850bf2544e4d035eae4c6cd97 Mon Sep 17 00:00:00 2001 From: Tim Johnson Date: Thu, 14 Dec 2017 16:58:25 -0500 Subject: [PATCH 1/2] Set host or address as resource for Net::HTTP Tracking external requests by host is more useful for understanding performance issues than tracking by HTTP method. By using the host we should have a smaller potential list than if we were tracking by URL/endpoint. We don't always have a hostname, so lets use logic that exists for falling back to address. --- lib/ddtrace/contrib/http/patcher.rb | 21 +++++++++++---------- test/contrib/http/miniapp_test.rb | 2 +- test/contrib/http/request_test.rb | 8 ++++---- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/lib/ddtrace/contrib/http/patcher.rb b/lib/ddtrace/contrib/http/patcher.rb index aa176c93595..3c8cfc59b12 100644 --- a/lib/ddtrace/contrib/http/patcher.rb +++ b/lib/ddtrace/contrib/http/patcher.rb @@ -108,12 +108,18 @@ def request(req, body = nil, &block) # :yield: +response+ pin.tracer.trace(NAME) do |span| begin + if req.respond_to?(:uri) && req.uri + host_address = req.uri.host + host_port = req.uri.port.to_s + else + host_address = @address + host_port = @port.to_s + end + span.service = pin.service span.span_type = Datadog::Ext::HTTP::TYPE - span.resource = req.method - # Using the method as a resource, as URL/path can trigger - # a possibly infinite number of resources. + span.resource = host_address span.set_tag(Datadog::Ext::HTTP::URL, req.path) span.set_tag(Datadog::Ext::HTTP::METHOD, req.method) @@ -133,13 +139,8 @@ def request(req, body = nil, &block) # :yield: +response+ response = request_without_datadog(req, body, &block) end span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, response.code) - if req.respond_to?(:uri) && req.uri - span.set_tag(Datadog::Ext::NET::TARGET_HOST, req.uri.host) - span.set_tag(Datadog::Ext::NET::TARGET_PORT, req.uri.port.to_s) - else - span.set_tag(Datadog::Ext::NET::TARGET_HOST, @address) - span.set_tag(Datadog::Ext::NET::TARGET_PORT, @port.to_s) - end + span.set_tag(Datadog::Ext::NET::TARGET_HOST, host_address) + span.set_tag(Datadog::Ext::NET::TARGET_PORT, host_port) case response.code.to_i / 100 when 4 diff --git a/test/contrib/http/miniapp_test.rb b/test/contrib/http/miniapp_test.rb index a696861468e..eb65b405e43 100644 --- a/test/contrib/http/miniapp_test.rb +++ b/test/contrib/http/miniapp_test.rb @@ -33,7 +33,7 @@ def check_span_page(span) def check_span_get(span, parent_id, trace_id) assert_equal('http.request', span.name) assert_equal('net/http', span.service) - assert_equal('GET', span.resource) + assert_equal(ELASTICSEARCH_HOST, span.resource) assert_equal('_cluster/health', span.get_tag('http.url')) assert_equal('GET', span.get_tag('http.method')) assert_equal('200', span.get_tag('http.status_code')) diff --git a/test/contrib/http/request_test.rb b/test/contrib/http/request_test.rb index 4cd56bdfcf8..ae63c92ffc4 100644 --- a/test/contrib/http/request_test.rb +++ b/test/contrib/http/request_test.rb @@ -36,7 +36,7 @@ def test_get_request span = spans[0] assert_equal('http.request', span.name) assert_equal('net/http', span.service) - assert_equal('GET', span.resource) + assert_equal(ELASTICSEARCH_HOST, span.resource) assert_equal('_cluster/health', span.get_tag('http.url')) assert_equal('GET', span.get_tag('http.method')) assert_equal('200', span.get_tag('http.status_code')) @@ -52,7 +52,7 @@ def test_post_request span = spans[0] assert_equal('http.request', span.name) assert_equal('net/http', span.service) - assert_equal('POST', span.resource) + assert_equal(ELASTICSEARCH_HOST, span.resource) assert_equal('/my/thing/42', span.get_tag('http.url')) assert_equal('POST', span.get_tag('http.method')) assert_equal('127.0.0.1', span.get_tag('out.host')) @@ -68,7 +68,7 @@ def test_404 span = spans[0] assert_equal('http.request', span.name) assert_equal('net/http', span.service) - assert_equal('GET', span.resource) + assert_equal(ELASTICSEARCH_HOST, span.resource) assert_equal('/admin.php?user=admin&passwd=123456', span.get_tag('http.url')) assert_equal('GET', span.get_tag('http.method')) assert_equal('404', span.get_tag('http.status_code')) @@ -93,7 +93,7 @@ def test_pin_block_call span = spans[0] assert_equal('http.request', span.name) assert_equal('net/http', span.service) - assert_equal('GET', span.resource) + assert_equal(ELASTICSEARCH_HOST, span.resource) assert_equal('/_cluster/health', span.get_tag('http.url')) assert_equal('GET', span.get_tag('http.method')) assert_equal('200', span.get_tag('http.status_code')) From 95f813ec213ae63b1f9befb2b358757b8e937bc8 Mon Sep 17 00:00:00 2001 From: Tim Johnson Date: Mon, 19 Mar 2018 17:41:14 -0400 Subject: [PATCH 2/2] Include Hostname and Port into net/http resource --- lib/ddtrace/contrib/http/patcher.rb | 2 +- test/contrib/http/miniapp_test.rb | 2 +- test/contrib/http/request_test.rb | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/ddtrace/contrib/http/patcher.rb b/lib/ddtrace/contrib/http/patcher.rb index 3c8cfc59b12..2a5c2910e3e 100644 --- a/lib/ddtrace/contrib/http/patcher.rb +++ b/lib/ddtrace/contrib/http/patcher.rb @@ -119,7 +119,7 @@ def request(req, body = nil, &block) # :yield: +response+ span.service = pin.service span.span_type = Datadog::Ext::HTTP::TYPE - span.resource = host_address + span.resource = "#{host_address}:#{host_port}" span.set_tag(Datadog::Ext::HTTP::URL, req.path) span.set_tag(Datadog::Ext::HTTP::METHOD, req.method) diff --git a/test/contrib/http/miniapp_test.rb b/test/contrib/http/miniapp_test.rb index eb65b405e43..effa0815501 100644 --- a/test/contrib/http/miniapp_test.rb +++ b/test/contrib/http/miniapp_test.rb @@ -33,7 +33,7 @@ def check_span_page(span) def check_span_get(span, parent_id, trace_id) assert_equal('http.request', span.name) assert_equal('net/http', span.service) - assert_equal(ELASTICSEARCH_HOST, span.resource) + assert_equal("#{ELASTICSEARCH_HOST}:#{ELASTICSEARCH_PORT}", span.resource) assert_equal('_cluster/health', span.get_tag('http.url')) assert_equal('GET', span.get_tag('http.method')) assert_equal('200', span.get_tag('http.status_code')) diff --git a/test/contrib/http/request_test.rb b/test/contrib/http/request_test.rb index ae63c92ffc4..ab542f6b440 100644 --- a/test/contrib/http/request_test.rb +++ b/test/contrib/http/request_test.rb @@ -36,7 +36,7 @@ def test_get_request span = spans[0] assert_equal('http.request', span.name) assert_equal('net/http', span.service) - assert_equal(ELASTICSEARCH_HOST, span.resource) + assert_equal("#{ELASTICSEARCH_HOST}:#{ELASTICSEARCH_PORT}", span.resource) assert_equal('_cluster/health', span.get_tag('http.url')) assert_equal('GET', span.get_tag('http.method')) assert_equal('200', span.get_tag('http.status_code')) @@ -52,7 +52,7 @@ def test_post_request span = spans[0] assert_equal('http.request', span.name) assert_equal('net/http', span.service) - assert_equal(ELASTICSEARCH_HOST, span.resource) + assert_equal("#{ELASTICSEARCH_HOST}:#{ELASTICSEARCH_PORT}", span.resource) assert_equal('/my/thing/42', span.get_tag('http.url')) assert_equal('POST', span.get_tag('http.method')) assert_equal('127.0.0.1', span.get_tag('out.host')) @@ -68,7 +68,7 @@ def test_404 span = spans[0] assert_equal('http.request', span.name) assert_equal('net/http', span.service) - assert_equal(ELASTICSEARCH_HOST, span.resource) + assert_equal("#{ELASTICSEARCH_HOST}:#{ELASTICSEARCH_PORT}", span.resource) assert_equal('/admin.php?user=admin&passwd=123456', span.get_tag('http.url')) assert_equal('GET', span.get_tag('http.method')) assert_equal('404', span.get_tag('http.status_code')) @@ -93,7 +93,7 @@ def test_pin_block_call span = spans[0] assert_equal('http.request', span.name) assert_equal('net/http', span.service) - assert_equal(ELASTICSEARCH_HOST, span.resource) + assert_equal("#{ELASTICSEARCH_HOST}:#{ELASTICSEARCH_PORT}", span.resource) assert_equal('/_cluster/health', span.get_tag('http.url')) assert_equal('GET', span.get_tag('http.method')) assert_equal('200', span.get_tag('http.status_code'))