feat(otlp-http): improve endpoint handling, add User-Agent header, an…

exporter/otlp-http/lib/opentelemetry/exporter/otlp/http/trace_exporter.rb

@@ -30,7 +30,9 @@ class TraceExporter # rubocop:disable Metrics/ClassLength
           ERROR_MESSAGE_INVALID_HEADERS = 'headers must be a String with comma-separated URL Encoded UTF-8 k=v pairs or a Hash'
           private_constant(:ERROR_MESSAGE_INVALID_HEADERS)
 
-          def initialize(endpoint: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_ENDPOINT', 'OTEL_EXPORTER_OTLP_ENDPOINT', default: 'http://localhost:4318/v1/traces'),
+          DEFAULT_USER_AGENT = "OTel-OTLP-Exporter-Ruby/#{OpenTelemetry::Exporter::OTLP::HTTP::VERSION} Ruby/#{RUBY_VERSION} (#{RUBY_PLATFORM}; #{RUBY_ENGINE}/#{RUBY_ENGINE_VERSION})".freeze
+
+          def initialize(endpoint: nil,
                          certificate_file: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE', 'OTEL_EXPORTER_OTLP_CERTIFICATE'),
                          client_certificate_file: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE', 'OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE'),
                          client_key_file: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_CLIENT_KEY', 'OTEL_EXPORTER_OTLP_CLIENT_KEY'),
@@ -39,24 +41,14 @@ def initialize(endpoint: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPOR
                          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 = if endpoint == ENV['OTEL_EXPORTER_OTLP_ENDPOINT']
-                     URI("#{endpoint}/v1/traces")
-                   else
-                     URI(endpoint)
-                   end
+            @uri = prepare_endpoint(endpoint)
 
             @http = http_connection(@uri, ssl_verify_mode, certificate_file, client_certificate_file, client_key_file)
 
             @path = @uri.path
-            @headers = case headers
-                       when String then parse_headers(headers)
-                       when Hash then headers
-                       else
-                         raise ArgumentError, ERROR_MESSAGE_INVALID_HEADERS
-                       end
+            @headers = prepare_headers(headers)
             @timeout = timeout.to_f
             @compression = compression
             @metrics_reporter = metrics_reporter || OpenTelemetry::SDK::Trace::Export::MetricsReporter
@@ -133,17 +125,21 @@ def around_request
           def send_bytes(bytes, timeout:) # rubocop:disable Metrics/MethodLength
             return FAILURE if bytes.nil?
 
+            @metrics_reporter.record_value('otel.otlp_exporter.message.uncompressed_size', value: bytes.bytesize)
+
             retry_count = 0
             timeout ||= @timeout
             start_time = OpenTelemetry::Common::Utilities.timeout_timestamp
             around_request do # rubocop:disable Metrics/BlockLength
               request = Net::HTTP::Post.new(@path)
-              request.body = if @compression == 'gzip'
-                               request.add_field('Content-Encoding', 'gzip')
-                               Zlib.gzip(bytes)
-                             else
-                               bytes
-                             end
+              if @compression == 'gzip'
+                request.add_field('Content-Encoding', 'gzip')
+                body = Zlib.gzip(bytes)
+                @metrics_reporter.record_value('otel.otlp_exporter.message.compressed_size', value: body.bytesize)
+              else
+                body = bytes
+              end
+              request.body = body
               request.add_field('Content-Type', 'application/x-protobuf')
               @headers.each { |key, value| request.add_field(key, value) }
 
@@ -168,6 +164,9 @@ def send_bytes(bytes, timeout:) # rubocop:disable Metrics/MethodLength
                 response.body # Read and discard body
                 redo if backoff?(retry_count: retry_count += 1, reason: response.code)
                 FAILURE
+              when Net::HTTPNotFound
+                log_request_failure(response.code)
+                FAILURE
               when Net::HTTPBadRequest, Net::HTTPClientError, Net::HTTPServerError
                 log_status(response.body)
                 @metrics_reporter.add_to_counter('otel.otlp_exporter.failure', labels: { 'reason' => response.code })
@@ -226,6 +225,11 @@ def log_status(body)
             OpenTelemetry.handle_error(exception: e, message: 'unexpected error decoding rpc.Status in OTLP::Exporter#log_status')
           end
 
+          def log_request_failure(response_code)
+            OpenTelemetry.handle_error(message: "OTLP exporter received http.code=#{response_code} for uri='#{@uri}' in OTLP::Exporter#send_bytes")
+            @metrics_reporter.add_to_counter('otel.otlp_exporter.failure', labels: { 'reason' => response_code })
+          end
+
           def measure_request_duration
             start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
             begin
@@ -265,6 +269,34 @@ def backoff?(retry_count:, reason:, retry_after: nil)
             true
           end
 
+          def prepare_headers(config_headers)
+            headers = case config_headers
+                      when String then parse_headers(config_headers)
+                      when Hash then config_headers.dup
+                      else
+                        raise ArgumentError, ERROR_MESSAGE_INVALID_HEADERS
+                      end
+
+            headers['User-Agent'] = "#{headers.fetch('User-Agent', '')} #{DEFAULT_USER_AGENT}".strip
+
+            headers
+          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 parse_headers(raw)
             entries = raw.split(',')
             raise ArgumentError, ERROR_MESSAGE_INVALID_HEADERS if entries.empty?

exporter/otlp-http/test/opentelemetry/exporter/otlp/http/trace_exporter_test.rb

@@ -11,11 +11,13 @@
   let(:success) { OpenTelemetry::SDK::Trace::Export::SUCCESS }
   let(:export_failure) { OpenTelemetry::SDK::Trace::Export::FAILURE }
 
+  DEFAULT_USER_AGENT = OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter::DEFAULT_USER_AGENT
+
   describe '#initialize' do
     it 'initializes with defaults' do
       exp = OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
       _(exp).wont_be_nil
-      _(exp.instance_variable_get(:@headers)).must_be_empty
+      _(exp.instance_variable_get(:@headers)).must_equal('User-Agent' => DEFAULT_USER_AGENT)
       _(exp.instance_variable_get(:@timeout)).must_equal 10.0
       _(exp.instance_variable_get(:@path)).must_equal '/v1/traces'
       _(exp.instance_variable_get(:@compression)).must_equal 'gzip'
@@ -27,6 +29,16 @@
       _(http.port).must_equal 4318
     end
 
+    it 'provides a useful, spec-compliant default user agent header' do
+      version = OpenTelemetry::Exporter::OTLP::HTTP::VERSION
+      # spec compliance: OTLP Exporter name and version
+      _(DEFAULT_USER_AGENT).must_match("OTel-OTLP-Exporter-Ruby/#{version}")
+      # bonus: incredibly useful troubleshooting information
+      _(DEFAULT_USER_AGENT).must_match("Ruby/#{RUBY_VERSION}")
+      _(DEFAULT_USER_AGENT).must_match(RUBY_PLATFORM)
+      _(DEFAULT_USER_AGENT).must_match("#{RUBY_ENGINE}/#{RUBY_ENGINE_VERSION}")
+    end
+
     it 'refuses invalid endpoint' do
       assert_raises ArgumentError do
         OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new(endpoint: 'not a url')
@@ -72,7 +84,7 @@
                                                 'OTEL_EXPORTER_OTLP_TIMEOUT' => '11') do
         OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
       end
-      _(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd')
+      _(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd', 'User-Agent' => DEFAULT_USER_AGENT)
       _(exp.instance_variable_get(:@timeout)).must_equal 11.0
       _(exp.instance_variable_get(:@path)).must_equal '/v1/traces'
       _(exp.instance_variable_get(:@compression)).must_equal 'gzip'
@@ -98,7 +110,7 @@
                                                                ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE,
                                                                timeout: 12)
       end
-      _(exp.instance_variable_get(:@headers)).must_equal('x' => 'y')
+      _(exp.instance_variable_get(:@headers)).must_equal('x' => 'y', 'User-Agent' => DEFAULT_USER_AGENT)
       _(exp.instance_variable_get(:@timeout)).must_equal 12.0
       _(exp.instance_variable_get(:@path)).must_equal ''
       _(exp.instance_variable_get(:@compression)).must_equal 'gzip'
@@ -110,12 +122,79 @@
       _(http.port).must_equal 4321
     end
 
+    it 'appends the correct path if OTEL_EXPORTER_OTLP_ENDPOINT has a trailing slash' do
+      exp = OpenTelemetry::TestHelpers.with_env(
+        'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234/'
+      ) do
+        OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
+      end
+      _(exp.instance_variable_get(:@path)).must_equal '/v1/traces'
+    end
+
+    it 'appends the correct path if OTEL_EXPORTER_OTLP_ENDPOINT does not have a trailing slash' do
+      exp = OpenTelemetry::TestHelpers.with_env(
+        'OTEL_EXPORTER_OTLP_ENDPOINT' => 'https://localhost:1234'
+      ) do
+        OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.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::HTTP::TraceExporter.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::HTTP::TraceExporter.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::HTTP::TraceExporter.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::HTTP::TraceExporter.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::HTTP::TraceExporter.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::HTTP::TraceExporter.new(headers: { 'token' => 'über' })
-      _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über')
+      _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über', 'User-Agent' => DEFAULT_USER_AGENT)
 
       exp = OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new(headers: 'token=%C3%BCber')
-      _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über')
+      _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über', 'User-Agent' => DEFAULT_USER_AGENT)
 
       error = _ do
         exp = OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new(headers: Object.new)
@@ -124,58 +203,68 @@
       _(error.message).must_match(/headers/i)
     end
 
+    it 'ignores later mutations of a headers Hash parameter' do
+      a_hash_to_mutate_later = { 'token' => 'über' }
+      exp = OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new(headers: a_hash_to_mutate_later)
+      _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über', 'User-Agent' => DEFAULT_USER_AGENT)
+
+      a_hash_to_mutate_later['token'] = 'unter'
+      a_hash_to_mutate_later['oops'] = 'i forgot to add this, too'
+      _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über', 'User-Agent' => DEFAULT_USER_AGENT)
+    end
+
     describe 'Headers Environment Variable' do
       it 'allows any number of the equal sign (=) characters in the value' do
         exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_HEADERS' => 'a=b,c=d==,e=f') do
           OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
         end
-        _(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd==', 'e' => 'f')
+        _(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd==', 'e' => 'f', 'User-Agent' => DEFAULT_USER_AGENT)
 
         exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_TRACES_HEADERS' => 'a=b,c=d==,e=f') do
           OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
         end
-        _(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd==', 'e' => 'f')
+        _(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd==', 'e' => 'f', 'User-Agent' => DEFAULT_USER_AGENT)
       end
 
       it 'trims any leading or trailing whitespaces in keys and values' do
         exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_HEADERS' => 'a =  b  ,c=d , e=f') do
           OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
         end
-        _(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd', 'e' => 'f')
+        _(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd', 'e' => 'f', 'User-Agent' => DEFAULT_USER_AGENT)
 
         exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_TRACES_HEADERS' => 'a =  b  ,c=d , e=f') do
           OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
         end
-        _(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd', 'e' => 'f')
+        _(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd', 'e' => 'f', 'User-Agent' => DEFAULT_USER_AGENT)
       end
 
       it 'decodes values as URL encoded UTF-8 strings' do
         exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_HEADERS' => 'token=%C3%BCber') do
           OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
         end
-        _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über')
+        _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über', 'User-Agent' => DEFAULT_USER_AGENT)
 
         exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_HEADERS' => '%C3%BCber=token') do
           OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
         end
-        _(exp.instance_variable_get(:@headers)).must_equal('über' => 'token')
+        _(exp.instance_variable_get(:@headers)).must_equal('über' => 'token', 'User-Agent' => DEFAULT_USER_AGENT)
 
         exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_TRACES_HEADERS' => 'token=%C3%BCber') do
           OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
         end
-        _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über')
+        _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über', 'User-Agent' => DEFAULT_USER_AGENT)
 
         exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_TRACES_HEADERS' => '%C3%BCber=token') do
           OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
         end
-        _(exp.instance_variable_get(:@headers)).must_equal('über' => 'token')
+        _(exp.instance_variable_get(:@headers)).must_equal('über' => 'token', 'User-Agent' => DEFAULT_USER_AGENT)
       end
 
       it 'prefers TRACES specific variable' do
         exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_HEADERS' => 'a=b,c=d==,e=f', 'OTEL_EXPORTER_OTLP_TRACES_HEADERS' => 'token=%C3%BCber') do
           OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new
         end
-        _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über')
+        _(exp.instance_variable_get(:@headers)).must_equal('token' => 'über', 'User-Agent' => DEFAULT_USER_AGENT)
       end
 
       it 'fails fast when header values are missing' do
@@ -287,6 +376,16 @@
       _(result).must_equal(success)
     end
 
+    it 'records metrics' do
+      metrics_reporter = Minitest::Mock.new
+      exporter = OpenTelemetry::Exporter::OTLP::HTTP::TraceExporter.new(metrics_reporter: metrics_reporter)
+      stub_request(:post, 'http://localhost:4318/v1/traces').to_timeout.then.to_return(status: 200)
+      metrics_reporter.expect(:record_value, nil) { |m, _, _| m == 'otel.otlp_exporter.message.uncompressed_size' }
+      metrics_reporter.expect(:record_value, nil) { |m, _, _| m == 'otel.otlp_exporter.message.compressed_size' }
+      metrics_reporter.expect(:add_to_counter, nil) { |m, _, _| m == 'otel.otlp_exporter.failure' }
+      exporter.export([OpenTelemetry::TestHelpers.create_span_data])
+    end
+
     it 'retries on timeout' do
       stub_request(:post, 'http://localhost:4318/v1/traces').to_timeout.then.to_return(status: 200)
       span_data = OpenTelemetry::TestHelpers.create_span_data
@@ -418,6 +517,25 @@
       OpenTelemetry.logger = logger
     end
 
+    it 'logs a specific message when there is a 404' do
+      log_stream = StringIO.new
+      logger = OpenTelemetry.logger
+      OpenTelemetry.logger = ::Logger.new(log_stream)
+
+      stub_request(:post, 'http://localhost:4318/v1/traces').to_return(status: 404, body: "Not Found\n")
+      span_data = OpenTelemetry::TestHelpers.create_span_data
+
+      result = exporter.export([span_data])
+
+      _(log_stream.string).must_match(
+        %r{ERROR -- : OpenTelemetry error: OTLP exporter received http\.code=404 for uri='http://localhost:4318/v1/traces'}
+      )
+
+      _(result).must_equal(export_failure)
+    ensure
+      OpenTelemetry.logger = logger
+    end
+
     it 'handles Zlib gzip compression errors' do
       stub_request(:post, 'http://localhost:4318/v1/traces').to_raise(Zlib::DataError.new('data error'))
       span_data = OpenTelemetry::TestHelpers.create_span_data