Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ module Ethon
end
end

require_relative 'ethon/http_helper'
require_relative 'ethon/instrumentation'
require_relative 'ethon/version'
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# frozen_string_literal: true

# Copyright The OpenTelemetry Authors
#
# SPDX-License-Identifier: Apache-2.0

module OpenTelemetry
module Instrumentation
module Ethon
# Helper module for HTTP method normalization
# @api private
module HttpHelper
# Lightweight struct to hold span creation attributes
SpanCreationAttributes = Struct.new(:span_name, :normalized_method, :original_method, keyword_init: true)

# Pre-computed mapping to avoid string allocations during normalization
METHOD_CACHE = {
'CONNECT' => 'CONNECT',
'DELETE' => 'DELETE',
'GET' => 'GET',
'HEAD' => 'HEAD',
'OPTIONS' => 'OPTIONS',
'PATCH' => 'PATCH',
'POST' => 'POST',
'PUT' => 'PUT',
'TRACE' => 'TRACE',
'connect' => 'CONNECT',
'delete' => 'DELETE',
'get' => 'GET',
'head' => 'HEAD',
'options' => 'OPTIONS',
'patch' => 'PATCH',
'post' => 'POST',
'put' => 'PUT',
'trace' => 'TRACE',
:connect => 'CONNECT',
:delete => 'DELETE',
:get => 'GET',
:head => 'HEAD',
:options => 'OPTIONS',
:patch => 'PATCH',
:post => 'POST',
:put => 'PUT',
:trace => 'TRACE'
}.freeze

# Pre-computed span names for old semantic conventions to avoid allocations
OLD_SPAN_NAMES = {
'CONNECT' => 'HTTP CONNECT',
'DELETE' => 'HTTP DELETE',
'GET' => 'HTTP GET',
'HEAD' => 'HTTP HEAD',
'OPTIONS' => 'HTTP OPTIONS',
'PATCH' => 'HTTP PATCH',
'POST' => 'HTTP POST',
'PUT' => 'HTTP PUT',
'TRACE' => 'HTTP TRACE'
}.freeze

private_constant :METHOD_CACHE, :OLD_SPAN_NAMES

# Prepares all span data for the specified semantic convention in a single call
# @param method [String, Symbol] The HTTP method
# @param semconv [Symbol] The semantic convention to use (:stable or :old)
# @return [SpanCreationAttributes] struct containing span_name, normalized_method, and original_method
def self.span_attrs_for(method, semconv: :stable)
normalized = METHOD_CACHE[method]
if normalized
span_name = semconv == :old ? OLD_SPAN_NAMES[normalized] : normalized
Comment on lines +66 to +69
Copy link
Contributor

Choose a reason for hiding this comment

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

@arielvalentin - I'm concerned about changing the span names and attributes for the "old" instrumentation. It seems to break the contract for the OTEL_SEMCONV_STABILITY_OPT_IN var:

The default behavior (in the absence of one of these values) is to continue emitting whatever version of the old experimental HTTP and networking conventions the instrumentation was emitting previously.
(source)

What do you think about reverting the changes for the old modules and leaving the changes for dup and new?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok that makes sense. I can leave the span name alone in follow up PRs.

Please consider that the existing old implementation does not follow the correct conventions for span names and should not have a prefix of HTTP:

HTTP spans MUST follow the overall guidelines for span names. HTTP server span names SHOULD be {http.method} {http.route} if there is a (low-cardinality) http.route available. HTTP server span names SHOULD be {http.method} if there is no (low-cardinality) http.route available. HTTP client spans have no http.route attribute since client-side instrumentation is not generally aware of the "route", and therefore HTTP client spans SHOULD use {http.method}. Instrumentation MUST NOT default to using URI path as span name, but MAY provide hooks to allow custom logic to override the default span name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

may provide hooks to allow custom logic to override the default span name is also something new to me 🤔

SpanCreationAttributes.new(
span_name: span_name,
normalized_method: normalized,
original_method: nil
)
else
SpanCreationAttributes.new(
span_name: 'HTTP',
normalized_method: '_OTHER',
original_method: method.to_s
)
end
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,11 @@ module Patches
module Dup
# Ethon::Easy patch for instrumentation
module Easy
ACTION_NAMES_TO_HTTP_METHODS = Hash.new do |h, k|
# #to_s is required because user input could be symbol or string
h[k] = k.to_s.upcase
end
HTTP_METHODS_TO_SPAN_NAMES = Hash.new do |h, k|
h[k] = k.to_s
h[k] = 'HTTP' if k == '_OTHER'
end

# Constant for the HTTP status range
HTTP_STATUS_SUCCESS_RANGE = (100..399)

def http_request(url, action_name, options = {})
@otel_method = ACTION_NAMES_TO_HTTP_METHODS[action_name]
@otel_method = action_name
super
end

Expand Down Expand Up @@ -77,12 +68,11 @@ def reset
end

def otel_before_request
method = '_OTHER' # Could be GET or not HTTP at all
method = @otel_method if instance_variable_defined?(:@otel_method) && !@otel_method.nil?
span_data = HttpHelper.span_attrs_for(@otel_method)

@otel_span = tracer.start_span(
HTTP_METHODS_TO_SPAN_NAMES[method],
attributes: span_creation_attributes(method),
span_data.span_name,
attributes: span_creation_attributes(span_data),
kind: :client
)

Expand All @@ -99,12 +89,12 @@ def otel_span_started?

private

def span_creation_attributes(method)
http_method = (method == '_OTHER' ? 'N/A' : method)
def span_creation_attributes(span_data)
instrumentation_attrs = {
'http.method' => http_method,
'http.request.method' => method
'http.method' => span_data.normalized_method,
'http.request.method' => span_data.normalized_method
}
instrumentation_attrs['http.request.method_original'] = span_data.original_method if span_data.original_method

uri = _otel_cleanse_uri(url)
if uri
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,11 @@ module Patches
module Old
# Ethon::Easy patch for instrumentation
module Easy
ACTION_NAMES_TO_HTTP_METHODS = Hash.new do |h, k|
# #to_s is required because user input could be symbol or string
h[k] = k.to_s.upcase
end
HTTP_METHODS_TO_SPAN_NAMES = Hash.new { |h, k| h[k] = "HTTP #{k}" }

# Constant for the HTTP status range
HTTP_STATUS_SUCCESS_RANGE = (100..399)

def http_request(url, action_name, options = {})
@otel_method = ACTION_NAMES_TO_HTTP_METHODS[action_name]
@otel_method = action_name
super
end

Expand Down Expand Up @@ -73,12 +67,11 @@ def reset
end

def otel_before_request
method = 'N/A' # Could be GET or not HTTP at all
method = @otel_method if instance_variable_defined?(:@otel_method) && !@otel_method.nil?
span_data = HttpHelper.span_attrs_for(@otel_method, semconv: :old)

@otel_span = tracer.start_span(
HTTP_METHODS_TO_SPAN_NAMES[method],
attributes: span_creation_attributes(method),
span_data.span_name,
attributes: span_creation_attributes(span_data),
kind: :client
)

Expand All @@ -95,9 +88,9 @@ def otel_span_started?

private

def span_creation_attributes(method)
def span_creation_attributes(span_data)
instrumentation_attrs = {
'http.method' => method
'http.method' => span_data.normalized_method
}

uri = _otel_cleanse_uri(url)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,11 @@ module Patches
module Stable
# Ethon::Easy patch for instrumentation
module Easy
ACTION_NAMES_TO_HTTP_METHODS = Hash.new do |h, k|
# #to_s is required because user input could be symbol or string
h[k] = k.to_s.upcase
end
HTTP_METHODS_TO_SPAN_NAMES = Hash.new do |h, k|
h[k] = k.to_s
h[k] = 'HTTP' if k == '_OTHER'
end

# Constant for the HTTP status range
HTTP_STATUS_SUCCESS_RANGE = (100..399)

def http_request(url, action_name, options = {})
@otel_method = ACTION_NAMES_TO_HTTP_METHODS[action_name]
@otel_method = action_name
super
end

Expand Down Expand Up @@ -76,12 +67,11 @@ def reset
end

def otel_before_request
method = '_OTHER' # Could be GET or not HTTP at all
method = @otel_method if instance_variable_defined?(:@otel_method) && !@otel_method.nil?
span_data = HttpHelper.span_attrs_for(@otel_method)

@otel_span = tracer.start_span(
HTTP_METHODS_TO_SPAN_NAMES[method],
attributes: span_creation_attributes(method),
span_data.span_name,
attributes: span_creation_attributes(span_data),
kind: :client
)

Expand All @@ -98,10 +88,11 @@ def otel_span_started?

private

def span_creation_attributes(method)
def span_creation_attributes(span_data)
instrumentation_attrs = {
'http.request.method' => method
'http.request.method' => span_data.normalized_method
}
instrumentation_attrs['http.request.method_original'] = span_data.original_method if span_data.original_method

uri = _otel_cleanse_uri(url)
if uri
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
easy.perform

_(span.name).must_equal 'HTTP'
_(span.attributes['http.method']).must_equal 'N/A'
_(span.attributes['http.method']).must_equal '_OTHER'
_(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'
Expand All @@ -92,7 +92,7 @@
# NOTE: check the finished spans since we expect to have closed it
span = exporter.finished_spans.first
_(span.name).must_equal 'HTTP'
_(span.attributes['http.method']).must_equal 'N/A'
_(span.attributes['http.method']).must_equal '_OTHER'
_(span.attributes['http.status_code']).must_be_nil
_(span.attributes['http.url']).must_equal 'http://example.com/test'
_(span.attributes['http.request.method']).must_equal '_OTHER'
Expand Down Expand Up @@ -121,7 +121,7 @@ def stub_response(options)
it 'when response is successful' do
stub_response(response_code: 200) do
_(span.name).must_equal 'HTTP'
_(span.attributes['http.method']).must_equal 'N/A'
_(span.attributes['http.method']).must_equal '_OTHER'
_(span.attributes['http.request.method']).must_equal '_OTHER'
_(span.attributes['http.status_code']).must_equal 200
_(span.attributes['http.response.status_code']).must_equal 200
Expand All @@ -137,7 +137,7 @@ def stub_response(options)
it 'when response is not successful' do
stub_response(response_code: 500) do
_(span.name).must_equal 'HTTP'
_(span.attributes['http.method']).must_equal 'N/A'
_(span.attributes['http.method']).must_equal '_OTHER'
_(span.attributes['http.request.method']).must_equal '_OTHER'
_(span.attributes['http.status_code']).must_equal 500
_(span.attributes['http.response.status_code']).must_equal 500
Expand All @@ -153,7 +153,7 @@ def stub_response(options)
it 'when request times out' do
stub_response(response_code: 0, return_code: :operation_timedout) do
_(span.name).must_equal 'HTTP'
_(span.attributes['http.method']).must_equal 'N/A'
_(span.attributes['http.method']).must_equal '_OTHER'
_(span.attributes['http.request.method']).must_equal '_OTHER'
_(span.attributes['http.status_code']).must_be_nil
_(span.attributes['http.response.status_code']).must_be_nil
Expand Down Expand Up @@ -232,7 +232,7 @@ def stub_response(options)
end

it 'cleans up @otel_method' do
_(easy.instance_eval { @otel_method }).must_equal 'PUT'
_(easy.instance_eval { @otel_method }).must_equal :put

easy.reset

Expand All @@ -256,6 +256,32 @@ def stub_response(options)
end
end
end

describe 'with unknown HTTP method' do
def stub_response(options)
easy.stub(:mirror, Ethon::Easy::Mirror.new(options)) do
easy.otel_before_request
# NOTE: perform calls complete
easy.complete

yield
end
end

it 'normalizes unknown HTTP methods' do
easy.http_request('http://example.com/purge', :purge)

stub_response(response_code: 200) do
_(exporter.finished_spans.size).must_equal 1
_(span.name).must_equal 'HTTP'
_(span.attributes['http.method']).must_equal '_OTHER'
_(span.attributes['http.url']).must_equal 'http://example.com/purge'
_(span.attributes['http.request.method']).must_equal '_OTHER'
_(span.attributes['http.request.method_original']).must_equal 'purge'
_(span.attributes['url.full']).must_equal 'http://example.com/purge'
end
end
end
end

describe 'multi' do
Expand Down
Loading