Merge branch 'instrument-analyzers' of https://github.com/shouichi/rails into shouichi-instrument-analyzers

activestorage/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Emit Active Support instrumentation events from Active Storage analyzers.
+
+    Fixes #42930
+
+    *Shouichi Kamiya*
+
 *   Add support for byte range requests
 
     *Tom Prats*

activestorage/lib/active_storage/analyzer.rb

@@ -40,5 +40,9 @@ module ActiveStorage
       def tmpdir # :doc:
         Dir.tmpdir
       end
+
+      def instrument(analyzer, &block) # :doc:
+        ActiveSupport::Notifications.instrument("analyze.active_storage", analyzer: analyzer, &block)
+      end
   end
 end

activestorage/lib/active_storage/analyzer/audio_analyzer.rb

@@ -42,14 +42,16 @@ module ActiveStorage
       end
 
       def probe_from(file)
-        IO.popen([ ffprobe_path,
-          "-print_format", "json",
-          "-show_streams",
-          "-show_format",
-          "-v", "error",
-          file.path
-        ]) do |output|
-          JSON.parse(output.read)
+        instrument(File.basename(ffprobe_path)) do
+          IO.popen([ ffprobe_path,
+            "-print_format", "json",
+            "-show_streams",
+            "-show_format",
+            "-v", "error",
+            file.path
+          ]) do |output|
+            JSON.parse(output.read)
+          end
         end
       rescue Errno::ENOENT
         logger.info "Skipping audio analysis because FFmpeg isn't installed"

activestorage/lib/active_storage/analyzer/image_analyzer/image_magick.rb

@@ -12,7 +12,10 @@ module ActiveStorage
       def read_image
         download_blob_to_tempfile do |file|
           require "mini_magick"
-          image = MiniMagick::Image.new(file.path)
+
+          image = instrument("mini_magick") do
+            MiniMagick::Image.new(file.path)
+          end
 
           if image.valid?
             yield image

activestorage/lib/active_storage/analyzer/image_analyzer/vips.rb

@@ -12,7 +12,10 @@ module ActiveStorage
       def read_image
         download_blob_to_tempfile do |file|
           require "ruby-vips"
-          image = ::Vips::Image.new_from_file(file.path, access: :sequential)
+
+          image = instrument("vips") do
+            ::Vips::Image.new_from_file(file.path, access: :sequential)
+          end
 
           if valid_image?(image)
             yield image

activestorage/lib/active_storage/analyzer/video_analyzer.rb

@@ -121,14 +121,16 @@ module ActiveStorage
       end
 
       def probe_from(file)
-        IO.popen([ ffprobe_path,
-          "-print_format", "json",
-          "-show_streams",
-          "-show_format",
-          "-v", "error",
-          file.path
-        ]) do |output|
-          JSON.parse(output.read)
+        instrument(File.basename(ffprobe_path)) do
+          IO.popen([ ffprobe_path,
+            "-print_format", "json",
+            "-show_streams",
+            "-show_format",
+            "-v", "error",
+            file.path
+          ]) do |output|
+            JSON.parse(output.read)
+          end
         end
       rescue Errno::ENOENT
         logger.info "Skipping video analysis because FFmpeg isn't installed"

activestorage/test/analyzer/audio_analyzer_test.rb

@@ -13,4 +13,14 @@ class ActiveStorage::Analyzer::AudioAnalyzerTest < ActiveSupport::TestCase
     assert_equal 0.914286, metadata[:duration]
     assert_equal 128000, metadata[:bit_rate]
   end
+
+  test "instrumenting analysis" do
+    events = subscribe_events_from("analyze.active_storage")
+
+    blob = create_file_blob(filename: "audio.mp3", content_type: "audio/mp3")
+    blob.analyze
+
+    assert_equal 1, events.size
+    assert_equal({ analyzer: "ffprobe" }, events.first.payload)
+  end
 end

activestorage/test/analyzer/image_analyzer/image_magick_test.rb

@@ -46,6 +46,18 @@ class ActiveStorage::Analyzer::ImageAnalyzer::ImageMagickTest < ActiveSupport::T
     end
   end
 
+  test "instrumenting analysis" do
+    analyze_with_image_magick do
+      events = subscribe_events_from("analyze.active_storage")
+
+      blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg")
+      blob.analyze
+
+      assert_equal 1, events.size
+      assert_equal({ analyzer: "mini_magick" }, events.first.payload)
+    end
+  end
+
   private
     def analyze_with_image_magick
       previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, :mini_magick

activestorage/test/analyzer/image_analyzer/vips_test.rb

@@ -46,6 +46,18 @@ class ActiveStorage::Analyzer::ImageAnalyzer::VipsTest < ActiveSupport::TestCase
     end
   end
 
+  test "instrumenting analysis" do
+    analyze_with_vips do
+      events = subscribe_events_from("analyze.active_storage")
+
+      blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg")
+      blob.analyze
+
+      assert_equal 1, events.size
+      assert_equal({ analyzer: "vips" }, events.first.payload)
+    end
+  end
+
   private
     def analyze_with_vips
       previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, :vips

activestorage/test/analyzer/video_analyzer_test.rb

@@ -76,4 +76,14 @@ class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase
     assert metadata[:video]
     assert_not metadata[:audio]
   end
+
+  test "instrumenting analysis" do
+    events = subscribe_events_from("analyze.active_storage")
+
+    blob = create_file_blob(filename: "video_without_audio_stream.mp4", content_type: "video/mp4")
+    blob.analyze
+
+    assert_equal 1, events.size
+    assert_equal({ analyzer: "ffprobe" }, events.first.payload)
+  end
 end

activestorage/test/test_helper.rb

@@ -157,6 +157,14 @@ class ActiveSupport::TestCase
       ActionController::Base.raise_on_open_redirects = old_raise_on_open_redirects
       ActiveStorage::Blob.service = old_service
     end
+
+    def subscribe_events_from(name)
+      events = []
+      ActiveSupport::Notifications.subscribe(name) do |*args|
+        events << ActiveSupport::Notifications::Event.new(*args)
+      end
+      events
+    end
 end
 
 require "global_id"

guides/source/active_support_instrumentation.md

@@ -690,6 +690,12 @@ INFO. The only ActiveStorage service that provides this hook so far is GCS.
 
 #### transform.active_storage
 
+#### analyze.active_storage
+
+| Key          | Value                          |
+| ------------ | ------------------------------ |
+| `:analyzer`  | Name of analyzer e.g., ffprobe |
+
 ### Railties
 
 #### load_config_initializer.railties