Merge branch 'main' into auto-instrumentation

Author: xuan-cao-swi

.github/workflows/ci-instrumentation-with-services.yml

@@ -157,7 +157,7 @@ jobs:
           ZOOKEEPER_CLIENT_PORT: 2181
           ZOOKEEPER_TICK_TIME: 2000
       kafka:
-        image: confluentinc/cp-kafka:latest
+        image: confluentinc/cp-kafka:7.9.1
         ports:
           - 9092:9092
           - 29092:29092

docker-compose.yml

@@ -226,7 +226,7 @@ services:
       ZOOKEEPER_TICK_TIME: 2000
 
   kafka:
-    image: confluentinc/cp-kafka:latest
+    image: confluentinc/cp-kafka:7.9.1
     ports:
       - "9092:9092"
       - "29092:29092"

instrumentation/action_pack/CHANGELOG.md

@@ -1,5 +1,13 @@
 # Release History: opentelemetry-instrumentation-action_pack
 
+### v0.12.3 / 2025-06-16
+
+* FIXED: Action_pack always assuming sdk spans
+
+### v0.12.2 / 2025-06-04
+
+* FIXED: Rack span class naming
+
 ### v0.12.1 / 2025-05-07
 
 * FIXED: Account for `nil` routes

instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb

@@ -23,35 +23,9 @@ def initialize(config)
           # @param payload [Hash] the payload passed as a method argument
           # @return [Hash] the payload passed as a method argument
           def start(_name, _id, payload)
-            span_name, attributes = to_span_name_and_attributes(payload)
-
             span = OpenTelemetry::Instrumentation::Rack.current_span
-            span.name = span_name
-            span.add_attributes(attributes)
-          rescue StandardError => e
-            OpenTelemetry.handle_error(exception: e)
-          end
-
-          # Invoked by ActiveSupport::Notifications at the end of the instrumentation block
-          #
-          # @param _name [String] of the event (unused)
-          # @param _id [String] of the event (unused)
-          # @param payload [Hash] the payload passed as a method argument
-          # @return [Hash] the payload passed as a method argument
-          def finish(_name, _id, payload)
-            span = OpenTelemetry::Instrumentation::Rack.current_span
-            span.record_exception(payload[:exception_object]) if payload[:exception_object]
-          rescue StandardError => e
-            OpenTelemetry.handle_error(exception: e)
-          end
-
-          private
+            return unless span.recording?
 
-          # Extracts the span name and attributes from the payload
-          #
-          # @param payload [Hash] the payload passed from ActiveSupport::Notifications
-          # @return [Array<String, Hash>] the span name and attributes
-          def to_span_name_and_attributes(payload)
             request = payload[:request]
             # It seems that there are cases in Rails functional tests where it bypasses the routing system and the `action_dispatch.route_uri_pattern` header not being set.
             # Our Test suite executes the routing system so we are unable to recreate this error case.
@@ -66,12 +40,33 @@ def to_span_name_and_attributes(payload)
             attributes[OpenTelemetry::SemanticConventions::Trace::HTTP_TARGET] = request.filtered_path if request.filtered_path != request.fullpath
 
             if @span_naming == :semconv
-              return ["#{request.method} #{http_route}", attributes] if http_route
-
-              return ["#{request.method} /#{payload.dig(:params, :controller)}/#{payload.dig(:params, :action)}", attributes]
+              span.name = if http_route
+                            "#{request.method} #{http_route}"
+                          else
+                            "#{request.method} /#{payload.dig(:params, :controller)}/#{payload.dig(:params, :action)}"
+                          end
+            # If there is an exception we want to keep the original span name
+            # so it is easier to see where the request was routed to.
+            elsif !request.env['action_dispatch.exception']
+              span.name = "#{payload[:controller]}##{payload[:action]}"
             end
 
-            ["#{payload[:controller]}##{payload[:action]}", attributes]
+            span.add_attributes(attributes)
+          rescue StandardError => e
+            OpenTelemetry.handle_error(exception: e)
+          end
+
+          # Invoked by ActiveSupport::Notifications at the end of the instrumentation block
+          #
+          # @param _name [String] of the event (unused)
+          # @param _id [String] of the event (unused)
+          # @param payload [Hash] the payload passed as a method argument
+          # @return [Hash] the payload passed as a method argument
+          def finish(_name, _id, payload)
+            span = OpenTelemetry::Instrumentation::Rack.current_span
+            span.record_exception(payload[:exception_object]) if payload[:exception_object]
+          rescue StandardError => e
+            OpenTelemetry.handle_error(exception: e)
           end
         end
       end

instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/version.rb

@@ -7,7 +7,7 @@
 module OpenTelemetry
   module Instrumentation
     module ActionPack
-      VERSION = '0.12.1'
+      VERSION = '0.12.3'
     end
   end
 end

instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb

@@ -215,11 +215,12 @@
 
   describe 'when the application has exceptions_app configured' do
     let(:rails_app) { AppConfig.initialize_app(use_exceptions_app: true) }
+    let(:config) { { span_naming: :class } }
 
     it 'does not overwrite the span name from the controller that raised' do
       get 'internal_server_error'
 
-      _(span.name).must_match(/^GET/)
+      _(span.name).must_equal 'ExampleController#internal_server_error'
       _(span.kind).must_equal :server
       _(span.status.ok?).must_equal false
 
@@ -235,6 +236,12 @@
       _(span.attributes['code.namespace']).must_equal 'ExceptionsController'
       _(span.attributes['code.function']).must_equal 'show'
     end
+
+    it 'does not raise with api/non recording spans' do
+      with_sampler(OpenTelemetry::SDK::Trace::Samplers::ALWAYS_OFF) do
+        get 'internal_server_error'
+      end
+    end
   end
 
   it 'sets filters `http.target`' do

instrumentation/action_pack/test/test_helper.rb

@@ -24,3 +24,11 @@
   c.use 'OpenTelemetry::Instrumentation::ActionPack'
   c.add_span_processor span_processor
 end
+
+def with_sampler(sampler)
+  previous_sampler = OpenTelemetry.tracer_provider.sampler
+  OpenTelemetry.tracer_provider.sampler = sampler
+  yield
+ensure
+  OpenTelemetry.tracer_provider.sampler = previous_sampler
+end

instrumentation/http/Appraisals

@@ -4,10 +4,19 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
-appraise 'http-4.4' do
-  gem 'http', '~> 4.4.0'
-end
+# To faclitate HTTP semantic convention stability migration, we are using
+# appraisal to test the different semantic convention modes along with different
+# HTTP gem versions. For more information on the semantic convention modes, see:
+# https://opentelemetry.io/docs/specs/semconv/non-normative/http-migration/
+
+semconv_stability = %w[dup stable old]
+
+semconv_stability.each do |mode|
+  appraise "http-4.4.0-#{mode}" do
+    gem 'http', '~> 4.4.0'
+  end
 
-appraise 'http-3.3.0' do
-  gem 'http', '~> 3.3.0'
+  appraise "http-3.3.0-#{mode}" do
+    gem 'http', '~> 3.3.0'
+  end
 end

instrumentation/http/README.md

@@ -64,3 +64,19 @@ The `opentelemetry-instrumentation-http` gem is distributed under the Apache 2.0
 [community-meetings]: https://github.com/open-telemetry/community#community-meetings
 [slack-channel]: https://cloud-native.slack.com/archives/C01NWKKMKMY
 [discussions-url]: https://github.com/open-telemetry/opentelemetry-ruby/discussions
+
+## HTTP semantic convention stability
+
+In the OpenTelemetry ecosystem, HTTP semantic conventions have now reached a stable state. However, the initial HTTP instrumentation was introduced before this stability was achieved, which resulted in HTTP attributes being based on an older version of the semantic conventions.
+
+To facilitate the migration to stable semantic conventions, you can use the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable. This variable allows you to opt-in to the new stable conventions, ensuring compatibility and future-proofing your instrumentation.
+
+When setting the value for `OTEL_SEMCONV_STABILITY_OPT_IN`, you can specify which conventions you wish to adopt:
+
+- `http` - Emits the stable HTTP and networking conventions and ceases emitting the old conventions previously emitted by the instrumentation.
+- `http/dup` - Emits both the old and stable HTTP and networking conventions, enabling a phased rollout of the stable semantic conventions.
+- Default behavior (in the absence of either value) is to continue emitting the old HTTP and networking conventions the instrumentation previously emitted.
+
+During the transition from old to stable conventions, HTTP instrumentation code comes in three patch versions: `dup`, `old`, and `stable`. These versions are identical except for the attributes they send. Any changes to HTTP instrumentation should consider all three patches.
+
+For additional information on migration, please refer to our [documentation](https://opentelemetry.io/docs/specs/semconv/non-normative/http-migration/).

instrumentation/http/lib/opentelemetry/instrumentation/http/instrumentation.rb

@@ -10,8 +10,9 @@ module HTTP
       # The Instrumentation class contains logic to detect and install the Http instrumentation
       class Instrumentation < OpenTelemetry::Instrumentation::Base
         install do |_config|
-          require_dependencies
-          patch
+          patch_type = determine_semconv
+          send(:"require_dependencies_#{patch_type}")
+          send(:"patch_#{patch_type}")
         end
 
         present do
@@ -20,14 +21,47 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base
 
         option :span_name_formatter, default: nil, validate: :callable
 
-        def patch
-          ::HTTP::Client.prepend(Patches::Client)
-          ::HTTP::Connection.prepend(Patches::Connection)
+        def determine_semconv
+          stability_opt_in = ENV.fetch('OTEL_SEMCONV_STABILITY_OPT_IN', '')
+          values = stability_opt_in.split(',').map(&:strip)
+
+          if values.include?('http/dup')
+            'dup'
+          elsif values.include?('http')
+            'stable'
+          else
+            'old'
+          end
+        end
+
+        def patch_old
+          ::HTTP::Client.prepend(Patches::Old::Client)
+          ::HTTP::Connection.prepend(Patches::Old::Connection)
+        end
+
+        def patch_dup
+          ::HTTP::Client.prepend(Patches::Dup::Client)
+          ::HTTP::Connection.prepend(Patches::Dup::Connection)
+        end
+
+        def patch_stable
+          ::HTTP::Client.prepend(Patches::Stable::Client)
+          ::HTTP::Connection.prepend(Patches::Stable::Connection)
+        end
+
+        def require_dependencies_dup
+          require_relative 'patches/dup/client'
+          require_relative 'patches/dup/connection'
+        end
+
+        def require_dependencies_old
+          require_relative 'patches/old/client'
+          require_relative 'patches/old/connection'
         end
 
-        def require_dependencies
-          require_relative 'patches/client'
-          require_relative 'patches/connection'
+        def require_dependencies_stable
+          require_relative 'patches/stable/client'
+          require_relative 'patches/stable/connection'
         end
       end
     end