Merge branch 'main' into add-rails-7-support

open-telemetry · Nov 2, 2021 · c61950c · c61950c
2 parents d2f100b + 47172f8
commit c61950c
Show file tree

Hide file tree

Showing 26 changed files with 244 additions and 56 deletions.
diff --git a/common/README.md b/common/README.md
@@ -40,8 +40,8 @@ tracer.in_span(
     'http.method' => req.method,
     'http.scheme' => USE_SSL_TO_SCHEME[use_ssl?],
     'http.target' => req.path,
-    'peer.hostname' => @address,
-    'peer.port' => @port
+    'net.peer.name' => @address,
+    'net.peer.port' => @port
   ),
   kind: :client
 ) do |span|

diff --git a/exporter/otlp/CHANGELOG.md b/exporter/otlp/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Release History: opentelemetry-exporter-otlp
 
+### v0.20.6 / 2021-10-29
+
+* FIXED: Add unexpected error handlign in BSP and OTLP exporter (#995)
+* FIXED: Handle otlp exporter race condition gzip errors with retry 
+
 ### v0.20.5 / 2021-09-29
 
 * (No significant changes)

diff --git a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb
@@ -143,6 +143,8 @@ def around_request
         end
 
         def send_bytes(bytes, timeout:) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity
+          return FAILURE if bytes.nil?
+
           retry_count = 0
           timeout ||= @timeout
           start_time = OpenTelemetry::Common::Utilities.timeout_timestamp
@@ -209,6 +211,10 @@ def send_bytes(bytes, timeout:) # rubocop:disable Metrics/AbcSize, Metrics/Cyclo
           rescue Zlib::DataError
             retry if backoff?(retry_count: retry_count += 1, reason: 'zlib_error')
             return FAILURE
+          rescue StandardError => e
+            OpenTelemetry.handle_error(exception: e, message: 'unexpected error in OTLP::Exporter#send_bytes')
+            @metrics_reporter.add_to_counter('otel.otlp_exporter.failure', labels: { 'reason' => e.class.to_s })
+            return FAILURE
           end
         ensure
           # Reset timeouts to defaults for the next call.
@@ -260,7 +266,7 @@ def backoff?(retry_after: nil, retry_count:, reason:)
           true
         end
 
-        def encode(span_data) # rubocop:disable Metrics/MethodLength
+        def encode(span_data) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
           Opentelemetry::Proto::Collector::Trace::V1::ExportTraceServiceRequest.encode(
             Opentelemetry::Proto::Collector::Trace::V1::ExportTraceServiceRequest.new(
               resource_spans: span_data
@@ -285,6 +291,9 @@ def encode(span_data) # rubocop:disable Metrics/MethodLength
                 end
             )
           )
+        rescue StandardError => e
+          OpenTelemetry.handle_error(exception: e, message: 'unexpected error in OTLP::Exporter#encode')
+          nil
         end
 
         def as_otlp_span(span_data) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength

diff --git a/exporter/otlp/lib/opentelemetry/exporter/otlp/version.rb b/exporter/otlp/lib/opentelemetry/exporter/otlp/version.rb
@@ -8,7 +8,7 @@ module OpenTelemetry
   module Exporter
     module OTLP
       ## Current OpenTelemetry OTLP exporter version
-      VERSION = '0.20.5'
+      VERSION = '0.20.6'
     end
   end
 end
diff --git a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb
@@ -285,6 +285,43 @@
       _(result).must_equal(TIMEOUT)
     end
 
+    it 'returns FAILURE on unexpected exceptions' do
+      log_stream = StringIO.new
+      logger = OpenTelemetry.logger
+      OpenTelemetry.logger = ::Logger.new(log_stream)
+
+      stub_request(:post, 'https://localhost:4318/v1/traces').to_raise('something unexpected')
+      span_data = create_span_data
+      result = exporter.export([span_data], timeout: 1)
+
+      _(log_stream.string).must_match(
+        /ERROR -- : OpenTelemetry error: unexpected error in OTLP::Exporter#send_bytes - something unexpected/
+      )
+
+      _(result).must_equal(FAILURE)
+    ensure
+      OpenTelemetry.logger = logger
+    end
+
+    it 'handles encoding failures' do
+      log_stream = StringIO.new
+      logger = OpenTelemetry.logger
+      OpenTelemetry.logger = ::Logger.new(log_stream)
+
+      stub_request(:post, 'https://localhost:4318/v1/traces').to_return(status: 200)
+      span_data = create_span_data
+
+      Opentelemetry::Proto::Collector::Trace::V1::ExportTraceServiceRequest.stub(:encode, ->(_) { raise 'a little hell' }) do
+        _(exporter.export([span_data], timeout: 1)).must_equal(FAILURE)
+      end
+
+      _(log_stream.string).must_match(
+        /ERROR -- : OpenTelemetry error: unexpected error in OTLP::Exporter#encode - a little hell/
+      )
+    ensure
+      OpenTelemetry.logger = logger
+    end
+
     it 'returns TIMEOUT on timeout after retrying' do
       stub_request(:post, 'https://localhost:4318/v1/traces').to_timeout.then.to_raise('this should not be reached')
       span_data = create_span_data

diff --git a/exporter/zipkin/lib/opentelemetry/exporter/zipkin/transformer.rb b/exporter/zipkin/lib/opentelemetry/exporter/zipkin/transformer.rb
@@ -29,10 +29,10 @@ module Transformer
         STATUS_ERROR = 'ERROR'
         STATUS_OK = 'OK'
         ATTRIBUTE_PEER_SERVICE = 'peer.service'
-        ATTRIBUTE_NET_PEER_IP = 'peer.ip'
-        ATTRIBUTE_NET_PEER_PORT = 'peer.port'
-        ATTRIBUTE_NET_HOST_IP = 'host.ip'
-        ATTRIBUTE_NET_HOST_PORT = 'host.port'
+        ATTRIBUTE_NET_PEER_IP = 'net.peer.ip'
+        ATTRIBUTE_NET_PEER_PORT = 'net.peer.port'
+        ATTRIBUTE_NET_HOST_IP = 'net.host.ip'
+        ATTRIBUTE_NET_HOST_PORT = 'net.host.port'
 
         DEFAULT_SERVICE_NAME = OpenTelemetry::SDK::Resources::Resource.default.attribute_enumerator.find { |k, _| k == SERVICE_NAME_ATTRIBUTE_KEY }&.last || 'unknown_service'
         private_constant(:KIND_MAP, :DEFAULT_SERVICE_NAME, :SERVICE_NAME_ATTRIBUTE_KEY, :ERROR_TAG_KEY, :STATUS_CODE_NAME, :STATUS_ERROR, :STATUS_OK, :ATTRIBUTE_PEER_SERVICE, :ATTRIBUTE_NET_PEER_IP, :ATTRIBUTE_NET_PEER_PORT, :ATTRIBUTE_NET_HOST_IP, :ATTRIBUTE_NET_HOST_PORT)

diff --git a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/client.rb
@@ -19,8 +19,8 @@ def perform(req, options) # rubocop:disable Metrics/AbcSize
               'http.scheme' => uri.scheme,
               'http.target' => uri.path,
               'http.url' => "#{uri.scheme}://#{uri.host}",
-              'peer.hostname' => uri.host,
-              'peer.port' => uri.port
+              'net.peer.name' => uri.host,
+              'net.peer.port' => uri.port
             }.merge(OpenTelemetry::Common::HTTP::ClientContext.attributes)
 
             tracer.in_span("HTTP #{request_method}", attributes: attributes, kind: :client) do |span|

diff --git a/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/connection.rb b/instrumentation/http/lib/opentelemetry/instrumentation/http/patches/connection.rb
@@ -12,8 +12,8 @@ module Patches
         module Connection
           def initialize(req, options)
             attributes = OpenTelemetry::Common::HTTP::ClientContext.attributes.merge(
-              'peer.hostname' => req.uri.host,
-              'peer.port' => req.uri.port
+              'net.peer.name' => req.uri.host,
+              'net.peer.port' => req.uri.port
             )
 
             tracer.in_span('HTTP CONNECT', attributes: attributes) do

diff --git a/instrumentation/http/test/instrumentation/http/patches/client_test.rb b/instrumentation/http/test/instrumentation/http/patches/client_test.rb
@@ -42,8 +42,8 @@
       _(span.attributes['http.scheme']).must_equal 'http'
       _(span.attributes['http.status_code']).must_equal 200
       _(span.attributes['http.target']).must_equal '/success'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 80
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 80
       assert_requested(
         :get,
         'http://example.com/success',
@@ -60,8 +60,8 @@
       _(span.attributes['http.scheme']).must_equal 'http'
       _(span.attributes['http.status_code']).must_equal 500
       _(span.attributes['http.target']).must_equal '/failure'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 80
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 80
       assert_requested(
         :post,
         'http://example.com/failure',
@@ -80,8 +80,8 @@
       _(span.attributes['http.scheme']).must_equal 'https'
       _(span.attributes['http.status_code']).must_be_nil
       _(span.attributes['http.target']).must_equal '/timeout'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 443
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 443
       _(span.status.code).must_equal(
         OpenTelemetry::Trace::Status::ERROR
       )
@@ -106,8 +106,8 @@
       _(span.attributes['http.scheme']).must_equal 'http'
       _(span.attributes['http.status_code']).must_equal 200
       _(span.attributes['http.target']).must_equal '/success'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 80
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 80
       _(span.attributes['peer.service']).must_equal 'foo'
       assert_requested(
         :get,

diff --git a/instrumentation/http/test/instrumentation/http/patches/connection_test.rb b/instrumentation/http/test/instrumentation/http/patches/connection_test.rb
@@ -36,8 +36,8 @@
 
       _(exporter.finished_spans.size).must_equal(2)
       _(span.name).must_equal 'HTTP CONNECT'
-      _(span.attributes['peer.hostname']).must_equal('localhost')
-      _(span.attributes['peer.port']).wont_be_nil
+      _(span.attributes['net.peer.name']).must_equal('localhost')
+      _(span.attributes['net.peer.port']).wont_be_nil
     ensure
       WebMock.disable_net_connect!
     end

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
@@ -21,8 +21,8 @@ def do_get_block(req, proxy, conn, &block) # rubocop:disable Metrics/AbcSize
               'http.scheme' => uri.scheme,
               'http.target' => uri.path,
               'http.url' => url,
-              'peer.hostname' => uri.host,
-              'peer.port' => uri.port
+              'net.peer.name' => uri.host,
+              'net.peer.port' => uri.port
             }.merge(OpenTelemetry::Common::HTTP::ClientContext.attributes)
 
             tracer.in_span("HTTP #{request_method}", attributes: attributes, kind: :client) do |span|

diff --git a/instrumentation/http_client/test/instrumentation/http_client/patches/client_test.rb b/instrumentation/http_client/test/instrumentation/http_client/patches/client_test.rb
@@ -44,8 +44,8 @@
       _(span.attributes['http.scheme']).must_equal 'http'
       _(span.attributes['http.status_code']).must_equal 200
       _(span.attributes['http.target']).must_equal '/success'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 80
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 80
       assert_requested(
         :get,
         'http://example.com/success',
@@ -64,8 +64,8 @@
       _(span.attributes['http.scheme']).must_equal 'http'
       _(span.attributes['http.status_code']).must_equal 500
       _(span.attributes['http.target']).must_equal '/failure'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 80
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 80
       assert_requested(
         :post,
         'http://example.com/failure',
@@ -86,8 +86,8 @@
       _(span.attributes['http.scheme']).must_equal 'https'
       _(span.attributes['http.status_code']).must_be_nil
       _(span.attributes['http.target']).must_equal '/timeout'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 443
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 443
       _(span.status.code).must_equal(
         OpenTelemetry::Trace::Status::ERROR
       )
@@ -114,8 +114,8 @@
       _(span.attributes['http.scheme']).must_equal 'http'
       _(span.attributes['http.status_code']).must_equal 200
       _(span.attributes['http.target']).must_equal '/success'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 80
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 80
       _(span.attributes['peer.service']).must_equal 'foo'
       assert_requested(
         :get,

diff --git a/...umentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb b/...umentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb
@@ -25,8 +25,8 @@ def request(req, body = nil, &block)
                   'http.method' => req.method,
                   'http.scheme' => USE_SSL_TO_SCHEME[use_ssl?],
                   'http.target' => req.path,
-                  'peer.hostname' => @address,
-                  'peer.port' => @port
+                  'net.peer.name' => @address,
+                  'net.peer.port' => @port
                 ),
                 kind: :client
               ) do |span|
@@ -51,8 +51,8 @@ def connect
 
               attributes = OpenTelemetry::Common::HTTP::ClientContext.attributes
               tracer.in_span('HTTP CONNECT', attributes: attributes.merge(
-                'peer.hostname' => conn_address,
-                'peer.port' => conn_port
+                'net.peer.name' => conn_address,
+                'net.peer.port' => conn_port
               )) do
                 super
               end

diff --git a/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/instrumentation_test.rb b/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/instrumentation_test.rb
@@ -48,8 +48,8 @@
       _(span.attributes['http.scheme']).must_equal 'http'
       _(span.attributes['http.status_code']).must_equal 200
       _(span.attributes['http.target']).must_equal '/success'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 80
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 80
       assert_requested(
         :get,
         'http://example.com/success',
@@ -66,8 +66,8 @@
       _(span.attributes['http.scheme']).must_equal 'http'
       _(span.attributes['http.status_code']).must_equal 500
       _(span.attributes['http.target']).must_equal '/failure'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 80
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 80
       assert_requested(
         :post,
         'http://example.com/failure',
@@ -86,8 +86,8 @@
       _(span.attributes['http.scheme']).must_equal 'https'
       _(span.attributes['http.status_code']).must_be_nil
       _(span.attributes['http.target']).must_equal '/timeout'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 443
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 443
       _(span.status.code).must_equal(
         OpenTelemetry::Trace::Status::ERROR
       )
@@ -112,8 +112,8 @@
       _(span.attributes['http.scheme']).must_equal 'http'
       _(span.attributes['http.status_code']).must_equal 200
       _(span.attributes['http.target']).must_equal '/success'
-      _(span.attributes['peer.hostname']).must_equal 'example.com'
-      _(span.attributes['peer.port']).must_equal 80
+      _(span.attributes['net.peer.name']).must_equal 'example.com'
+      _(span.attributes['net.peer.port']).must_equal 80
       _(span.attributes['peer.service']).must_equal 'foo'
       assert_requested(
         :get,
@@ -138,8 +138,8 @@
 
       _(exporter.finished_spans.size).must_equal(2)
       _(span.name).must_equal 'HTTP CONNECT'
-      _(span.attributes['peer.hostname']).must_equal('localhost')
-      _(span.attributes['peer.port']).wont_be_nil
+      _(span.attributes['net.peer.name']).must_equal('localhost')
+      _(span.attributes['net.peer.port']).wont_be_nil
     ensure
       WebMock.disable_net_connect!
     end
@@ -153,8 +153,8 @@
 
       _(exporter.finished_spans.size).must_equal(1)
       _(span.name).must_equal 'HTTP CONNECT'
-      _(span.attributes['peer.hostname']).must_equal('localhost')
-      _(span.attributes['peer.port']).must_equal(99_999)
+      _(span.attributes['net.peer.name']).must_equal('localhost')
+      _(span.attributes['net.peer.port']).must_equal(99_999)
 
       span_event = span.events.first
       _(span_event.name).must_equal 'exception'

diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware.rb
@@ -18,7 +18,13 @@ class TracerMiddleware # rubocop:disable Metrics/ClassLength
           class << self
             def allowed_rack_request_headers
               @allowed_rack_request_headers ||= Array(config[:allowed_request_headers]).each_with_object({}) do |header, memo|
-                memo["HTTP_#{header.to_s.upcase.gsub(/[-\s]/, '_')}"] = build_attribute_name('http.request.headers.', header)
+                key = header.to_s.upcase.gsub(/[-\s]/, '_')
+                case key
+                when 'CONTENT_TYPE', 'CONTENT_LENGTH'
+                  memo[key] = build_attribute_name('http.request.headers.', header)
+                else
+                  memo["HTTP_#{key}"] = build_attribute_name('http.request.headers.', header)
+                end
               end
             end