fix(active-record): avoid error spans for handled RecordInvalid

instrumentation/active_record/README.md

@@ -22,6 +22,19 @@ OpenTelemetry::SDK.configure do |c|
 end
 ```
 
+To configure which exceptions should be re-raised after span closure without
+recording exception events for bang persistence methods (`save!`, `create!`, and
+`update!`), set `handled_exceptions`:
+
+```ruby
+OpenTelemetry::SDK.configure do |c|
+  c.use 'OpenTelemetry::Instrumentation::ActiveRecord',
+        handled_exceptions: ['ActiveRecord::RecordInvalid', 'ActiveRecord::RecordNotFound']
+end
+```
+
+`handled_exceptions` defaults to `['ActiveRecord::RecordInvalid']`.
+
 Alternatively, you can also call `use_all` to install all the available instrumentation.
 
 ```ruby

instrumentation/active_record/lib/opentelemetry/instrumentation/active_record/instrumentation.rb

@@ -11,6 +11,8 @@ module ActiveRecord
       class Instrumentation < OpenTelemetry::Instrumentation::Base
         MINIMUM_VERSION = Gem::Version.new('7')
 
+        option :handled_exceptions, default: ['ActiveRecord::RecordInvalid'], validate: :array
+
         install do |_config|
           require_dependencies
           patch_activerecord
@@ -32,6 +34,7 @@ def gem_version
 
         def require_dependencies
           require 'active_support/lazy_load_hooks'
+          require_relative 'patches/handled_exceptions'
           require_relative 'patches/querying'
           require_relative 'patches/persistence'
           require_relative 'patches/persistence_class_methods'

instrumentation/active_record/lib/opentelemetry/instrumentation/active_record/patches/handled_exceptions.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+# Copyright The OpenTelemetry Authors
+#
+# SPDX-License-Identifier: Apache-2.0
+
+module OpenTelemetry
+  module Instrumentation
+    module ActiveRecord
+      module Patches
+        # Shared exception matching for cases where specific exceptions are
+        # intentionally re-raised after the span closes.
+        module HandledExceptions
+          DEFAULT_HANDLED_EXCEPTIONS = ['ActiveRecord::RecordInvalid'].freeze
+
+          private
+
+          def handled_exception?(exception)
+            handled_exceptions.any? do |handled_exception|
+              case handled_exception
+              when Class
+                exception.is_a?(handled_exception)
+              when String, Symbol
+                exception.class.ancestors.any? do |ancestor|
+                  ancestor.name == handled_exception.to_s
+                end
+              else
+                false
+              end
+            end
+          end
+
+          def handled_exceptions
+            ActiveRecord::Instrumentation.instance.config.fetch(
+              :handled_exceptions,
+              DEFAULT_HANDLED_EXCEPTIONS
+            )
+          end
+        end
+      end
+    end
+  end
+end

instrumentation/active_record/lib/opentelemetry/instrumentation/active_record/patches/persistence.rb

@@ -11,6 +11,8 @@ module Patches
         # Module to prepend to ActiveRecord::Base for instrumentation
         # contains the ActiveRecord::Persistence methods to be patched
         module Persistence
+          include HandledExceptions
+
           def delete
             tracer.in_span("#{self.class}#delete") do
               super
@@ -54,9 +56,17 @@ def update(...)
           end
 
           def update!(...)
-            tracer.in_span("#{self.class}#update!") do
+            handled_exception = nil
+            result = tracer.in_span("#{self.class}#update!") do
               super
+            rescue StandardError => e
+              raise e unless handled_exception?(e)
+
+              handled_exception = e
             end
+            raise handled_exception if handled_exception
+
+            result
           end
 
           def update_column(...)

instrumentation/active_record/lib/opentelemetry/instrumentation/active_record/patches/persistence_class_methods.rb

@@ -18,16 +18,26 @@ class << base
 
           # Contains ActiveRecord::Persistence::ClassMethods to be patched
           module ClassMethods
+            include HandledExceptions
+
             def create(...)
               tracer.in_span("#{self}.create") do
                 super
               end
             end
 
             def create!(...)
-              tracer.in_span("#{self}.create!") do
+              handled_exception = nil
+              result = tracer.in_span("#{self}.create!") do
                 super
+              rescue StandardError => e
+                raise e unless handled_exception?(e)
+
+                handled_exception = e
               end
+              raise handled_exception if handled_exception
+
+              result
             end
 
             def update(...)

instrumentation/active_record/lib/opentelemetry/instrumentation/active_record/patches/validations.rb

@@ -14,16 +14,26 @@ module Patches
         # https://github.com/rails/rails/blob/v5.2.4.5/activerecord/lib/active_record/validations.rb#L42-L53
         # Contains the ActiveRecord::Validations methods to be patched
         module Validations
+          include HandledExceptions
+
           def save(...)
             tracer.in_span("#{self.class}#save") do
               super
             end
           end
 
           def save!(...)
-            tracer.in_span("#{self.class}#save!") do
+            handled_exception = nil
+            result = tracer.in_span("#{self.class}#save!") do
               super
+            rescue StandardError => e
+              raise e unless handled_exception?(e)
+
+              handled_exception = e
             end
+            raise handled_exception if handled_exception
+
+            result
           end
 
           private

instrumentation/active_record/test/instrumentation/active_record/patches/persistence_class_methods_test.rb

@@ -39,6 +39,15 @@
       _(create_span_event.attributes['exception.type']).must_equal('ActiveModel::UnknownAttributeError')
       _(create_span_event.attributes['exception.message']).must_include('unknown attribute \'attreeboot\' for User.')
     end
+
+    it 'does not add an exception event if it raises a handled validation error' do
+      _(-> { User.create!(name: 'not otel') }).must_raise(ActiveRecord::RecordInvalid)
+
+      create_span = spans.find { |s| s.name == 'User.create!' }
+      _(create_span).wont_be_nil
+      _(create_span.events).must_be_nil
+      _(create_span.status.code).must_equal(OpenTelemetry::Trace::Status::UNSET)
+    end
   end
 
   describe '.update' do

instrumentation/active_record/test/instrumentation/active_record/patches/persistence_test.rb

@@ -30,14 +30,42 @@
       _(save_span).wont_be_nil
     end
 
-    it 'adds an exception event if it raises' do
-      _(-> { User.new(name: 'not otel').save! }).must_raise(ActiveRecord::RecordInvalid)
+    it 'adds an exception event if it raises an unhandled error' do
+      user = User.new
+      user.define_singleton_method(:create_or_update) { |*_, **_| raise RuntimeError, 'boom' }
+
+      _(-> { user.save! }).must_raise(RuntimeError)
 
       save_span = spans.find { |s| s.name == 'User#save!' }
       _(save_span).wont_be_nil
       save_span_event = save_span.events.first
-      _(save_span_event.attributes['exception.type']).must_equal('ActiveRecord::RecordInvalid')
-      _(save_span_event.attributes['exception.message']).must_equal('Validation failed: must be otel')
+      _(save_span_event.attributes['exception.type']).must_equal('RuntimeError')
+      _(save_span_event.attributes['exception.message']).must_equal('boom')
+    end
+
+    it 'does not add an exception event for configured handled exceptions' do
+      allow(OpenTelemetry::Instrumentation::ActiveRecord::Instrumentation.instance)
+        .to receive(:config)
+        .and_return(handled_exceptions: ['ActiveRecord::RecordNotFound'])
+
+      user = User.new
+      user.define_singleton_method(:create_or_update) { |*_, **_| raise ActiveRecord::RecordNotFound, 'not found' }
+
+      _(-> { user.save! }).must_raise(ActiveRecord::RecordNotFound)
+
+      save_span = spans.find { |s| s.name == 'User#save!' }
+      _(save_span).wont_be_nil
+      _(save_span.events).must_be_nil
+      _(save_span.status.code).must_equal(OpenTelemetry::Trace::Status::UNSET)
+    end
+
+    it 'does not add an exception event if it raises a handled validation error' do
+      _(-> { User.new(name: 'not otel').save! }).must_raise(ActiveRecord::RecordInvalid)
+
+      save_span = spans.find { |s| s.name == 'User#save!' }
+      _(save_span).wont_be_nil
+      _(save_span.events).must_be_nil
+      _(save_span.status.code).must_equal(OpenTelemetry::Trace::Status::UNSET)
     end
   end
 
@@ -106,6 +134,29 @@
       update_span = spans.find { |s| s.name == 'User#update!' }
       _(update_span).wont_be_nil
     end
+
+    it 'adds an exception event if it raises an unhandled error' do
+      user = User.create!(name: 'otel')
+
+      _(-> { user.update!(attreeboot: 1) }).must_raise(ActiveModel::UnknownAttributeError)
+
+      update_span = spans.find { |s| s.name == 'User#update!' }
+      _(update_span).wont_be_nil
+      update_span_event = update_span.events.first
+      _(update_span_event.attributes['exception.type']).must_equal('ActiveModel::UnknownAttributeError')
+      _(update_span_event.attributes['exception.message']).must_include("unknown attribute 'attreeboot' for User.")
+    end
+
+    it 'does not add an exception event if it raises a handled validation error' do
+      user = User.create!(name: 'otel')
+
+      _(-> { user.update!(name: 'not otel') }).must_raise(ActiveRecord::RecordInvalid)
+
+      update_span = spans.find { |s| s.name == 'User#update!' }
+      _(update_span).wont_be_nil
+      _(update_span.events).must_be_nil
+      _(update_span.status.code).must_equal(OpenTelemetry::Trace::Status::UNSET)
+    end
   end
 
   describe '#update_column' do