From 224f1ead210ff04eaef68013b3325bdf94f91bb3 Mon Sep 17 00:00:00 2001 From: Victor Date: Sat, 19 Oct 2024 04:25:28 +0100 Subject: [PATCH 1/4] Main Created a readable constant in 8 files to freeze ranges 100..399 . Relate to #1172 --- .../instrumentation/ethon/patches/easy.rb | 5 ++++- .../excon/middlewares/tracer_middleware.rb | 14 ++++---------- .../faraday/middlewares/tracer_middleware.rb | 7 +++++-- .../instrumentation/http/patches/client.rb | 5 ++++- .../instrumentation/http_client/patches/client.rb | 5 ++++- .../opentelemetry/instrumentation/httpx/plugin.rb | 5 ++++- .../net/http/patches/instrumentation.rb | 5 ++++- .../instrumentation/restclient/patches/request.rb | 8 +++++--- 8 files changed, 34 insertions(+), 20 deletions(-) diff --git a/instrumentation/ethon/lib/opentelemetry/instrumentation/ethon/patches/easy.rb b/instrumentation/ethon/lib/opentelemetry/instrumentation/ethon/patches/easy.rb index 74791818e..925699f4c 100644 --- a/instrumentation/ethon/lib/opentelemetry/instrumentation/ethon/patches/easy.rb +++ b/instrumentation/ethon/lib/opentelemetry/instrumentation/ethon/patches/easy.rb @@ -16,6 +16,9 @@ module Easy 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).freeze + def http_request(url, action_name, options = {}) @otel_method = ACTION_NAMES_TO_HTTP_METHODS[action_name] super @@ -42,7 +45,7 @@ def complete @otel_span.status = OpenTelemetry::Trace::Status.error("Request has failed: #{message}") else @otel_span.set_attribute('http.status_code', response_code) - @otel_span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(response_code.to_i) + @otel_span.status = OpenTelemetry::Trace::Status.error unless HTTP_STATUS_SUCCESS_RANGE.cover?(response_code.to_i) end ensure @otel_span&.finish diff --git a/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb b/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb index 592fdb793..e14e64f44 100644 --- a/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb +++ b/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true - # Copyright The OpenTelemetry Authors # # SPDX-License-Identifier: Apache-2.0 @@ -21,11 +20,13 @@ class TracerMiddleware < ::Excon::Middleware::Base hash[uppercase_method] ||= "HTTP #{uppercase_method}" end.freeze + # Constant for the HTTP status range + HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + def request_call(datum) return @stack.request_call(datum) if untraced?(datum) http_method = HTTP_METHODS_TO_UPPERCASE[datum[:method]] - attributes = { OpenTelemetry::SemanticConventions::Trace::HTTP_HOST => datum[:host], OpenTelemetry::SemanticConventions::Trace::HTTP_METHOD => http_method, @@ -35,19 +36,14 @@ def request_call(datum) OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => datum[:hostname], OpenTelemetry::SemanticConventions::Trace::NET_PEER_PORT => datum[:port] } - peer_service = Excon::Instrumentation.instance.config[:peer_service] attributes[OpenTelemetry::SemanticConventions::Trace::PEER_SERVICE] = peer_service if peer_service attributes.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes) - span = tracer.start_span(HTTP_METHODS_TO_SPAN_NAMES[http_method], attributes: attributes, kind: :client) ctx = OpenTelemetry::Trace.context_with_span(span) - datum[:otel_span] = span datum[:otel_token] = OpenTelemetry::Context.attach(ctx) - OpenTelemetry.propagation.inject(datum[:headers]) - @stack.request_call(datum) end @@ -68,7 +64,6 @@ def self.around_default_stack # If the default stack contains a version of the trace middleware already... existing_trace_middleware = default_stack.find { |m| m <= TracerMiddleware } default_stack.delete(existing_trace_middleware) if existing_trace_middleware - # Inject after the ResponseParser middleware response_middleware_index = default_stack.index(::Excon::Middleware::ResponseParser).to_i default_stack.insert(response_middleware_index + 1, self) @@ -84,7 +79,7 @@ def handle_response(datum) if datum.key?(:response) response = datum[:response] 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) + span.status = OpenTelemetry::Trace::Status.error unless HTTP_STATUS_SUCCESS_RANGE.cover?(response[:status].to_i) end if datum.key?(:error) @@ -93,7 +88,6 @@ def handle_response(datum) end span.finish - OpenTelemetry::Context.detach(datum.delete(:otel_token)) if datum.include?(:otel_token) end rescue StandardError => e diff --git a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb index 1b4452ac3..fea0ded50 100644 --- a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb +++ b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb @@ -9,7 +9,7 @@ module Instrumentation module Faraday module Middlewares # TracerMiddleware propagates context and instruments Faraday requests - # by way of its middlware system + # by way of its middleware system class TracerMiddleware < ::Faraday::Middleware HTTP_METHODS_SYMBOL_TO_STRING = { connect: 'CONNECT', @@ -23,6 +23,9 @@ class TracerMiddleware < ::Faraday::Middleware trace: 'TRACE' }.freeze + # Constant for the HTTP status range + HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + def call(env) http_method = HTTP_METHODS_SYMBOL_TO_STRING[env.method] config = Faraday::Instrumentation.instance.config @@ -68,7 +71,7 @@ def tracer def trace_response(span, status) span.set_attribute('http.status_code', status) - span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(status.to_i) + span.status = OpenTelemetry::Trace::Status.error unless HTTP_STATUS_SUCCESS_RANGE.cover?(status.to_i) end end end diff --git a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb index f814e1a65..e28ac3bcd 100644 --- a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb +++ b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb @@ -10,6 +10,9 @@ module HTTP module Patches # Module to prepend to HTTP::Client for instrumentation module Client + # Constant for the HTTP status range + HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + def perform(req, options) uri = req.uri request_method = req.verb.to_s.upcase @@ -43,7 +46,7 @@ def annotate_span_with_response!(span, response) 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) + span.status = OpenTelemetry::Trace::Status.error unless HTTP_STATUS_SUCCESS_RANGE.cover?(status_code) end def create_request_span_name(request_method, request_path) diff --git a/instrumentation/http_client/lib/opentelemetry/instrumentation/http_client/patches/client.rb b/instrumentation/http_client/lib/opentelemetry/instrumentation/http_client/patches/client.rb index a71c8ad7b..73ae32a17 100644 --- a/instrumentation/http_client/lib/opentelemetry/instrumentation/http_client/patches/client.rb +++ b/instrumentation/http_client/lib/opentelemetry/instrumentation/http_client/patches/client.rb @@ -10,6 +10,9 @@ module HttpClient module Patches # Module to prepend to HTTPClient for instrumentation module Client + # Constant for the HTTP status range + HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + private def do_get_block(req, proxy, conn, &block) @@ -42,7 +45,7 @@ def annotate_span_with_response!(span, response) status_code = response.status_code.to_i span.set_attribute('http.status_code', status_code) - span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(status_code.to_i) + span.status = OpenTelemetry::Trace::Status.error unless HTTP_STATUS_SUCCESS_RANGE.cover?(status_code) end def tracer diff --git a/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb b/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb index 137fca976..069ecb2dd 100644 --- a/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb +++ b/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb @@ -11,6 +11,9 @@ module Plugin # Instruments around HTTPX's request/response lifecycle in order to generate # an OTEL trace. class RequestTracer + # Constant for the HTTP status range + HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + def initialize(request) @request = request end @@ -54,7 +57,7 @@ def finish(response) @span.status = Trace::Status.error("Unhandled exception of type: #{response.error.class}") else @span.set_attribute(OpenTelemetry::SemanticConventions::Trace::HTTP_STATUS_CODE, response.status) - @span.status = Trace::Status.error unless (100..399).cover?(response.status) + @span.status = Trace::Status.error unless HTTP_STATUS_SUCCESS_RANGE.cover?(response.status) end OpenTelemetry::Context.detach(@trace_token) if @trace_token diff --git a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb index 725d4c546..b8b08f1c7 100644 --- a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb +++ b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb @@ -14,6 +14,9 @@ module Instrumentation HTTP_METHODS_TO_SPAN_NAMES = Hash.new { |h, k| h[k] = "HTTP #{k}" } USE_SSL_TO_SCHEME = { false => 'http', true => 'https' }.freeze + # Constant for the HTTP status range + HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + def request(req, body = nil, &block) # Do not trace recursive call for starting the connection return super unless started? @@ -78,7 +81,7 @@ def annotate_span_with_response!(span, response) status_code = response.code.to_i span.set_attribute(OpenTelemetry::SemanticConventions::Trace::HTTP_STATUS_CODE, status_code) - span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(status_code.to_i) + span.status = OpenTelemetry::Trace::Status.error unless HTTP_STATUS_SUCCESS_RANGE.cover?(status_code) end def tracer diff --git a/instrumentation/restclient/lib/opentelemetry/instrumentation/restclient/patches/request.rb b/instrumentation/restclient/lib/opentelemetry/instrumentation/restclient/patches/request.rb index afb2fdf11..acc3b39ea 100644 --- a/instrumentation/restclient/lib/opentelemetry/instrumentation/restclient/patches/request.rb +++ b/instrumentation/restclient/lib/opentelemetry/instrumentation/restclient/patches/request.rb @@ -10,6 +10,9 @@ module RestClient module Patches # Module to prepend to RestClient::Request for instrumentation module Request + # Constant for the HTTP status range + HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + def execute(&block) trace_request do |_span| super @@ -49,13 +52,12 @@ def trace_request # If so, add additional attributes. if response.is_a?(::RestClient::Response) span.set_attribute('http.status_code', response.code) - span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(response.code.to_i) + span.status = OpenTelemetry::Trace::Status.error unless HTTP_STATUS_SUCCESS_RANGE.cover?(response.code.to_i) end end rescue ::RestClient::ExceptionWithResponse => e span.set_attribute('http.status_code', e.http_code) - span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(e.http_code.to_i) - + span.status = OpenTelemetry::Trace::Status.error unless HTTP_STATUS_SUCCESS_RANGE.cover?(e.http_code.to_i) raise e ensure span.finish From 47c9ff0875cf2e5d92e98d487a0d4d918415742d Mon Sep 17 00:00:00 2001 From: Victor Date: Sat, 2 Nov 2024 01:26:22 +0100 Subject: [PATCH 2/4] refactor: Create a readable constant in 8 files to freeze ranges 100..399 . Reviewed-by: kaylareopelle Refs: #1172 --- docker-compose.yml | 2 -- .../httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index b77ad0e9d..a2fb5633f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,5 +1,3 @@ -version: "3.5" - x-shared-config: base: &base command: /bin/bash diff --git a/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb b/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb index 069ecb2dd..878b09520 100644 --- a/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb +++ b/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb @@ -92,4 +92,4 @@ def send(request) end end end -end +end \ No newline at end of file From 194e6f987e0eb163e56eddb2fa20eb65abbf0246 Mon Sep 17 00:00:00 2001 From: Victor Date: Sat, 9 Nov 2024 02:38:15 +0100 Subject: [PATCH 3/4] refactor: Fix rubocop ruby linter failures Reviewed-by: kaylareopelle Refs: #1172 --- .../opentelemetry/instrumentation/http_client/patches/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/http_client/lib/opentelemetry/instrumentation/http_client/patches/client.rb b/instrumentation/http_client/lib/opentelemetry/instrumentation/http_client/patches/client.rb index 73ae32a17..46ad72142 100644 --- a/instrumentation/http_client/lib/opentelemetry/instrumentation/http_client/patches/client.rb +++ b/instrumentation/http_client/lib/opentelemetry/instrumentation/http_client/patches/client.rb @@ -11,7 +11,7 @@ module Patches # Module to prepend to HTTPClient for instrumentation module Client # Constant for the HTTP status range - HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + HTTP_STATUS_SUCCESS_RANGE = (100..399) private From 6b07cb5d6bb7b5dc142021fe3134e34295ddf46c Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 17 Nov 2024 01:28:35 +0100 Subject: [PATCH 4/4] refactor: Fix rubocop ruby linter failures Reviewed-by: kaylareopelle Refs: #1172 --- .../lib/opentelemetry/instrumentation/ethon/patches/easy.rb | 2 +- .../instrumentation/excon/middlewares/tracer_middleware.rb | 3 ++- .../instrumentation/faraday/middlewares/tracer_middleware.rb | 2 +- .../lib/opentelemetry/instrumentation/http/patches/client.rb | 2 +- .../httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb | 4 ++-- .../instrumentation/net/http/patches/instrumentation.rb | 2 +- .../instrumentation/restclient/patches/request.rb | 2 +- 7 files changed, 9 insertions(+), 8 deletions(-) diff --git a/instrumentation/ethon/lib/opentelemetry/instrumentation/ethon/patches/easy.rb b/instrumentation/ethon/lib/opentelemetry/instrumentation/ethon/patches/easy.rb index 925699f4c..7850ac565 100644 --- a/instrumentation/ethon/lib/opentelemetry/instrumentation/ethon/patches/easy.rb +++ b/instrumentation/ethon/lib/opentelemetry/instrumentation/ethon/patches/easy.rb @@ -17,7 +17,7 @@ module Easy 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).freeze + HTTP_STATUS_SUCCESS_RANGE = (100..399) def http_request(url, action_name, options = {}) @otel_method = ACTION_NAMES_TO_HTTP_METHODS[action_name] diff --git a/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb b/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb index e14e64f44..b17bfc662 100644 --- a/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb +++ b/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + # Copyright The OpenTelemetry Authors # # SPDX-License-Identifier: Apache-2.0 @@ -21,7 +22,7 @@ class TracerMiddleware < ::Excon::Middleware::Base end.freeze # Constant for the HTTP status range - HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + HTTP_STATUS_SUCCESS_RANGE = (100..399) def request_call(datum) return @stack.request_call(datum) if untraced?(datum) diff --git a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb index fea0ded50..981e44e26 100644 --- a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb +++ b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb @@ -24,7 +24,7 @@ class TracerMiddleware < ::Faraday::Middleware }.freeze # Constant for the HTTP status range - HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + HTTP_STATUS_SUCCESS_RANGE = (100..399) def call(env) http_method = HTTP_METHODS_SYMBOL_TO_STRING[env.method] diff --git a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb index e28ac3bcd..1d98cb5c1 100644 --- a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb +++ b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb @@ -11,7 +11,7 @@ module Patches # Module to prepend to HTTP::Client for instrumentation module Client # Constant for the HTTP status range - HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + HTTP_STATUS_SUCCESS_RANGE = (100..399) def perform(req, options) uri = req.uri diff --git a/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb b/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb index 878b09520..2635a39e6 100644 --- a/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb +++ b/instrumentation/httpx/lib/opentelemetry/instrumentation/httpx/plugin.rb @@ -12,7 +12,7 @@ module Plugin # an OTEL trace. class RequestTracer # Constant for the HTTP status range - HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + HTTP_STATUS_SUCCESS_RANGE = (100..399) def initialize(request) @request = request @@ -92,4 +92,4 @@ def send(request) end end end -end \ No newline at end of file +end diff --git a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb index b8b08f1c7..6cdbfdb2b 100644 --- a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb +++ b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb @@ -15,7 +15,7 @@ module Instrumentation USE_SSL_TO_SCHEME = { false => 'http', true => 'https' }.freeze # Constant for the HTTP status range - HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + HTTP_STATUS_SUCCESS_RANGE = (100..399) def request(req, body = nil, &block) # Do not trace recursive call for starting the connection diff --git a/instrumentation/restclient/lib/opentelemetry/instrumentation/restclient/patches/request.rb b/instrumentation/restclient/lib/opentelemetry/instrumentation/restclient/patches/request.rb index acc3b39ea..1507948f7 100644 --- a/instrumentation/restclient/lib/opentelemetry/instrumentation/restclient/patches/request.rb +++ b/instrumentation/restclient/lib/opentelemetry/instrumentation/restclient/patches/request.rb @@ -11,7 +11,7 @@ module Patches # Module to prepend to RestClient::Request for instrumentation module Request # Constant for the HTTP status range - HTTP_STATUS_SUCCESS_RANGE = (100..399).freeze + HTTP_STATUS_SUCCESS_RANGE = (100..399) def execute(&block) trace_request do |_span|