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.

Sign up for GitHub

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

feat: Harmonize HTTP client instrumentation #1013

Closed
wants to merge 6 commits into from
Closed

feat: Harmonize HTTP client instrumentation #1013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
39221d1
Use constants when available
tt Jun 16, 2024
2ac3f83
Sort attributes alphabetically
tt Jun 16, 2024
a42aa74
Split to multiple lines
tt Jun 16, 2024
49d0d63
Verify port equivalence
tt Jun 16, 2024
ea385d7
Move test file to match library structure
tt Jun 16, 2024
6a6d46d
Adopt latest semantic conventions
tt Jun 16, 2024
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
16 changes: 11 additions & 5 deletions instrumentation/ethon/lib/opentelemetry/instrumentation/ethon/patches/easy.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ def complete
message = return_code ? ::Ethon::Curl.easy_strerror(return_code) : 'unknown reason'
@otel_span.status = OpenTelemetry::Trace::Status.error("Request has failed: #{message}")
else
@otel_span.set_attribute('http.status_code', response_code)
@otel_span.set_attribute('http.response.status_code', response_code)
@otel_span.set_attribute(OpenTelemetry::SemanticConventions::Trace::HTTP_STATUS_CODE, response_code)
@otel_span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(response_code.to_i)
end
ensure
Expand Down Expand Up @@ -84,17 +85,22 @@ def otel_span_started?

def span_creation_attributes(method)
instrumentation_attrs = {
'http.method' => method
OpenTelemetry::SemanticConventions::Trace::HTTP_METHOD => method
'http.request.method' => method
}

uri = _otel_cleanse_uri(url)
if uri
instrumentation_attrs['http.url'] = uri.to_s
instrumentation_attrs['net.peer.name'] = uri.host if uri.host
instrumentation_attrs[OpenTelemetry::SemanticConventions::Trace::HTTP_URL] = uri.to_s
instrumentation_attrs[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME] = uri.host if uri.host
instrumentation_attrs['server.address'] = uri.host if uri.host
instrumentation_attrs['server.port'] = uri.port if uri.port
instrumentation_attrs['url.full'] = uri.to_s
instrumentation_attrs['url.scheme'] = uri.scheme if uri.scheme
end

config = Ethon::Instrumentation.instance.config
instrumentation_attrs['peer.service'] = config[:peer_service] if config[:peer_service]
instrumentation_attrs[OpenTelemetry::SemanticConventions::Trace::PEER_SERVICE] = config[:peer_service] if config[:peer_service]
instrumentation_attrs.merge!(
OpenTelemetry::Common::HTTP::ClientContext.attributes
)
Expand Down
23 changes: 22 additions & 1 deletion instrumentation/ethon/test/opentelemetry/instrumentation/ethon/instrumentation_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,15 @@

_(span.name).must_equal 'HTTP N/A'
_(span.attributes['http.method']).must_equal 'N/A'
_(span.attributes['http.request.method']).must_equal 'N/A'
_(span.attributes['http.response.status_code']).must_be_nil
_(span.attributes['http.status_code']).must_be_nil
_(span.attributes['http.url']).must_equal 'http://example.com/test'
_(span.attributes['net.peer.name']).must_equal 'example.com'
_(span.attributes['server.address']).must_equal 'example.com'
_(span.attributes['server.port']).must_equal 80
_(span.attributes['url.full']).must_equal 'http://example.com/test'
_(span.attributes['url.scheme']).must_equal 'http'
end
end
end
Expand All @@ -92,8 +98,11 @@ def stub_response(options)
stub_response(response_code: 200) do
_(span.name).must_equal 'HTTP N/A'
_(span.attributes['http.method']).must_equal 'N/A'
_(span.attributes['http.request.method']).must_equal 'N/A'
_(span.attributes['http.response.status_code']).must_equal 200
_(span.attributes['http.status_code']).must_equal 200
_(span.attributes['http.url']).must_equal 'http://example.com/test'
_(span.attributes['url.full']).must_equal 'http://example.com/test'
_(easy.instance_eval { @otel_span }).must_be_nil
_(
easy.instance_eval { @otel_original_headers['traceparent'] }
Expand All @@ -105,8 +114,11 @@ def stub_response(options)
stub_response(response_code: 500) do
_(span.name).must_equal 'HTTP N/A'
_(span.attributes['http.method']).must_equal 'N/A'
_(span.attributes['http.request.method']).must_equal 'N/A'
_(span.attributes['http.response.status_code']).must_equal 500
_(span.attributes['http.status_code']).must_equal 500
_(span.attributes['http.url']).must_equal 'http://example.com/test'
_(span.attributes['url.full']).must_equal 'http://example.com/test'
_(easy.instance_eval { @otel_span }).must_be_nil
_(
easy.instance_eval { @otel_original_headers['traceparent'] }
Expand All @@ -118,8 +130,11 @@ def stub_response(options)
stub_response(response_code: 0, return_code: :operation_timedout) do
_(span.name).must_equal 'HTTP N/A'
_(span.attributes['http.method']).must_equal 'N/A'
_(span.attributes['http.request.method']).must_equal 'N/A'
_(span.attributes['http.response.status_code']).must_be_nil
_(span.attributes['http.status_code']).must_be_nil
_(span.attributes['http.url']).must_equal 'http://example.com/test'
_(span.attributes['url.full']).must_equal 'http://example.com/test'
_(span.status.code).must_equal(
OpenTelemetry::Trace::Status::ERROR
)
Expand All @@ -140,8 +155,10 @@ def stub_response(options)
OpenTelemetry::Common::HTTP::ClientContext.with_attributes(client_context_attrs) do
stub_response(response_code: 200) do
_(span.attributes['http.method']).must_equal 'OVERRIDE'
_(span.attributes['test.attribute']).must_equal 'test.value'
_(span.attributes['http.request.method']).must_equal 'N/A'
_(span.attributes['http.url']).must_equal 'http://example.com/test'
_(span.attributes['test.attribute']).must_equal 'test.value'
_(span.attributes['url.full']).must_equal 'http://example.com/test'
end
end
end
Expand Down Expand Up @@ -278,8 +295,12 @@ def stub_response(options)
_(exporter.finished_spans.size).must_equal 2
_(exporter.finished_spans[0].attributes['http.url']).must_be_nil
_(exporter.finished_spans[0].attributes['net.peer.name']).must_be_nil
_(exporter.finished_spans[0].attributes['server.address']).must_be_nil
_(exporter.finished_spans[0].attributes['url.full']).must_be_nil
_(exporter.finished_spans[1].attributes['http.url']).must_equal 'test'
_(exporter.finished_spans[1].attributes['net.peer.name']).must_be_nil
_(exporter.finished_spans[1].attributes['server.address']).must_be_nil
_(exporter.finished_spans[1].attributes['url.full']).must_equal 'test'
end
end
end
Expand Down
10 changes: 8 additions & 2 deletions ...umentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,17 @@ def request_call(datum)
http_method = HTTP_METHODS_TO_UPPERCASE[datum[:method]]

attributes = {
OpenTelemetry::SemanticConventions::Trace::HTTP_HOST => datum[:host],
OpenTelemetry::SemanticConventions::Trace::HTTP_METHOD => http_method,
'http.request.method' => http_method,
OpenTelemetry::SemanticConventions::Trace::HTTP_SCHEME => datum[:scheme],
OpenTelemetry::SemanticConventions::Trace::HTTP_TARGET => datum[:path],
OpenTelemetry::SemanticConventions::Trace::HTTP_HOST => datum[:host],
OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => datum[:hostname],
OpenTelemetry::SemanticConventions::Trace::NET_PEER_PORT => datum[:port]
OpenTelemetry::SemanticConventions::Trace::NET_PEER_PORT => datum[:port],
'server.address' => datum[:hostname],
'server.port' => datum[:port],
'url.full' => OpenTelemetry::Common::Utilities.cleanse_url(::Excon::Utils.request_uri(datum)),
'url.scheme' => datum[:scheme]
}

peer_service = Excon::Instrumentation.instance.config[:peer_service]
Expand Down Expand Up @@ -82,6 +87,7 @@ def handle_response(datum)

if datum.key?(:response)
response = datum[:response]
span.set_attribute('http.response.status_code', response[:status])
span.set_attribute(OpenTelemetry::SemanticConventions::Trace::HTTP_STATUS_CODE, response[:status])
span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(response[:status].to_i)
end
Expand Down
7 changes: 6 additions & 1 deletion instrumentation/excon/lib/opentelemetry/instrumentation/excon/patches/socket.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ def connect
conn_port = @port
end

attributes = { OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => conn_address, OpenTelemetry::SemanticConventions::Trace::NET_PEER_PORT => conn_port }.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes)
attributes = {
OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => conn_address,
OpenTelemetry::SemanticConventions::Trace::NET_PEER_PORT => conn_port,
'server.address' => conn_address,
'server.port' => conn_port
}.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes)

if is_a?(::Excon::SSLSocket) && @data[:proxy]
span_name = 'HTTP CONNECT'
Expand Down
71 changes: 59 additions & 12 deletions instrumentation/excon/test/opentelemetry/instrumentation/excon/instrumentation_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,17 @@

_(exporter.finished_spans.size).must_equal 1
_(span.name).must_equal 'HTTP GET'
_(span.attributes['http.host']).must_equal 'example.com'
_(span.attributes['http.method']).must_equal 'GET'
_(span.attributes['http.status_code']).must_equal 200
_(span.attributes['http.request.method']).must_equal 'GET'
_(span.attributes['http.response.status_code']).must_equal 200
_(span.attributes['http.scheme']).must_equal 'http'
_(span.attributes['http.host']).must_equal 'example.com'
_(span.attributes['http.status_code']).must_equal 200
_(span.attributes['http.target']).must_equal '/success'
_(span.attributes['server.address']).must_equal 'example.com'
_(span.attributes['server.port']).must_equal 80
_(span.attributes['url.full']).must_equal 'http://example.com/success'
_(span.attributes['url.scheme']).must_equal 'http'
assert_requested(
:get,
'http://example.com/success',
Expand All @@ -63,6 +69,7 @@
specify 'after request with capital-letters HTTP method' do
Excon.new('http://example.com/success').request(method: 'GET')

_(span.attributes['http.request.method']).must_equal 'GET'
_(span.attributes['http.method']).must_equal 'GET'
end

Expand All @@ -71,11 +78,17 @@

_(exporter.finished_spans.size).must_equal 1
_(span.name).must_equal 'HTTP GET'
_(span.attributes['http.host']).must_equal 'example.com'
_(span.attributes['http.method']).must_equal 'GET'
_(span.attributes['http.status_code']).must_equal 500
_(span.attributes['http.request.method']).must_equal 'GET'
_(span.attributes['http.response.status_code']).must_equal 500
_(span.attributes['http.scheme']).must_equal 'http'
_(span.attributes['http.host']).must_equal 'example.com'
_(span.attributes['http.status_code']).must_equal 500
_(span.attributes['http.target']).must_equal '/failure'
_(span.attributes['server.address']).must_equal 'example.com'
_(span.attributes['server.port']).must_equal 80
_(span.attributes['url.full']).must_equal 'http://example.com/failure'
_(span.attributes['url.scheme']).must_equal 'http'
assert_requested(
:get,
'http://example.com/failure',
Expand All @@ -90,10 +103,15 @@

_(exporter.finished_spans.size).must_equal 1
_(span.name).must_equal 'HTTP GET'
_(span.attributes['http.host']).must_equal 'example.com'
_(span.attributes['http.method']).must_equal 'GET'
_(span.attributes['http.request.method']).must_equal 'GET'
_(span.attributes['http.scheme']).must_equal 'http'
_(span.attributes['http.host']).must_equal 'example.com'
_(span.attributes['http.target']).must_equal '/timeout'
_(span.attributes['server.address']).must_equal 'example.com'
_(span.attributes['server.port']).must_equal 80
_(span.attributes['url.full']).must_equal 'http://example.com/timeout'
_(span.attributes['url.scheme']).must_equal 'http'
_(span.status.code).must_equal(
OpenTelemetry::Trace::Status::ERROR
)
Expand All @@ -119,12 +137,18 @@

_(exporter.finished_spans.size).must_equal 1
_(span.name).must_equal 'HTTP GET'
_(span.attributes['http.host']).must_equal 'example.com'
_(span.attributes['http.method']).must_equal 'OVERRIDE'
_(span.attributes['http.status_code']).must_equal 200
_(span.attributes['http.request.method']).must_equal 'GET'
_(span.attributes['http.response.status_code']).must_equal 200
_(span.attributes['http.scheme']).must_equal 'http'
_(span.attributes['http.host']).must_equal 'example.com'
_(span.attributes['http.status_code']).must_equal 200
_(span.attributes['http.target']).must_equal '/success'
_(span.attributes['server.address']).must_equal 'example.com'
_(span.attributes['server.port']).must_equal 80
_(span.attributes['test.attribute']).must_equal 'test.value'
_(span.attributes['url.full']).must_equal 'http://example.com/success'
_(span.attributes['url.scheme']).must_equal 'http'
assert_requested(
:get,
'http://example.com/success',
Expand Down Expand Up @@ -186,8 +210,11 @@

_(exporter.finished_spans.size).must_equal 1
_(span.name).must_equal 'HTTP GET'
_(span.attributes['http.method']).must_equal 'GET'
_(span.attributes['http.host']).must_equal 'example.com'
_(span.attributes['http.method']).must_equal 'GET'
_(span.attributes['http.request.method']).must_equal 'GET'
_(span.attributes['server.address']).must_equal 'example.com'
_(span.attributes['server.port']).must_equal 80
end

it 'creates a span on connect for a non-ignored request' do
Expand All @@ -200,6 +227,8 @@
_(span.kind).must_equal(:internal)
_(span.attributes['net.peer.name']).must_equal('example.com')
_(span.attributes['net.peer.port']).must_equal(80)
_(span.attributes['server.address']).must_equal('example.com')
_(span.attributes['server.port']).must_equal(80)
end
end

Expand All @@ -215,6 +244,8 @@
end

it 'emits span on connect' do
port = nil

TCPServer.open('localhost', 0) do |server|
Thread.start do
server.accept
Expand All @@ -231,8 +262,12 @@
_(span.name).must_equal 'connect'
_(span.attributes['net.peer.name']).must_equal('localhost')
_(span.attributes['net.peer.port']).wont_be_nil
_(span.attributes['net.peer.port']).must_equal(port)
_(span.attributes['server.address']).must_equal('localhost')
_(span.attributes['server.port']).wont_be_nil
_(span.attributes['server.port']).must_equal(port)

assert_http_spans(target: '/example', exception: 'Excon::Error::Timeout')
assert_http_spans(port: port, target: '/example', exception: 'Excon::Error::Timeout')
end

it 'captures errors' do
Expand All @@ -242,13 +277,15 @@
_(span.name).must_equal 'connect'
_(span.attributes['net.peer.name']).must_equal('invalid.com')
_(span.attributes['net.peer.port']).must_equal(99_999)
_(span.attributes['server.address']).must_equal('invalid.com')
_(span.attributes['server.port']).must_equal(99_999)

span_event = span.events.first
_(span_event.name).must_equal 'exception'
# Depending on the Ruby and Excon Version this will be a SocketError, Socket::ResolutionError or Resolv::ResolvError
_(span_event.attributes['exception.type']).must_match(/(Socket|Resolv)/)

assert_http_spans(host: 'invalid.com', target: '/example')
assert_http_spans(host: 'invalid.com', port: 99_999, target: '/example')
end

it '[BUG] fails to emit an HTTP CONNECT span when connecting through an SSL proxy for an HTTP service' do
Expand All @@ -259,6 +296,8 @@
_(span.kind).must_equal(:internal)
_(span.attributes['net.peer.name']).must_equal('localhost')
_(span.attributes['net.peer.port']).must_equal(443)
_(span.attributes['server.address']).must_equal('localhost')
_(span.attributes['server.port']).must_equal(443)

assert_http_spans
end
Expand All @@ -271,6 +310,8 @@
_(span.kind).must_equal(:client)
_(span.attributes['net.peer.name']).must_equal('localhost')
_(span.attributes['net.peer.port']).must_equal(443)
_(span.attributes['server.address']).must_equal('localhost')
_(span.attributes['server.port']).must_equal(443)

assert_http_spans(scheme: 'https')
end
Expand All @@ -283,6 +324,8 @@
_(span.kind).must_equal(:internal)
_(span.attributes['net.peer.name']).must_equal('localhost')
_(span.attributes['net.peer.port']).must_equal(443)
_(span.attributes['server.address']).must_equal('localhost')
_(span.attributes['server.port']).must_equal(443)

assert_http_spans(exception: 'Excon::Error::Socket')
end
Expand All @@ -296,13 +339,17 @@
end
end

def assert_http_spans(scheme: 'http', host: 'localhost', target: '/', exception: nil)
def assert_http_spans(scheme: 'http', host: 'localhost', port: nil, target: '/', exception: nil)
exporter.finished_spans[1..].each do |http_span|
_(http_span.name).must_equal 'HTTP GET'
_(http_span.attributes['http.host']).must_equal host
_(http_span.attributes['http.method']).must_equal 'GET'
_(http_span.attributes['http.request.method']).must_equal 'GET'
_(http_span.attributes['http.scheme']).must_equal scheme
_(http_span.attributes['http.host']).must_equal host
_(http_span.attributes['http.target']).must_equal target
_(http_span.attributes['server.address']).must_equal host
_(http_span.attributes['url.full']).must_equal "#{scheme}://#{host}#{port&.to_s&.prepend(':')}#{target}"
_(http_span.attributes['url.scheme']).must_equal scheme
_(http_span.status.code).must_equal(
OpenTelemetry::Trace::Status::ERROR
)
Expand Down
16 changes: 11 additions & 5 deletions ...tation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,17 @@ def call(env)

def span_creation_attributes(http_method:, url:, config:)
instrumentation_attrs = {
'http.method' => http_method,
'http.url' => OpenTelemetry::Common::Utilities.cleanse_url(url.to_s),
OpenTelemetry::SemanticConventions::Trace::HTTP_METHOD => http_method,
'http.request.method' => http_method,
OpenTelemetry::SemanticConventions::Trace::HTTP_URL => OpenTelemetry::Common::Utilities.cleanse_url(url.to_s),
'url.full' => OpenTelemetry::Common::Utilities.cleanse_url(url.to_s),
'url.scheme' => url.scheme,
'faraday.adapter.name' => app.class.name
}
instrumentation_attrs['net.peer.name'] = url.host if url.host
instrumentation_attrs['peer.service'] = config[:peer_service] if config[:peer_service]
instrumentation_attrs[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME] = url.host if url.host
instrumentation_attrs[OpenTelemetry::SemanticConventions::Trace::PEER_SERVICE] = config[:peer_service] if config[:peer_service]
instrumentation_attrs['server.address'] = url.host if url.host
instrumentation_attrs['server.port'] = url.port if url.port

instrumentation_attrs.merge!(
OpenTelemetry::Common::HTTP::ClientContext.attributes
Expand All @@ -67,7 +72,8 @@ def tracer
end

def trace_response(span, status)
span.set_attribute('http.status_code', status)
span.set_attribute('http.response.status_code', status)
span.set_attribute(OpenTelemetry::SemanticConventions::Trace::HTTP_STATUS_CODE, status)
span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(status.to_i)
end
end
Expand Down
Loading
Loading