From 62a310cadb83eb9553a86c1d85aa9c60e1f072a6 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Tue, 1 Aug 2023 14:58:13 +0200 Subject: [PATCH 1/5] fix(exporter/otlp): env var handling --- .../opentelemetry/exporter/otlp/exporter.rb | 44 +++++++++++++++---- .../exporter/otlp/exporter_test.rb | 29 ++++++++++++ 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb index a6296aa063..d81f1a4803 100644 --- a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb +++ b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb @@ -47,22 +47,16 @@ def self.ssl_verify_mode end end - def initialize(endpoint: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_ENDPOINT', 'OTEL_EXPORTER_OTLP_ENDPOINT', default: 'http://localhost:4318/v1/traces'), + def initialize(endpoint: nil, certificate_file: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE', 'OTEL_EXPORTER_OTLP_CERTIFICATE'), ssl_verify_mode: Exporter.ssl_verify_mode, headers: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_HEADERS', 'OTEL_EXPORTER_OTLP_HEADERS', default: {}), compression: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_COMPRESSION', 'OTEL_EXPORTER_OTLP_COMPRESSION', default: 'gzip'), timeout: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_TIMEOUT', 'OTEL_EXPORTER_OTLP_TIMEOUT', default: 10), metrics_reporter: nil) - raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" unless OpenTelemetry::Common::Utilities.valid_url?(endpoint) + @uri = determine_url(endpoint) raise ArgumentError, "unsupported compression key #{compression}" unless compression.nil? || %w[gzip none].include?(compression) - @uri = if endpoint == ENV['OTEL_EXPORTER_OTLP_ENDPOINT'] - URI.join(endpoint, 'v1/traces') - else - URI(endpoint) - end - @http = http_connection(@uri, ssl_verify_mode, certificate_file) @path = @uri.path @@ -386,6 +380,40 @@ def as_otlp_any_value(value) result end + # Returns the url used by the exporter based on the precedence defined in the specification: + # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#endpoint-urls-for-otlphttp + # + # @param [String, nil] endpoint + # @return [URI::Generic] + def determine_url(endpoint) + if !endpoint.nil? + # Use endpoint provided via argument as-is + parse_url_or_raise(endpoint) + elsif !ENV['OTEL_EXPORTER_OTLP_TRACES_ENDPOINT'].nil? + # Use signal-specific endpoint as-is + parse_url_or_raise(ENV['OTEL_EXPORTER_OTLP_TRACES_ENDPOINT']) + elsif !ENV['OTEL_EXPORTER_OTLP_ENDPOINT'].nil? + # Append v1/traces only if the non-signal specific env var is set. + non_signal_specific_endpoint = ENV['OTEL_EXPORTER_OTLP_ENDPOINT'] + begin + URI.join(non_signal_specific_endpoint, 'v1/traces') + rescue URI::InvalidURIError + raise ArgumentError, "invalid url for OTLP::Exporter #{non_signal_specific_endpoint}" + end + else + # use default in all other cases + parse_url_or_raise('http://localhost:4318/v1/traces') + end + end + + def parse_url_or_raise(endpoint) + raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" if endpoint.nil? || endpoint.strip.empty? + + URI(endpoint) + rescue URI::InvalidURIError + raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" + end + def prepare_headers(config_headers) headers = case config_headers when String then parse_headers(config_headers) diff --git a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb index affd79e78b..ac604d6541 100644 --- a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb +++ b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb @@ -139,6 +139,35 @@ _(exp.instance_variable_get(:@path)).must_equal '/v1/traces' end + it 'does not join endpoint with v1/traces if endpoint and OTEL_EXPORTER_OTLP_ENDPOINT are equal' do + exp = OpenTelemetry::TestHelpers.with_env( + 'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234/custom/path' + ) do + OpenTelemetry::Exporter::OTLP::Exporter.new(endpoint: 'https://localhost:1234/custom/path') + end + _(exp.instance_variable_get(:@path)).must_equal '/custom/path' + end + + it 'does not append v1/traces if OTEL_EXPORTER_OTLP_ENDPOINT and OTEL_EXPORTER_OTLP_TRACES_ENDPOINT both equal' do + exp = OpenTelemetry::TestHelpers.with_env( + 'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234/custom/path', + 'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT' => 'https://localhost:1234/custom/path' + ) do + OpenTelemetry::Exporter::OTLP::Exporter.new() + end + _(exp.instance_variable_get(:@path)).must_equal '/custom/path' + end + + it 'uses OTEL_EXPORTER_OTLP_TRACES_ENDPOINT over OTEL_EXPORTER_OTLP_ENDPOINT' do + exp = OpenTelemetry::TestHelpers.with_env( + 'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234/non/specific/custom/path', + 'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT' => 'https://localhost:1234/specific/custom/path' + ) do + OpenTelemetry::Exporter::OTLP::Exporter.new() + end + _(exp.instance_variable_get(:@path)).must_equal '/specific/custom/path' + end + it 'restricts explicit headers to a String or Hash' do exp = OpenTelemetry::Exporter::OTLP::Exporter.new(headers: { 'token' => 'über' }) _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über', 'User-Agent' => DEFAULT_USER_AGENT) From c262ced244d1ffe6f4b03c818703e12e410a8c56 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Tue, 1 Aug 2023 16:08:45 +0200 Subject: [PATCH 2/5] fix(exporter/otlp): handle case where non-signal-specific endpoint ends without '/' --- .../opentelemetry/exporter/otlp/exporter.rb | 2 ++ .../exporter/otlp/exporter_test.rb | 30 +++++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb index d81f1a4803..7ae82b8999 100644 --- a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb +++ b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb @@ -395,6 +395,8 @@ def determine_url(endpoint) elsif !ENV['OTEL_EXPORTER_OTLP_ENDPOINT'].nil? # Append v1/traces only if the non-signal specific env var is set. non_signal_specific_endpoint = ENV['OTEL_EXPORTER_OTLP_ENDPOINT'] + # append '/' if not present + non_signal_specific_endpoint += '/' unless non_signal_specific_endpoint.end_with?('/') begin URI.join(non_signal_specific_endpoint, 'v1/traces') rescue URI::InvalidURIError diff --git a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb index ac604d6541..ac2f62356b 100644 --- a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb +++ b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb @@ -125,7 +125,7 @@ exp = OpenTelemetry::TestHelpers.with_env( 'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234/' ) do - OpenTelemetry::Exporter::OTLP::Exporter.new() + OpenTelemetry::Exporter::OTLP::Exporter.new end _(exp.instance_variable_get(:@path)).must_equal '/v1/traces' end @@ -134,12 +134,22 @@ exp = OpenTelemetry::TestHelpers.with_env( 'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234' ) do - OpenTelemetry::Exporter::OTLP::Exporter.new() + OpenTelemetry::Exporter::OTLP::Exporter.new end _(exp.instance_variable_get(:@path)).must_equal '/v1/traces' end - it 'does not join endpoint with v1/traces if endpoint and OTEL_EXPORTER_OTLP_ENDPOINT are equal' do + it 'appends the correct path if OTEL_EXPORTER_OTLP_ENDPOINT does have a path without a trailing slash' do + exp = OpenTelemetry::TestHelpers.with_env( + # simulate OTLP endpoints built on top of an exiting API + 'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234/api/v2/otlp' + ) do + OpenTelemetry::Exporter::OTLP::Exporter.new + end + _(exp.instance_variable_get(:@path)).must_equal '/api/v2/otlp/v1/traces' + end + + it 'does not join endpoint with v1/traces if endpoint is set and is equal to OTEL_EXPORTER_OTLP_ENDPOINT' do exp = OpenTelemetry::TestHelpers.with_env( 'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234/custom/path' ) do @@ -153,7 +163,7 @@ 'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234/custom/path', 'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT' => 'https://localhost:1234/custom/path' ) do - OpenTelemetry::Exporter::OTLP::Exporter.new() + OpenTelemetry::Exporter::OTLP::Exporter.new end _(exp.instance_variable_get(:@path)).must_equal '/custom/path' end @@ -163,11 +173,21 @@ 'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234/non/specific/custom/path', 'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT' => 'https://localhost:1234/specific/custom/path' ) do - OpenTelemetry::Exporter::OTLP::Exporter.new() + OpenTelemetry::Exporter::OTLP::Exporter.new end _(exp.instance_variable_get(:@path)).must_equal '/specific/custom/path' end + it 'uses endpoint over OTEL_EXPORTER_OTLP_TRACES_ENDPOINT and OTEL_EXPORTER_OTLP_ENDPOINT' do + exp = OpenTelemetry::TestHelpers.with_env( + 'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234/non/specific/custom/path', + 'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT' => 'https://localhost:1234/specific/custom/path' + ) do + OpenTelemetry::Exporter::OTLP::Exporter.new(endpoint: 'https://localhost:1234/endpoint/custom/path') + end + _(exp.instance_variable_get(:@path)).must_equal '/endpoint/custom/path' + end + it 'restricts explicit headers to a String or Hash' do exp = OpenTelemetry::Exporter::OTLP::Exporter.new(headers: { 'token' => 'über' }) _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über', 'User-Agent' => DEFAULT_USER_AGENT) From 40204f970c45710608f2f8fbae95d0fbe4e557dc Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 10 Aug 2023 13:56:18 +0200 Subject: [PATCH 3/5] fix(exporter/otlp): move url logic closer to constructor --- .../opentelemetry/exporter/otlp/exporter.rb | 51 +++++-------------- 1 file changed, 14 insertions(+), 37 deletions(-) diff --git a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb index 7ae82b8999..51f38517f5 100644 --- a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb +++ b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb @@ -54,7 +54,20 @@ def initialize(endpoint: nil, compression: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_COMPRESSION', 'OTEL_EXPORTER_OTLP_COMPRESSION', default: 'gzip'), timeout: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_TIMEOUT', 'OTEL_EXPORTER_OTLP_TIMEOUT', default: 10), metrics_reporter: nil) - @uri = determine_url(endpoint) + endpoint ||= ENV['OTEL_EXPORTER_OTLP_TRACES_ENDPOINT'] + @uri = begin + if endpoint.nil? + endpoint = ENV['OTEL_EXPORTER_OTLP_ENDPOINT'] || 'http://localhost:4318' + endpoint += '/' unless endpoint.end_with?('/') + URI.join(endpoint, 'v1/traces') + elsif endpoint.strip.empty? + raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" + else + URI(endpoint) + end + rescue URI::Error + raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" + end raise ArgumentError, "unsupported compression key #{compression}" unless compression.nil? || %w[gzip none].include?(compression) @http = http_connection(@uri, ssl_verify_mode, certificate_file) @@ -380,42 +393,6 @@ def as_otlp_any_value(value) result end - # Returns the url used by the exporter based on the precedence defined in the specification: - # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#endpoint-urls-for-otlphttp - # - # @param [String, nil] endpoint - # @return [URI::Generic] - def determine_url(endpoint) - if !endpoint.nil? - # Use endpoint provided via argument as-is - parse_url_or_raise(endpoint) - elsif !ENV['OTEL_EXPORTER_OTLP_TRACES_ENDPOINT'].nil? - # Use signal-specific endpoint as-is - parse_url_or_raise(ENV['OTEL_EXPORTER_OTLP_TRACES_ENDPOINT']) - elsif !ENV['OTEL_EXPORTER_OTLP_ENDPOINT'].nil? - # Append v1/traces only if the non-signal specific env var is set. - non_signal_specific_endpoint = ENV['OTEL_EXPORTER_OTLP_ENDPOINT'] - # append '/' if not present - non_signal_specific_endpoint += '/' unless non_signal_specific_endpoint.end_with?('/') - begin - URI.join(non_signal_specific_endpoint, 'v1/traces') - rescue URI::InvalidURIError - raise ArgumentError, "invalid url for OTLP::Exporter #{non_signal_specific_endpoint}" - end - else - # use default in all other cases - parse_url_or_raise('http://localhost:4318/v1/traces') - end - end - - def parse_url_or_raise(endpoint) - raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" if endpoint.nil? || endpoint.strip.empty? - - URI(endpoint) - rescue URI::InvalidURIError - raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" - end - def prepare_headers(config_headers) headers = case config_headers when String then parse_headers(config_headers) From f2748b7f89d29c61f6d255d051ddcbee208692fc Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 7 Sep 2023 17:22:26 +0200 Subject: [PATCH 4/5] fix(exporter/otlp): address rubocop errors --- .../opentelemetry/exporter/otlp/exporter.rb | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb index 51f38517f5..41a05daac1 100644 --- a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb +++ b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb @@ -54,20 +54,12 @@ def initialize(endpoint: nil, compression: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_COMPRESSION', 'OTEL_EXPORTER_OTLP_COMPRESSION', default: 'gzip'), timeout: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_TIMEOUT', 'OTEL_EXPORTER_OTLP_TIMEOUT', default: 10), metrics_reporter: nil) - endpoint ||= ENV['OTEL_EXPORTER_OTLP_TRACES_ENDPOINT'] @uri = begin - if endpoint.nil? - endpoint = ENV['OTEL_EXPORTER_OTLP_ENDPOINT'] || 'http://localhost:4318' - endpoint += '/' unless endpoint.end_with?('/') - URI.join(endpoint, 'v1/traces') - elsif endpoint.strip.empty? - raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" - else - URI(endpoint) - end - rescue URI::Error - raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" - end + prepare_endpoint(endpoint) + rescue ArgumentError => e + raise ArgumentError, e.message + end + raise ArgumentError, "unsupported compression key #{compression}" unless compression.nil? || %w[gzip none].include?(compression) @http = http_connection(@uri, ssl_verify_mode, certificate_file) @@ -393,6 +385,21 @@ def as_otlp_any_value(value) result end + def prepare_endpoint(endpoint) + endpoint ||= ENV['OTEL_EXPORTER_OTLP_TRACES_ENDPOINT'] + if endpoint.nil? + endpoint = ENV['OTEL_EXPORTER_OTLP_ENDPOINT'] || 'http://localhost:4318' + endpoint += '/' unless endpoint.end_with?('/') + URI.join(endpoint, 'v1/traces') + elsif endpoint.strip.empty? + raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" + else + URI(endpoint) + end + rescue URI::InvalidURIError + raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" + end + def prepare_headers(config_headers) headers = case config_headers when String then parse_headers(config_headers) From e4d31d6b58d35ad1f30201df74a08387915f483f Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Tue, 26 Sep 2023 14:55:52 +0200 Subject: [PATCH 5/5] Update exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb Co-authored-by: Ariel Valentin --- exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb index 41a05daac1..696ea5bb9b 100644 --- a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb +++ b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb @@ -54,11 +54,7 @@ def initialize(endpoint: nil, compression: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_COMPRESSION', 'OTEL_EXPORTER_OTLP_COMPRESSION', default: 'gzip'), timeout: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_TIMEOUT', 'OTEL_EXPORTER_OTLP_TIMEOUT', default: 10), metrics_reporter: nil) - @uri = begin - prepare_endpoint(endpoint) - rescue ArgumentError => e - raise ArgumentError, e.message - end + @uri = prepare_endpoint(endpoint) raise ArgumentError, "unsupported compression key #{compression}" unless compression.nil? || %w[gzip none].include?(compression)