update syntax for sentry instrument + Appsignal transaction name convension

Author: HNKhoi2410

lib/idempotency.rb

@@ -71,7 +71,7 @@ def use_cache(request, request_identifiers, lock_duration: nil, action: nil)
     duration_start = Process.clock_gettime(::Process::CLOCK_MONOTONIC)
     action_name = action || "#{request.request_method}:#{request.path}"
 
-    with_apm_instrumentation('idempotency.use_cache', action: action_name) do
+    with_apm_instrumentation('idempotency.use_cache', action_name) do
       return yield unless cache_request?(request)
 
       request_headers = request.env
@@ -150,30 +150,25 @@ def unquote(str)
   end
 
   # rubocop:disable Metrics/AbcSize
-  def with_apm_instrumentation(name, tags = {}, &block)
+  def with_apm_instrumentation(name, action, &block)
     # Build nested instrumentation layers from innermost to outermost
     instrumented_block = block
 
     # Wrap with Sentry if enabled
     if config.observability.sentry_enabled && defined?(Sentry)
       instrumented_block = lambda do
-        transaction = Sentry.start_transaction(name: name, op: 'idempotency', **tags)
-        Sentry.get_current_scope.set_span(transaction)
-
-        begin
+        Sentry.with_child_span(op: name, description: action) do |_span|
           block.call
-        rescue StandardError => e
-          Sentry.capture_exception(e)
-          raise
-        ensure
-          transaction.finish
         end
+      rescue StandardError => e
+        Sentry.capture_exception(e)
+        raise
       end
     end
 
     # Wrap with AppSignal if enabled (outermost layer)
     if config.observability.appsignal_enabled && defined?(Appsignal)
-      Appsignal.monitor_transaction(name, tags) do
+      Appsignal.instrument(name, action) do
         instrumented_block.call
       rescue StandardError => e
         Appsignal.set_error(e)

spec/idempotency/apm_instrumentation_spec.rb

@@ -31,7 +31,7 @@
       end
 
       it 'executes the block without instrumentation' do
-        result = idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
+        result = idempotency.send(:with_apm_instrumentation, 'test.operation', 'test') do
           test_block.call
         end
 
@@ -42,6 +42,7 @@
     context 'when AppSignal is enabled' do
       before do
         stub_const('Appsignal', double('Appsignal'))
+        allow(Appsignal).to receive(:instrument).and_yield
 
         Idempotency.configure do |config|
           config.redis_pool = redis_pool
@@ -51,13 +52,12 @@
         end
       end
 
-      it 'wraps execution in AppSignal transaction' do
-        expect(Appsignal).to receive(:monitor_transaction).with(
-          'test.operation',
-          { action: 'test' }
+      it 'wraps execution in AppSignal instrumentation' do
+        expect(Appsignal).to receive(:instrument).with(
+          'test.operation', 'test'
         ).and_yield
 
-        result = idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
+        result = idempotency.send(:with_apm_instrumentation, 'test.operation', 'test') do
           test_block.call
         end
 
@@ -67,15 +67,14 @@
       it 'reports errors to AppSignal and re-raises' do
         test_error = StandardError.new('test error')
 
-        expect(Appsignal).to receive(:monitor_transaction).with(
-          'test.operation',
-          { action: 'test' }
+        expect(Appsignal).to receive(:instrument).with(
+          'test.operation', 'test'
         ).and_yield
 
         expect(Appsignal).to receive(:set_error).with(test_error)
 
         expect do
-          idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
+          idempotency.send(:with_apm_instrumentation, 'test.operation', 'test') do
             raise test_error
           end
         end.to raise_error(StandardError, 'test error')
@@ -84,30 +83,26 @@
       it 'handles exceptions during error reporting gracefully' do
         test_error = StandardError.new('test error')
 
-        expect(Appsignal).to receive(:monitor_transaction).with(
-          'test.operation',
-          { action: 'test' }
+        expect(Appsignal).to receive(:instrument).with(
+          'test.operation', 'test'
         ).and_yield
 
         expect(Appsignal).to receive(:set_error).with(test_error).and_raise('AppSignal error')
 
         expect do
-          idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
+          idempotency.send(:with_apm_instrumentation, 'test.operation', 'test') do
             raise test_error
           end
         end.to raise_error('AppSignal error')
       end
     end
 
     context 'when Sentry is enabled' do
-      let(:mock_transaction) { double('Sentry::Transaction', finish: true) }
-      let(:mock_scope) { double('Sentry::Scope') }
+      let(:mock_span) { double('Sentry::Span') }
 
       before do
         stub_const('Sentry', double('Sentry'))
-        allow(Sentry).to receive(:start_transaction).and_return(mock_transaction)
-        allow(Sentry).to receive(:get_current_scope).and_return(mock_scope)
-        allow(mock_scope).to receive(:set_span)
+        allow(Sentry).to receive(:with_child_span).and_yield(mock_span)
 
         Idempotency.configure do |config|
           config.redis_pool = redis_pool
@@ -117,18 +112,13 @@
         end
       end
 
-      it 'wraps execution in Sentry transaction' do
-        expect(Sentry).to receive(:start_transaction).with(
-          name: 'test.operation',
-          op: 'idempotency',
-          action: 'test'
-        ).and_return(mock_transaction)
+      it 'wraps execution in Sentry child span' do
+        expect(Sentry).to receive(:with_child_span).with(
+          op: 'test.operation',
+          description: 'test'
+        ).and_yield(mock_span)
 
-        expect(Sentry).to receive(:get_current_scope).and_return(mock_scope)
-        expect(mock_scope).to receive(:set_span).with(mock_transaction)
-        expect(mock_transaction).to receive(:finish)
-
-        result = idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
+        result = idempotency.send(:with_apm_instrumentation, 'test.operation', 'test') do
           test_block.call
         end
 
@@ -138,35 +128,15 @@
       it 'captures exceptions with Sentry and re-raises' do
         test_error = StandardError.new('test error')
 
-        expect(Sentry).to receive(:start_transaction).with(
-          name: 'test.operation',
-          op: 'idempotency',
-          action: 'test'
-        ).and_return(mock_transaction)
+        expect(Sentry).to receive(:with_child_span).with(
+          op: 'test.operation',
+          description: 'test'
+        ).and_yield(mock_span)
 
-        expect(Sentry).to receive(:get_current_scope).and_return(mock_scope)
-        expect(mock_scope).to receive(:set_span).with(mock_transaction)
         expect(Sentry).to receive(:capture_exception).with(test_error)
-        expect(mock_transaction).to receive(:finish)
 
         expect do
-          idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
-            raise test_error
-          end
-        end.to raise_error(StandardError, 'test error')
-      end
-
-      it 'ensures transaction is finished even when error occurs' do
-        test_error = StandardError.new('test error')
-
-        expect(Sentry).to receive(:start_transaction).and_return(mock_transaction)
-        expect(Sentry).to receive(:get_current_scope).and_return(mock_scope)
-        expect(mock_scope).to receive(:set_span).with(mock_transaction)
-        expect(Sentry).to receive(:capture_exception).with(test_error)
-        expect(mock_transaction).to receive(:finish)
-
-        expect do
-          idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
+          idempotency.send(:with_apm_instrumentation, 'test.operation', 'test') do
             raise test_error
           end
         end.to raise_error(StandardError, 'test error')
@@ -186,7 +156,7 @@
       end
 
       it 'falls back to executing without instrumentation' do
-        result = idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
+        result = idempotency.send(:with_apm_instrumentation, 'test.operation', 'test') do
           test_block.call
         end
 
@@ -207,7 +177,7 @@
       end
 
       it 'falls back to executing without instrumentation' do
-        result = idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
+        result = idempotency.send(:with_apm_instrumentation, 'test.operation', 'test') do
           test_block.call
         end
 
@@ -216,15 +186,13 @@
     end
 
     context 'when both AppSignal and Sentry are enabled' do
-      let(:mock_transaction) { double('Sentry::Transaction', finish: true) }
-      let(:mock_scope) { double('Sentry::Scope') }
+      let(:mock_span) { double('Sentry::Span') }
 
       before do
         stub_const('Appsignal', double('Appsignal'))
         stub_const('Sentry', double('Sentry'))
-        allow(Sentry).to receive(:start_transaction).and_return(mock_transaction)
-        allow(Sentry).to receive(:get_current_scope).and_return(mock_scope)
-        allow(mock_scope).to receive(:set_span)
+        allow(Appsignal).to receive(:instrument).and_yield
+        allow(Sentry).to receive(:with_child_span).and_yield(mock_span)
 
         Idempotency.configure do |config|
           config.redis_pool = redis_pool
@@ -236,23 +204,18 @@
 
       it 'instruments in both AppSignal and Sentry (nested)' do
         # Expect Sentry to be set up (inner layer)
-        expect(Sentry).to receive(:start_transaction).with(
-          name: 'test.operation',
-          op: 'idempotency',
-          action: 'test'
-        ).and_return(mock_transaction)
-
-        expect(Sentry).to receive(:get_current_scope).and_return(mock_scope)
-        expect(mock_scope).to receive(:set_span).with(mock_transaction)
-        expect(mock_transaction).to receive(:finish)
+        expect(Sentry).to receive(:with_child_span).with(
+          op: 'test.operation',
+          description: 'test'
+        ).and_yield(mock_span)
 
         # Expect AppSignal to wrap everything (outer layer)
-        expect(Appsignal).to receive(:monitor_transaction).with(
+        expect(Appsignal).to receive(:instrument).with(
           'test.operation',
-          { action: 'test' }
+          'test'
         ).and_yield
 
-        result = idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
+        result = idempotency.send(:with_apm_instrumentation, 'test.operation', 'test') do
           test_block.call
         end
 
@@ -262,19 +225,22 @@
       it 'reports errors to both AppSignal and Sentry' do
         test_error = StandardError.new('test error')
 
-        # Expect Sentry to capture the exception
-        expect(Sentry).to receive(:start_transaction).and_return(mock_transaction)
-        expect(Sentry).to receive(:get_current_scope).and_return(mock_scope)
-        expect(mock_scope).to receive(:set_span).with(mock_transaction)
+        # Expect Sentry to capture the exception (inner layer)
+        expect(Sentry).to receive(:with_child_span).with(
+          op: 'test.operation',
+          description: 'test'
+        ).and_yield(mock_span)
         expect(Sentry).to receive(:capture_exception).with(test_error)
-        expect(mock_transaction).to receive(:finish)
 
-        # Expect AppSignal to also capture the exception
-        expect(Appsignal).to receive(:monitor_transaction).and_yield
+        # Expect AppSignal to also capture the exception (outer layer)
+        expect(Appsignal).to receive(:instrument).with(
+          'test.operation',
+          'test'
+        ).and_yield
         expect(Appsignal).to receive(:set_error).with(test_error)
 
         expect do
-          idempotency.send(:with_apm_instrumentation, 'test.operation', action: 'test') do
+          idempotency.send(:with_apm_instrumentation, 'test.operation', 'test') do
             raise test_error
           end
         end.to raise_error(StandardError, 'test error')