From 6d0da5a34d2ea1b331787e017d5384434ebbfc50 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Fri, 13 Sep 2024 11:34:35 -0700 Subject: [PATCH 1/3] feat: add sem_conv opt in --- .../instrumentation/http/instrumentation.rb | 3 ++ .../instrumentation/http/patches/client.rb | 28 +++++++---- .../http/patches/connection.rb | 13 ++++-- ...opentelemetry-instrumentation-http.gemspec | 1 + .../http/patches/client_test.rb | 46 ++++++++++++++++++- 5 files changed, 76 insertions(+), 15 deletions(-) diff --git a/instrumentation/http/lib/opentelemetry/instrumentation/http/instrumentation.rb b/instrumentation/http/lib/opentelemetry/instrumentation/http/instrumentation.rb index dfe3951bf..cc92c9cae 100644 --- a/instrumentation/http/lib/opentelemetry/instrumentation/http/instrumentation.rb +++ b/instrumentation/http/lib/opentelemetry/instrumentation/http/instrumentation.rb @@ -9,9 +9,12 @@ module Instrumentation module HTTP # The Instrumentation class contains logic to detect and install the Http instrumentation class Instrumentation < OpenTelemetry::Instrumentation::Base + attr_reader :sem_conv + install do |_config| require_dependencies patch + @sem_conv = OpenTelemetry::SemanticConventions::StabilityMode.new end present do diff --git a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb index f814e1a65..351518f22 100644 --- a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb +++ b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb @@ -15,14 +15,15 @@ def perform(req, options) request_method = req.verb.to_s.upcase span_name = create_request_span_name(request_method, uri.path) - attributes = { - 'http.method' => request_method, - 'http.scheme' => uri.scheme, - 'http.target' => uri.path, - 'http.url' => "#{uri.scheme}://#{uri.host}", - 'net.peer.name' => uri.host, - 'net.peer.port' => uri.port - }.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes) + attributes = {} + sem_conv.set_http_method(attributes, request_method) + sem_conv.set_http_scheme(attributes, uri.scheme) + sem_conv.set_http_target(attributes, uri.path, uri.query) + sem_conv.set_http_url(attributes, "#{uri.scheme}://#{uri.host}") + sem_conv.set_http_net_peer_name_client(attributes, uri.host) + sem_conv.set_http_peer_port_client(attributes, uri.port) + + attributes.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes) tracer.in_span(span_name, attributes: attributes, kind: :client) do |span| OpenTelemetry.propagation.inject(req.headers) @@ -42,8 +43,11 @@ def annotate_span_with_response!(span, response) return unless response&.status status_code = response.status.to_i - span.set_attribute('http.status_code', status_code) - span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(status_code.to_i) + sem_conv_status_code = {} + sem_conv.set_http_status_code(sem_conv_status_code, status_code) + span.add_attributes(sem_conv_status_code) + + span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(status_code) end def create_request_span_name(request_method, request_path) @@ -60,6 +64,10 @@ def create_request_span_name(request_method, request_path) def tracer HTTP::Instrumentation.instance.tracer end + + def sem_conv + HTTP::Instrumentation.instance.sem_conv + end end end end diff --git a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/connection.rb b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/connection.rb index 6e162886f..e5b3e66d8 100644 --- a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/connection.rb +++ b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/connection.rb @@ -11,10 +11,11 @@ module Patches # Module to prepend to HTTP::Connection for instrumentation module Connection def initialize(req, options) - attributes = { - 'net.peer.name' => req.uri.host, - 'net.peer.port' => req.uri.port - }.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes) + attributes = {} + + sem_conv.set_http_net_peer_name_client(attributes, req.uri.host) + sem_conv.set_http_peer_port_client(attributes, req.uri.port) + attributes.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes) tracer.in_span('HTTP CONNECT', attributes: attributes) do super @@ -26,6 +27,10 @@ def initialize(req, options) def tracer HTTP::Instrumentation.instance.tracer end + + def sem_conv + HTTP::Instrumentation.instance.sem_conv + end end end end diff --git a/instrumentation/http/opentelemetry-instrumentation-http.gemspec b/instrumentation/http/opentelemetry-instrumentation-http.gemspec index f7ceae25c..fed66c274 100644 --- a/instrumentation/http/opentelemetry-instrumentation-http.gemspec +++ b/instrumentation/http/opentelemetry-instrumentation-http.gemspec @@ -27,6 +27,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'opentelemetry-api', '~> 1.0' spec.add_dependency 'opentelemetry-instrumentation-base', '~> 0.22.1' + spec.add_dependency 'opentelemetry-semantic_conventions', '>= 1.8.0' spec.add_development_dependency 'appraisal', '~> 2.5' spec.add_development_dependency 'bundler', '~> 2.4' diff --git a/instrumentation/http/test/instrumentation/http/patches/client_test.rb b/instrumentation/http/test/instrumentation/http/patches/client_test.rb index 8d62a5b07..daaeb6540 100644 --- a/instrumentation/http/test/instrumentation/http/patches/client_test.rb +++ b/instrumentation/http/test/instrumentation/http/patches/client_test.rb @@ -20,7 +20,7 @@ end let(:span_name_formatter) { nil } - before do + def reset exporter.reset @orig_propagation = OpenTelemetry.propagation propagator = OpenTelemetry::Trace::Propagation::TraceContext.text_map_propagator @@ -33,6 +33,10 @@ stub_request(:get, 'https://example.com/timeout').to_timeout end + before do + reset + end + after do # Force re-install of instrumentation instrumentation.instance_variable_set(:@installed, false) @@ -182,5 +186,45 @@ ) end end + + describe 'Semantic conventions http stability' do + it 'reports stable http attributes when OTEL_SEMCONV_STABILITY_OPT_IN = `http`' do + OpenTelemetry::TestHelpers.with_env('OTEL_SEMCONV_STABILITY_OPT_IN' => 'http') do + reset + HTTP.get('http://example.com/success') + + _(exporter.finished_spans.size).must_equal(1) + _(span.name).must_equal 'HTTP GET' + _(span.attributes['http.request.method']).must_equal 'GET' + _(span.attributes['url.scheme']).must_equal 'http' + _(span.attributes['http.response.status_code']).must_equal 200 + _(span.attributes['url.path']).must_equal '/success' + _(span.attributes['server.address']).must_equal 'example.com' + _(span.attributes['server.port']).must_equal 80 + end + end + + it 'reports stable http attributes when OTEL_SEMCONV_STABILITY_OPT_IN = `http/dup`' do + OpenTelemetry::TestHelpers.with_env('OTEL_SEMCONV_STABILITY_OPT_IN' => 'http/dup') do + reset + HTTP.get('http://example.com/success') + + _(exporter.finished_spans.size).must_equal(1) + _(span.name).must_equal 'HTTP GET' + _(span.attributes['http.method']).must_equal 'GET' + _(span.attributes['http.request.method']).must_equal 'GET' + _(span.attributes['http.scheme']).must_equal 'http' + _(span.attributes['url.scheme']).must_equal 'http' + _(span.attributes['http.status_code']).must_equal 200 + _(span.attributes['http.response.status_code']).must_equal 200 + _(span.attributes['http.target']).must_equal '/success' + _(span.attributes['url.path']).must_equal '/success' + _(span.attributes['net.peer.name']).must_equal 'example.com' + _(span.attributes['server.address']).must_equal 'example.com' + _(span.attributes['net.peer.port']).must_equal 80 + _(span.attributes['server.port']).must_equal 80 + end + end + end end end From 8f28f640b115222d0313fc48391fb44b86e6b51b Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Mon, 16 Sep 2024 09:59:24 -0700 Subject: [PATCH 2/3] Cleanup code with `tap` --- .../instrumentation/http/patches/client.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb index 351518f22..e3c351f5f 100644 --- a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb +++ b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb @@ -16,12 +16,14 @@ def perform(req, options) span_name = create_request_span_name(request_method, uri.path) attributes = {} - sem_conv.set_http_method(attributes, request_method) - sem_conv.set_http_scheme(attributes, uri.scheme) - sem_conv.set_http_target(attributes, uri.path, uri.query) - sem_conv.set_http_url(attributes, "#{uri.scheme}://#{uri.host}") - sem_conv.set_http_net_peer_name_client(attributes, uri.host) - sem_conv.set_http_peer_port_client(attributes, uri.port) + attributes.tap do |attrs| + sem_conv.set_http_method(attrs, request_method) + sem_conv.set_http_scheme(attrs, uri.scheme) + sem_conv.set_http_target(attrs, uri.path, uri.query) + sem_conv.set_http_url(attrs, "#{uri.scheme}://#{uri.host}") + sem_conv.set_http_net_peer_name_client(attrs, uri.host) + sem_conv.set_http_peer_port_client(attrs, uri.port) + end attributes.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes) From c6b45fbf978b06c718696dd53f5390d37aa9e226 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com> Date: Mon, 16 Sep 2024 16:07:03 -0700 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com> --- .../http/test/instrumentation/http/patches/client_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/http/test/instrumentation/http/patches/client_test.rb b/instrumentation/http/test/instrumentation/http/patches/client_test.rb index daaeb6540..0455fa6a7 100644 --- a/instrumentation/http/test/instrumentation/http/patches/client_test.rb +++ b/instrumentation/http/test/instrumentation/http/patches/client_test.rb @@ -204,7 +204,7 @@ def reset end end - it 'reports stable http attributes when OTEL_SEMCONV_STABILITY_OPT_IN = `http/dup`' do + it 'reports stable http attributes and old http attributes when OTEL_SEMCONV_STABILITY_OPT_IN = `http/dup`' do OpenTelemetry::TestHelpers.with_env('OTEL_SEMCONV_STABILITY_OPT_IN' => 'http/dup') do reset HTTP.get('http://example.com/success')