chore: feedback

Author: scbjans

.toys/.data/releases.yml

@@ -142,8 +142,7 @@ gems:
 
   - name: opentelemetry-instrumentation-active_model_serializers
     directory: instrumentation/active_model_serializers
-    version_constant:
-      [OpenTelemetry, Instrumentation, ActiveModelSerializers, VERSION]
+    version_constant: [OpenTelemetry, Instrumentation, ActiveModelSerializers, VERSION]
 
   - name: opentelemetry-instrumentation-concurrent_ruby
     directory: instrumentation/concurrent_ruby
@@ -237,8 +236,7 @@ gems:
 
   - name: opentelemetry-propagator-google_cloud_trace_context
     directory: propagator/google_cloud_trace_context
-    version_constant:
-      [OpenTelemetry, Propagator, GoogleCloudTraceContext, VERSION]
+    version_constant: [OpenTelemetry, Propagator, GoogleCloudTraceContext, VERSION]
 
   - name: opentelemetry-propagator-ottrace
     directory: propagator/ottrace
@@ -270,8 +268,7 @@ gems:
   - name: opentelemetry-resource-detector-google_cloud_platform
     directory: resources/google_cloud_platform
     version_rb_path: lib/opentelemetry/resource/detector/google_cloud_platform/version.rb
-    version_constant:
-      [OpenTelemetry, Resource, Detector, GoogleCloudPlatform, VERSION]
+    version_constant: [OpenTelemetry, Resource, Detector, GoogleCloudPlatform, VERSION]
 
   - name: opentelemetry-instrumentation-grpc
     directory: instrumentation/grpc

instrumentation/net_ldap/lib/opentelemetry/instrumentation/net/ldap/instrumentation_service.rb

@@ -28,18 +28,19 @@ def instrument(event, payload)
               OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => host || hosts,
               OpenTelemetry::SemanticConventions::Trace::NET_PEER_PORT => port,
               OpenTelemetry::SemanticConventions::Trace::PEER_SERVICE => instrumentation_config[:peer_service]
-            }.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes)
+            }
+            attributes.delete_if { |_key, value| value.nil? }
 
             tracer.in_span(
               event,
-              attributes: attributes.compact,
+              attributes: attributes,
               kind: :client
             ) do |span|
               yield(payload).tap do |response|
-                annotate_span_with_response!(span, response)
+                annotate_span_with_response(span, response) if response
               end
-            rescue StandardError => e
-              span.record_exception(e)
+            rescue ::Net::LDAP::Error => e
+              span.set_attribute('ldap.error_message', "#{e.class}: #{e.message}")
               span.status = OpenTelemetry::Trace::Status.error
               raise e
             end
@@ -57,9 +58,7 @@ def instrumentation_config
             LDAP::Instrumentation.instance.config
           end
 
-          def annotate_span_with_response!(span, response)
-            return unless response
-
+          def annotate_span_with_response(span, response)
             status_code = ::Net::LDAP::ResultCodeSuccess
             message = ''
             error_message = ''
@@ -69,12 +68,15 @@ def annotate_span_with_response!(span, response)
               error_message = response.error_message.to_s
               message = ::Net::LDAP.result2string(status_code)
             end
-            span.set_attribute('ldap.status_code', status_code)
-            span.set_attribute('ldap.message', message) unless message.empty?
+            attributes = {
+              'ldap.status_code' => status_code
+            }
+            attributes['ldap.message'] = message unless message.empty?
+            attributes['ldap.error_message'] = error_message unless error_message.empty?
+            span.add_attributes(attributes)
 
             return if ::Net::LDAP::ResultCodesNonError.include?(status_code)
 
-            span.record_exception(error_message) unless error_message.empty?
             span.status = OpenTelemetry::Trace::Status.error
           end
         end

instrumentation/net_ldap/test/opentelemetry/instrumentation/net/ldap/instrumentation_test.rb

@@ -110,10 +110,9 @@ def add(args)
         _(span.attributes['ldap.base']).must_equal 'dc=com'
         _(span.attributes['ldap.encryption']).must_equal '{"method":"simple_tls","tls_options":{"foo":"bar"}}'
         _(span.attributes['ldap.payload']).must_equal '{}'
+        _(span.attributes['ldap.error_message']).must_equal 'Net::LDAP::Error: Connection timed out - user specified timeout'
         _(span.attributes['net.peer.name']).must_equal 'test.mocked.com'
         _(span.attributes['net.peer.port']).must_equal 636
-        _(span.events.first.attributes['exception.type']).must_equal 'Net::LDAP::Error'
-        _(span.events.first.attributes['exception.message']).must_equal 'Connection timed out - user specified timeout'
       end
     end
   end

release-please-config.json

@@ -231,4 +231,4 @@
       "version-file": "lib/opentelemetry/resource/detector/google_cloud_platform/version.rb"
     }
   }
-}
+}