fix(exporter/otlp): align endpoint environment varaible handling with…

… spec (#1506) * fix(exporter/otlp): env var handling * fix(exporter/otlp): handle case where non-signal-specific endpoint ends without '/' * fix(exporter/otlp): move url logic closer to constructor * fix(exporter/otlp): address rubocop errors * Update exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb Co-authored-by: Ariel Valentin <[email protected]> --------- Co-authored-by: Robert <[email protected]> Co-authored-by: Ariel Valentin <[email protected]> Co-authored-by: Matthew Wear <[email protected]>
open-telemetry · Oct 7, 2023 · afc7bc5 · afc7bc5
1 parent cbe70c4
commit afc7bc5
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 10 deletions.
diff --git a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb
@@ -47,21 +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)
-          raise ArgumentError, "unsupported compression key #{compression}" unless compression.nil? || %w[gzip none].include?(compression)
+          @uri = prepare_endpoint(endpoint)
 
-          @uri = if endpoint == ENV['OTEL_EXPORTER_OTLP_ENDPOINT']
-                   URI.join(endpoint, 'v1/traces')
-                 else
-                   URI(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)
 
@@ -386,6 +381,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)

diff --git 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,11 +134,60 @@
       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 '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
+        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 '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)